Bug 17981 - Add the ability to preview generated notice templates
Summary: Add the ability to preview generated notice templates
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Josef Moravec
URL:
Keywords:
Depends on: 17961
Blocks: 15969 29490 18007
  Show dependency treegraph
 
Reported: 2017-01-24 15:19 UTC by Jonathan Druart
Modified: 2021-11-16 12:24 UTC (History)
12 users (show)

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


Attachments
Bug 17981: Move svc/letters to svc/letters/get (1.77 KB, patch)
2017-01-24 15:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Add a 'letter' param to GetPreparedLetter (1.94 KB, patch)
2017-01-24 15:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Add a preview mode for notice templates (24.32 KB, patch)
2017-01-24 15:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Better error handling (3.23 KB, patch)
2017-01-24 16:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Move the divs to the top of the modal (2.54 KB, patch)
2017-01-24 17:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Move svc/letters to svc/letters/get (1.79 KB, patch)
2017-06-05 17:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Add a 'letter' param to GetPreparedLetter (1.94 KB, patch)
2017-06-05 17:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Add a preview mode for notice templates (24.35 KB, patch)
2017-06-05 17:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Better error handling (3.24 KB, patch)
2017-06-05 17:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Move the divs to the top of the modal (2.56 KB, patch)
2017-06-05 17:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Add a preview mode for notice templates (24.37 KB, patch)
2017-08-08 18:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Better error handling (3.24 KB, patch)
2017-08-08 18:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Move the divs to the top of the modal (2.56 KB, patch)
2017-08-08 18:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Move svc/letters to svc/letters/get (1.83 KB, patch)
2017-08-09 11:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 17981: Add a 'letter' param to GetPreparedLetter (1.99 KB, patch)
2017-08-09 11:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 17981: Add a preview mode for notice templates (24.39 KB, patch)
2017-08-09 11:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 17981: Better error handling (3.28 KB, patch)
2017-08-09 11:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 17981: Move the divs to the top of the modal (2.60 KB, patch)
2017-08-09 11:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 17981: Move svc/letters to svc/letters/get (1.84 KB, patch)
2017-09-15 14:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Add a 'letter' param to GetPreparedLetter (2.00 KB, patch)
2017-09-15 14:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Add a preview mode for notice templates (24.42 KB, patch)
2017-09-15 14:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Better error handling (3.29 KB, patch)
2017-09-15 14:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Move the divs to the top of the modal (2.61 KB, patch)
2017-09-15 14:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Remove uneeded module JS var (1.97 KB, patch)
2017-09-15 15:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Move svc/letters to svc/letters/get (1.91 KB, patch)
2018-01-05 16:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Add a 'letter' param to GetPreparedLetter (2.00 KB, patch)
2018-01-05 16:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Add a preview mode for notice templates (24.69 KB, patch)
2018-01-05 16:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Better error handling (3.29 KB, patch)
2018-01-05 16:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Move the divs to the top of the modal (2.64 KB, patch)
2018-01-05 16:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Remove uneeded module JS var (2.04 KB, patch)
2018-01-05 16:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Move svc/letters to svc/letters/get (1.91 KB, patch)
2018-03-23 16:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Add a 'letter' param to GetPreparedLetter (2.00 KB, patch)
2018-03-23 16:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Add a preview mode for notice templates (24.86 KB, patch)
2018-03-23 16:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Better error handling (3.29 KB, patch)
2018-03-23 16:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Move the divs to the top of the modal (2.64 KB, patch)
2018-03-23 16:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Remove uneeded module JS var (2.04 KB, patch)
2018-03-23 16:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Add a preview mode for notice templates (21.73 KB, patch)
2018-04-10 15:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Better error handling (3.29 KB, patch)
2018-04-10 15:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Move the divs to the top of the modal (2.64 KB, patch)
2018-04-10 15:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Remove uneeded module JS var (2.04 KB, patch)
2018-04-10 15:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Do not crash if the template is already using TT syntax (864 bytes, patch)
2018-04-11 18:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17981: Move svc/letters to svc/letters/get (1.95 KB, patch)
2018-04-11 19:26 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17981: Add a 'letter' param to GetPreparedLetter (2.05 KB, patch)
2018-04-11 19:26 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17981: Add a preview mode for notice templates (21.76 KB, patch)
2018-04-11 19:26 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17981: Better error handling (3.34 KB, patch)
2018-04-11 19:26 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17981: Move the divs to the top of the modal (2.68 KB, patch)
2018-04-11 19:26 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17981: Remove uneeded module JS var (2.08 KB, patch)
2018-04-11 19:26 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17981: Do not crash if the template is already using TT syntax (912 bytes, patch)
2018-04-11 19:27 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17981: (QA follow-up) Fix and update pod in svc/letters/get (757 bytes, patch)
2018-04-11 19:27 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2017-01-24 15:19:44 UTC

    
Comment 1 Jonathan Druart 2017-01-24 15:34:59 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2017-01-24 15:35:07 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2017-01-24 15:35:20 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2017-01-24 15:37:14 UTC
These patches will not apply due to the big dependency tree.
Use this remote branch instead: https://github.com/joubu/Koha/tree/bug_17981
Comment 5 Jonathan Druart 2017-01-24 16:13:33 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2017-01-24 17:23:20 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2017-03-24 18:19:01 UTC
I have rebased this branch, that was a nightmare, really...
There are too many patches in the queue, please help me to move things toward master.
Comment 8 Jonathan Druart 2017-06-05 17:06:00 UTC
Created attachment 63978 [details] [review]
Bug 17981: Move svc/letters to svc/letters/get

