Bug 21898 - Add basket info available for ACQORDER
Summary: Add basket info available for ACQORDER
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-28 11:12 UTC by Katrin Fischer
Modified: 2020-11-16 21:24 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
This enhancement adds the aqbasket variable to the AQORDER notice processor. This allows users to utilise basket details in the subject and content of their `AQORDER` notices.
Version(s) released in:
20.11.00


Attachments
Bug 21898: Add aqbasket to available tables for AQORDER (1.77 KB, patch)
2020-10-01 08:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21898: Add aqbasket to available tables for AQORDER (2.24 KB, patch)
2020-10-01 08:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21898: Add aqbasket to available tables for AQORDER (2.38 KB, patch)
2020-10-20 11:38 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 21898: Add aqbasket to available tables for AQORDER (2.44 KB, patch)
2020-10-20 13:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 21898: Add aqbasket to available tables for ACQORDER (2.55 KB, patch)
2020-11-04 09:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21898: Add basket for TT syntax (1.03 KB, patch)
2020-11-04 09:58 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2018-11-28 11:12:50 UTC
It would be nice if the order number or basket name could be included in the subject of the ACQORDER email. At the moment, the <<aqbasket.basketname>> is not replaced when sending the email.
Comment 1 Martin Renvoize 2020-10-01 08:05:59 UTC
Created attachment 111024 [details] [review]
Bug 21898: Add aqbasket to available tables for AQORDER
Comment 2 Martin Renvoize 2020-10-01 08:12:28 UTC
I really wish we'd bite the bullet and work through all notices converting them to TT and then deprecate the old syntax.. It's really challenging to maintain both and features are not carrying across between the two.
Comment 3 Martin Renvoize 2020-10-01 08:17:06 UTC
Created attachment 111025 [details] [review]
Bug 21898: Add aqbasket to available tables for AQORDER

This patch should allow <<aqbasket.$field>> replacements to work in both
email subject and email content fields for notices.

Test plan
1/ Add <<aqbasket.basketname>> into the AQORDER notice (In the subject
line and content box)
2/ Trigger said notice to send (before applying the patch)
3/ Note that the replacements have not taken place..
<<aqbasket.basketname>> is present as is.
4/ Apply patch
5/ Trigger the notice again
6/ Note that the replacements are now made.
7/ Signoff
Comment 4 Séverine Queune 2020-10-20 11:38:38 UTC
Created attachment 112027 [details] [review]
Bug 21898: Add aqbasket to available tables for AQORDER

This patch should allow <<aqbasket.$field>> replacements to work in both
email subject and email content fields for notices.

Test plan
1/ Add <<aqbasket.basketname>> into the AQORDER notice (In the subject
line and content box)
2/ Trigger said notice to send (before applying the patch)
3/ Note that the replacements have not taken place..
<<aqbasket.basketname>> is present as is.
4/ Apply patch
5/ Trigger the notice again
6/ Note that the replacements are now made.
7/ Signoff

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 5 Séverine Queune 2020-10-20 11:43:38 UTC
Patch works fine !
Note I didn't have problem on master on step 3 : basketname was correctly displayed on the content of email.
Comment 6 Katrin Fischer 2020-10-20 13:36:48 UTC
Created attachment 112039 [details] [review]
Bug 21898: Add aqbasket to available tables for AQORDER

This patch should allow <<aqbasket.$field>> replacements to work in both
email subject and email content fields for notices.

Test plan
1/ Add <<aqbasket.basketname>> into the AQORDER notice (In the subject
line and content box)
2/ Trigger said notice to send (before applying the patch)
3/ Note that the replacements have not taken place..
<<aqbasket.basketname>> is present as is.
4/ Apply patch
5/ Trigger the notice again
6/ Note that the replacements are now made.
7/ Signoff

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Martin Renvoize 2020-10-21 11:13:18 UTC
Bug 26745 would go really nicely with this one ;)
Comment 8 Jonathan Druart 2020-11-04 09:51:51 UTC
Created attachment 112979 [details] [review]
Bug 21898: Add aqbasket to available tables for ACQORDER

This patch should allow <<aqbasket.$field>> replacements to work in both
email subject and email content fields for notices.

Test plan
1/ Add <<aqbasket.basketname>> into the ACQORDER notice (In the subject
line and content box)
2/ Trigger said notice to send (before applying the patch)
3/ Note that the replacements have not taken place..
<<aqbasket.basketname>> is present as is.
4/ Apply patch
5/ Trigger the notice again
6/ Note that the replacements are now made.
7/ Signoff

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

JD amended patch: fix alignment in hashref, edit commit message to fix
letter code "AQORDER" vs "ACQORDER"
Comment 9 Jonathan Druart 2020-11-04 09:58:45 UTC
Created attachment 112980 [details] [review]
Bug 21898: Add basket for TT syntax

Without this patch the basket's info won't be available for the TT
syntax.

With this patch you can now use [% basket.basketname %]
Comment 10 Jonathan Druart 2020-11-04 12:02:50 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 11 Jonathan Druart 2020-11-04 13:38:28 UTC
There is a failing test, can you have a look, Martin?

#   Failed test 'SendAlerts - claimissue'
#   at t/db_dependent/Letters.t line 642.
C4::Letters::_substitute_tables(): DBI Exception: DBD::mysql::st execute failed: called with 2 bind variables when 1 are needed [for Statement "SELECT * FROM aqbasket WHERE       basketno = ?" with ParamValues: 0=26] at /kohadevbox/koha/C4/Letters.pm line 679
Comment 12 Jonathan Druart 2020-11-05 11:11:14 UTC
(In reply to Jonathan Druart from comment #11)
> There is a failing test, can you have a look, Martin?
> 
> #   Failed test 'SendAlerts - claimissue'
> #   at t/db_dependent/Letters.t line 642.
> C4::Letters::_substitute_tables(): DBI Exception: DBD::mysql::st execute
> failed: called with 2 bind variables when 1 are needed [for Statement
> "SELECT * FROM aqbasket WHERE       basketno = ?" with ParamValues: 0=26] at
> /kohadevbox/koha/C4/Letters.pm line 679

Fixed on bug 26935.
Comment 13 Lucas Gass 2020-11-16 21:24:10 UTC
enhancement will not be backported to 20.05.x