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

Replace conditions with scope #144

Merged
merged 1 commit into from Dec 6, 2017

Conversation

alexander-demicev
Copy link

@alexander-demicev alexander-demicev commented Nov 6, 2017

Replace sql conditions with scopes.

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

@@ -28,11 +28,11 @@ def service_group_names
end

def service_group_services_running
service_group_services.where(SystemService.running_systemd_services_condition)
SystemService.running_systemd
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the PR! Just a note: we want use an association instead of SystemService model to find matching system services.

@miq-bot miq-bot added the wip label Nov 6, 2017
@alexander-demicev alexander-demicev changed the title [WIP] Replace conditions with scope Replace conditions with scope Nov 25, 2017
@miq-bot miq-bot removed the wip label Nov 25, 2017
@@ -28,11 +28,11 @@ def service_group_names
end

def service_group_services_running
service_group_services.where(SystemService.running_systemd_services_condition)
service_group_services.where("(\"system_services\".\"systemd_active\" = 'running' AND \"system_services\".\"systemd_sub\" = 'running')")
Copy link
Member

@Fryguy Fryguy Nov 27, 2017

Choose a reason for hiding this comment

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

👎

You should just let ActiveRecord do the escaping for you.

.where(:system_services => {:systemd_active => "running", :systemd_sub => "running"})

end

def service_group_services_failed
service_group_services.where(SystemService.failed_systemd_services_condition)
service_group_services.where("(\"system_services\".\"systemd_active\" = 'failed' OR \"system_services\".\"systemd_sub\" = 'failed')")
Copy link
Member

Choose a reason for hiding this comment

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

Same

@aufi
Copy link
Member

aufi commented Nov 29, 2017

@alexander-demichev Hi, could check the failing Travis?

@alexander-demicev
Copy link
Author

@aufi Hi, Travis fails because of unmerged PR in main repo.

@miq-bot
Copy link
Member

miq-bot commented Nov 30, 2017

Checked commit alexander-demicev@e6ee2a0 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. 🍰

@aufi
Copy link
Member

aufi commented Nov 30, 2017

Looks good to me!

@aufi aufi merged commit 88ec0d4 into ManageIQ:master Dec 6, 2017
@aufi aufi added this to the Sprint 75 Ending Dec 11, 2017 milestone Dec 6, 2017
@aufi
Copy link
Member

aufi commented Dec 6, 2017

This PR depends on ManageIQ/manageiq#16399 (merged) and is needed by ManageIQ/manageiq-ui-classic#2638 (merge pending)

@simaishi
Copy link
Contributor

simaishi commented Jan 9, 2018

FYI... for backport, waiting for someone to confirm the dependent PR ManageIQ/manageiq#16399 can be gaprindashvili/yes

ManageIQ/manageiq#16399 (comment)

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.

None yet

5 participants