Bug 36815 - Add the option to 'Reset to default' in the notices editor
Summary: Add the option to 'Reset to default' in the notices editor
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P1 - high enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on:
Blocks: 15278 15279
  Show dependency treegraph
 
Reported: 2024-05-08 13:03 UTC by Martin Renvoize
Modified: 2024-05-17 08:25 UTC (History)
9 users (show)

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


Attachments
Bug 36815: Add 'get_default' method to Koha::Notice::Template (2.51 KB, patch)
2024-05-08 16:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36815: WIP - Expose to the UI (4.45 KB, patch)
2024-05-08 16:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36815: Add 'get_default' method to Koha::Notice::Template (2.51 KB, patch)
2024-05-09 09:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36815: Expose to the UI (4.94 KB, patch)
2024-05-09 09:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36815: Add 'get_default' method to Koha::Notice::Template (2.58 KB, patch)
2024-05-09 20:51 UTC, Emily Lamancusa
Details | Diff | Splinter Review
Bug 36815: Expose to the UI (5.00 KB, patch)
2024-05-09 20:51 UTC, Emily Lamancusa
Details | Diff | Splinter Review
Bug 36815: (follow-up) Fix logic for new languages (1.67 KB, patch)
2024-05-10 12:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36815: Add 'get_default' method to Koha::Notice::Template (2.58 KB, patch)
2024-05-14 09:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36815: Expose to the UI (5.01 KB, patch)
2024-05-14 09:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36815: (follow-up) Fix logic for new languages (4.08 KB, patch)
2024-05-14 09:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36815: Add 'get_default' method to Koha::Notice::Template (2.58 KB, patch)
2024-05-14 12:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36815: Expose to the UI (5.42 KB, patch)
2024-05-14 12:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36815: (follow-up) Fix logic for new languages (4.49 KB, patch)
2024-05-14 12:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36815: Unit tests (2.57 KB, patch)
2024-05-14 12:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36815: Add 'get_default' method to Koha::Notice::Template (2.61 KB, patch)
2024-05-14 16:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36815: Unit tests (2.61 KB, patch)
2024-05-14 16:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36815: Expose to the UI (5.42 KB, patch)
2024-05-14 16:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36815: (follow-up) Fix logic for new languages (4.49 KB, patch)
2024-05-14 16:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36815: Add 'get_default' method to Koha::Notice::Template (2.67 KB, patch)
2024-05-15 10:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 36815: Unit tests (2.66 KB, patch)
2024-05-15 10:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 36815: Expose to the UI (5.48 KB, patch)
2024-05-15 10:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 36815: (follow-up) Fix logic for new languages (4.54 KB, patch)
2024-05-15 10:32 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2024-05-08 13:03:59 UTC
It would be nice to have the option to reset to the default notice text from the UI.

This would allow for a reset to catch changes at upgrades or a reset for when you've installed a translation after the fact.

I imagine a preview before reset might be helpful too?
Comment 1 Martin Renvoize 2024-05-08 16:08:42 UTC
Created attachment 166429 [details] [review]
Bug 36815: Add 'get_default' method to Koha::Notice::Template

This patch adds a new 'get_default' method to the Koha::Notice::Template
object.  The idea is that it returns the template content as defined by
the sample_notices.yml file for the language of your choosing.
Comment 2 Martin Renvoize 2024-05-08 16:08:45 UTC
Created attachment 166430 [details] [review]
Bug 36815: WIP - Expose to the UI

This patch begins to expose the sample notices to the UI in the notices
editor.

It doesn't look pretty yet, we need to treat the output as code as
aposed to rendering it as html.. and we want to add the 'copy to
content' functoinality to allow resetting from it.
Comment 3 Martin Renvoize 2024-05-08 16:09:44 UTC
If anyone wants to continue here I'd be more than happy to share the work.
Comment 4 Martin Renvoize 2024-05-08 16:24:57 UTC
To test this, just apply the patches and then navigate to any notice template from the tools and look for a new 'View default' button that appears when editing a template if a sample exists for that code + language + message_transport_type combination.

The button will display a modal containing the sample template content.. it is very rough around the edges right now.
Comment 5 Martin Renvoize 2024-05-09 09:44:30 UTC
Created attachment 166449 [details] [review]
Bug 36815: Add 'get_default' method to Koha::Notice::Template

This patch adds a new 'get_default' method to the Koha::Notice::Template
object.  The idea is that it returns the template content as defined by
the sample_notices.yml file for the language of your choosing.
Comment 6 Martin Renvoize 2024-05-09 09:44:33 UTC
Created attachment 166450 [details] [review]
Bug 36815: Expose to the UI

