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

Use scope instead of a query to get soft-deleted items #1462

Conversation

Ladas
Copy link
Contributor

@Ladas Ladas commented May 30, 2017

Depends on:

Use scope instead of a query to get soft-deleted items. The rule
of thumb should be to never do specific queries and rather
move the logic to the models.

@cben
Copy link
Contributor

cben commented Jun 1, 2017

There is also some controllers testing deleted_on, eg.
process_show_list(:where_clause => 'container_groups.deleted_on IS NULL')
This will actually keep working without change, but let's switch that too to use scopes.

@Ladas
Copy link
Contributor Author

Ladas commented Jun 1, 2017

@cben nice catch, yes, it's best to use 1 scope (there is also a perf impact, since deleted is indexed and deleted_on is not)

instead of
process_show_list(:where_clause => 'container_groups.deleted_on IS NULL')
we can do
process_show_list(:named_scope => :not_deleted)

Ladas added 3 commits June 5, 2017 10:05
Use scope instead of a query to get soft-deleted items. The rule
of thumb should be to never do specific queries and rather
move the logic to the models.
Use named scope instead of a custom query
Rename :not_deleted scope to :active
@Ladas Ladas force-pushed the use_scope_for_getting_soft_deleted_records_instead_of_a_query branch from dd4630a to b1ac6cb Compare June 5, 2017 08:42
@miq-bot
Copy link
Member

miq-bot commented Jun 5, 2017

Checked commits Ladas/manageiq-ui-classic@aabe976~...b1ac6cb with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
5 files checked, 0 offenses detected
Everything looks fine. 🍪

@chessbyte chessbyte changed the title Use scope instead of a query to get soft-deleted items [WIP] Use scope instead of a query to get soft-deleted items Jun 7, 2017
@Ladas Ladas changed the title [WIP] Use scope instead of a query to get soft-deleted items Use scope instead of a query to get soft-deleted items Jul 12, 2017
@Ladas
Copy link
Contributor Author

Ladas commented Jul 12, 2017

@miq-bot remove_label wip, pending core

@Ladas
Copy link
Contributor Author

Ladas commented Jul 12, 2017

@cben can you give me a last review on this? The dependency was merged

@cben
Copy link
Contributor

cben commented Jul 12, 2017

LGTM 👍

@cben
Copy link
Contributor

cben commented Jul 18, 2017

ping @martinpovolny this is 👍 from containers side, can someone review/merge?

@Ladas can you toggle travis, miq-syntax-checker require was fixed few days ago

@Ladas Ladas closed this Jul 18, 2017
@Ladas Ladas reopened this Jul 18, 2017
@Ladas Ladas closed this Aug 4, 2017
@Ladas Ladas reopened this Aug 4, 2017
@martinpovolny martinpovolny added this to the Sprint 67 Ending Aug 21, 2017 milestone Aug 9, 2017
@martinpovolny martinpovolny merged commit d2c2af2 into ManageIQ:master Aug 9, 2017
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.

6 participants