Skip to content

Enforce array for subform values#16733

Merged
mbabker merged 1 commit intojoomla:stagingfrom
bembelimen:patch-28
Aug 6, 2017
Merged

Enforce array for subform values#16733
mbabker merged 1 commit intojoomla:stagingfrom
bembelimen:patch-28

Conversation

@bembelimen
Copy link
Copy Markdown
Contributor

@bembelimen bembelimen commented Jun 17, 2017

Summary of Changes

This PR enforce, that the value is an array, because I had the case, that I get an object and the field stopped working.

Testing Instructions

Apply the patch, it should work like before.

@coolcat-creations
Copy link
Copy Markdown
Contributor

I have tested this item ✅ successfully on fb04de3

Now the values save correctly and are also shown in the subform field


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16733.

1 similar comment
@coolcat-creations
Copy link
Copy Markdown
Contributor

I have tested this item ✅ successfully on fb04de3

Now the values save correctly and are also shown in the subform field


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16733.

@laoneo
Copy link
Copy Markdown
Member

laoneo commented Jun 17, 2017

Did you get here an object produced in core or by an extension? Because there should never arrive here an object if all is working as it should. If there is code in core which passes an object, then the bug is probably on that edge.

@bembelimen
Copy link
Copy Markdown
Contributor Author

bembelimen commented Jun 18, 2017

Hi @laoneo is there any subform in the core?
The "problem" is, that you can give an array or an object to JForm, so somewhere has to be a object => array converter

@laoneo
Copy link
Copy Markdown
Member

laoneo commented Jun 19, 2017

Yes the list based custom fields.

@coolcat-creations
Copy link
Copy Markdown
Contributor

Sorry offtopic: #15723 I thought Custom Fields don´t support subforms. But true - the listfield is one. But why does this work but custom CustomFields not?

@bembelimen
Copy link
Copy Markdown
Contributor Author

@laoneo could you please evaluate your thoughts?
My view is: at the moment it's possible, that values could be non-arrays, it doesn't matter where they come from, and the field has to take care for. But I'm open for all suggestions...

@HRIT-Florian
Copy link
Copy Markdown
Contributor

I have tested this item ✅ successfully on fb04de3


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16733.

@HRIT-Florian
Copy link
Copy Markdown
Contributor

I have tested this item ✅ successfully on fb04de3


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16733.

@ghost
Copy link
Copy Markdown

ghost commented Jun 24, 2017

RTC after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jun 24, 2017
@zero-24 zero-24 added this to the Joomla 3.7.4 milestone Jun 24, 2017
@laoneo
Copy link
Copy Markdown
Member

laoneo commented Jun 25, 2017

I just fear that it can have unexpected behaviors if an object is passed. @bembelimen where and how do you pass the object, perhaps I do miss something here.

@rdeutz rdeutz modified the milestones: Joomla 3.8.0, Joomla 3.7.4 Jul 11, 2017
@mbabker
Copy link
Copy Markdown
Contributor

mbabker commented Jul 29, 2017

@bembelimen any feedback on Allon's last comment?

@bembelimen
Copy link
Copy Markdown
Contributor Author

I'm not sure, what I should answer here?
If the data is an object, you'll get an error.

The object is passed via $form->bind($data);

The question is: what should break here, because the field expect an array...

@joomla-cms-bot joomla-cms-bot added PR-staging and removed RTC This Pull Request is Ready To Commit labels Aug 6, 2017
izharaazmi added a commit to izharaazmi/joomla-cms that referenced this pull request Aug 9, 2017
* staging: (148 commits)
  Correcting non-escaped double quotes in en-GB.plg_sampledata_testing.ini (joomla#17455)
  Correct namespace reference (Fix joomla#17448)
  Correcting Jalali/Persian calendar popup (joomla#17432)
  Adding russian calendar language file (joomla#17443)
  Reset for dev
  Prepare 3.8 Beta release
  Fix covers tags
  Fix file paths
  Move library files to just libraries/src as it should be (joomla#17441)
  Add a default empty array for the session queue (joomla#16943)
  [3.8] Restructure version constants (joomla#16169)
  Adjusting copyright and versions and two remaining "sampledata" (joomla#17435)
  PHP 7.2 has branched, update Travis config to reflect
  PHP 7.2 count warning (joomla#16840)
  Enforce array for subform values (joomla#16733)
  System URL menu link (joomla#17419)
  Don't use array merge here. (joomla#17391)
  add the checked attribute (joomla#17336)
  [RFC] Mod sample data (joomla#7680)
  Rename Page to Menu Item (joomla#17409)
  ...
izharaazmi added a commit to izharaazmi/joomla-cms that referenced this pull request Aug 9, 2017
* staging: (148 commits)
  Correcting non-escaped double quotes in en-GB.plg_sampledata_testing.ini (joomla#17455)
  Correct namespace reference (Fix joomla#17448)
  Correcting Jalali/Persian calendar popup (joomla#17432)
  Adding russian calendar language file (joomla#17443)
  Reset for dev
  Prepare 3.8 Beta release
  Fix covers tags
  Fix file paths
  Move library files to just libraries/src as it should be (joomla#17441)
  Add a default empty array for the session queue (joomla#16943)
  [3.8] Restructure version constants (joomla#16169)
  Adjusting copyright and versions and two remaining "sampledata" (joomla#17435)
  PHP 7.2 has branched, update Travis config to reflect
  PHP 7.2 count warning (joomla#16840)
  Enforce array for subform values (joomla#16733)
  System URL menu link (joomla#17419)
  Don't use array merge here. (joomla#17391)
  add the checked attribute (joomla#17336)
  [RFC] Mod sample data (joomla#7680)
  Rename Page to Menu Item (joomla#17409)
  ...
@bembelimen bembelimen deleted the patch-28 branch August 13, 2018 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants