-
Notifications
You must be signed in to change notification settings - Fork 356
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Reset button for tag page in Services #2680
Fix Reset button for tag page in Services #2680
Conversation
@miq-bot add_label bug |
@miq-bot add_label gaprindashvili/yes |
@@ -111,7 +111,8 @@ def tagging_edit_tags_reset | |||
@title = _('Tag Assignment') | |||
if tagging_explorer_controller? | |||
@refresh_partial = "layouts/tagging" | |||
replace_right_cell(:action => @sb[:action]) if params[:button] | |||
action = x_active_tree == :svcs_tree && params[:button] == "reset" ? "tag" : @sb[:action] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the problem here was that @sb[:action]
in this particular case was not set properly? Or was not set at all? Do I understand the problem correctly?
If so, shouldn't we rather fix @sb[:action]
so that it's set correctly? The above fix would just introduce special behavior for one specific tree into a code, which otherwise is written in a generic fashion (more or less).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sb[:action]
was not set at all here. But maybe since I opened this PR, "things" may have changed. I will check it now again and will update this PR. Thank you for good questions! :)
Just for explanation, as I remember, fixing @sb[:action]
was ...hell for me. And it looks like you understand it much more than I. So another hints/ideas, how to fix the bug in a better way, are appreciated 👼
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not saying I necessarily have better understanding of the code. I'd just like to avoid creating special if (or condition if you will) for one specific invocation of the tagging code.
What I'd do is look at tagging of different type of objects, where the reset works correctly and see what's different and where the @sb[:action]
is being set in there.
b7e198d
to
1457687
Compare
The PR changes code in services controller to use common |
1457687
to
ee91ba7
Compare
b10e10d
to
a680c9c
Compare
fixing https://bugzilla.redhat.com/show_bug.cgi?id=1445313 Fix Reset button for tag page opened from Service item detail page in Services -> My Services.
a680c9c
to
e7eef4d
Compare
Checked commits hstastna/manageiq-ui-classic@63f214f~...cee2b70 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0 |
Fix Reset button for tag page in Services (cherry picked from commit 1ad41f8) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1532354 Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1532355
Gaprindashvili backport details:
|
I am going to revert this most likely. It breaks stuff from #2636 And the use of the common (messy) |
I agree with @martinpovolny to revert this PR. We need to fix tagging the other way. I will create a new PR for it once I will have some solution and we can discuss it :) |
@simaishi yes, I can. It looks like the bug is still present in Fine. |
fixing:
(1) https://bugzilla.redhat.com/show_bug.cgi?id=1445313
(2) https://bugzilla.redhat.com/show_bug.cgi?id=1445303
Fix Reset button for tag page opened from Service item detail page in Services -> My Services.
The problem was that Reset button did nothing but produced an error after choosing Edit Tags under Policy , making a change and resetting the change, in Service item detail page in Services -> My Services. The action was not set properly for Services which lead to unwanted behavior.
Before: (1)
![r1](https://user-images.githubusercontent.com/13417815/32607461-6be91a92-c559-11e7-9059-480cff80b154.png)
![r3](https://user-images.githubusercontent.com/13417815/32607820-718ecd38-c55a-11e7-8d72-2dd327416a60.png)
After: (1)
![r2](https://user-images.githubusercontent.com/13417815/32607292-be660a60-c558-11e7-8e5c-89d96d70bd84.png)
Before: (2)
![rr1](https://user-images.githubusercontent.com/13417815/32609534-4b41c210-c560-11e7-85bf-46704d7886a6.png)
After: (2)
![rr2](https://user-images.githubusercontent.com/13417815/32609416-f5662520-c55f-11e7-872e-c3303983f835.png)