-
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
Add scopes for MiqRequest #3274
Conversation
cc @martinpovolny |
95b4525
to
8a84800
Compare
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.
@tumido nice work 👏 👏 , it looks like tough refactoring, untangling this.
@martinpovolny I wonder if the specs are enough to say this all works? Do we have also some integration tests somewhere? Or should we click it through?
I merged the backend PR. Here I am waiting for the specs that we discussed today. Thx! |
@martinpovolny 👍 I'm on it! 😉 |
97bdce3
to
b722be2
Compare
badcdcd
to
1ead08f
Compare
@miq-bot remove_label wip |
@@ -118,7 +118,8 @@ def report_data_request_data(options) | |||
# Fires a POST request to the current controller's /report_data action | |||
# | |||
def report_data_request(options) | |||
post :report_data, :params => report_data_request_data(options) | |||
request.headers['Content-Type'] = 'application/json' | |||
post :report_data, report_data_request_data(options) |
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.
The lines above are required, otherwise the report_data
post request is broken. Rails can't handle nested arrays on request properly. (When sending the request.) This is the way to bypass it.
If :named_scope
is sent the old way this is what happens:
Sent data (includes):
"named_scope" => [["with_requester", 80000000000202]]
Received by controller:
"named_scope" => [["with_requester"], [80000000000202]]
1ead08f
to
cd812b1
Compare
Some comments on commit tumido@cd812b1 spec/controllers/miq_request_controller_spec.rb
|
Checked commit tumido@cd812b1 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0 app/controllers/miq_request_controller.rb
spec/support/controller_helper.rb
|
Change the MiqRequest query builder to use
named_scopes
instead offilter
.Depends on:
ManageIQ/manageiq#16843Depends on: ManageIQ/manageiq#16939
Depends on: ManageIQ/manageiq#16950
filter
, usescopes
insteadpage_display_options
are used (the first two seems redundant and the third seems unused)request_types_for_model
magic to something readible