This patch adds a modal to preview the sample notices in the UI and
allow replacing the template content with that content of that modal.
Comment 7 Martin Renvoize 2024-05-09 09:48:41 UTC
This is all working pretty well for a first draft now so I'm setting to NSO.

We'll need a unit test for the new method introduced to Koha::Notice::Template.. but honestly, I'm not entirely sure yet how to achieve that.

Anyway.. I'd love some feedback and testing.

Test plan:
1) Apply patches and restart plack (Or just use a sandbox)
2) Login to the staff client and navigate to 'Tools > Notices and Slips'
3) 'Edit' a notice.
4) Note the new 'View default' button on some notices where we ship a default in the 'sample_notices' file.
5) Click the button to view the sample notice
6) Confirm 'Close' just closes the modal
7) Confirm 'Copy' copies the sample notice into the editor field
Comment 8 Martin Renvoize 2024-05-09 09:49:41 UTC
For bonus points.. confirm that translated sample notices also work ;)
Comment 9 Emily Lamancusa 2024-05-09 20:51:32 UTC
Created attachment 166475 [details] [review]
Bug 36815: Add 'get_default' method to Koha::Notice::Template

This patch adds a new 'get_default' method to the Koha::Notice::Template
object.  The idea is that it returns the template content as defined by
the sample_notices.yml file for the language of your choosing.

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 10 Emily Lamancusa 2024-05-09 20:51:35 UTC
Created attachment 166476 [details] [review]
Bug 36815: Expose to the UI

This patch adds a modal to preview the sample notices in the UI and
allow replacing the template content with that content of that modal.

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 11 Emily Lamancusa 2024-05-09 20:52:10 UTC
That looks really nice! Everything seems to be working as intended without translations. I tested a few different notices, and it correctly displayed/copied the default notice text for email, SMS, and print notices if a corresponding default was defined.

It didn't seem to work for translated notices, though I'm not 100% sure I set that up correctly.
Here's what I did:
1. Install Spanish by running koha-translate --install es-ES from inside the kohashell
2. restart all
3. Enable Spanish and set it to the default language in both the staff interface and OPAC
4. Set TranslateNotices to Allow
5. Verify that the Spanish-language sample_notices.yml exists
6. Open a notice that has at least one Spanish default notice defined

The Default language tab for the notice says "Default language: Español", but the "View default" button shows the English notice text. The Español (es-ES) and English (en) tabs don't show the "View default" button at all. On the other hand, the text editors for those tabs are also completely blank, so it's entirely possible that I missed a step for importing the translated notices at all.

I also tried a full reset of KTD and then repeated steps 3-6 to see if that would automagically import the default Spanish notices, but the results were the same.


Signing off since the main test plan worked and I'm not sure I did the "bonus" part right anyway. Feel free to set back if needed.
Comment 12 Martin Renvoize 2024-05-10 08:53:08 UTC
Thanks for the testing Emily.  You've highlighted a fun little challenge with the translated notices which I'm working on now.
Comment 13 Marcel de Rooy 2024-05-10 09:31:01 UTC
 WARN   tools/letter.pl
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 125, now: 127)


Module changes require tests. Could you please add a test?
Comment 14 Martin Renvoize 2024-05-10 12:17:11 UTC
Created attachment 166535 [details] [review]
Bug 36815: (follow-up) Fix logic for new languages

If a notice doesn't already have any data in it we weren't able to use
the object to lookup the sample.. with this patch we now always load the
samples if they exist for each installed and enabled language,
regardless of whether there's already a notice stored in that language.
Comment 15 Martin Renvoize 2024-05-10 14:05:17 UTC
Follow-up should work but doesn't.. I'll revisit this with a fresher brain some time.
Comment 16 Martin Renvoize 2024-05-10 14:05:55 UTC
        # Fetch sample notices
        use Data::Printer;
        for my $lang_key ( keys %letters ) {
            for my $mtt_key ( keys %{ $letters{$lang_key}{templates} } ) {
                my $object = Koha::Notice::Template->new(
                    { module => $module, code => $code, message_transport_type => $mtt_key, lang => $lang_key } );
                $letters{$lang_key}{templates}{$mtt_key}{sample} = $object->get_default;
                $letters{$lang_key}{templates}{$mtt_key}{id}     = lc( "$module" . "_" . $code . "_" . $mtt_key . "_" . $lang_key );
                p $letters{$lang_key}{templates}{$mtt_key}, as => $lang_key . ":" . $mtt_key;
            }
        }

