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

Add status and state scopes, fix time for MiqTask list #16365

Merged
merged 1 commit into from
Nov 2, 2017

Conversation

tumido
Copy link
Member

@tumido tumido commented Oct 31, 2017

Fix a regression brought by: ManageIQ/manageiq-ui-classic#2521
Related: ManageIQ/manageiq-ui-classic#2585

Adding :state and :status based filtering for tasks view and fix the :updated_on scope, since after a switch to reportdata it stopped working properly (Ruby date range can't be passed as string).

@tumido
Copy link
Member Author

tumido commented Oct 31, 2017

cc @martinpovolny

scope :completed_warn, -> { finished.where(:status => 'Warn') }
scope :completed_error, -> { finished.where(:status => 'Error') }
scope :no_status_selected, -> { running.where.not(:status => %(Ok Error Warn)) }
scope :with_status_in, ->(scopes) { scopes[1..-1].reduce(MiqTask.send(scopes.first)) { |chain, s| chain.or(MiqTask.send(s)) } }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the most handsome scope out there, I have to admit that. It allows us to chain scopes to get a query like:

scope1 AND scope2 AND (scope3 OR scope4 OR scope5)

using scopes (matching to example above)

:named_scopes => [:scope1, :scope2, [:with_status_in, [:scope3, :scope4, :scope5]]]

Copy link
Member

@martinpovolny martinpovolny Oct 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an alternative to what previously was an SQL fragment passed around. So while this is not perfect, it's a significant improvement on the current situation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The later patch uses splat instead of slices, which is a bit better code...

@miq-bot
Copy link
Member

miq-bot commented Oct 31, 2017

Checked commit tumido@f8f555e with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@chessbyte
Copy link
Member

LGTM 👍

@martinpovolny martinpovolny merged commit d24d6d7 into ManageIQ:master Nov 2, 2017
@martinpovolny martinpovolny added this to the Sprint 73 Ending Nov 13, 2017 milestone Nov 2, 2017
simaishi pushed a commit that referenced this pull request Nov 3, 2017
Add status and state scopes, fix time for MiqTask list
(cherry picked from commit d24d6d7)

https://bugzilla.redhat.com/show_bug.cgi?id=1509429
@simaishi
Copy link
Contributor

simaishi commented Nov 3, 2017

Gaprindashvili backport details:

$ git log -1
commit 37f79f7eb62ba7ba12cbfa68ba677673c1c7fec1
Author: Martin Povolny <mpovolny@redhat.com>
Date:   Thu Nov 2 11:57:55 2017 +0100

    Merge pull request #16365 from tumido/miq_task_status_as_scope
    
    Add status and state scopes, fix time for MiqTask list
    (cherry picked from commit d24d6d7950a80f38f5e30d8de4db5b6784459e97)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1509429

@tumido tumido deleted the miq_task_status_as_scope branch June 26, 2018 14:17
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.

5 participants