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 Service request filtering #2830

Merged
merged 4 commits into from
Nov 27, 2017
Merged

Conversation

martinpovolny
Copy link
Member

@martinpovolny martinpovolny commented Nov 27, 2017

Reproducer

  1. Services -> Requests
  2. Select only Approved, or only Denied requests
  3. Press Apply

BZ:
https://bugzilla.redhat.com/show_bug.cgi?id=1512719

Closes: #2742

@mzazrivec : PTAL

TODO:

  • Looking as some strange stuff in the case statement, will be done in a separate PR.
  • looking at what code to remove removed just a bit
  • specs

Explanation:

The filters are composed from SQL fragments. So no scopes for now :-(.

It's handled (statefully) in /report_data via the "exceptions". This one is for "MiqRequest" and calls page_params.

So we can keep the code that updates the session when form fields are changed. (That needs a rework no soner then when it's redone to client side.)

And all that is really needed is the reload of the GTL grid -- that will reload /report_data and apply the changed filter (remember, it's in the session). So doing just that.

@miq-bot
Copy link
Member

miq-bot commented Nov 27, 2017

Some comments on commits martinpovolny/manageiq-ui-classic@d159956~...54c3eec

spec/controllers/miq_request_controller_spec.rb

  • ⚠️ - 212 - Detected expect_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.

@miq-bot
Copy link
Member

miq-bot commented Nov 27, 2017

Checked commits martinpovolny/manageiq-ui-classic@d159956~...54c3eec with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. ⭐

@martinpovolny martinpovolny removed the wip label Nov 27, 2017
@martinpovolny martinpovolny changed the title [WIP] Fix Service request filtering Fix Service request filtering Nov 27, 2017
@mzazrivec
Copy link
Contributor

I'm OK with the changes. @karelhala ?

@mzazrivec mzazrivec self-assigned this Nov 27, 2017
@martinpovolny martinpovolny added this to the Sprint 74 Ending Nov 27, 2017 milestone Nov 27, 2017
@martinpovolny
Copy link
Member Author

@karelhala loves it we discussed f2f ;-)

@martinpovolny martinpovolny removed this from the Sprint 74 Ending Nov 27, 2017 milestone Nov 27, 2017
@mzazrivec mzazrivec added this to the Sprint 74 Ending Nov 27, 2017 milestone Nov 27, 2017
@mzazrivec mzazrivec merged commit 7089223 into ManageIQ:master Nov 27, 2017
simaishi pushed a commit that referenced this pull request Nov 28, 2017
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit ee6c37001555d13012300fa3e376516a10e01140
Author: Milan Zázrivec <mzazrivec@redhat.com>
Date:   Mon Nov 27 20:12:20 2017 +0100

    Merge pull request #2830 from martinpovolny/miq_request_fix
    
    Fix Service request filtering
    (cherry picked from commit 7089223b58c197d41cb4423c5aa294a622c57cbd)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1518302

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants