Bug 17962 - TT syntax for notices - Prove that ACQ_NOTIF_ON_RECEIV is compatible
Summary: TT syntax for notices - Prove that ACQ_NOTIF_ON_RECEIV is compatible
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 17958
Blocks: 17961 17963
  Show dependency treegraph
 
Reported: 2017-01-23 13:10 UTC by Jonathan Druart
Modified: 2017-12-07 22:15 UTC (History)
2 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 17962: TT syntax for notices - Prove that ACQ_NOTIF_ON_RECEIV is compatible (6.14 KB, patch)
2017-01-23 15:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17962: Add Koha::Tmp::Order[s] (3.05 KB, patch)
2017-01-23 15:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17962: TT syntax for notices - Prove that ACQ_NOTIF_ON_RECEIV is compatible (6.21 KB, patch)
2017-01-24 19:13 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17962: Add Koha::Tmp::Order[s] (3.12 KB, patch)
2017-01-24 19:13 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-23 13:10:03 UTC

    
Comment 1 Jonathan Druart 2017-01-23 15:25:33 UTC
Created attachment 59428 [details] [review]
Bug 17962: TT syntax for notices - Prove that ACQ_NOTIF_ON_RECEIV is compatible

To make ACQ_NOTIF_ON_RECEIV TT compatible, we need to expose data from
the aqorders table. We already have a package for it in the Koha
namespace but it is based on Koha::Object[s].
The other path creates dummy Koha::Tmp::Order[s] packages to make it
usable. Of course we should use a valid Koha::Acquisition::Order[s]
based on Koha::Object, but it's outside the scope of this bug report.

This notice template is quite simple, and it's a good one to start.
From C4::Acq::NotifyOrderUsers, GetPreparedLetter is called with 4
elements: the library, the patron to notify, the biblio and the order
information.
Note that prior to this patch aqorders was filled from GetOrder, which
retrieved a lot of information from the acquisition table (aqbasket,
aqbookseller). The idea with the TT syntax is to access the data from
where it really exists. So if a user wants to display the basket name,
[% order.basket.basketname %] should be used instead.
Note that this will not work at the moment, the basket method is not
defined in the order package.

However the basic template should work as before.
The test added to TemplateToolkit proves that.

Test plan:
Use the default ACQ_NOTIF_ON_RECEIV to notify a patron that an order has
been received.
That generated template should be exactly the same as prior to this
patch.
Comment 2 Jonathan Druart 2017-01-23 15:25:41 UTC
Created attachment 59429 [details] [review]
Bug 17962: Add Koha::Tmp::Order[s]

These packages should not be used outside the TT syntax scope.
A rewrite of Koha::Acquisition::Order[s] should be considered instead.
Comment 3 Josef Moravec 2017-01-24 19:13:22 UTC
Created attachment 59525 [details] [review]
[SIGNED-OFF] Bug 17962: TT syntax for notices - Prove that ACQ_NOTIF_ON_RECEIV is compatible

To make ACQ_NOTIF_ON_RECEIV TT compatible, we need to expose data from
the aqorders table. We already have a package for it in the Koha
namespace but it is based on Koha::Object[s].
The other path creates dummy Koha::Tmp::Order[s] packages to make it
usable. Of course we should use a valid Koha::Acquisition::Order[s]
based on Koha::Object, but it's outside the scope of this bug report.

This notice template is quite simple, and it's a good one to start.
From C4::Acq::NotifyOrderUsers, GetPreparedLetter is called with 4
elements: the library, the patron to notify, the biblio and the order
information.
Note that prior to this patch aqorders was filled from GetOrder, which
retrieved a lot of information from the acquisition table (aqbasket,
aqbookseller). The idea with the TT syntax is to access the data from
where it really exists. So if a user wants to display the basket name,
[% order.basket.basketname %] should be used instead.
Note that this will not work at the moment, the basket method is not
defined in the order package.

However the basic template should work as before.
The test added to TemplateToolkit proves that.

Test plan:
Use the default ACQ_NOTIF_ON_RECEIV to notify a patron that an order has
been received.
That generated template should be exactly the same as prior to this
patch.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 4 Josef Moravec 2017-01-24 19:13:29 UTC
Created attachment 59526 [details] [review]
[SIGNED-OFF] Bug 17962: Add Koha::Tmp::Order[s]

These packages should not be used outside the TT syntax scope.
A rewrite of Koha::Acquisition::Order[s] should be considered instead.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 5 Kyle M Hall 2017-02-17 11:47:43 UTC
Pushed to master for 17.05, thanks Jonathan!
Comment 6 Katrin Fischer 2017-02-19 20:20:03 UTC
This won't get ported back to 16.11.x as it is an enhancement.