Bug 30159

Summary: Fix display of validation of important fields when biblio cataloguing
Product: Koha Reporter: Aleisha Amohia <aleisha>
Component: CatalogingAssignee: Adam Styles <adam.styles>
Status: RESOLVED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: andrewfh, kyle, m.de.rooy
Version: master   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This patch adds a check for both mandatory and important fields when validating bibliographic records during cataloguing.
Version(s) released in:
22.05.00,21.11.05
Bug Depends on: 28694    
Bug Blocks: 30482    
Attachments: Popup box showing HTML dump of validation for important fields
Bug 30159: Fix Check() to validate both mandatory and important fields
Bug 30159: Fix Check() to validate both mandatory and important fields
Bug 30159: (follow-up) Make warning messages consistent
Bug 30159: Fix Check() to validate both mandatory and important fields
Bug 30159: (follow-up) Make warning messages consistent
Bug 30159: (QA follow-up) Fix translatability of alert message

Description Aleisha Amohia 2022-02-22 22:32:45 UTC
Created attachment 131029 [details]
Popup box showing HTML dump of validation for important fields

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.
Comment 1 Aleisha Amohia 2022-02-22 22:41:55 UTC
Created attachment 131030 [details] [review]
Bug 30159: Fix Check() to validate both mandatory and important fields

This patch adds a check for both mandatory and important fields when
validating bibliographic records during cataloguing.

To test:

1. Go to Admin -> Biblio frameworks. View the MARC structure of your
default framework.
2. Search for tag 082. Edit the subfields for this tag.
3. Check the 'important' checkbox for subfield a and Save.
4. Go to add or edit a new biblio record under Cataloguing.
5. Ensure the 082$a field is empty and hit Save.
6. Notice the confirmation pop-up is an unreadable display of HTML.
7. Click Cancel on the pop-up box.
8. Apply the patch and refresh the editor page.
9. Ensure the 082$a field is empty and hit Save.
10. Notice the confirmation pop-up is more helpful this time. Click
Cancel on the pop-up box.
11. Notice the 082$a field is displayed at the top of the page as an
important field that is missing information. Use the 'go to field' link
and confirm this works as expected.
12. Leave 082$a empty and hit Save.
13. This time click OK on the confirmation pop-up. Confirm you are able
to save your record as normal.

Sponsored-by: Education Services Australia SCIS
Comment 2 Aleisha Amohia 2022-02-22 22:43:24 UTC
I've attached this patch on behalf of Adam Styles, who is the assignee. The author line on the patch is correct.
Comment 3 Owen Leonard 2022-02-23 11:54:04 UTC
Created attachment 131034 [details] [review]
Bug 30159: Fix Check() to validate both mandatory and important fields

This patch adds a check for both mandatory and important fields when
validating bibliographic records during cataloguing.

To test:

1. Go to Admin -> Biblio frameworks. View the MARC structure of your
   default framework.
2. Search for tag 082. Edit the subfields for this tag.
3. Check the 'important' checkbox for subfield a and Save.
4. Go to add or edit a new biblio record under Cataloguing.
5. Ensure the 082$a field is empty and hit Save.
6. Notice the confirmation pop-up is an unreadable display of HTML.
7. Click Cancel on the pop-up box.
8. Apply the patch and refresh the editor page.
9. Ensure the 082$a field is empty and hit Save.
10. Notice the confirmation pop-up is more helpful this time. Click
    Cancel on the pop-up box.
11. Notice the 082$a field is displayed at the top of the page as an
    important field that is missing information. Use the 'go to field'
    link and confirm this works as expected.
12. Leave 082$a empty and hit Save.
13. This time click OK on the confirmation pop-up. Confirm you are able
    to save your record as normal.

Sponsored-by: Education Services Australia SCIS

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 4 Aleisha Amohia 2022-03-03 21:43:34 UTC
Created attachment 131365 [details] [review]
Bug 30159: (follow-up) Make warning messages consistent
Comment 5 Katrin Fischer 2022-03-06 13:44:18 UTC
As the follow-up is only a text change, I am resetting to signed off.
Comment 6 Katrin Fischer 2022-03-06 13:58:28 UTC
Created attachment 131411 [details] [review]
Bug 30159: Fix Check() to validate both mandatory and important fields

This patch adds a check for both mandatory and important fields when
validating bibliographic records during cataloguing.

To test:

1. Go to Admin -> Biblio frameworks. View the MARC structure of your
   default framework.
2. Search for tag 082. Edit the subfields for this tag.
3. Check the 'important' checkbox for subfield a and Save.
4. Go to add or edit a new biblio record under Cataloguing.
5. Ensure the 082$a field is empty and hit Save.
6. Notice the confirmation pop-up is an unreadable display of HTML.
7. Click Cancel on the pop-up box.
8. Apply the patch and refresh the editor page.
9. Ensure the 082$a field is empty and hit Save.
10. Notice the confirmation pop-up is more helpful this time. Click
    Cancel on the pop-up box.
11. Notice the 082$a field is displayed at the top of the page as an
    important field that is missing information. Use the 'go to field'
    link and confirm this works as expected.
12. Leave 082$a empty and hit Save.
13. This time click OK on the confirmation pop-up. Confirm you are able
    to save your record as normal.

Sponsored-by: Education Services Australia SCIS

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Katrin Fischer 2022-03-06 13:58:32 UTC
Created attachment 131412 [details] [review]
Bug 30159: (follow-up) Make warning messages consistent

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Katrin Fischer 2022-03-06 13:58:36 UTC
Created attachment 131413 [details] [review]
Bug 30159: (QA follow-up) Fix translatability of alert message

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Katrin Fischer 2022-03-06 13:58:44 UTC
Hi Adam, 

thx for your patch!

Be careful when working with the JavaScript message. We do need the _("") or __("") in JS files to make strings translatable.

I fixed this line in my follow-up:
+                var r=confirm("Important fields(s) are not filled. Are you sure you want to save?");
Comment 10 Fridolin Somers 2022-03-16 08:31:52 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 11 Aleisha Amohia 2022-03-16 20:19:41 UTC
Woohoo! Congrats Adam on your first patch!
Comment 12 Kyle M Hall 2022-03-25 12:16:35 UTC
Pushed to 21.11.x for 21.11.05
Comment 13 Andrew Fuerste-Henry 2022-04-25 20:02:32 UTC
Missing dependency, not backported to 21.05