Bug 34728 - HTML notices should not be pre-formatted
Summary: HTML notices should not be pre-formatted
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial
Assignee: Matt Blenkinsop
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 34704
  Show dependency treegraph
 
Reported: 2023-09-06 11:11 UTC by Matt Blenkinsop
Modified: 2024-07-04 20:40 UTC (History)
4 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.11.00,23.05.04,22.11.11
Circulation function:


Attachments
Spacing before (27.14 KB, image/png)
2023-09-06 11:11 UTC, Matt Blenkinsop
Details
Spacing after (23.86 KB, image/png)
2023-09-06 11:12 UTC, Matt Blenkinsop
Details
Bug 34728: Make pre tag conditional (3.39 KB, patch)
2023-09-06 11:33 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 34728: Make pre tag conditional (3.42 KB, patch)
2023-09-13 22:52 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 34728: Make pre tag conditional (3.52 KB, patch)
2023-09-15 07:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34728: (QA follow-up) Remove o modifier, bit more compact (1.17 KB, patch)
2023-09-15 07:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Blenkinsop 2023-09-06 11:11:49 UTC
Created attachment 155263 [details]
Spacing before

Print notices that are html templates are wrapped in a <pre> tag which causes inconsistencies in formatting. Extra spaces are introduced that shouldn't be, this is solved by making the <pre> tag conditional on the content type of the message.
Comment 1 Matt Blenkinsop 2023-09-06 11:12:05 UTC
Created attachment 155264 [details]
Spacing after
Comment 2 Matt Blenkinsop 2023-09-06 11:33:06 UTC
Created attachment 155265 [details] [review]
Bug 34728: Make pre tag conditional

This patch makes the pre-formatting of notice templates conditional on the content type.

Test plan:
1) Navigate to tools > Notices and slips
2) Edit the notice with the code of HOLD
3) Under print, paste the following
<p>Please collect within 10 days from the date of this letter and don't forget to bring your library card with you.</p>
<p>Be aware that opening hours vary at different library branches so if in doubt double check the times before you visit.</p>
<p>Should you no longer require this item please contact us so we can make it available for another customer.</p>

<p>Thank you</p>
4) Check the box that says HTML message and save the notice
5) In Patrons, select a patron and in Patron messaging preferences, check the box in the Email column for Hold filled
6) Find a record in the catalogue
7) Copy the barcode for one of the items
8) Place a hold on the item for the patron you selected
9) Navigate to Circulation > Check-in
10) Paste the barcode in the Check-in box and click Check-in
11) In the pop-up box, click Confirm hold
12) In the command line, run perl misc/cronjobs/gather_print_notices.pl --letter_code=HOLD --html /tmp
13) cd /tmp
14) There should be a file in there called 'notices_HOLD-date.html
15) cat this file
16) The html you pasted in the template will be wrapped in a <pre> tag
17) Apply patch and restart_all
18) rm the notices file that was created
19) cd /kohadevbox/koha
20) Select a new record from the catalogue and repeat steps 7-15 for this record, using the same patron
21) This time, the html file should not contain the <pre> tag
22) Sign off!
Comment 3 Lucas Gass (lukeg) 2023-09-13 22:52:58 UTC
Created attachment 155599 [details] [review]
Bug 34728: Make pre tag conditional

This patch makes the pre-formatting of notice templates conditional on the content type.

Test plan:
1) Navigate to tools > Notices and slips
2) Edit the notice with the code of HOLD
3) Under print, paste the following
<p>Please collect within 10 days from the date of this letter and don't forget to bring your library card with you.</p>
<p>Be aware that opening hours vary at different library branches so if in doubt double check the times before you visit.</p>
<p>Should you no longer require this item please contact us so we can make it available for another customer.</p>

<p>Thank you</p>
4) Check the box that says HTML message and save the notice
5) In Patrons, select a patron and in Patron messaging preferences, check the box in the Email column for Hold filled
6) Find a record in the catalogue
7) Copy the barcode for one of the items
8) Place a hold on the item for the patron you selected
9) Navigate to Circulation > Check-in
10) Paste the barcode in the Check-in box and click Check-in
11) In the pop-up box, click Confirm hold
12) In the command line, run perl misc/cronjobs/gather_print_notices.pl --letter_code=HOLD --html /tmp
13) cd /tmp
14) There should be a file in there called 'notices_HOLD-date.html
15) cat this file
16) The html you pasted in the template will be wrapped in a <pre> tag
17) Apply patch and restart_all
18) rm the notices file that was created
19) cd /kohadevbox/koha
20) Select a new record from the catalogue and repeat steps 7-15 for this record, using the same patron
21) This time, the html file should not contain the <pre> tag
22) Sign off!

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 4 Marcel de Rooy 2023-09-15 07:05:15 UTC
Created attachment 155639 [details] [review]
Bug 34728: Make pre tag conditional

This patch makes the pre-formatting of notice templates conditional on the content type.

Test plan:
1) Navigate to tools > Notices and slips
2) Edit the notice with the code of HOLD
3) Under print, paste the following
<p>Please collect within 10 days from the date of this letter and don't forget to bring your library card with you.</p>
<p>Be aware that opening hours vary at different library branches so if in doubt double check the times before you visit.</p>
<p>Should you no longer require this item please contact us so we can make it available for another customer.</p>

<p>Thank you</p>
4) Check the box that says HTML message and save the notice
5) In Patrons, select a patron and in Patron messaging preferences, check the box in the Email column for Hold filled
6) Find a record in the catalogue
7) Copy the barcode for one of the items
8) Place a hold on the item for the patron you selected
9) Navigate to Circulation > Check-in
10) Paste the barcode in the Check-in box and click Check-in
11) In the pop-up box, click Confirm hold
12) In the command line, run perl misc/cronjobs/gather_print_notices.pl --letter_code=HOLD --html /tmp
13) cd /tmp
14) There should be a file in there called 'notices_HOLD-date.html
15) cat this file
16) The html you pasted in the template will be wrapped in a <pre> tag
17) Apply patch and restart_all
18) rm the notices file that was created
19) cd /kohadevbox/koha
20) Select a new record from the catalogue and repeat steps 7-15 for this record, using the same patron
21) This time, the html file should not contain the <pre> tag
22) Sign off!

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Marcel de Rooy 2023-09-15 07:05:17 UTC
Created attachment 155640 [details] [review]
Bug 34728: (QA follow-up) Remove o modifier, bit more compact

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Tomás Cohen Arazi (tcohen) 2023-09-18 18:55:05 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 7 Fridolin Somers 2023-09-18 21:45:06 UTC
Pushed to 23.05.x for 23.05.04
Comment 8 Matt Blenkinsop 2023-10-12 16:01:09 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x
Comment 9 Katrin Fischer 2023-11-07 22:08:39 UTC
 WARN	misc/cronjobs/gather_print_notices.pl
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 92, now: 93)

Fixed in amended last patch.