To allow another script under svc/letters
Comment 9 Jonathan Druart 2017-06-05 17:06:06 UTC
Created attachment 63979 [details] [review]
Bug 17981: Add a 'letter' param to GetPreparedLetter

This will allow us to call GetPreparedLetter with a letter content that
does not exist in the DB
Comment 10 Jonathan Druart 2017-06-05 17:06:12 UTC
Created attachment 63980 [details] [review]
Bug 17981: Add a preview mode for notice templates

This patch is a first step to provide a preview mode for notice
templates.

CHECKIN, CHECKOUT and HOLD_SLIP are supported so far.
Maybe more, but I have not tested yet and the interface will not allow
you to generate the preview.

The idea is to provide an idea of how will render the messages generated
from a notice template.

A new "Preview" button is added close to each textarea on the editing
notice templates view.
For each notice template code (letter_code), we will need some input
data to produce the preview.

For instance, for CHECKIN we need an barcode. From the barcode we
can guess all the other data.
For CHECKOUT we will need the borrowernumber and the barcode.
Note that the way to enter the data for the preview is not really
user-friendly, for CHECKOUT you will have to fill
'borrowernumber|barcode', but the placeholder will help you to know how
and what to fill.
In the modal window, you will see 4 blocks:
1/ the content of the letter (with the placeholds << >>)
2/ the generated message (with the data filled)
3/ if the letter contained historical syntax markers, the screen will
try to generate a notice template using the TT syntax
4/ the generated message from this TT syntax
=> You will be able to compare the 2 generated messages.

What is the goal of this first patchset:
- Show this first POC and get feedback from other developpers
- Add a way to easily visualise the differences between the 2 syntaxes
- Confort users with the TT syntax and the migration step from the
historical syntax.

I'd like to get opinions before going further.

