Skip to content
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 selecting ALL filter and changing it to default on Physical Infra #3797

Conversation

hstastna
Copy link

@hstastna hstastna commented Apr 19, 2018

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1567416


Fix selecting ALL filter and changing it to Default on Physical Infra page
(in Compute -> Physical Infrastructure -> Servers), also on Hosts page.

Steps to Reproduce:

  1. Go to
    Compute -> Physical Infrastructure -> Servers or
    Compute -> Infrastructure -> Hosts
  2. In accordion, select a filter other than the ALL (Default)
  3. Click on the Set Default button to set the selected filter as Default
  4. Click on the ALL filter again
    => nothing happened, previously selected filter remained applied and it was not possible to set ALL back to ALL (Default)

Step 4 from steps to reproduce:
filter_set_all_default

Before:
filter_before

After:
filter_after


Details:
Removing unnecessary line of the code fixed the bug. The problem was that default search was loaded (load_default_search method) when clicking on ALL filter because @edit[:selected] was set to false. As I remember, it was me who added the same line to the code some time ago. Adding it was not necessary so I am sure we can remove it.

@hstastna
Copy link
Author

@miq-bot add_label bug, gaprindashvili/yes

@hstastna
Copy link
Author

hstastna commented May 4, 2018

@martinpovolny @mzazrivec Could you please look at this simple fix? Thanks! ;)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1567416

Fix selecting ALL filter and changing it to default on Physical Infra page
(in Compute -> Physical Infrastructure -> Servers), also on Hosts page.
@hstastna hstastna force-pushed the Impossible_select_ALL_filter_default_Physical_Infra branch from f158394 to 679df2c Compare May 4, 2018 13:51
@miq-bot
Copy link
Member

miq-bot commented May 4, 2018

Checked commit hstastna@679df2c with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🍪

@hstastna
Copy link
Author

hstastna commented May 4, 2018

What do you think about this fix, @himdel ? :)

@saulotoledo
Copy link
Member

I confirm that the fix solves the problem I reported.

@himdel
Copy link
Contributor

himdel commented May 9, 2018

The line was introduced in #2225 .. doesn't look like it was to fix a bug, so hopefully this won't break anything.. :)

@hstastna
Copy link
Author

hstastna commented May 9, 2018

@himdel I think it will not break anything. I added the line in that PR because of comment in https://github.com/ManageIQ/manageiq-ui-classic/pull/2225/files#diff-08a3013ef490c3273c6b0ffc7c84a0eaR184 I thought I was doing the right thing. Apparently, not :D Now I see I did not understand well what this line or variable was for.

@himdel himdel self-assigned this May 18, 2018
@himdel
Copy link
Contributor

himdel commented May 18, 2018

LGTM, works for me :)

@himdel himdel merged commit a0126da into ManageIQ:master May 18, 2018
@himdel himdel added this to the Sprint 86 Ending May 21, 2018 milestone May 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants