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

WIP for validation of complex properties #170

Draft
wants to merge 841 commits into
base: develop
Choose a base branch
from
Draft

Conversation

ghost
Copy link

@ghost ghost commented Jan 3, 2020

(144)

This PR does the following:

  1. Abstracts rules around requiredness/validation into a YAML file, loaded by the model so they can be accessed outside of ComplexValidation
  2. Adds custom classes on HTML form elements built by the nested attribute input code.
  3. Adds custom javascript to add the 'required' flag once a condition is met, and thus toggle the 'green tick' to a cross in the requirements panel on the right.

The code meets the 'required if any' case, and is configured to support the following cases:

  1. complex_person ... if anything other than name is populated, make name required
  2. complex_organization ... if anything other than organization is populated, make organization required
  3. complex_identifier ... if anything other than identifier is populated, make identifier required
  4. complex_date ... if anything other than date is populated, make date required
  5. complex_rights ... if anything other than rights is populated, make rights required

Where the role is 'operator' the condition is skipped as this is pre-populated in the instrument tab of dataset and we do no want this field to be required by default.

The code meets the 'required if conditional' , and is configured to support the following case:

  1. complex_person ... if name is populated, make role required

CAVEAT: I would caution against the over-use of conditional requirements, it makes the form very busy - the javascript has to loop through the form multiple times and I am concerned about performance if it is doing this for mulitiple different conditions. I have added role as an example.

I have observed 'unresponsive' errors in the browser. The needs testing in a more production-like environment where the JS is precompiled.

TODO:

  • add tests

martyn-w and others added 30 commits September 23, 2019 17:55
skip parsing date if its format is invalid (usually it is year)
refactor of complex_person spec
add remember_token to users table
disable citation template temporarily
remove pry from complex_fields_behaviour.rb
nabeta and others added 23 commits January 27, 2020 22:43
add Handle identifier to the authority file
make some methods supporting multi-value
…nd-specimens

Expanded temporary validation on complex instrument and specimen to stop form…
Use default thumbnail for .txt, .csv and .tsv
Fix issue with "search for works" user profile
add .mvn directory to store local jvm.config
@ghost ghost force-pushed the i144-form-control branch 2 times, most recently from 2cf4932 to 5192989 Compare March 4, 2020 12:48
roll out the required-if-any code to all inputs; add condition to skip operator (as thats prefilled)

add conditional; extended required fields
@ghost ghost force-pushed the i144-form-control branch from 9107c04 to b4afa55 Compare April 15, 2020 12:45
@@ -22,5 +22,5 @@
// Required by Blacklight
//= require blacklight/blacklight

//= require_tree .
Copy link
Collaborator

Choose a reason for hiding this comment

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

can I ask why this change + the require almond below?

Copy link
Author

Choose a reason for hiding this comment

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

I seem to recall needing it, but I'll test without.

@nabeta nabeta force-pushed the i144-form-control branch from e7ddd13 to 3296db7 Compare October 10, 2024 07:13
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

Successfully merging this pull request may close these issues.

5 participants