Don't x_node_set on a tree which doesn't exist #1657
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
Calling `x_node_set` when a tree doesn't exist works, but actually creates the entry that `tree_exists?` checks to see if such a tree exists.
When the user doesn't have the right to see such a tree, that's problematic, since it breaks the assumption that `tree_exists?` returns true only for trees that were actually initialized.
The leads to the calling `lock_tree` on a nonexistent tree, leading to a JS exceptions ("No element has been found").
This makes us actually check if the tree already exists before calling `x_node_set` on it.
(Would prefer to fix `x_node_set` instead, but.. this is a bugfix, and I really don't want to introduce more bugs. :))
https://bugzilla.redhat.com/show_bug.cgi?id=1467146
|
Checked commit https://github.com/himdel/manageiq-ui-classic/commit/5d36640dacc79b2aa3ac3978ddc34de37d74575c with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
|
looks good. |
|
@miq-bot add_label blocker |
simaishi
pushed a commit
that referenced
this pull request
Jul 6, 2017
Don't x_node_set on a tree which doesn't exist (cherry picked from commit 63093bc) https://bugzilla.redhat.com/show_bug.cgi?id=1468336 https://bugzilla.redhat.com/show_bug.cgi?id=1468337
|
Fine backport details: |
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.
Calling
x_node_setwhen a tree doesn't exist works, but actually creates the entry thattree_exists?checks to see if such a tree exists.When the user doesn't have the right to see such a tree, that's problematic, since it breaks the assumption that
tree_exists?returns true only for trees that were actually initialized.That leads to calling
lock_treeon a nonexistent tree, leading to a JS exceptions ("No element has been found").This makes us actually check if the tree already exists before calling
x_node_seton it - specifically forroles_tree(that'sTreeBuilderReportRoles).(I would prefer to fix
x_node_setinstead, but.. this is a bugfix, and I really don't want to introduce more bugs. :))https://bugzilla.redhat.com/show_bug.cgi?id=1467146
EDIT: also fixes https://bugzilla.redhat.com/show_bug.cgi?id=1448332