+++ This bug was initially created as a clone of Bug #30159 +++ Fields that are marked as important in a biblio framework will trigger a confirmation box to the cataloguer if not filled, asking if they would still like to save. Since Bug 28694, the validation for mandatory fields is nice and pretty, but it appears the validation for important fields was not tested. The confirmation pop-up now spits out the HTML markup and is unreadable. This should be fixed.
We run AreFieldsNotOk() twice and concatenate the result when showing errors on the screen. If the second run of AreFieldsNotOk() returns false, the string concatenation adds "0"
Created attachment 133061 [details] Screenshot of an extra "0" output with a validation error
Created attachment 133062 [details] [review] Bug 30482: Potential for bad string concatenation in cataloging validation error message In validating the basic MARC editor form before submission, we run AreFieldsNotOk() twice (once with the parameter "false") and concatenate the result for output. This creates the potential for the error string to be appended with "0" if AreFieldsNotOk() returns false. This patch improves the logic around building the error string. To test, apply the patch and make sure one of your MARC frameworks contains at least one mandatory field and at least one important field. - Edit or create a MARC record in the basic MARC editor. - Submit the form in various states of completion: - If a mandatory and an important field are empty, you should see two error messages at the top. "The following mandatory subfields aren't filled" and "The following important subfields aren’t filled." - If a mandatory OR an important field is empty, you should see a single message. - If it's the important field which is empty, a confirmation will ask if you want to save the record anyway. Test that both answers to this confirmation work correctly. - If no mandatory or important fields are empty the form should submit.
Created attachment 133506 [details] [review] Bug 30482: Potential for bad string concatenation in cataloging validation error message In validating the basic MARC editor form before submission, we run AreFieldsNotOk() twice (once with the parameter "false") and concatenate the result for output. This creates the potential for the error string to be appended with "0" if AreFieldsNotOk() returns false. This patch improves the logic around building the error string. To test, apply the patch and make sure one of your MARC frameworks contains at least one mandatory field and at least one important field. - Edit or create a MARC record in the basic MARC editor. - Submit the form in various states of completion: - If a mandatory and an important field are empty, you should see two error messages at the top. "The following mandatory subfields aren't filled" and "The following important subfields aren’t filled." - If a mandatory OR an important field is empty, you should see a single message. - If it's the important field which is empty, a confirmation will ask if you want to save the record anyway. Test that both answers to this confirmation work correctly. - If no mandatory or important fields are empty the form should submit. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 133675 [details] [review] Bug 30482: Potential for bad string concatenation in cataloging validation error message In validating the basic MARC editor form before submission, we run AreFieldsNotOk() twice (once with the parameter "false") and concatenate the result for output. This creates the potential for the error string to be appended with "0" if AreFieldsNotOk() returns false. This patch improves the logic around building the error string. To test, apply the patch and make sure one of your MARC frameworks contains at least one mandatory field and at least one important field. - Edit or create a MARC record in the basic MARC editor. - Submit the form in various states of completion: - If a mandatory and an important field are empty, you should see two error messages at the top. "The following mandatory subfields aren't filled" and "The following important subfields aren’t filled." - If a mandatory OR an important field is empty, you should see a single message. - If it's the important field which is empty, a confirmation will ask if you want to save the record anyway. Test that both answers to this confirmation work correctly. - If no mandatory or important fields are empty the form should submit. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Pushed to master for 22.05, thanks to everybody involved [U+1F984]
Pushed to 21.11.x for 21.11.06
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.