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

Skip seeding of categories if their creation is invalid #16568

Merged
merged 1 commit into from
Nov 30, 2017

Conversation

chrisarcand
Copy link
Member

@chrisarcand chrisarcand commented Nov 30, 2017

Warning: This is somewhat hacky and definitely an exception in what we've deemed 'standard practice' with our unusual seeding procedure.

This change skips seeding of categories in the event that creating them would be invalid - and they would be invalid because people would like to use their own categories with the same name. Because categories here in seeding are looked up via name (delegated to tag), a new category with the same name is attempted to be made, fails silently, and gives a weird error when entries are attempted to be added to an initialized but unpersisted category record.

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

cc/ @jrafanie

Warning: This is somewhat hacky and definitely an exception in what
we've deemed 'standard practice' with our unusual seeding procedure.

This change skips seeding of categories in the event that creating them
would be invalid - and they would be invalid because people would like
to use their own categories with the same name. Because categories here
in seeding are looked up via name (delegated to tag), a new category
with the same name is attempted to be made, fails silently, and gives a
weird error when entries are attempted to be added to an initialized but
unpersisted category record.

https://bugzilla.redhat.com/show_bug.cgi?id=1507240
@miq-bot
Copy link
Member

miq-bot commented Nov 30, 2017

Checked commit chrisarcand@b093eda with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 2 offenses detected

app/models/classification.rb

spec/models/classification_spec.rb

@gtanzillo
Copy link
Member

Thanks @chrisarcand 👍

@gtanzillo gtanzillo added this to the Sprint 75 Ending Dec 11, 2017 milestone Nov 30, 2017
@gtanzillo gtanzillo merged commit 48846ec into ManageIQ:master Nov 30, 2017
# This tests that if seeding category and it's invalid (due to uniqueness constraints)
# then seeding still doesn't fail.
cat = Classification.find_by!(:description => "Cost Center", :parent_id => 0)
allow(YAML).to receive(:load_file).and_call_original
Copy link
Member

Choose a reason for hiding this comment

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

☝️ 😆 Good times.

Copy link
Member Author

Choose a reason for hiding this comment

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

😬

@jrafanie
Copy link
Member

Looks like code.

@chrisarcand chrisarcand deleted the fix-classification-seeding branch November 30, 2017 17:21
simaishi pushed a commit that referenced this pull request Dec 1, 2017
Skip seeding of categories if their creation is invalid
(cherry picked from commit 48846ec)

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

simaishi commented Dec 1, 2017

Gaprindashvili backport details:

$ git log -1
commit 7eecd58992e02055d8b799aad83e80b94c6e83e0
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Thu Nov 30 12:07:46 2017 -0500

    Merge pull request #16568 from chrisarcand/fix-classification-seeding
    
    Skip seeding of categories if their creation is invalid
    (cherry picked from commit 48846ec50d8fee8be7d894148903b56a9285d855)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1519875

simaishi pushed a commit that referenced this pull request Dec 7, 2017
Skip seeding of categories if their creation is invalid
(cherry picked from commit 48846ec)

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

simaishi commented Dec 7, 2017

Fine backport details:

$ git log -1
commit da99517007ca83933174db0760e3296cb255f830
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Thu Nov 30 12:07:46 2017 -0500

    Merge pull request #16568 from chrisarcand/fix-classification-seeding
    
    Skip seeding of categories if their creation is invalid
    (cherry picked from commit 48846ec50d8fee8be7d894148903b56a9285d855)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1523402

d-m-u pushed a commit to d-m-u/manageiq that referenced this pull request Jun 6, 2018
…-seeding

Skip seeding of categories if their creation is invalid
(cherry picked from commit 48846ec)

https://bugzilla.redhat.com/show_bug.cgi?id=1523402
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