Inconsistent behaviour in UserUpdater class

[fzngagan (Faizaan Gagan)]

@eviltrout

Repro steps:

1. Let’s create a user field and it has id 1.
2. Run the second line of the below code twice.

Case 1

up = UserUpdater.new(Discourse.system_user, User.find(1))
up.update({:custom_fields=>{:user_field_1=>"abc"}})

3. You’ll see abc twice when you go to /my/preferences/profile

Surprisingly, when the profile is updated via /my/preferences/profile, it does so perfectly fine.

Case 2

This seems to work fine

up = UserUpdater.new(Discourse.system_user, User.find(1))
params = ActionController::Parameters.new({:custom_fields=>{:user_field_1=>"abc"}})
up.update(params.permit!)

What happens is, in Case 1, the value is pushed to an array and is displayed as comma-separated values on /my/preferences/profile.

Case 2 seems to do the correct thing i.e. it replaces the current value with the new one but wrapping it in ActionController::Parameters is unnatural.
https://github.com/discourse/discourse/blob/3e49c5b4d875654f3357583828086fe8fee104be/app/controllers/users_controller.rb#L160

[eviltrout (Robin Ward)]

This does seem like a valid bug and should be fixed, thanks for reporting it!

[fzngagan (Faizaan Gagan)]

My analysis comes down to this.

# case 1
u1 = User.find(1)
u1.custom_fields[:user_field_1] = "abc"
u1.save

# case 2
u1.custom_fields["user_field_1"] = "abc"
u1.save

In case 1, A new UserCustomField is created per call to u1.save

In case 2, All UserCustomField with the name ‘user_field_1’ are deleted from the db
except the specified.

All and all the symbol and string versions of the same key are treated differently in the HasCustomFields mixin.

[fzngagan (Faizaan Gagan)]

Ok, I went for it. Here’s the PR which fixes the issue

https://github.com/discourse/discourse/pull/10486

[fzngagan (Faizaan Gagan)]

Ok, so this one is merged.