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

Remove MiqWidget title unique validation #11431

Merged

Conversation

carbonin
Copy link
Member

This validation was causing issues when someone deleted a default widget then created a new widget with the same title as the default one, but a different description.

When we restart the server (on an update, for example) we attempt to recreate the default widgets if we can't find them by description, this leads to a unique validation error on the title.

This PR removes the validation and changes one additional lookup by title to a lookup by description.
Also we had to change how the schedule for a widget was created because schedule has a unique validation on name which would cause a similar problem during seeding (the widget would get created, but fail to create the schedule).

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

@miq-bot add_label bug
@gtanzillo please review

Some alternatives

I'm okay going either of these ways as well

  • If there is a good reason we had a unique constraint on the widget title, another option is to not allow users to remove the default widgets in the first place (especially as they will be recreated anyway).
  • Yet another option is to look up the existing widget by title or description, but I feel like calling two widgets the same widget because their titles are the same isn't quite right.

This allows a user to remove a default widget and recreate a new
one with the same title without having the seed blow up with a
unique validation error when it runs again.

https://bugzilla.redhat.com/show_bug.cgi?id=1375313
This is more reliable now that description is the only unique
field.
@miq-bot miq-bot added the bug label Sep 21, 2016
@carbonin carbonin changed the title Remove miq widget title unique validation Remove MiqWidget title unique validation Sep 21, 2016
@miq-bot
Copy link
Member

miq-bot commented Sep 21, 2016

Checked commits carbonin/manageiq@0f60e7d~...673be4b with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
2 files checked, 1 offense detected

app/models/miq_widget.rb

  • ❗ - Line 21, Col 3 - Rails/Validation - Prefer the new style validations validates :column, uniqueness: value over validates_uniqueness_of.

@gtanzillo
Copy link
Member

👍 I"m good with this change.

@gtanzillo gtanzillo added this to the Sprint 47 Ending Oct 3, 2016 milestone Sep 23, 2016
@gtanzillo gtanzillo merged commit ba6fd67 into ManageIQ:master Sep 23, 2016
@chessbyte
Copy link
Member

@carbonin @gtanzillo This did not make it into Euwe branch. Please label euwe/yes or euwe/no, as appropriate.

@carbonin
Copy link
Member Author

@miq-bot add_label darga/yes

chessbyte pushed a commit that referenced this pull request Sep 29, 2016
…e_validation

Remove MiqWidget title unique validation
(cherry picked from commit ba6fd67)
@chessbyte
Copy link
Member

Euwe Backport details:

$ git log
commit 6532a7cc2802a492a69564cdba30b6ddff7dda8a
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Fri Sep 23 16:55:15 2016 -0400

    Merge pull request #11431 from carbonin/remove_miq_widget_title_unique_validation

    Remove MiqWidget title unique validation
    (cherry picked from commit ba6fd6712d5679f41e1aecd1d1baf0d372b90d06)

@chessbyte
Copy link
Member

chessbyte pushed a commit that referenced this pull request Oct 7, 2016
…e_validation

Remove MiqWidget title unique validation
(cherry picked from commit ba6fd67)

https://bugzilla.redhat.com/show_bug.cgi?id=1379728
@chessbyte
Copy link
Member

Darga Backport details:

$ git log
commit 1e978fb02d3a92738a7cd436bc69df3f9d89214e
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Fri Sep 23 16:55:15 2016 -0400

    Merge pull request #11431 from carbonin/remove_miq_widget_title_unique_validation

    Remove MiqWidget title unique validation
    (cherry picked from commit ba6fd6712d5679f41e1aecd1d1baf0d372b90d06)

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

@carbonin carbonin deleted the remove_miq_widget_title_unique_validation branch November 2, 2016 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.

None yet

4 participants