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

Improved request_spec by removing direct filtering by Classification#parent_id #611

Conversation

yrudman
Copy link
Contributor

@yrudman yrudman commented Jun 14, 2019

get rid of direct filtering by 'Classification#parent_id' in rspec

Improvement suggested by @kbrock to #610

@miq-bot add-label test

@miq-bot miq-bot added the test label Jun 14, 2019
@yrudman
Copy link
Contributor Author

yrudman commented Jun 14, 2019

@miq-bot add-label bug

@miq-bot miq-bot added the bug label Jun 14, 2019
@yrudman
Copy link
Contributor Author

yrudman commented Jun 14, 2019

/cc @gtanzillo @kbrock

@@ -258,7 +258,7 @@

FactoryBot.create(:classification_department_with_tags)

t = Classification.where(:description => 'Department', :parent_id => 0).includes(:tag).first
t = Classification.where(:description => 'Department', :parent_id => nil).includes(:tag).first
Copy link
Member

Choose a reason for hiding this comment

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

This looks a bit strange because the category should have :parent_id => 0 in Classifications. Is it possible that the factory can be fixed to ensure that the category is created that way instead of this change?

Copy link
Member

@kbrock kbrock left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this.

lets get parent_id => 0 (or now it is parent_id => nil) out of our code base

thanks

spec/requests/requests_spec.rb Outdated Show resolved Hide resolved
spec/requests/requests_spec.rb Outdated Show resolved Hide resolved
@yrudman yrudman force-pushed the fixed-rspec-for-request-with-classification-parent-nil branch from d82fe80 to 68f14b0 Compare June 19, 2019 12:14
…- get rid of direct filtering by Classification#parent_id
@yrudman yrudman force-pushed the fixed-rspec-for-request-with-classification-parent-nil branch from 68f14b0 to 95bd358 Compare June 19, 2019 12:27
@miq-bot
Copy link
Member

miq-bot commented Jun 19, 2019

Checked commit yrudman@95bd358 with ruby 2.3.3, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🍰

@yrudman yrudman changed the title Fixed request_spec as follow-up of fixing Classification factory Improved request_spec by removing direct filtering by Classification#parent_id Jun 19, 2019
@yrudman
Copy link
Contributor Author

yrudman commented Jun 19, 2019

changed PR description and title, thank you @kbrock and @lpichler

@lpichler lpichler added this to the Sprint 114 Ending Jun 24, 2019 milestone Jun 19, 2019
Copy link
Contributor

@lpichler lpichler left a comment

Choose a reason for hiding this comment

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

thanks 👍

@lpichler lpichler merged commit 2acedd0 into ManageIQ:master Jun 19, 2019
@yrudman yrudman deleted the fixed-rspec-for-request-with-classification-parent-nil branch June 19, 2019 14:01
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