We've had cases where user has been able to save patron without address. It turned out that if one collapses patron entry fields validating those fields no longer works even if they are marked as mandatory. To reproduce: 1. Make sure you have "Main address" fields set as mandatory with BorrowerMandatoryField syspref (address, city, zipcode) 2. Add new patron, fill in some info, but leave "Main address" fields empty. 3. Attempt to save. => "This field is mandatory" should be displayed next to the address fields. 4. Collapse "Main address" section. 5. Attempt to save again. => Patron is saved successfully without address information. I'm using address section as an example here, but this happens with every other fields as well. I was able to save patron without surname and date of birth this way.
Adding "ignore" rule as "" for entry forms validation (members.js line 242) does the trick. However "This field is required." errors aren't visible when fields are collapsed. Either there should be some error displayed next to field sets name or some sort of "There are some required fields missing. Please check form." text displayed. Or maybe collapsed fields should be opened if they contain required fields.
Created attachment 155643 [details] [review] Bug 34671: Show error message if fieldset has invalid input If fieldset is collapsed and it contains required fields patron can be saved without filling these fields. This patch adds rule ignore as "" in form validation and displayes error message in fieldsets containing missing required inputs. To test: 1. Make sure you have "Main address" fields set as mandatory with BorrowerMandatoryField syspref (address, city, zipcode) 2. Add new patron, fill in some info, but leave "Main address" fields empty. 3. Attempt to save. => "This field is mandatory" should be displayed next to the address fields. 4. Collapse "Main address" fieldset. 5. Attempt to save again. => Patron is saved successfully without address information. 6. Apply this patch. 7. Edit patron and attempt to save again with "Main address" fieldset collapsed. => Error message "There are some required fields missing from this field set. Please fill them." should be displayed and patron isn't saved. 8. Fill missing fields, attempt to save. => Saving should now be successfull. Sponsored-by: Koha-Suomi Oy
Currently this breaks child patron modification. Relationship input is always mandatory if borrowerRelationship syspref is set and it's also always hidden.
Created attachment 156145 [details] [review] Bug 34671: Ignore new_guarantor_relationship input if it's hidden Validating patron forms hidden fields causes problems when adding/modifying guarantee patrons since class new_guarantor_relationship is always hidden and mandatory if syspref "borrowerRelationship" is set. This patch adds class ignore_validation to relationship input if it's hidden to ignore validatation and removes it when input is displayed, so it is validated. To test: 1. Apply previous patch. 2. Add or modify guarantee patron. 3. Do not add or modify guarantor. 4. Attempt to save patron. => Pressing save button does nothing and no error is displayed. 5. Apply this patch. 6. Again add or modify guarantee patron and don't add or modify guarantor. 7. Attempt to save. => Saving should now be successfull. 8. Again, add or modify guarantee patron, but this time also add or modify guarantor. 9. Leave relationship input empty. 10. Attempt to save. => Saving should fail and error "There are some required fields missing..." should be displayed on top of "Patron guarantor" field set. 11. Add relationship for guarantor and save. => Saving should now be successfull. Sponsored-by: Koha-Suomi Oy
Upon testing this, I did see the error message under the collapsed address field, so that works successfully. However, I did not see the same error on the Guarantor Collapsed field. I can not save the form without the relationship of the guarantor, but that does not show if the field is collapsed.
Created attachment 156263 [details] [review] Bug 34671: Show error message if fieldset has invalid input If fieldset is collapsed and it contains required fields patron can be saved without filling these fields. This patch adds rule ignore as "" in form validation and displayes error message in fieldsets containing missing required inputs. To test: 1. Make sure you have "Main address" fields set as mandatory with BorrowerMandatoryField syspref (address, city, zipcode) 2. Add new patron, fill in some info, but leave "Main address" fields empty. 3. Attempt to save. => "This field is mandatory" should be displayed next to the address fields. 4. Collapse "Main address" fieldset. 5. Attempt to save again. => Patron is saved successfully without address information. 6. Apply this patch. 7. Edit patron and attempt to save again with "Main address" fieldset collapsed. => Error message "There are some required fields missing from this field set. Please fill them." should be displayed and patron isn't saved. 8. Fill missing fields, attempt to save. => Saving should now be successfull. Sponsored-by: Koha-Suomi Oy
Created attachment 156264 [details] [review] Bug 34671: Ignore new_guarantor_relationship input if it's hidden Validating patron forms hidden fields causes problems when adding/modifying guarantee patrons since class new_guarantor_relationship is always hidden and mandatory if syspref "borrowerRelationship" is set. This patch adds class ignore_validation to relationship input if it's hidden to ignore validatation and removes it when input is displayed, so it is validated. To test: 1. Apply previous patch. 2. Add or modify guarantee patron. 3. Do not add or modify guarantor. 4. Attempt to save patron. => Pressing save button does nothing and no error is displayed. 5. Apply this patch. 6. Again add or modify guarantee patron and don't add or modify guarantor. 7. Attempt to save. => Saving should now be successfull. 8. Again, add or modify guarantee patron, but this time also add or modify guarantor. 9. Leave relationship input empty. 10. Attempt to save. => Saving should fail and error "There are some required fields missing..." should be displayed on top of "Patron guarantor" field set. 11. Add relationship for guarantor and save. => Saving should now be successfull. Sponsored-by: Koha-Suomi Oy
Fixed issue Kelly pointed out. There was also problem when adding guarantee via "Add guarantee" option. Just a node, I made these two patches for mainly testing purposes. They can be squashed together if needed.
I would suggest this change to the text: "There are some required fields missing from this section. Please complete them." And I think it looks better when the new message appears after the <legend> not before it.
Created attachment 156453 [details] [review] Bug 34671: Show error message if fieldset has invalid input If fieldset is collapsed and it contains required fields patron can be saved without filling these fields. This patch adds rule ignore as "" in form validation and displayes error message in fieldsets containing missing required inputs. To test: 1. Make sure you have "Main address" fields set as mandatory with BorrowerMandatoryField syspref (address, city, zipcode) 2. Add new patron, fill in some info, but leave "Main address" fields empty. 3. Attempt to save. => "This field is mandatory" should be displayed next to the address fields. 4. Collapse "Main address" fieldset. 5. Attempt to save again. => Patron is saved successfully without address information. 6. Apply this patch. 7. Edit patron and attempt to save again with "Main address" fieldset collapsed. => Error message "There are some required fields missing from this field set. Please fill them." should be displayed and patron isn't saved. 8. Fill missing fields, attempt to save. => Saving should now be successfull. Sponsored-by: Koha-Suomi Oy
Created attachment 156454 [details] [review] Bug 34671: Ignore new_guarantor_relationship input if it's hidden adding/modifying guarantee patrons since class new_guarantor_relationship is always hidden and mandatory if syspref "borrowerRelationship" is set. This patch adds class ignore_validation to relationship input if it's hidden to ignore validatation and removes it when input is displayed, so it is validated. To test: 1. Apply previous patch. 2. Add or modify guarantee patron. 3. Do not add or modify guarantor. 4. Attempt to save patron. => Pressing save button does nothing and no error is displayed. 5. Apply this patch. 6. Again add or modify guarantee patron and don't add or modify guarantor. 7. Attempt to save. => Saving should now be successfull. 8. Again, add or modify guarantee patron, but this time also add or modify guarantor. 9. Leave relationship input empty. 10. Attempt to save. => Saving should fail and error "There are some required fields missing..." should be displayed on top of "Patron guarantor" field set. 11. Add relationship for guarantor and save. => Saving should now be successfull. Sponsored-by: Koha-Suomi Oy
(In reply to Owen Leonard from comment #9) > I would suggest this change to the text: > > "There are some required fields missing from this section. Please complete > them." > > And I think it looks better when the new message appears after the <legend> > not before it. Adjusted as suggested. And it does look better now :D
There's still a problem with how this behaves with patron attribute types set as required. Even if they aren't needed for certain type of patron and are hidden from form, they're still required and prevent save.
Created attachment 158168 [details] [review] Bug 34671: Ignore validation if patron attribute field is hidden Patron attributes not used by patron category are still present in form as hidden and marked as required if they are set as mandatory. This prevents saving patron when validation is required also for hidden fields. This patch adds and removes class "ignore_validation" for these fields based on their visibility. To test: 1. Add patron attribute for patron category A and set it as mandatory. 2. Add patron with category B and fill in required fields. 3. Attempt to save. => Error is displayed above "Additional attributes and identifiers" field set. 4. Apply this patch. 5. Repeat steps 2 and 3. => Save successfully. 6. Add new patron with category B, but this time change patrons category as A from patron form. => Confirm that madatory patron attribute is displayed correctly and without it patron cannot be saved. Sponsored-by: Koha-Suomi Oy
This still needs some work.
Created attachment 158299 [details] [review] Bug 34671: Ignore validation if patron attribute field is hidden Patron attributes not used by patron category are still present in form as hidden and marked as required if they are set as mandatory. This prevents saving patron when validation is required also for hidden fields. This patch adds and removes class "ignore_validation" for these fields based on their visibility. To test: 1. Add patron attribute for patron category A and set it as mandatory. 2. Add patron with category B and fill in required fields. 3. Attempt to save. => Error is displayed above "Additional attributes and identifiers" field set. 4. Apply this patch. 5. Repeat steps 2 and 3. => Save successfully. 6. Add new patron with category B, but this time change patrons category as A from patron form. => Confirm that madatory patron attribute is displayed correctly and without it patron cannot be saved. Sponsored-by: Koha-Suomi Oy
Created attachment 158524 [details] [review] Bug 34671: Show error message if fieldset has invalid input If fieldset is collapsed and it contains required fields patron can be saved without filling these fields. This patch adds rule ignore as "" in form validation and displayes error message in fieldsets containing missing required inputs. To test: 1. Make sure you have "Main address" fields set as mandatory with BorrowerMandatoryField syspref (address, city, zipcode) 2. Add new patron, fill in some info, but leave "Main address" fields empty. 3. Attempt to save. => "This field is mandatory" should be displayed next to the address fields. 4. Collapse "Main address" fieldset. 5. Attempt to save again. => Patron is saved successfully without address information. 6. Apply this patch. 7. Edit patron and attempt to save again with "Main address" fieldset collapsed. => Error message "There are some required fields missing from this field set. Please fill them." should be displayed and patron isn't saved. 8. Fill missing fields, attempt to save. => Saving should now be successfull. Sponsored-by: Koha-Suomi Oy Signed-off-by: Kelly <kelly@bywatersolutions.com>
Created attachment 158525 [details] [review] Bug 34671: Ignore new_guarantor_relationship input if it's hidden Validating patron forms hidden fields causes problems when adding/modifying guarantee patrons since class new_guarantor_relationship is always hidden and mandatory if syspref "borrowerRelationship" is set. This patch adds class ignore_validation to relationship input if it's hidden to ignore validatation and removes it when input is displayed, so it is validated. To test: 1. Apply previous patch. 2. Add or modify guarantee patron. 3. Do not add or modify guarantor. 4. Attempt to save patron. => Pressing save button does nothing and no error is displayed. 5. Apply this patch. 6. Again add or modify guarantee patron and don't add or modify guarantor. 7. Attempt to save. => Saving should now be successfull. 8. Again, add or modify guarantee patron, but this time also add or modify guarantor. 9. Leave relationship input empty. 10. Attempt to save. => Saving should fail and error "There are some required fields missing..." should be displayed on top of "Patron guarantor" field set. 11. Add relationship for guarantor and save. => Saving should now be successfull. Sponsored-by: Koha-Suomi Oy Signed-off-by: Kelly <kelly@bywatersolutions.com>
Created attachment 158526 [details] [review] Bug 34671: Ignore validation if patron attribute field is hidden Patron attributes not used by patron category are still present in form as hidden and marked as required if they are set as mandatory. This prevents saving patron when validation is required also for hidden fields. This patch adds and removes class "ignore_validation" for these fields based on their visibility. To test: 1. Add patron attribute for patron category A and set it as mandatory. 2. Add patron with category B and fill in required fields. 3. Attempt to save. => Error is displayed above "Additional attributes and identifiers" field set. 4. Apply this patch. 5. Repeat steps 2 and 3. => Save successfully. 6. Add new patron with category B, but this time change patrons category as A from patron form. => Confirm that madatory patron attribute is displayed correctly and without it patron cannot be saved. Sponsored-by: Koha-Suomi Oy Signed-off-by: Kelly <kelly@bywatersolutions.com>
+ ignore: "", Default should be :hidden. Does this make a difference? Comparing with + if($(".new_guarantor_relationship").is(":hidden")){ + $(".new_guarantor_relationship").addClass("ignore_validation"); + } - ignore: "", + ignore: ".ignore_validation", Also third patch. My first thought here is that we are doing things too complicated?
Failing QA for feedback on comment#20.
(In reply to Marcel de Rooy from comment #20) > + ignore: "", > Default should be :hidden. Does this make a difference? Hmm, if ignore setting isn't set then yes, default is ":hidden". That's why it has to be set as ignore: "" so that all fields are validated no matter if they're hidden or not. If we add class to this setting, only those fields are ignored. > Comparing with > + if($(".new_guarantor_relationship").is(":hidden")){ > + $(".new_guarantor_relationship").addClass("ignore_validation"); > + } > > - ignore: "", > + ignore: ".ignore_validation", > > Also third patch. > My first thought here is that we are doing things too complicated? How form validation works does make things complicated because the way it works (as stated above). I did at one point test if we could leave class "required" out if field is hidden but I can't remember if it's even possible or less complicated to achieve than new class.