Skip to content

Commit

Permalink
Allow group settings with string keys
Browse files Browse the repository at this point in the history
This fixes an issue where settings are created/updated in the JSON
API, which has no notion of symbols as implemented by
Ruby. Consequently, these settings get ignored when looked up by
symbol. My solution was to override the `settings` method to return a
`HashWithIndifferentAccess`.

I am personally not a fan of `HashWithIndifferentAccess`, but
considered this the best of several options. Alternatives considered:

* update everywhere that we might be sending hashes with string keys,
  convert them to symbols
* override the setter to accept hashes with symbols or strings, but
  convert them to all symbols prior to writing.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1424842
  • Loading branch information
imtayadeway committed Oct 6, 2017
1 parent 874cad6 commit c0a662b
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 0 deletions.
4 changes: 4 additions & 0 deletions app/models/miq_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ def name
description
end

def settings
super && super.with_indifferent_access
end

def self.with_allowed_roles_for(user_or_group)
includes(:miq_user_role).where.not({:miq_user_roles => {:name => user_or_group.disallowed_roles}})
end
Expand Down
9 changes: 9 additions & 0 deletions spec/models/miq_group_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,15 @@
ws3 = FactoryGirl.create(:miq_widget_set, :name => 'B2', :owner => group)
expect(group.ordered_widget_sets).to eq([ws1, ws3, ws2])
end

it "works when settings use strings" do
ws1 = FactoryGirl.create(:miq_widget_set, :name => 'A1', :owner => group)
_ws2 = FactoryGirl.create(:miq_widget_set, :name => 'C3', :owner => group)
ws3 = FactoryGirl.create(:miq_widget_set, :name => 'B2', :owner => group)
group.update_attributes(:settings => {"dashboard_order" => [ws3.id.to_s, ws1.id.to_s]})

expect(group.ordered_widget_sets).to eq([ws3, ws1])
end
end

context ".sort_by_desc" do
Expand Down

0 comments on commit c0a662b

Please sign in to comment.