[#18701] - Prevent PlgSystemFields::onContentAfterSave from deleting user values from #__fields_values
- Closed
- 8 Dec 2017
- Medium
- Build: staging
- # 18701
- Diff
- stutteringp0et:patch-4
User tests: Successful: Unsuccessful:
Changing user groups via JUserHelper without first setting a task input variable to activate, block or unblock deletes ALL user values from #__fields_values. $model->setFieldValue deletes records where the value is null. This change tests for a null value before calling setFieldValue to prevent that deletion.
Pull Request for Issue # .
Summary of Changes
Test for null value to prevent the fields model from deleting user values from the table.
Testing Instructions
Create a system plugin that calls JUserHelper::setUserGroups
Before the patch, all user field values for the updated user will be deleted.
After the patch, all user field values for the updated user will remain.
Prior to this change, I had to JFactory::getApplication()->input->set('task','unblock'); in order to update user groups without losing all of their field data.
Expected result
User fields remain
Actual result
User fields are deleted
Documentation Changes Required
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Front End Plugins |
| Labels |
Added:
?
|
||
| Title |
|
||||||
What if the new value is 0 (zero)? The "if($value)" test would fail and the field would not be updated. Even a string 0 fails that test.
Of course, good point!
On 5 Dec 2017, 03:03 +0000, Michael Richey notifications@github.com, wrote:
What if the new value is 0 (zero)? The "if($value)" test would fail and the field would not be updated.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
To explain my thinking on this - when a field doesn't exist in the request, PlgSystemFields sets it to null. There is no way for a user to submit a null, so the only way for it to be null is if it is missing from the request. This PR is just ignoring the nulls.
@stutteringp0et Just for your information, the problem is more difficult than your propose fix because we have to deal with Checkboxes, Checkbox custom field types. You can see the two PRs try to deal with it:
That's a lot of eyeballs on the problem. I hadn't considered checkboxes. Closing this PR.
Thanks for pointing me to the others.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-12-08 07:36:21 |
| Closed_By | ⇒ | stutteringp0et |
plg_system_pr18701.zip
Attached: A system plugin to test this PR.
This plugin has 3 configurations. User, Group, and Method.
Choose a user to manipulate
Choose a group to toggle membership
Choose a method to perform the group update
You must first create at least one user custom field and fill it with a value for the test user.
This plugin toggles the group membership for the selected user on every page load. Regardless of the method, all field values are lost for the user before this PR.
Note, this plugin does not do anything when saving a user profile, only on page refreshes/loads.
Once this PR is applied, this plugin has no effect on field values for the user.