Bug 32914 - Use template wrapper for batch record deletion and modification templates
Summary: Use template wrapper for batch record deletion and modification templates
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Owen Leonard
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on: 32571
Blocks: 33624
  Show dependency treegraph
 
Reported: 2023-02-08 17:44 UTC by Owen Leonard
Modified: 2023-12-28 20:47 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.07


Attachments
Bug 32914: Use template wrapper for batch record deletion and modification templates (12.75 KB, patch)
2023-02-08 18:58 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 32914: Use template wrapper for batch record deletion and modification templates (12.83 KB, patch)
2023-02-09 09:38 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 32914: Use template wrapper for batch record deletion and modification templates (12.75 KB, patch)
2023-03-07 12:19 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 32914: (follow-up) Add CSS to improve tab layout (5.66 KB, patch)
2023-03-07 12:19 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 32914: Use template wrapper for batch record deletion and modification templates (12.75 KB, patch)
2023-03-28 12:01 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 32914: (follow-up) Add CSS to improve tab layout (5.56 KB, patch)
2023-03-28 12:01 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 32914: Use template wrapper for batch record deletion and modification templates (15.36 KB, patch)
2023-04-14 11:34 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 32914: (follow-up) Improve handling of list tab visibility (4.34 KB, patch)
2023-04-25 15:34 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 32914: Use template wrapper for batch record deletion and modification templates (15.42 KB, patch)
2023-04-27 07:13 UTC, Philip Orr
Details | Diff | Splinter Review
Bug 32914: (follow-up) Improve handling of list tab visibility (4.39 KB, patch)
2023-04-27 07:13 UTC, Philip Orr
Details | Diff | Splinter Review
k (4.45 KB, patch)
2023-05-09 11:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32914: Use template wrapper for batch record deletion and modification templates (15.47 KB, patch)
2023-05-09 11:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32914: (follow-up) Improve handling of list tab visibility (4.45 KB, patch)
2023-05-09 11:55 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2023-02-08 17:44:33 UTC
Continuing the work of Bug 32571, the batch record deletion and modification templates should be updated to use the new WRAPPER for tabs.
Comment 1 Owen Leonard 2023-02-08 18:58:49 UTC Comment hidden (obsolete)
Comment 2 Matt Blenkinsop 2023-02-09 09:38:15 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2023-02-24 21:43:47 UTC
There is a small display issue on 
http://localhost:8081/cgi-bin/koha/tools/batch_record_modification.pl
with the tabs being further left than the corresponding content block below.
Comment 4 Owen Leonard 2023-03-07 12:19:07 UTC Comment hidden (obsolete)
Comment 5 Owen Leonard 2023-03-07 12:19:09 UTC Comment hidden (obsolete)
Comment 6 Philip Orr 2023-03-28 07:45:33 UTC
Hi Owen,
I wanted to test this bug but after creating a list and a MARC modification template, when I clicked on "Batch record modification", I got an error: template process failed.

"Template process failed: file error - parse error - /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_record_modification.tt line 158: unexpected token (ELSIF)
  [% ELSIF view == 'list' %] at /kohadevbox/koha/C4/Templates.pm line 127"
Comment 7 Owen Leonard 2023-03-28 10:58:54 UTC
Thanks Philip, please don't hesitate to mark something FAILED QA if you run into a problem like that when testing
Comment 8 Philip Orr 2023-03-28 10:59:58 UTC
OK, will do! :)
Comment 9 Owen Leonard 2023-03-28 12:01:43 UTC Comment hidden (obsolete)
Comment 10 Owen Leonard 2023-03-28 12:01:45 UTC Comment hidden (obsolete)
Comment 11 Philip Orr 2023-03-28 14:33:46 UTC
The display issue is now fixed!

All three tabs display correctly.
However when submitting a batch for modification only the tab "Enter a list of record numbers" actually leads to a batch record modification; both other tabs "Select a list of records" and "Upload a file" lead to the message "No records were modified".
Comment 12 Owen Leonard 2023-04-14 11:34:38 UTC
Created attachment 149670 [details] [review]
Bug 32914: Use template wrapper for batch record deletion and modification templates

This patch updates the batch record modification and batch record
deletion templates to replace tab markup with the use of WRAPPERs.

