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

Fields are sometimes marked as required when they are not #1799

Closed
pablobm opened this issue Oct 15, 2020 · 2 comments
Closed

Fields are sometimes marked as required when they are not #1799

pablobm opened this issue Oct 15, 2020 · 2 comments

Comments

@pablobm
Copy link
Collaborator

pablobm commented Oct 15, 2020

Merging #1521 gave Administrate the ability to detect "required" fields and mark them as such.

However, this detection is not always perfect. On occasion, there will be :if/:unless conditions attached to the validation rule. These are not correctly interepreted by Administrate. So for example:

  validates :amount, :currency, presence: true, if: ->{ stripe? }

Screenshot 2020-10-14 at 18 11 58

Originally posted by @sedubois in #1521 (comment)

jmeinerz pushed a commit that referenced this issue Oct 23, 2020
We were assuming that all fields were required when a presence
validation existed. While that makes sense, it's also possible for
validations to be conditional.

Take the following validation as an example:

        validates :phone_number, presence: true, if: :egyptian?

Before this commit, the UI would flag phone_number as required, even for
records who were not egyptian.

We now always flag these fields as optional. This is a bit misleading
too, but it's impossible to know these things when the page is rendered,
and marking them as optional makes for a slightly better user interface,
as the user will most likely be prompted with detailed validation errors
after trying to persist an invalid item, rather than be led to fill some
fields that are, in fact, not mandatory.
jmeinerz pushed a commit that referenced this issue Oct 28, 2020
We were assuming that all fields were required when a presence
validation existed. While that makes sense, it's also possible for
validations to be conditional.

Take the following validation as an example:

        validates :phone_number, presence: true, if: :egyptian?

Before this commit, the UI would flag phone_number as required, even for
records who were not egyptian.

We now always flag these fields as optional. This is a bit misleading
too, but it's impossible to know these things when the page is rendered,
and marking them as optional makes for a slightly better user interface,
as the user will most likely be prompted with detailed validation errors
after trying to persist an invalid item, rather than be led to fill some
fields that are, in fact, not mandatory.
jmeinerz pushed a commit that referenced this issue Oct 28, 2020
We were assuming that all fields were required when a presence
validation existed. While that makes sense, it's also possible for
validations to be conditional.

Take the following validation as an example:

        validates :phone_number, presence: true, if: :egyptian?

Before this commit, the UI would flag phone_number as required, even for
records who were not egyptian.

We now always flag these fields as optional. This is a bit misleading
too, but it's impossible to know these things when the page is rendered,
and marking them as optional makes for a slightly better user interface,
as the user will most likely be prompted with detailed validation errors
after trying to persist an invalid item, rather than be led to fill some
fields that are, in fact, not mandatory.
jmeinerz pushed a commit that referenced this issue Oct 28, 2020
We were assuming that all fields were required when a presence
validation existed. While that makes sense, it's also possible for
validations to be conditional.

Take the following validation as an example:

        validates :phone_number, presence: true, if: :egyptian?

Before this commit, the UI would flag phone_number as required, even for
records who were not egyptian.

We now always flag these fields as optional. This is a bit misleading
too, but it's impossible to know these things when the page is rendered,
and marking them as optional makes for a slightly better user interface,
as the user will most likely be prompted with detailed validation errors
after trying to persist an invalid item, rather than be led to fill some
fields that are, in fact, not mandatory.
@jmeinerz
Copy link
Contributor

@pablobm this should be solved by #1808 (which you could argue open a new issue...)

@pablobm
Copy link
Collaborator Author

pablobm commented Oct 29, 2020

Fixed by #1808

@pablobm pablobm closed this as completed Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants