Project

General

Profile

Actions

Bug #7450

closed

We mark not-required fields as required

Added by Marek Hulán over 10 years ago. Updated over 6 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Web Interface
Target version:
Difficulty:
Triaged:
Fixed in Releases:
Found in Releases:

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 3 (1 open2 closed)

Related to Foreman - Feature #6400: mark required fields in formClosedJoseph Magen06/26/2014Actions
Related to Foreman - Bug #7560: add :required => true on host fields that have conditional validation if host.managed?ClosedJoseph Magen09/21/2014Actions
Related to Foreman - Feature #21678: Mark required fields conditionally based on other fieldsNew11/15/2017Actions
Actions #1

Updated by Marek Hulán over 10 years ago

Actions #2

Updated by Dominic Cleal over 10 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 :)

Actions #3

Updated by Marek Hulán over 10 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

Actions #4

Updated by Dominic Cleal over 10 years ago

Indeed, that should use options[:required] unless it's nil, and only then fall back to is_required.

Actions #5

Updated by Marek Hulán over 10 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?

Actions #6

Updated by Dominic Cleal over 10 years ago

Sounds good!

Actions #7

Updated by The Foreman Bot over 10 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 ()
Actions #8

Updated by Marek Hulán over 10 years ago

  • Assignee set to Marek Hulán
Actions #9

Updated by Marek Hulán over 10 years ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100
Actions #10

Updated by Dominic Cleal over 10 years ago

  • Translation missing: en.field_release set to 21
Actions #11

Updated by Dominic Cleal over 10 years ago

  • Related to Bug #7560: add :required => true on host fields that have conditional validation if host.managed? added
Actions #12

Updated by Marek Hulán about 7 years ago

  • Related to Feature #21678: Mark required fields conditionally based on other fields added
Actions

Also available in: Atom PDF