#        while ( my ( $lang_key, $lang_value ) = each %letters ) {
#            p $lang_value, as => $lang_key;
#            while ( my ( $mtt_key, $mtt_value ) = each %{ $lang_value->{templates} } ) {
#                my $object = Koha::Notice::Template->new(
#                    { module => $module, code => $code, message_transport_type => $mtt_key, lang => $lang_key } );
#                $mtt_value->{sample} = $object->get_default;
#                $mtt_value->{id}     = lc( "$module" . "_" . $code . "_" . $mtt_key . "_" . $lang_key );
#            }
#            warn "\n\n";
#        }
Comment 17 Martin Renvoize 2024-05-14 09:45:22 UTC
Created attachment 166678 [details] [review]
Bug 36815: Add 'get_default' method to Koha::Notice::Template

This patch adds a new 'get_default' method to the Koha::Notice::Template
object.  The idea is that it returns the template content as defined by
the sample_notices.yml file for the language of your choosing.

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 18 Martin Renvoize 2024-05-14 09:45:25 UTC
Created attachment 166679 [details] [review]
Bug 36815: Expose to the UI

This patch adds a modal to preview the sample notices in the UI and
allow replacing the template content with that content of that modal.

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 19 Martin Renvoize 2024-05-14 09:45:28 UTC
Created attachment 166680 [details] [review]
Bug 36815: (follow-up) Fix logic for new languages

If a notice doesn't already have any data in it we weren't able to use
the object to lookup the sample.. with this patch we now always load the
samples if they exist for each installed and enabled language,
regardless of whether there's already a notice stored in that language.
Comment 20 Martin Renvoize 2024-05-14 09:47:13 UTC
I finally got to the bottom of my referencing/de-referencing challenge!

This should be ready to go now.. I'm sure we can further improve it down the line but as a first go I think it's a great little enhancement.
Comment 21 Martin Renvoize 2024-05-14 12:28:00 UTC
Created attachment 166684 [details] [review]
Bug 36815: Add 'get_default' method to Koha::Notice::Template

This patch adds a new 'get_default' method to the Koha::Notice::Template
object.  The idea is that it returns the template content as defined by
the sample_notices.yml file for the language of your choosing.

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 22 Martin Renvoize 2024-05-14 12:28:03 UTC
Created attachment 166685 [details] [review]
Bug 36815: Expose to the UI

This patch adds a modal to preview the sample notices in the UI and
allow replacing the template content with that content of that modal.

Test plan
1) Apply patches and restart plack (Or just use a sandbox)
2) Login to the staff client and navigate to 'Tools > Notices and Slips'
3) 'Edit' a notice.
4) Note the new 'View default' button on some notices where we ship a
   default in the 'sample_notices' file.
5) Click the button to view the sample notice
6) Confirm 'Close' just closes the modal
7) Confirm 'Copy' copies the sample notice into the editor field

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 23 Martin Renvoize 2024-05-14 12:28:06 UTC
Created attachment 166686 [details] [review]
Bug 36815: (follow-up) Fix logic for new languages

If a notice doesn't already have any data in it we weren't able to use
the object to lookup the sample.. with this patch we now always load the
samples if they exist for each installed and enabled language,
regardless of whether there's already a notice stored in that language.

Test plan
1. Install Spanish by running koha-translate --install es-ES from inside
   the kohashell
2. restart all
3. Set TranslateNotices to Allow
4. Verify that the Spanish-language sample_notices.yml exists
5. Open a notice that has at least one Spanish default notice defined
6. Confirm you see the 'View default' button for the spanish notice and
   that the displayed notice is indeed the spanish translation
Comment 24 Martin Renvoize 2024-05-14 12:52:47 UTC
Created attachment 166689 [details] [review]
Bug 36815: Unit tests

Add a pretty sparse integration test that relies on the default shipped
sample_notices.yaml.  This should catch if the format of that file
changes, but it doesn't test any more than that.
Comment 25 Martin Renvoize 2024-05-14 13:23:47 UTC
Hmm.. and now I'm struggling to get the unit test to pass.. this is the bug that just keeps giving.
Comment 26 Emily Lamancusa 2024-05-14 15:55:22 UTC
(In reply to Martin Renvoize from comment #25)
> Hmm.. and now I'm struggling to get the unit test to pass.. this is the bug
> that just keeps giving.

Shooting in the dark a bit here...but could it be a newline character mismatch?

> $content = join "\r\n", @{ $template->{content} };

versus the multiline string in the test file, which might just be encoding the line breaks as '\n'?
Comment 27 Martin Renvoize 2024-05-14 16:21:08 UTC
Created attachment 166693 [details] [review]
Bug 36815: Add 'get_default' method to Koha::Notice::Template

This patch adds a new 'get_default' method to the Koha::Notice::Template
object.  The idea is that it returns the template content as defined by
the sample_notices.yml file for the language of your choosing.

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 28 Martin Renvoize 2024-05-14 16:21:11 UTC
Created attachment 166694 [details] [review]
Bug 36815: Unit tests

Add a pretty sparse integration test that relies on the default shipped
sample_notices.yaml.  This should catch if the format of that file
changes, but it doesn't test any more than that.
Comment 29 Martin Renvoize 2024-05-14 16:21:13 UTC
Created attachment 166695 [details] [review]
Bug 36815: Expose to the UI

This patch adds a modal to preview the sample notices in the UI and
allow replacing the template content with that content of that modal.

Test plan
1) Apply patches and restart plack (Or just use a sandbox)
2) Login to the staff client and navigate to 'Tools > Notices and Slips'
3) 'Edit' a notice.
4) Note the new 'View default' button on some notices where we ship a
   default in the 'sample_notices' file.