The possibilities:
- Mock data to get fully working generated messages for any notice
templates. For instance, for CHECKIN and CHECKOUT, the item is not
checked in/out yet. So we cannot access the issue's information.
(I have no idea how to do that)
- Browse the data to get the ones we want to use for the preview (big).
Comment 11 Jonathan Druart 2017-06-05 17:06:17 UTC
Created attachment 63981 [details] [review]
Bug 17981: Better error handling
Comment 12 Jonathan Druart 2017-06-05 17:06:21 UTC
Created attachment 63982 [details] [review]
Bug 17981: Move the divs to the top of the modal
Comment 13 Kyle M Hall 2017-06-09 10:20:07 UTC
Is this patch set in a testable state yet? I applied the patches to master and did not see the preview button. Can you add a test plan?
Comment 14 Jonathan Druart 2017-06-09 11:27:41 UTC
(In reply to Kyle M Hall from comment #13)
> Is this patch set in a testable state yet? I applied the patches to master
> and did not see the preview button. Can you add a test plan?

You need to apply all patches under bug 17961, then test for CHECKIN, CHECKOUT or HOLD_SLIP.
Then the commit message should contain all the information to test the patch.
Comment 15 Lee Jamison 2017-06-10 13:29:21 UTC
I believe the issue Kyle speaks of is an issue related to the cache. When I first applied it to master, I did not see the Preview button either on any of the three notices. However, I cleared the cache and then they appeared. I'd like to do additional testing before signing off.
Comment 16 Lee Jamison 2017-06-11 05:10:30 UTC
Also, this is one of my favorite enhancements. Our clerk who handles all the notices will be very grateful for this feature!
Comment 17 Séverine Queune 2017-06-28 08:45:16 UTC
I tested with the HOLD_SLIP notice on BibLibre's sandbox03.

When I applied bug 17975, a new box appeared to preview the notice for "biblionumber|borrowernumber".
I tested with record #3194 and 2 different patrons (#5903 with the hold waiting for her and #6587 who is the second in priority list) but for none of them I succeeded to preview the notice, it doesn't stop loading.
Maybe I misunderstood something in the way to use it.

I couldn't test it for CHECKIN and CHECKout, patch 17964 didn't apply.
Comment 18 Jonathan Druart 2017-07-06 15:53:12 UTC
I do not think you can test this patch and all the dependencies on a sandbox.
Comment 19 Jonathan Druart 2017-08-08 18:56:38 UTC
Created attachment 65688 [details] [review]
Bug 17981: Add a preview mode for notice templates

This patch is a first step to provide a preview mode for notice
templates.

CHECKIN, CHECKOUT and HOLD_SLIP are supported so far.
Maybe more, but I have not tested yet and the interface will not allow
you to generate the preview.

The idea is to provide an idea of how will render the messages generated
from a notice template.

A new "Preview" button is added close to each textarea on the editing
notice templates view.
For each notice template code (letter_code), we will need some input
data to produce the preview.

For instance, for CHECKIN we need an barcode. From the barcode we
can guess all the other data.
For CHECKOUT we will need the borrowernumber and the barcode.
Note that the way to enter the data for the preview is not really
user-friendly, for CHECKOUT you will have to fill
'borrowernumber|barcode', but the placeholder will help you to know how
and what to fill.
In the modal window, you will see 4 blocks:
1/ the content of the letter (with the placeholds << >>)
2/ the generated message (with the data filled)
3/ if the letter contained historical syntax markers, the screen will
try to generate a notice template using the TT syntax
4/ the generated message from this TT syntax
=> You will be able to compare the 2 generated messages.

What is the goal of this first patchset:
- Show this first POC and get feedback from other developpers
- Add a way to easily visualise the differences between the 2 syntaxes
- Confort users with the TT syntax and the migration step from the
historical syntax.

I'd like to get opinions before going further.

The possibilities:
- Mock data to get fully working generated messages for any notice
templates. For instance, for CHECKIN and CHECKOUT, the item is not
checked in/out yet. So we cannot access the issue's information.
(I have no idea how to do that)
- Browse the data to get the ones we want to use for the preview (big).
Comment 20 Jonathan Druart 2017-08-08 18:56:44 UTC
Created attachment 65689 [details] [review]
Bug 17981: Better error handling
Comment 21 Jonathan Druart 2017-08-08 18:56:48 UTC
Created attachment 65690 [details] [review]
Bug 17981: Move the divs to the top of the modal
Comment 22 Kyle M Hall 2017-08-09 10:57:32 UTC
> The possibilities:
> - Mock data to get fully working generated messages for any notice
> templates. For instance, for CHECKIN and CHECKOUT, the item is not
> checked in/out yet. So we cannot access the issue's information.

At first glance, I'm not seeing any big problems with this. I suppose we could have the librarian pass in an old_issues issue_id, but that seems like it would be a fair bit less user friendly.

> - Browse the data to get the ones we want to use for the preview (big).

That's a really great idea, but it does feel like it will be quite a lot of work, and typically librarians will probably just set up dummy patrons / titles for testing, so I doubt there is an immediate need for that.
Comment 23 Kyle M Hall 2017-08-09 10:59:42 UTC
I also agree the current way of filling in the preview data is not user friendly ; )
Hopefully it won't be too much harder to put those data needed in separate form inputs.
Comment 24 Kyle M Hall 2017-08-09 11:00:09 UTC
Created attachment 65701 [details] [review]
Bug 17981: Move svc/letters to svc/letters/get

