Bug 35285

Summary: Centralise notice content wrapping for HTML output
Product: Koha Reporter: Martin Renvoize (ashimema) <martin.renvoize>
Component: NoticesAssignee: Martin Renvoize (ashimema) <martin.renvoize>
Status: RESOLVED FIXED QA Contact: Testopia <testopia>
Severity: trivial    
Priority: P5 - low CC: fridolin.somers, jonathan.druart, lucas, matt.blenkinsop, oleonard
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.06
Circulation function:
Bug Depends on: 30287, 34704    
Bug Blocks: 30723, 33260, 35286, 35459, 36808, 36809    
Attachments: Bug 35285: Add non-html template support to html_content wrapping
Bug 35285: Remove switch in notices.tt
Bug 35285: Add non-html template support to html_content wrapping
Bug 35285: Remove switch in notices.tt
Bug 35285: Unit tests
Bug 35285: Add non-html template support to html_content wrapping
Bug 35285: Remove switch in notices.tt
Bug 35285: Unit tests
Bug 35285: Add non-html template support to html_content wrapping
Bug 35285: Remove switch in notices.tt
Bug 35285: Unit tests
Bug 35285: Add non-html template support to html_content wrapping
Bug 35285: Remove switch in notices.tt
Bug 35285: Unit tests
Bug 35285: Add non-html template support to html_content wrapping
Bug 35285: Remove switch in notices.tt
Bug 35285: Unit tests
Bug 35285: Add non-html template support to html_content wrapping
Bug 35285: Remove switch in notices.tt
Bug 35285: Unit tests
Bug 35285: Add non-html template support to html_content wrapping
Bug 35285: Remove switch in notices.tt
Bug 35285: Unit tests
Bug 35285: (QA follow-up) Tidy

Description Martin Renvoize (ashimema) 2023-11-08 09:44:01 UTC
Whilst working through some notice bugs I noted that we now have a few different places where we wrap the content for output in various instants.

