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 selector for MiqTasks #2521
Conversation
|
||
statuses = (opts.keys & status_scopes.keys) | ||
if statuses.any? | ||
statuses.each { |status| scope += status_scope[status] } |
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.
You can and should use reduce here.
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.
@skateman, sure I will, give me 1 hour. :P
bde686d
to
3ccbe2a
Compare
@skateman, @mzazrivec, @tumido : tested? working?
Looks related :-( |
@skateman : otherwise tested, reviewed? |
@tumido I was wrapping my head around this and I understand it more or less, but I think this would need some extra description above the method and some specs. I was also clicking through the UI and found out that the filtering doesn't work at all with your changes 😞 It seems like the |
@@ -79,7 +79,8 @@ def list_jobs | |||
when "tasks_2", "alltasks_2" then @layout = "all_tasks" | |||
end | |||
|
|||
@view, @pages = get_view(MiqTask, :conditions => tasks_condition(@tasks_options[@tabform])) | |||
binding.pry |
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.
☠️ ☠️ ☠️ 🚒 🚽
95bcba5
to
9b47459
Compare
Newly depends on ManageIQ/manageiq#16323 |
Since we can't effectively build scoped queried for
If you look closely you can see we have here ANDed queries for |
what is the status of this PR? is it needed for the upcoming release? |
@abonas, yes. But not by our team, It's needed by @martinpovolny. |
so is it waiting for review/merge? |
@abonas : this is needed for the GTLs to work correctly. This needs to go to the release. There's a set of > 10 PRs and this one is one of those. Once travis is green, this is merged. |
Copy the `tasks_condition` behavior, but return `:named_scopes` instead of `:conditions`. In future replace the `tasks_condition` method, when possible.
9b47459
to
5277449
Compare
Checked commits tumido/manageiq-ui-classic@f6f2dc4~...5277449 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0 app/controllers/miq_task_controller.rb
|
Ignoring failed chargeback test. |
Copy the
tasks_condition
behavior, but return:named_scopes
instead of:conditions
.In future replace the
tasks_condition
method, when possible.Depends on: ManageIQ/manageiq/pull/16308, ManageIQ/manageiq#16323
Should help with: #2383