FIX: Update only present fields in request (PR #14310)

Some category fields were always updated, even if they were not present in the request. When this happened, these field were erased.

GitHub

          put "/categories/#{category.id}.json"

          expect(response.status).to eq(200)

Testing for the response helds to provide better feedback in the event that a failure occurs.

          expect(category.tags.blank?).to eq(true)
          expect(category.tags.blank?).to eq(true)
          expect(category.tag_groups.blank?).to eq(true)

Will the || [] ever be called?

[3] pry(main)> nil.presence || [] if nil
=> nil
[4] pry(main)> true.presence || [] if true
=> true
[5] pry(main)> false.presence || [] if false
=> nil

If params[:allowed_tags] is nil, it’ll not pass the conditional so params[:allowed_tags].presence in this case can never be false or nil

We can just pluck the id here. Also, I think we should provide better feedback if reviewable_by_group_id is an invalid group id.

Sorry just realized this is not related to your change but since we’re making changes here we might as well fix it grin

Yes, because not all true values are also present.

When there are no more allowed_tags, params[:allowed_tags] will be equal to “”. The if makes sure that allowed_tags will be updated and and the .presence and || make sure that the blank string is converted to an empty array. It also works well when params[:allowed_tags] is equal to an array of strings.

ahhh I didn’t know presence actually returns nil for blank strings. All good here then :+1: