find_records_with_rbac raises exception if ids contains nil #3131
Conversation
This file contains hidden or 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
|
@miq-bot assign @romanblanco |
himdel
reviewed
Dec 21, 2017
| @@ -123,6 +123,7 @@ def find_record_with_rbac(klass, id, options = {}) | |||
| # either sets flash or raises exception | |||
| # | |||
| def find_records_with_rbac(klass, ids, options = {}) | |||
| raise(_("Can't access records with nil id")) if ids.include?(nil) | |||
There was a problem hiding this comment.
Array.wrap(nil).include?(nil) # false
PanSpagetka
force-pushed
the
change-find-rbac-for-nil
branch
2 times, most recently
from
60ab347 to
5cafa74
Compare
romanblanco
reviewed
Dec 21, 2017
There was a problem hiding this comment.
@PanSpagetka Could you add some tests to test both find_records_with_rbac and find_record_with_rbac, checking that if you send a nil the exception is raised?
PanSpagetka
force-pushed
the
change-find-rbac-for-nil
branch
from
265a427 to
decc9cc
Compare
romanblanco
approved these changes
Dec 21, 2017
|
@PanSpagetka: travis seems related. But restarting anyway. |
|
@PanSpagetka : please, fix the specs. |
PanSpagetka
force-pushed
the
change-find-rbac-for-nil
branch
from
decc9cc to
9ac47f7
Compare
PanSpagetka
force-pushed
the
change-find-rbac-for-nil
branch
3 times, most recently
from
9eb9dbc to
f8699f3
Compare
PanSpagetka
force-pushed
the
change-find-rbac-for-nil
branch
from
f8699f3 to
43e629b
Compare
…ord::RecordNotFound
…ler specs accoding to find_records_with_rbac changes
PanSpagetka
force-pushed
the
change-find-rbac-for-nil
branch
from
43e629b to
20d6907
Compare
|
Checked commits PanSpagetka/manageiq-ui-classic@ccca932~...20d6907 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0 |
|
@martinpovolny ready to merge. |
|
@miq-bot assign @martinpovolny |
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.
Change
find_records_with_rbac(klass, ids, options = {})method behavior if ids contains nil. Now when you call this method with ids contains nil or empty array, it just returns nil or empty array.After this change
find_records_with_rbac(klass, ids, options = {})will raise exception if ids contains nil value. Because when you want to access record with nil id something is probably wrong and this method shouldn't hide it.Works analogically for
find_record_with_rbacwhich usesfind_records_with_rbac