I think we could centralise on the html_content method I introduce in bug 30287 and add support for the 'pre' (or perhaps replace pre with <div style="white-space: pre-wrap;"> as suggested in https://stackoverflow.com/questions/248011/how-do-i-wrap-text-in-a-pre-tag) for non html encoded templates.

Thoughts?
Comment 1 Martin Renvoize (ashimema) 2023-11-08 10:40:16 UTC
Created attachment 158657 [details] [review]
Bug 35285: Add non-html template support to html_content wrapping

This patch adds support for messages generated using non-html formatted
notice templates to the html_content method of Koha::Notice::Message.

We continue to wrap content for html generated messages with the
appropriate headers, css and title.

For non-html generated content we wrap in the <div style="white-space:
pre-wrap"> block to maintain text formatting as defined in the original
plaintext template.
Comment 2 Martin Renvoize (ashimema) 2023-11-08 10:40:57 UTC
Work in progress
Comment 3 Martin Renvoize (ashimema) 2023-11-08 10:46:23 UTC
Created attachment 158658 [details] [review]
Bug 35285: Remove switch in notices.tt

We now wrap appropriately for non-html formatted messages as
part of the html_content method. This means we can remove the
case from members/notices.tt and rely on html_content doing
the right thing for notice previews.
Comment 4 Martin Renvoize (ashimema) 2023-11-13 16:48:35 UTC
Created attachment 158889 [details] [review]
Bug 35285: Add non-html template support to html_content wrapping

This patch adds support for messages generated using non-html formatted
notice templates to the html_content method of Koha::Notice::Message.

We continue to wrap content for html generated messages with the
appropriate headers, css and title.

For non-html generated content we wrap in the <div style="white-space:
pre-wrap"> block to maintain text formatting as defined in the original
plaintext template.

Test
Follow the test plan for bug 30287, nothing should outwardly change.
Comment 5 Martin Renvoize (ashimema) 2023-11-13 16:48:37 UTC
Created attachment 158890 [details] [review]
Bug 35285: Remove switch in notices.tt

We now wrap appropriately for non-html formatted messages as
part of the html_content method. This means we can remove the
case from members/notices.tt and rely on html_content doing
the right thing for notice previews.
Comment 6 Martin Renvoize (ashimema) 2023-11-13 16:48:40 UTC
Created attachment 158891 [details] [review]
Bug 35285: Unit tests

This patch adds unit tests for the new 'is_html' function introduced in
Koha::Notice::Message and update the tests to include the new plaintext
handling of html_content.
Comment 7 David Nind 2023-11-15 09:16:21 UTC
Created attachment 158960 [details] [review]
Bug 35285: Add non-html template support to html_content wrapping

This patch adds support for messages generated using non-html formatted
notice templates to the html_content method of Koha::Notice::Message.

We continue to wrap content for html generated messages with the
appropriate headers, css and title.

For non-html generated content we wrap in the <div style="white-space:
pre-wrap"> block to maintain text formatting as defined in the original
plaintext template.

Test
Follow the test plan for bug 30287, nothing should outwardly change.

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2023-11-15 09:16:24 UTC
Created attachment 158961 [details] [review]
Bug 35285: Remove switch in notices.tt

We now wrap appropriately for non-html formatted messages as
part of the html_content method. This means we can remove the
case from members/notices.tt and rely on html_content doing
the right thing for notice previews.

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2023-11-15 09:16:27 UTC
Created attachment 158962 [details] [review]
Bug 35285: Unit tests

This patch adds unit tests for the new 'is_html' function introduced in
Koha::Notice::Message and update the tests to include the new plaintext
handling of html_content.

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 Martin Renvoize (ashimema) 2024-05-07 18:25:52 UTC Comment hidden (obsolete)
Comment 11 Martin Renvoize (ashimema) 2024-05-07 18:25:55 UTC Comment hidden (obsolete)
Comment 12 Martin Renvoize (ashimema) 2024-05-07 18:25:58 UTC Comment hidden (obsolete)
Comment 13 Martin Renvoize (ashimema) 2024-05-07 18:26:17 UTC
Just a quick rebase
Comment 14 Martin Renvoize (ashimema) 2024-05-07 18:30:51 UTC Comment hidden (obsolete)
Comment 15 Martin Renvoize (ashimema) 2024-05-07 18:30:53 UTC Comment hidden (obsolete)
Comment 16 Martin Renvoize (ashimema) 2024-05-07 18:30:57 UTC Comment hidden (obsolete)
Comment 17 Martin Renvoize (ashimema) 2024-05-08 06:03:41 UTC Comment hidden (obsolete)
Comment 18 Martin Renvoize (ashimema) 2024-05-08 06:03:44 UTC Comment hidden (obsolete)
Comment 19 Martin Renvoize (ashimema) 2024-05-08 06:03:46 UTC Comment hidden (obsolete)
Comment 20 Martin Renvoize (ashimema) 2024-05-08 08:27:35 UTC Comment hidden (obsolete)
Comment 21 Martin Renvoize (ashimema) 2024-05-08 08:27:38 UTC Comment hidden (obsolete)
Comment 22 Martin Renvoize (ashimema) 2024-05-08 08:27:41 UTC Comment hidden (obsolete)
Comment 23 Nick Clemens (kidclamp) 2024-05-14 18:05:40 UTC
Created attachment 166719 [details] [review]
Bug 35285: Add non-html template support to html_content wrapping

This patch adds support for messages generated using non-html formatted
notice templates to the html_content method of Koha::Notice::Message.

We continue to wrap content for html generated messages with the
appropriate headers, css and title.

For non-html generated content we wrap in the <div style="white-space:
pre-wrap"> block to maintain text formatting as defined in the original
plaintext template.

Test
Follow the test plan for bug 30287, nothing should outwardly change.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 24 Nick Clemens (kidclamp) 2024-05-14 18:05:42 UTC
Created attachment 166720 [details] [review]
Bug 35285: Remove switch in notices.tt

We now wrap appropriately for non-html formatted messages as
part of the html_content method. This means we can remove the
case from members/notices.tt and rely on html_content doing
the right thing for notice previews.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 25 Nick Clemens (kidclamp) 2024-05-14 18:05:44 UTC
Created attachment 166721 [details] [review]
Bug 35285: Unit tests

This patch adds unit tests for the new 'is_html' function introduced in
Koha::Notice::Message and update the tests to include the new plaintext
handling of html_content.

Signed-off-by: David Nind <david@davidnind.com>

test

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 26 Nick Clemens (kidclamp) 2024-05-14 18:05:47 UTC
Created attachment 166722 [details] [review]
Bug 35285: (QA follow-up) Tidy

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 27 Katrin Fischer 2024-05-17 08:22:43 UTC
This one seems at least in a grey area for being an enh. I am going to push it as it's mostly affecting display in the staff interface right now.
Comment 28 Katrin Fischer 2024-05-17 08:37:16 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 29 Fridolin Somers 2024-05-29 13:18:34 UTC
Pushed to 23.11.x for 23.11.06
Comment 30 Lucas Gass (lukeg) 2024-05-31 13:05:31 UTC
This feels like an enhancement to me. I'm not going to include this in 23.05.x.