To allow another script under svc/letters

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 25 Kyle M Hall 2017-08-09 11:00:17 UTC
Created attachment 65702 [details] [review]
Bug 17981: Add a 'letter' param to GetPreparedLetter

This will allow us to call GetPreparedLetter with a letter content that
does not exist in the DB

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 26 Kyle M Hall 2017-08-09 11:00:21 UTC
Created attachment 65703 [details] [review]
Bug 17981: Add a preview mode for notice templates

This patch is a first step to provide a preview mode for notice
templates.

CHECKIN, CHECKOUT and HOLD_SLIP are supported so far.
Maybe more, but I have not tested yet and the interface will not allow
you to generate the preview.

The idea is to provide an idea of how will render the messages generated
from a notice template.

A new "Preview" button is added close to each textarea on the editing
notice templates view.
For each notice template code (letter_code), we will need some input
data to produce the preview.

For instance, for CHECKIN we need an barcode. From the barcode we
can guess all the other data.
For CHECKOUT we will need the borrowernumber and the barcode.
Note that the way to enter the data for the preview is not really
user-friendly, for CHECKOUT you will have to fill
'borrowernumber|barcode', but the placeholder will help you to know how
and what to fill.
In the modal window, you will see 4 blocks:
1/ the content of the letter (with the placeholds << >>)
2/ the generated message (with the data filled)
3/ if the letter contained historical syntax markers, the screen will
try to generate a notice template using the TT syntax
4/ the generated message from this TT syntax
=> You will be able to compare the 2 generated messages.

What is the goal of this first patchset:
- Show this first POC and get feedback from other developpers
- Add a way to easily visualise the differences between the 2 syntaxes
- Confort users with the TT syntax and the migration step from the
historical syntax.

I'd like to get opinions before going further.

The possibilities:
- Mock data to get fully working generated messages for any notice
templates. For instance, for CHECKIN and CHECKOUT, the item is not
checked in/out yet. So we cannot access the issue's information.
(I have no idea how to do that)
- Browse the data to get the ones we want to use for the preview (big).

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 27 Kyle M Hall 2017-08-09 11:00:24 UTC
Created attachment 65704 [details] [review]
Bug 17981: Better error handling

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 28 Kyle M Hall 2017-08-09 11:00:28 UTC
Created attachment 65705 [details] [review]
Bug 17981: Move the divs to the top of the modal

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 29 Jonathan Druart 2017-09-15 14:23:54 UTC
Created attachment 67144 [details] [review]
Bug 17981: Move svc/letters to svc/letters/get

To allow another script under svc/letters

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 30 Jonathan Druart 2017-09-15 14:23:58 UTC
Created attachment 67145 [details] [review]
Bug 17981: Add a 'letter' param to GetPreparedLetter