5) Click the button to view the sample notice
6) Confirm 'Close' just closes the modal
7) Confirm 'Copy' copies the sample notice into the editor field

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 30 Martin Renvoize 2024-05-14 16:21:16 UTC
Created attachment 166696 [details] [review]
Bug 36815: (follow-up) Fix logic for new languages

If a notice doesn't already have any data in it we weren't able to use
the object to lookup the sample.. with this patch we now always load the
samples if they exist for each installed and enabled language,
regardless of whether there's already a notice stored in that language.

Test plan
1. Install Spanish by running koha-translate --install es-ES from inside
   the kohashell
2. restart all
3. Set TranslateNotices to Allow
4. Verify that the Spanish-language sample_notices.yml exists
5. Open a notice that has at least one Spanish default notice defined
6. Confirm you see the 'View default' button for the spanish notice and
   that the displayed notice is indeed the spanish translation
Comment 31 Martin Renvoize 2024-05-14 16:22:09 UTC
(In reply to Emily Lamancusa from comment #26)
> (In reply to Martin Renvoize from comment #25)
> > Hmm.. and now I'm struggling to get the unit test to pass.. this is the bug
> > that just keeps giving.
> 
> Shooting in the dark a bit here...but could it be a newline character
> mismatch?
> 
> > $content = join "\r\n", @{ $template->{content} };
> 
> versus the multiline string in the test file, which might just be encoding
> the line breaks as '\n'?

Of course, you were correct there.. clearly it's been a long day ;P
Comment 32 David Cook 2024-05-14 23:46:43 UTC
This sounds like an awesome idea
Comment 33 Kyle M Hall 2024-05-15 10:32:38 UTC
Created attachment 166763 [details] [review]
Bug 36815: Add 'get_default' method to Koha::Notice::Template

This patch adds a new 'get_default' method to the Koha::Notice::Template
object.  The idea is that it returns the template content as defined by
the sample_notices.yml file for the language of your choosing.

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 34 Kyle M Hall 2024-05-15 10:32:46 UTC
Created attachment 166764 [details] [review]
Bug 36815: Unit tests

Add a pretty sparse integration test that relies on the default shipped
sample_notices.yaml.  This should catch if the format of that file
changes, but it doesn't test any more than that.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 35 Kyle M Hall 2024-05-15 10:32:48 UTC
Created attachment 166765 [details] [review]
Bug 36815: Expose to the UI

This patch adds a modal to preview the sample notices in the UI and
allow replacing the template content with that content of that modal.

Test plan
1) Apply patches and restart plack (Or just use a sandbox)
2) Login to the staff client and navigate to 'Tools > Notices and Slips'
3) 'Edit' a notice.
4) Note the new 'View default' button on some notices where we ship a
   default in the 'sample_notices' file.
5) Click the button to view the sample notice
6) Confirm 'Close' just closes the modal
7) Confirm 'Copy' copies the sample notice into the editor field

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 36 Kyle M Hall 2024-05-15 10:32:50 UTC
Created attachment 166766 [details] [review]
Bug 36815: (follow-up) Fix logic for new languages

If a notice doesn't already have any data in it we weren't able to use
the object to lookup the sample.. with this patch we now always load the
samples if they exist for each installed and enabled language,
regardless of whether there's already a notice stored in that language.

Test plan
1. Install Spanish by running koha-translate --install es-ES from inside
   the kohashell
2. restart all
3. Set TranslateNotices to Allow
4. Verify that the Spanish-language sample_notices.yml exists
5. Open a notice that has at least one Spanish default notice defined
6. Confirm you see the 'View default' button for the spanish notice and
   that the displayed notice is indeed the spanish translation

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 37 Katrin Fischer 2024-05-17 08:25:06 UTC
While I love the idea and the feature, I think we are keeping this for 24.11 or an early backport.