This patch adds a "clearfix" snippet of CSS to _mixins.scss copied from
Bootstrap. Applied to the active tab pane, this helps in situations
where Bootstrap tabs follow a floated element.

The patch also wraps tab label strings in <span> to ensure they can be
translated.

Unrelated: The patch removes an extra </option> from the batch record
modification template.

To test you should have at least one list and at least one MARC
modification template defined.

- Apply the patch and go to Cataloging -> Batch record modification
- You should see three tabs, "Upload a file," "Select a list of
  records," and "Enter a list of record numbers."
- The tabs should look correct and work correctly.
- Checking the "Authorities" radio button should hide the lists tab.
- Test that submissions from each form work correctly.

- Go to Cataloging -> Batch record deletion and perform the same tests.
Comment 13 Philip Orr 2023-04-14 12:44:24 UTC
in Batch Record Modification, all forms now look and perform correctly for Record type: Biblios. The tab "Select a list of records" also disappears when you select Record type: Authorities. 
However, if Record Type: Biblios is still selected and you first select your list from the drop down under "Select a list of records" and afterwards you then switch from Record type: Biblios to Record type: Authorities, your list actually stays selected and is still shown (and will indeed be modified if you proceed).
Comment 14 Owen Leonard 2023-04-25 14:33:33 UTC
(In reply to Philip Orr from comment #13)
> then switch from Record type: Biblios to Record type: Authorities, your list
> actually stays selected and is still shown (and will indeed be modified if
> you proceed).

In my testing this is a bug which exists in master and previous versions. I'm not sure if the bug should be resolved as part of this patch or not
Comment 15 Owen Leonard 2023-04-25 15:34:00 UTC
Created attachment 150214 [details] [review]
Bug 32914: (follow-up) Improve handling of list tab visibility

This patch makes minor changes to the way this page handles switching
between authority and bibliographic record batch operation in order to
ensure that the list tab is correctly shown or hidden and that the list
tab selection isn't transmitted when an authority batch is submitted.

This patch also changes the labels on the "record type" fields to fix
the incorrect use of the abbreviated "biblios" in favor of
"Bibliographic."

To test, apply the patch and go to Cataloging -> Batch record
modification

- With the "Bibliographic records" selected, click the "Select a list
  of records" tab."
- Select the "Authority records" radio button.
- The "Select a list of records" tab should disappear, and the "Upload a
  file" tab should now be selected."
- Select the "Bibliographic records radio button.
- The "Select a list of records" tab should reappear, and the "Upload a
  file" tab should be selected.
- Test all combinations of form submissions to confirm that each works
  correctly:
  - Bibliographic records by upload, list, and biblionumber entry
  - Authory records by upload and authority record number entry
Comment 16 Owen Leonard 2023-04-25 15:35:00 UTC
> I'm not sure if the bug should be resolved as part of this patch or not

I decided that since the patch includes changes affecting how tabs are shown and hidden it makes sense to roll this fix in at the same time. Hopefully my follow-up takes care of it.
Comment 17 Philip Orr 2023-04-26 08:23:45 UTC
Thanks Owen for going above and beyond on this one :)
Everything now works as expected and displays correctly. Although...
I hope I'm not being too much of a nuisance, but I did find another bug.

If you enter batch record modification, select Authority records, select a template and then hit continue without actually uploading a file or entering record numbers, the error message "No records were modified." will be displayed. All correct so far. 
If you then don't click on "New batch record modification" and instead use the back button in Firefox to go back one page, you will land in batch record modification with Authority records selected but with the tab "Select a list of records" now again displayed and usable.
Comment 18 Owen Leonard 2023-04-26 15:19:30 UTC
(In reply to Philip Orr from comment #17)
> If you then don't click on "New batch record modification" and instead use
> the back button in Firefox to go back one page, you will land in batch
> record modification with Authority records selected but with the tab "Select
> a list of records" now again displayed and usable.

This is an existing bug not caused by my patch. I would ask that this one be covered in another bug report.
Comment 19 Philip Orr 2023-04-27 07:08:27 UTC
will do! :)
Comment 20 Philip Orr 2023-04-27 07:13:43 UTC
Created attachment 150285 [details] [review]
Bug 32914: Use template wrapper for batch record deletion and modification templates

This patch updates the batch record modification and batch record
deletion templates to replace tab markup with the use of WRAPPERs.