This will allow us to call GetPreparedLetter with a letter content that
does not exist in the DB

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 31 Jonathan Druart 2017-09-15 14:24:03 UTC
Created attachment 67146 [details] [review]
Bug 17981: Add a preview mode for notice templates

This patch is a first step to provide a preview mode for notice
templates.

CHECKIN, CHECKOUT and HOLD_SLIP are supported so far.
Maybe more, but I have not tested yet and the interface will not allow
you to generate the preview.

The idea is to provide an idea of how will render the messages generated
from a notice template.

A new "Preview" button is added close to each textarea on the editing
notice templates view.
For each notice template code (letter_code), we will need some input
data to produce the preview.

For instance, for CHECKIN we need an barcode. From the barcode we
can guess all the other data.
For CHECKOUT we will need the borrowernumber and the barcode.
Note that the way to enter the data for the preview is not really
user-friendly, for CHECKOUT you will have to fill
'borrowernumber|barcode', but the placeholder will help you to know how
and what to fill.
In the modal window, you will see 4 blocks:
1/ the content of the letter (with the placeholds << >>)
2/ the generated message (with the data filled)
3/ if the letter contained historical syntax markers, the screen will
try to generate a notice template using the TT syntax
4/ the generated message from this TT syntax
=> You will be able to compare the 2 generated messages.

What is the goal of this first patchset:
- Show this first POC and get feedback from other developpers
- Add a way to easily visualise the differences between the 2 syntaxes
- Confort users with the TT syntax and the migration step from the
historical syntax.

I'd like to get opinions before going further.

The possibilities:
- Mock data to get fully working generated messages for any notice
templates. For instance, for CHECKIN and CHECKOUT, the item is not
checked in/out yet. So we cannot access the issue's information.
(I have no idea how to do that)
- Browse the data to get the ones we want to use for the preview (big).

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 32 Jonathan Druart 2017-09-15 14:24:07 UTC
Created attachment 67147 [details] [review]
Bug 17981: Better error handling

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 33 Jonathan Druart 2017-09-15 14:24:11 UTC
Created attachment 67148 [details] [review]
Bug 17981: Move the divs to the top of the modal

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 34 Owen Leonard 2017-09-15 14:58:09 UTC
While rebasing Bug 18007 I noticed something in this update:

In the JavaScript there are couple of instances of a "module" variable which doesn't appear to ever be initialized. Is it superfluous, or should it be created somewhere?
Comment 35 Jonathan Druart 2017-09-15 15:49:40 UTC
(In reply to Owen Leonard from comment #34)
> While rebasing Bug 18007 I noticed something in this update:
> 
> In the JavaScript there are couple of instances of a "module" variable which
> doesn't appear to ever be initialized. Is it superfluous, or should it be
> created somewhere?

Yes you are right, it could be removed for now.
Comment 36 Jonathan Druart 2017-09-15 15:51:18 UTC
Created attachment 67152 [details] [review]
Bug 17981: Remove uneeded module JS var

As we only support CHECKIN and CHECKOUT for now, we do not need this
module variable. It may be useful to re-add it later.
Comment 37 Jonathan Druart 2018-01-05 16:03:32 UTC
Created attachment 70300 [details] [review]
Bug 17981: Move svc/letters to svc/letters/get

To allow another script under svc/letters

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 38 Jonathan Druart 2018-01-05 16:03:40 UTC
Created attachment 70301 [details] [review]
Bug 17981: Add a 'letter' param to GetPreparedLetter

This will allow us to call GetPreparedLetter with a letter content that
does not exist in the DB

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 39 Jonathan Druart 2018-01-05 16:03:46 UTC
Created attachment 70302 [details] [review]
Bug 17981: Add a preview mode for notice templates

This patch is a first step to provide a preview mode for notice
templates.

CHECKIN, CHECKOUT and HOLD_SLIP are supported so far.
Maybe more, but I have not tested yet and the interface will not allow
you to generate the preview.

The idea is to provide an idea of how will render the messages generated
from a notice template.

A new "Preview" button is added close to each textarea on the editing
notice templates view.
For each notice template code (letter_code), we will need some input
data to produce the preview.

For instance, for CHECKIN we need an barcode. From the barcode we
can guess all the other data.
For CHECKOUT we will need the borrowernumber and the barcode.
Note that the way to enter the data for the preview is not really
user-friendly, for CHECKOUT you will have to fill
'borrowernumber|barcode', but the placeholder will help you to know how
and what to fill.
In the modal window, you will see 4 blocks:
1/ the content of the letter (with the placeholds << >>)
2/ the generated message (with the data filled)
3/ if the letter contained historical syntax markers, the screen will
try to generate a notice template using the TT syntax
4/ the generated message from this TT syntax
=> You will be able to compare the 2 generated messages.

What is the goal of this first patchset:
- Show this first POC and get feedback from other developpers
- Add a way to easily visualise the differences between the 2 syntaxes
- Confort users with the TT syntax and the migration step from the
historical syntax.

I'd like to get opinions before going further.

The possibilities:
- Mock data to get fully working generated messages for any notice
templates. For instance, for CHECKIN and CHECKOUT, the item is not
checked in/out yet. So we cannot access the issue's information.
(I have no idea how to do that)
- Browse the data to get the ones we want to use for the preview (big).

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 40 Jonathan Druart 2018-01-05 16:03:55 UTC
Created attachment 70303 [details] [review]
Bug 17981: Better error handling

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 41 Jonathan Druart 2018-01-05 16:04:01 UTC
Created attachment 70304 [details] [review]
Bug 17981: Move the divs to the top of the modal

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 42 Jonathan Druart 2018-01-05 16:04:11 UTC
Created attachment 70305 [details] [review]
Bug 17981: Remove uneeded module JS var

As we only support CHECKIN and CHECKOUT for now, we do not need this
module variable. It may be useful to re-add it later.
Comment 43 Katrin Fischer 2018-03-18 00:41:44 UTC
Please rebase!
Comment 44 Jonathan Druart 2018-03-23 16:32:56 UTC
Created attachment 73201 [details] [review]
Bug 17981: Move svc/letters to svc/letters/get

To allow another script under svc/letters

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 45 Jonathan Druart 2018-03-23 16:33:01 UTC
Created attachment 73202 [details] [review]
Bug 17981: Add a 'letter' param to GetPreparedLetter

This will allow us to call GetPreparedLetter with a letter content that
does not exist in the DB

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 46 Jonathan Druart 2018-03-23 16:33:06 UTC
Created attachment 73203 [details] [review]
Bug 17981: Add a preview mode for notice templates

This patch is a first step to provide a preview mode for notice
templates.

CHECKIN, CHECKOUT and HOLD_SLIP are supported so far.
Maybe more, but I have not tested yet and the interface will not allow
you to generate the preview.

The idea is to provide an idea of how will render the messages generated
from a notice template.

A new "Preview" button is added close to each textarea on the editing
notice templates view.
For each notice template code (letter_code), we will need some input
data to produce the preview.

For instance, for CHECKIN we need an barcode. From the barcode we
can guess all the other data.
For CHECKOUT we will need the borrowernumber and the barcode.
Note that the way to enter the data for the preview is not really
user-friendly, for CHECKOUT you will have to fill
'borrowernumber|barcode', but the placeholder will help you to know how
and what to fill.
In the modal window, you will see 4 blocks:
1/ the content of the letter (with the placeholds << >>)
2/ the generated message (with the data filled)
3/ if the letter contained historical syntax markers, the screen will
try to generate a notice template using the TT syntax
4/ the generated message from this TT syntax
=> You will be able to compare the 2 generated messages.

What is the goal of this first patchset:
- Show this first POC and get feedback from other developpers
- Add a way to easily visualise the differences between the 2 syntaxes
- Confort users with the TT syntax and the migration step from the
historical syntax.

I'd like to get opinions before going further.

The possibilities:
- Mock data to get fully working generated messages for any notice
templates. For instance, for CHECKIN and CHECKOUT, the item is not
checked in/out yet. So we cannot access the issue's information.
(I have no idea how to do that)
- Browse the data to get the ones we want to use for the preview (big).

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 47 Jonathan Druart 2018-03-23 16:33:11 UTC
Created attachment 73204 [details] [review]
Bug 17981: Better error handling

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 48 Jonathan Druart 2018-03-23 16:33:15 UTC
Created attachment 73205 [details] [review]
Bug 17981: Move the divs to the top of the modal

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 49 Jonathan Druart 2018-03-23 16:33:20 UTC
Created attachment 73206 [details] [review]
Bug 17981: Remove uneeded module JS var

As we only support CHECKIN and CHECKOUT for now, we do not need this
module variable. It may be useful to re-add it later.
Comment 50 Julian Maurice 2018-04-10 05:47:02 UTC
Patches do not apply on master. Please rebase.

> Applying: Bug 17981: Move svc/letters to svc/letters/get
> Applying: Bug 17981: Add a 'letter' param to GetPreparedLetter
> Applying: Bug 17981: Add a preview mode for notice templates
> error: sha1 information is lacking or useless (C4/Letters.pm).
> error: could not build fake ancestor
> Patch failed at 0001 Bug 17981: Add a preview mode for notice templates
Comment 51 Jonathan Druart 2018-04-10 15:17:43 UTC
Created attachment 73949 [details] [review]
Bug 17981: Add a preview mode for notice templates

This patch is a first step to provide a preview mode for notice
templates.

CHECKIN, CHECKOUT and HOLD_SLIP are supported so far.
Maybe more, but I have not tested yet and the interface will not allow
you to generate the preview.

The idea is to provide an idea of how will render the messages generated
from a notice template.

A new "Preview" button is added close to each textarea on the editing
notice templates view.
For each notice template code (letter_code), we will need some input
data to produce the preview.

For instance, for CHECKIN we need an barcode. From the barcode we
can guess all the other data.
For CHECKOUT we will need the borrowernumber and the barcode.
Note that the way to enter the data for the preview is not really
user-friendly, for CHECKOUT you will have to fill
'borrowernumber|barcode', but the placeholder will help you to know how
and what to fill.
In the modal window, you will see 4 blocks:
1/ the content of the letter (with the placeholds << >>)
2/ the generated message (with the data filled)
3/ if the letter contained historical syntax markers, the screen will
try to generate a notice template using the TT syntax
4/ the generated message from this TT syntax
=> You will be able to compare the 2 generated messages.

What is the goal of this first patchset:
- Show this first POC and get feedback from other developpers
- Add a way to easily visualise the differences between the 2 syntaxes
- Confort users with the TT syntax and the migration step from the
historical syntax.

I'd like to get opinions before going further.

The possibilities:
- Mock data to get fully working generated messages for any notice
templates. For instance, for CHECKIN and CHECKOUT, the item is not
checked in/out yet. So we cannot access the issue's information.
(I have no idea how to do that)
- Browse the data to get the ones we want to use for the preview (big).

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 52 Jonathan Druart 2018-04-10 15:17:48 UTC
Created attachment 73950 [details] [review]
Bug 17981: Better error handling

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 53 Jonathan Druart 2018-04-10 15:17:52 UTC
Created attachment 73951 [details] [review]
Bug 17981: Move the divs to the top of the modal

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 54 Jonathan Druart 2018-04-10 15:17:56 UTC
Created attachment 73952 [details] [review]
Bug 17981: Remove uneeded module JS var

As we only support CHECKIN and CHECKOUT for now, we do not need this
module variable. It may be useful to re-add it later.
Comment 55 Josef Moravec 2018-04-11 18:22:08 UTC
Looks good, just one thing so far:

When the notice is in TT syntax, the svc/letters/preview and with ISE: Can't use string ("") as a HASH ref while "strict refs" in use at /home/vagrant/kohaclone/svc/letters/preview line 238.
Comment 56 Jonathan Druart 2018-04-11 18:55:26 UTC
Created attachment 74025 [details] [review]
Bug 17981: Do not crash if the template is already using TT syntax
Comment 57 Josef Moravec 2018-04-11 19:26:22 UTC
Created attachment 74027 [details] [review]
Bug 17981: Move svc/letters to svc/letters/get

To allow another script under svc/letters

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 58 Josef Moravec 2018-04-11 19:26:29 UTC
Created attachment 74028 [details] [review]
Bug 17981: Add a 'letter' param to GetPreparedLetter

This will allow us to call GetPreparedLetter with a letter content that
does not exist in the DB

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 59 Josef Moravec 2018-04-11 19:26:36 UTC
Created attachment 74029 [details] [review]
Bug 17981: Add a preview mode for notice templates

This patch is a first step to provide a preview mode for notice
templates.

CHECKIN, CHECKOUT and HOLD_SLIP are supported so far.
Maybe more, but I have not tested yet and the interface will not allow
you to generate the preview.

The idea is to provide an idea of how will render the messages generated
from a notice template.

A new "Preview" button is added close to each textarea on the editing
notice templates view.
For each notice template code (letter_code), we will need some input
data to produce the preview.

For instance, for CHECKIN we need an barcode. From the barcode we
can guess all the other data.
For CHECKOUT we will need the borrowernumber and the barcode.
Note that the way to enter the data for the preview is not really
user-friendly, for CHECKOUT you will have to fill
'borrowernumber|barcode', but the placeholder will help you to know how
and what to fill.
In the modal window, you will see 4 blocks:
1/ the content of the letter (with the placeholds << >>)
2/ the generated message (with the data filled)
3/ if the letter contained historical syntax markers, the screen will
try to generate a notice template using the TT syntax
4/ the generated message from this TT syntax
=> You will be able to compare the 2 generated messages.

What is the goal of this first patchset:
- Show this first POC and get feedback from other developpers
- Add a way to easily visualise the differences between the 2 syntaxes
- Confort users with the TT syntax and the migration step from the
historical syntax.

I'd like to get opinions before going further.

The possibilities:
- Mock data to get fully working generated messages for any notice
templates. For instance, for CHECKIN and CHECKOUT, the item is not
checked in/out yet. So we cannot access the issue's information.
(I have no idea how to do that)
- Browse the data to get the ones we want to use for the preview (big).

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 60 Josef Moravec 2018-04-11 19:26:43 UTC
Created attachment 74030 [details] [review]
Bug 17981: Better error handling

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 61 Josef Moravec 2018-04-11 19:26:49 UTC
Created attachment 74031 [details] [review]
Bug 17981: Move the divs to the top of the modal

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 62 Josef Moravec 2018-04-11 19:26:55 UTC
Created attachment 74032 [details] [review]
Bug 17981: Remove uneeded module JS var

As we only support CHECKIN and CHECKOUT for now, we do not need this
module variable. It may be useful to re-add it later.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 63 Josef Moravec 2018-04-11 19:27:01 UTC
Created attachment 74033 [details] [review]
Bug 17981: Do not crash if the template is already using TT syntax

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 64 Josef Moravec 2018-04-11 19:27:07 UTC
Created attachment 74034 [details] [review]
Bug 17981: (QA follow-up) Fix and update pod in svc/letters/get

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 65 Jonathan Druart 2018-04-12 15:05:53 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 66 Nick Clemens 2018-05-14 19:46:01 UTC
Enhancement, not backported for 17.11.x
Comment 67 David Cook 2020-05-18 23:50:30 UTC
Going to double-check but this might be broken in 19.11...
Comment 68 David Cook 2020-07-01 04:54:42 UTC
(In reply to David Cook from comment #67)
> Going to double-check but this might be broken in 19.11...

I must've been mistaken as it looks fine now.