Allow group settings with string keys #16142
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
imtayadeway
force-pushed
the
fix-group-settings-access
branch
from
0084c5b
to
c0a662b
Compare
|
Checked commit imtayadeway@c0a662b with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0 |
h-kataria
added a commit
to h-kataria/manageiq-ui-classic
that referenced
this pull request
Dec 1, 2017
Also fixed saving of reports menus for a selected group that broke with changes in ManageIQ/manageiq#16142 https://bugzilla.redhat.com/show_bug.cgi?id=1517073
h-kataria
added a commit
to h-kataria/manageiq-ui-classic
that referenced
this pull request
Dec 1, 2017
Also fixed saving of reports menus for a selected group that broke with changes in ManageIQ/manageiq#16142 https://bugzilla.redhat.com/show_bug.cgi?id=1517073
h-kataria
added a commit
to h-kataria/manageiq-ui-classic
that referenced
this pull request
Dec 1, 2017
Also fixed saving of reports menus for a selected group that broke with changes in ManageIQ/manageiq#16142 https://bugzilla.redhat.com/show_bug.cgi?id=1517073
h-kataria
added a commit
to h-kataria/manageiq-ui-classic
that referenced
this pull request
Dec 1, 2017
Also fixed saving of reports menus for a selected group that broke with changes in ManageIQ/manageiq#16142 https://bugzilla.redhat.com/show_bug.cgi?id=1517073
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
settingsmethod to return aHashWithIndifferentAccess.I am personally not a fan of
HashWithIndifferentAccess, butconsidered this the best of several options. Alternatives considered:
convert them to symbols
convert them to all symbols prior to writing.
I could be talked into one of these alternatives (or one I hadn't considered) if you have strong opinions ™️ about it.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1424842
@miq-bot add-label bug
@miq-bot assign @gtanzillo