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.

Sign up for GitHub

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

Jump to bottom

Add scopes for MiqRequest #16843

Merged
merged 2 commits into from
Jan 31, 2018
Merged

Add scopes for MiqRequest #16843

merged 2 commits into from
Jan 31, 2018

Conversation

tumido
Copy link
Member

@tumido tumido commented Jan 18, 2018 •
edited
Loading

In order to fetch MiqRequest via report_data properly, it's required to get rid of filter and use scopes instead. For more info please consult the dependent PR.

Required by: ManageIQ/manageiq-ui-classic#3274

@tumido tumido mentioned this pull request Jan 18, 2018
Merged
4 tasks
@tumido
Copy link
Member Author

tumido commented Jan 18, 2018

@miq-bot add_label wip

@miq-bot miq-bot changed the title Add scopes for MiqRequest [WIP] Add scopes for MiqRequest Jan 18, 2018
@miq-bot miq-bot added the wip label Jan 18, 2018
@tumido
Copy link
Member Author

tumido commented Jan 18, 2018

cc @martinpovolny

@miq-bot
Copy link
Member

miq-bot commented Jan 22, 2018

Checked commits tumido/manageiq@9f04ed5~...4afaebb with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

@tumido
Copy link
Member Author

tumido commented Jan 22, 2018

@miq-bot remove_label wip
@miq-bot add_label core, enhancement

@tumido tumido changed the title [WIP] Add scopes for MiqRequest Add scopes for MiqRequest Jan 22, 2018
martinpovolny
martinpovolny approved these changes Jan 30, 2018
View reviewed changes
Copy link
Member

@martinpovolny martinpovolny left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The def self.with_reason_like(reason) might seem strange, but we need that functionality and I believe the implementation is safe and far better than what we have now.

Overall I approve. Ping @Fryguy, @Ladas : any objections or can I merge this one?

Ladas
Ladas approved these changes Jan 30, 2018
View reviewed changes
Copy link
Contributor

@Ladas Ladas left a comment

Choose a reason for hiding this comment

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

Looks great. 👍 It's a nice cleanup for the controllers.

@martinpovolny martinpovolny merged commit 15cab00 into ManageIQ:master Jan 31, 2018
@martinpovolny martinpovolny added this to the Sprint 79 Ending Feb 12, 2018 milestone Jan 31, 2018
@tumido tumido mentioned this pull request Feb 5, 2018
Merged
@tumido tumido mentioned this pull request Mar 7, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ladas Ladas Ladas approved these changes

@martinpovolny martinpovolny martinpovolny approved these changes

Assignees

@Fryguy Fryguy

Labels
core enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tumido @miq-bot @martinpovolny @Ladas @Fryguy