This patch adds a "clearfix" snippet of CSS to _mixins.scss copied from
Bootstrap. Applied to the active tab pane, this helps in situations
where Bootstrap tabs follow a floated element.

The patch also wraps tab label strings in <span> to ensure they can be
translated.

Unrelated: The patch removes an extra </option> from the batch record
modification template.

To test you should have at least one list and at least one MARC
modification template defined.

- Apply the patch and go to Cataloging -> Batch record modification
- You should see three tabs, "Upload a file," "Select a list of
  records," and "Enter a list of record numbers."
- The tabs should look correct and work correctly.
- Checking the "Authorities" radio button should hide the lists tab.
- Test that submissions from each form work correctly.

- Go to Cataloging -> Batch record deletion and perform the same tests.

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 21 Philip Orr 2023-04-27 07:13:46 UTC
Created attachment 150286 [details] [review]
Bug 32914: (follow-up) Improve handling of list tab visibility

This patch makes minor changes to the way this page handles switching
between authority and bibliographic record batch operation in order to
ensure that the list tab is correctly shown or hidden and that the list
tab selection isn't transmitted when an authority batch is submitted.

This patch also changes the labels on the "record type" fields to fix
the incorrect use of the abbreviated "biblios" in favor of
"Bibliographic."

To test, apply the patch and go to Cataloging -> Batch record
modification

- With the "Bibliographic records" selected, click the "Select a list
  of records" tab."
- Select the "Authority records" radio button.
- The "Select a list of records" tab should disappear, and the "Upload a
  file" tab should now be selected."
- Select the "Bibliographic records radio button.
- The "Select a list of records" tab should reappear, and the "Upload a
  file" tab should be selected.
- Test all combinations of form submissions to confirm that each works
  correctly:
  - Bibliographic records by upload, list, and biblionumber entry
  - Authory records by upload and authority record number entry

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Comment 22 Katrin Fischer 2023-05-09 11:55:26 UTC
Created attachment 150856 [details] [review]
k
Comment 23 Katrin Fischer 2023-05-09 11:55:57 UTC
Created attachment 150857 [details] [review]
Bug 32914: Use template wrapper for batch record deletion and modification templates

This patch updates the batch record modification and batch record
deletion templates to replace tab markup with the use of WRAPPERs.

This patch adds a "clearfix" snippet of CSS to _mixins.scss copied from
Bootstrap. Applied to the active tab pane, this helps in situations
where Bootstrap tabs follow a floated element.

The patch also wraps tab label strings in <span> to ensure they can be
translated.

Unrelated: The patch removes an extra </option> from the batch record
modification template.

To test you should have at least one list and at least one MARC
modification template defined.

- Apply the patch and go to Cataloging -> Batch record modification
- You should see three tabs, "Upload a file," "Select a list of
  records," and "Enter a list of record numbers."
- The tabs should look correct and work correctly.
- Checking the "Authorities" radio button should hide the lists tab.
- Test that submissions from each form work correctly.

- Go to Cataloging -> Batch record deletion and perform the same tests.

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 24 Katrin Fischer 2023-05-09 11:55:59 UTC
Created attachment 150858 [details] [review]
Bug 32914: (follow-up) Improve handling of list tab visibility

This patch makes minor changes to the way this page handles switching
between authority and bibliographic record batch operation in order to
ensure that the list tab is correctly shown or hidden and that the list
tab selection isn't transmitted when an authority batch is submitted.

This patch also changes the labels on the "record type" fields to fix
the incorrect use of the abbreviated "biblios" in favor of
"Bibliographic."

To test, apply the patch and go to Cataloging -> Batch record
modification

- With the "Bibliographic records" selected, click the "Select a list
  of records" tab."
- Select the "Authority records" radio button.
- The "Select a list of records" tab should disappear, and the "Upload a
  file" tab should now be selected."
- Select the "Bibliographic records radio button.
- The "Select a list of records" tab should reappear, and the "Upload a
  file" tab should be selected.
- Test all combinations of form submissions to confirm that each works
  correctly:
  - Bibliographic records by upload, list, and biblionumber entry
  - Authory records by upload and authority record number entry

Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 25 Tomás Cohen Arazi 2023-05-16 18:19:11 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 26 Matt Blenkinsop 2023-06-08 16:08:30 UTC
Nice work everyone!

Pushed to stable for 22.11.x