Bug #7450
We mark not-required fields as required
Description
If the validator has some condition (:if => $smt, :unless => $smt) we shouldn't mark the field as required. In this case we simply can't tell without AJAX call because validity may depend on another field of the form. Btw there was a project translating AR validations to javascript (client side validations) but IIRC it's discontinued. Also it was quite big thing.
Related issues
Associated revisions
History
#1
Updated by Marek Hulán over 8 years ago
- Related to Feature #6400: mark required fields in form added
#2
Updated by Dominic Cleal over 8 years ago
Do you have an example field where this is wrong? There's an override option which can be passed into the form helper to disable it, we just need to know which field(s) require it :)
#3
Updated by Marek Hulán over 8 years ago
I'm afraid we can just enforce required to be true. Looking to the code I see
required_mark = ' *' if is_required?(f, attr) || options[:required]
see https://github.com/theforeman/foreman/blob/develop/app/helpers/layout_helper.rb#L153 and https://github.com/theforeman/foreman/blob/develop/app/helpers/layout_helper.rb#L167 for more details.
I have no existing example since it's in form I'm currently working on for bonding.
something like
<%= text_f f, :attached_to, :help_inline => _("Identifier of the interface to which this interface belongs, e.g. eth1") %>
adding :required => false does not help obviously since is_required?(f, attr) is always true
#4
Updated by Dominic Cleal over 8 years ago
Indeed, that should use options[:required] unless it's nil, and only then fall back to is_required.
#5
Updated by Marek Hulán over 8 years ago
Yup, will send a PR shortly. I thought it would be also useful to make is_required?
aware of conditions and if some is present, return false (probably makes sense in most cases?). Thoughts?
#6
Updated by Dominic Cleal over 8 years ago
Sounds good!
#7
Updated by The Foreman Bot over 8 years ago
- Status changed from New to Ready For Testing
- Target version set to 1.7.3
- Pull request https://github.com/theforeman/foreman/pull/1766 added
- Pull request deleted (
)
#8
Updated by Marek Hulán over 8 years ago
- Assignee set to Marek Hulán
#9
Updated by Marek Hulán over 8 years ago
- Status changed from Ready For Testing to Closed
- % Done changed from 0 to 100
Applied in changeset 3b87444aac04cf78e073bcb6e7f7778ef72f9c8e.
#10
Updated by Dominic Cleal over 8 years ago
- Legacy Backlogs Release (now unused) set to 21
#11
Updated by Dominic Cleal over 8 years ago
- Related to Bug #7560: add :required => true on host fields that have conditional validation if host.managed? added
#12
Updated by Marek Hulán over 5 years ago
- Related to Feature #21678: Mark required fields conditionally based on other fields added
Fixes #7450 - do not mark non-required fields
If validation is conditional we don't mark fields as required by
default. Also fixes disabling by override using :required option.