Display Cancel after applying changes in non-explorer Compare screen #6542
Conversation
|
@miq-bot add_label bug |
hstastna
changed the title
hstastna
force-pushed
the
Cancel_after_applying_changes_Compare
branch
2 times, most recently
from
dffe306
to
a7d67a6
Compare
hstastna
changed the title
hstastna
force-pushed
the
Cancel_after_applying_changes_Compare
branch
from
a7d67a6
to
586577c
Compare
|
Checked commit hstastna@586577c with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 |
|
Tested in UI. Fixes the problem in all possible nested list cases. LGTM 👍 |
| @@ -420,7 +420,7 @@ def drift_same | |||
| # AJAX driven routine to check for changes in ANY field on the form | |||
| def sections_field_changed | |||
| @keep_compare = true | |||
| @explorer = %w[VMs Templates].include?(session[:db_title]) | |||
| @explorer = %w[VMs Templates].include?(session[:db_title]) && @display.nil? | |||
There was a problem hiding this comment.
Fixes the issue, but I think the condition is wrong anyway.
What we usually do to determine explorer:
if BASE_MODEL_EXPLORER_CLASSES.include?(cls)if request.xml_http_request?if @edit[:explorer]
Can any of those work instead?
There was a problem hiding this comment.
The old logic of "VM" or "Template" is in title => explorer, is wrong, so this is fixing a wrong thing by adding stuff instead of replacing to logic to make sense.
Perhaps the place which renders the template should be setting @edit[:explorer] and then we can get it from session.
There was a problem hiding this comment.
I've tried the second two of them, before I made the change in this PR, of course, but it was not good enough (it fixed the issue for non explorer screens but stopped working for explorer ones).
if @edit[:explorer] did not work, we need @edit to be set, for this case - and this is not always done even when we are in an explorer screen. Even when I've set @edit[:explorer] to true or false at the beginning of comparing items (after user clicked on Compare action), later right after calling sections_field_changed @edit became nil, the information 'got lost'.
And request.xml_http_request? returned something even when we weren't in explorer screen (this happens to me sometimes also for other places and controllers).
There was a problem hiding this comment.
OK, merging as is then, sounds like a future refactoring :).
(created #6580 )
There was a problem hiding this comment.
Also checking some classes was not enough to decide if we are in a explorer screen or not, because of nested lists which are mostly in non-explorer screens.
Issue: #6481
Cancel button "disappeared" after applying changes in Comparison Sections while comparing selected items displayed in a non-explorer screen. This happened because
@explorerwas accidentaly set totrue.The problem was that
%w[VMs Templates].include?(session[:db_title])condition insections_field_changedwas simply not enough to set@explorervariable properly. The condition did not care about if we are in a nested list of VMs/Templates. And if we are, usually@displayis set to'vms'or'instances', so I used this variable for setting@explorerproperly while making/applying changes in in Comparison Sections. And we need@explorerto be set properly because of this.Before:
After:
