Bug 34728

Summary: HTML notices should not be pre-formatted
Product: Koha Reporter: Matt Blenkinsop <matt.blenkinsop>
Component: PatronsAssignee: Matt Blenkinsop <matt.blenkinsop>
Status: Pushed to oldstable --- QA Contact: Marcel de Rooy <m.de.rooy>
Severity: trivial    
Priority: P5 - low CC: fridolin.somers, gmcharlt, kyle.m.hall, m.de.rooy
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33478
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22131
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34704
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
Bug Depends on:    
Bug Blocks: 34704    
Attachments: Spacing before
Spacing after
Bug 34728: Make pre tag conditional
Bug 34728: Make pre tag conditional
Bug 34728: Make pre tag conditional
Bug 34728: (QA follow-up) Remove o modifier, bit more compact

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 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 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.