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

Validate bools with inclusion #18914

Merged

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Jun 25, 2019

... since the presence fails because false.blank? => true

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

@d-m-u
Copy link
Contributor Author

d-m-u commented Jun 25, 2019

@miq-bot add_label bug
@miq-bot add_reviewer @eclarizio
@miq-bot assign @tinaafitz

@d-m-u d-m-u force-pushed the fixing_dialog_field_visible_validation branch from 947ceb9 to 76aed49 Compare June 26, 2019 10:50
@d-m-u
Copy link
Contributor Author

d-m-u commented Jun 26, 2019

@bdunne please review
@tinaafitz please review

@d-m-u d-m-u force-pushed the fixing_dialog_field_visible_validation branch from 76aed49 to 65e367a Compare June 26, 2019 14:07
@d-m-u
Copy link
Contributor Author

d-m-u commented Jun 27, 2019

@jrafanie any chance you want to review please?

@d-m-u
Copy link
Contributor Author

d-m-u commented Jun 27, 2019

@bdunne sorry but could i get you to re-re-re-review please?

@bdunne bdunne merged commit 36676ab into ManageIQ:master Jun 27, 2019
@bdunne bdunne assigned bdunne and unassigned tinaafitz Jun 27, 2019
@bdunne bdunne added this to the Sprint 115 Ending Jul 8, 2019 milestone Jun 27, 2019
@@ -30,7 +30,7 @@ class DialogField < ApplicationRecord

default_value_for :required, false
default_value_for(:visible) { true }
validates :visible, :presence => true
validates :visible, inclusion: { in: [ true, false ] }
Copy link
Member

Choose a reason for hiding this comment

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

👍

@d-m-u
Copy link
Contributor Author

d-m-u commented Jun 28, 2019

@miq-bot add_label hammer/yes

@simaishi
Copy link
Contributor

simaishi commented Jul 1, 2019

@d-m-u #18778 isn't in hammer branch and backporting this PR conflicts.

diff --cc app/models/dialog_field.rb
index e78ac49364,8b87d46f0c..0000000000
--- a/app/models/dialog_field.rb
+++ b/app/models/dialog_field.rb
@@@ -29,7 -29,9 +29,13 @@@ class DialogField < ApplicationRecor
                                    :message => "Field Name %{value} is reserved."}
  
    default_value_for :required, false
++<<<<<<< HEAD
 +  default_value_for(:visible, :allows_nil => false) { true }
++=======
+   default_value_for(:visible) { true }
+   validates :visible, inclusion: { in: [ true, false ] }
+   default_value_for :load_values_on_init, true
++>>>>>>> 36676ab875... Merge pull request #18914 from d-m-u/fixing_dialog_field_visible_validation

@d-m-u
Copy link
Contributor Author

d-m-u commented Jul 1, 2019

Yeah, sorry @simaishi
@miq-bot add_label hammer/no
doesn't need to be backported cause #18778 wasn't

@d-m-u d-m-u deleted the fixing_dialog_field_visible_validation branch September 26, 2019 10:37
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.

7 participants