Updating a user via API sets user profile to public if private_profile field is not provided
We've identified an issue while synchronising users with our internal directory in which the user profile is always set to public if we don't provide the field in the API PUT request.
The behaviour we observe is that:
- If the field is provided with a boolean value, it is updated correctly
- If the field is provided with an explicit
nil
value, it is set to false - If not provided, it is updated to
false
, no matter which value it had
Our expectation would be that:
- If the field is provided with a valid boolean value, it's updated with the provided value
- If the field is provided with an explicit
nil
value, it is set tofalse
- If not provided, it's left as it is, except if never previously set in the model, in which case it will be set to
false
Initial analysis and tests show that the culprit is a default value in the API user params in lib/api/users.rb
:
params :optional_attributes do
...
optional :private_profile, type: Boolean, default: false, desc: 'Flag indicating the user has a private profile'
...
end
It seems unnecessary to have this default value since the user model model is taking care of it:
before_save :default_private_profile_to_false
...
def default_private_profile_to_false
return unless private_profile_changed? && private_profile.nil?
self.private_profile = false
end
Is there any (historical) reason why this is the only field with a default value?
Furthermore, the specs seem to be wrong for this case in spec/requests/api/users_spec.rb
since the put request will not see the updated value:
it "updates private profile when nil is given to false" do
# This should be `user.update...`
admin.update(private_profile: true)
# The next request will see `user.private_profile? == false`
put api("/users/#{user.id}", admin), params: { private_profile: nil }
expect(user.reload.private_profile).to eq(false)
end
We'll be submitting an MR shortly to tackle this but wanted to give you a heads-up, since maybe there's some background reason for this behaviour.