Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.9] Correctly handle objects when storing data in tables with _jsonEncode being set #33633

Merged

Conversation

richard67
Copy link
Member

@richard67 richard67 commented May 7, 2021

Pull Request for Issue #33553 .

Summary of Changes

Back-integrate from 4.0-dev to here (staging) the following changes from PR #25761 for fixing bind() with objects in the JTable class:

  • Change order of processing in the bind() routine and add a get_object_vars if the $src parameter is an object, so that it is:
  1. Check if the source value is an array or object and throw an exception if not.
  2. If the ignore value is a string, explode it over spaces.
  3. If the source value is an object, get its accessible properties with get_object_vars.
  4. JSON encode any fields if required.
  5. Bind the source value, excluding the ignored fields.
  • Add a unit test for bind() with an object.

I'm wondering why the 4th step in the above list currently comes before the 1st without this PR.

Testing Instructions

Step 1: On a current staging branch or latest 3.9.x nightly build, switch on ""Debug System" and set "Error Reporting" to "Maximum" or "Development".

Step 2: Add the following code to some administrator view:

// Test PR-33558 begin
JTable::addIncludePath(JPATH_ROOT . '/administrator/components/com_contact/tables');

$row1 = JTable::getInstance('Contact', 'ContactTable');

$contact1 = [
	'name'      => 'Bat Man',
	'alias'     => 'bat-man',
	'catid'     => 4,
	'params'    => ['show_contact_list' => 0, 'show_tags' => ''],
	'language'  => '*',
	'published' => 1,
	'access'    => 1,
];

$row1->bind($contact1);
$row1->store();

echo '<div>Contact 1 saved.<div>';

$row2 = JTable::getInstance('Contact', 'ContactTable');

$contact2            = new stdClass;
$contact2->name      = 'John Doe';
$contact2->alias     = 'john-doe';
$contact2->catid     = 4;
$contact2->params    = ['show_contact_list' => 0, 'show_tags' => ''];
$contact2->language  = '*';
$contact2->published = 1;
$contact2->access    = 1;

$row2->bind($contact2);
$row2->store();

echo '<div>Contact 2 saved.<div>';
// Test PR-33558 end

E.g. you can add it just before the following line in the users view's default template:
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_users/views/users/tmpl/default.php#L20

The code creates or updates 2 new contact records in database, using bind() and store(), one contact given as array and the other one as object (stdClass).

Step 3: Navigate to the view modified in the previous step.
In my example: "Users -> Manage" (administrator/index.php?option=com_users&view=users).

Result: Error 'Cannot use object of type stdClass as array', see issue #33553 and section "Actual result BEFORE applying this Pull Request" below.

Step 4: Check in database if the two contacts have been created.

Result: Only the first contact "Bat Man" has been created.

Step 5: Delete that contact.

Step 6: Apply the patch of this PR.

Step 7: Navigate again to the view modified in the previous step, or reload the page if still there.

Result: No error, the debug output is shown, see section "Expected result AFTER applying this Pull Request" below.

Step 8: Check in database if the two contacts have been created.

Result: Both contacts "Bat Man" and "John Doe" have been created.

Actual result BEFORE applying this Pull Request

Storing data fails with error 'Cannot use object of type stdClass as array'.

2021-05-06_1

The line number may be different to the one shown in the screenshot because that screenshot was made when testing PR #33558 which I had made for 3.10).

Expected result AFTER applying this Pull Request

Storing data succeeds.

The debug output is shown.

2021-05-06_2

Documentation Changes Required

None.

Additional information

This change doesn't need to be merged up into 4.0-dev since it has been applied there already, see PR #25761 . The unit tests are implemented differently there.

I.e. when merging this up and getting conflicts, just chose to use the existing 4.0-dev code for these when resolving.

@joomdonation
Copy link
Contributor

I have tested this item ✅ successfully on 7424cc4


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

@alikon
Copy link
Contributor

alikon commented May 11, 2021

I have tested this item ✅ successfully on 7424cc4


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

@alikon
Copy link
Contributor

alikon commented May 11, 2021

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 11, 2021
@richard67 richard67 added this to the Joomla! 3.9.27 milestone May 11, 2021
@HLeithner HLeithner merged commit 0c0becc into joomla:staging May 15, 2021
@HLeithner
Copy link
Member

thanks

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 15, 2021
@richard67 richard67 deleted the staging-support-object-in-table-bind branch May 15, 2021 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants