User Settings Bug

I've posted this on the modxcms github issues list, but no one has replied, so I'll try here (probably should have done here first, but oh well).

When updating a user setting via Manage -> Users, the setting entry in the database that was entered first is updated regardless of the setting key or user id.

Step to reproduce

1: Create User testuser. Provide full name, email, set to active, press save.
2: Once user is created, make a user settings entry via the User Settings tab called test_id, name of Test ID, with value of 123 (all other fields default)
3: Check database table modx_user_settings. You should see that there is a setting key test_id for a user with the value 123
4: Create a new user setting via the User Settings tab for the same user called some_id, name of Some ID, set the value to 456 (all other fields default)
5: Check database table modx_user_settings. You should see that setting key test_id and some_id for the user with values 123 and 456 respectively.
6: In the User Settings tab, update the setting some_id with new value of 789. Settings Tab should say test_id has value 123 and some_id has 789
7: Check database table modx_user_settings. Instead of some_id having the value of 789, it will have a value of 456 and it is the test_id key that has the value of 789. You can confirm by refreshing the User Settings tab.

This occurs on a fresh install of ModX.

Also, if you create more users with their own settings, then try to update those settings, it will always be the first entry test_id that gets updated.

Can anyone replicate this? I've checked across three installs; one 2.4.3 and 2.4.4 with the same outcomes. Never noticed it before as I've never used the User Settings abilities before.

No, I cannot replicate this issue, with a new installation of 2.5.0-pl

Still occurs for me after upgrading to 2.5.0 from 2.4.4. Also occurs on fresh install of 2.5.0. I have tried to track it through the ModX model, and have confirmed that the correct setting values (along with key and user id) are sent to the right controllers, but I lose track of where the actual updating occurs.

You will also notice in the 4th image above that the name of the setting gets changed as well.

The three images attached to this post:

1: After the above setting changed, the name of the setting for the first entry also changed. In fact, a whole new setting name entry was created.
2: I reset the name of the setting_test_id entry back to Test ID
3: I then changed the setting name for the 'Another ID' entry in the grid. The database entry for setting_test_id is updated instead.

Ok, so I've tracked down where it changes the setting object it will update.

In the file core/model/modx/modprocessor.class.php, there's an abstract class:



In the initialize method, it calls

Before this line, $this->object references the correct setting object, after this call it's changed to the first instance of a user setting found in the database (for example, if I delete the test_id setting, leaving only 'another_id', then create a whole new setting, then any change to the new setting is made to 'another_id' instead). Commenting out this line results in the correct object being updated in the database.

It's a bit weird, one because why re-initialise the object if it already exists, and two the $primaryKey actually references the correct setting key but returns the wrong object.

The modObjectUpdateProcessor initialize method is called down the chain by the core/model/modx/processors/security/user/setting/update.class.php.

Hmmm. One value is not enough to identify a particular setting. As you've seen, it will always get the first setting for that user (or vice versa, depending).

It takes a minimum of two values to identify a setting: the user ID and the key. The user_settings table has a compound index for both of them.

I believe that this would be the safest fix:

At the lowest level, in the update.class.php processor file, change this:



To this:



I think the intention was to override the parent behavior (because of the need for two fields), but calling parent::initialize() defeats the purpose. There's no need to call the parent method because the update processor does everything the parent does, so the parent would just be repeating the tests (and trashing the object).

If this works, please file a bug report at GitHub (and consider doing a pull request).

That did the trick. Confirmed it working from 2.4.3 to 2.5.0. I'll update my bug report on github.

Much appreaciated.

I can't create a pull request on github but have updated the bug at https://github.com/modxcms/revolution/issues/12966