Bug 28374

Summary: Convert pos/printreceipt.pl to use GetPreparedLetter
Product: Koha Reporter: Martin Renvoize (ashimema) <martin.renvoize>
Component: Architecture, internals, and plumbingAssignee: David Nind <david>
Status: CLOSED FIXED QA Contact: Tomás Cohen Arazi (tcohen) <tomascohen>
Severity: enhancement    
Priority: P5 - low CC: andrew, david, jonathan.druart, kyle, lucas, nick, sally.healey, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26787
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This patch converts the point of sale receipt printer controller to using GetPreparedLetter instead of calling getletter directly.
Version(s) released in:
21.11.00
Circulation function:
Bug Depends on: 28368    
Bug Blocks: 13985    
Attachments: Bug 28373: Convert printreceipt to use GetPreparedLetter
Bug 28374: Convert printreceipt to use GetPreparedLetter
Bug 28374: Convert printreceipt to use GetPreparedLetter
Bug 28374: Convert printreceipt to use GetPreparedLetter
Bug 28374: Update existing notices
Bug 28374 - Print receipt is blank after patches applied
Bug 28374: Convert printreceipt to use GetPreparedLetter
Bug 28374: Update existing notices
Bug 28374: Convert printreceipt to use GetPreparedLetter
Bug 28374: Update existing notices
Bug 28374: Convert printreceipt to use GetPreparedLetter
Bug 28374: Update existing notices
Bug 28374: (QA follow-up) Avoid adding duplicate USE

Description Martin Renvoize (ashimema) 2021-05-18 15:08:30 UTC
It looks like somehow a patch got dropped from bug 26734 that converted the POS receipt printer to use GetPrepparedLetter too.
Comment 1 Martin Renvoize (ashimema) 2021-05-18 15:36:29 UTC
Created attachment 121116 [details] [review]
Bug 28373: Convert printreceipt to use GetPreparedLetter

This patch converts the point of sale receipt printer controller to
using GetPreparedLetter instead of calling getletter directly.

Test plan
1/ Try printing a point of sale receipt before and after applying the
patch
2/ No change should result.
Comment 2 Martin Renvoize (ashimema) 2021-05-18 15:37:29 UTC
Created attachment 121117 [details] [review]
Bug 28374: Convert printreceipt to use GetPreparedLetter

This patch converts the point of sale receipt printer controller to
using GetPreparedLetter instead of calling getletter directly.

Test plan
1/ Try printing a point of sale receipt before and after applying the
patch
2/ No change should result.
Comment 3 Martin Renvoize (ashimema) 2021-07-06 12:20:56 UTC
Created attachment 122613 [details] [review]
Bug 28374: Convert printreceipt to use GetPreparedLetter

This patch converts the point of sale receipt printer controller to
using GetPreparedLetter instead of calling getletter directly.

Test plan
1/ Try printing a point of sale receipt before and after applying the
patch
2/ No change should result.
Comment 4 Nick Clemens (kidclamp) 2021-07-07 16:42:37 UTC
I get an error after these patches - you remove the USE statements from the page template, but the letter expects them. They can be added to the notice, but I think we need a DB update then.

Additionally, the default letter is not marked as HTML, but it is formatted as html - if I don't change the notice to HTML then I get all the HTML tags
Comment 5 Martin Renvoize (ashimema) 2021-11-05 14:55:13 UTC
Created attachment 127367 [details] [review]
Bug 28374: Convert printreceipt to use GetPreparedLetter

This patch converts the point of sale receipt printer controller to
using GetPreparedLetter instead of calling getletter directly.

Test plan
1/ Try printing a point of sale receipt before and after applying the
patch
2/ No change should result.
Comment 6 Martin Renvoize (ashimema) 2021-11-05 14:55:17 UTC
Created attachment 127368 [details] [review]
Bug 28374: Update existing notices

This patch updates the default slip to include KohaDates, Branches and
Price plugins as required and prepends the same to existing notices at
upgrade time.  We also, switch the HTML flag on.
Comment 7 Martin Renvoize (ashimema) 2021-11-05 14:56:13 UTC
Sorry.. it took me ages to get back to this.. the fix was trivial too :(
Comment 8 David Nind 2021-11-07 19:59:01 UTC
Created attachment 127415 [details]
Bug 28374 - Print receipt is blank after patches applied

After the patches are applied (and database update) the receipt has no details - see the attachment.

Testing notes (koha-testing-docker):

1. Setup for testing:
   i.  Enable EnablePointOfSale and UseCashRegisters system preferences
   ii. Add a new cash register (Administration > Accounting > Cash registers)
   ii. Add one or two products for sale (Administration > Accounting > Debit types, make sure 'Can be sold?' is ticked)
2. Go to Home > Point of sale
3. Add some items for purchase to a sale
4. Click confirm
5. Click on 'Print receipt' and save the PDF file somewhere
6. Apply patches
7. Update database: updatedatabase
8. Repeat steps 2-5
9. Compare receipt PDFs - should be no changes
10. Check that the default RECEIPT is updated - for print, HTML should be ticked (Tools > 
11. Sign off!
Comment 9 Martin Renvoize (ashimema) 2021-11-08 10:52:18 UTC
Created attachment 127434 [details] [review]
Bug 28374: Convert printreceipt to use GetPreparedLetter

This patch converts the point of sale receipt printer controller to
using GetPreparedLetter instead of calling getletter directly.

Test plan
1. Setup for testing:
   i.   Enable EnablePointOfSale and UseCashRegisters system preferences
   ii.  Add a new cash register
        (Administration > Accounting > Cash registers)
   iii. Add one or two products for sale
        (Administration > Accounting > Debit types, make sure 'Can be sold?' is ticked)
2. Go to Home > Point of sale
3. Add some items for purchase to a sale
4. Click confirm
5. Click on 'Print receipt' and save the PDF file somewhere
6. Apply patches
7. Update database: updatedatabase
8. Repeat steps 2-5
9. Compare receipt PDFs - should be no changes
10. Check that the default RECEIPT is updated - for print, HTML should be ticked
11. Sign off!
Comment 10 Martin Renvoize (ashimema) 2021-11-08 10:52:21 UTC
Created attachment 127435 [details] [review]
Bug 28374: Update existing notices

This patch updates the default slip to include KohaDates, Branches and
Price plugins as required and prepends the same to existing notices at
upgrade time.  We also, switch the HTML flag on.
Comment 11 Martin Renvoize (ashimema) 2021-11-08 10:54:06 UTC
Thankyou so much for testing David!.. I'm quite embarrassed I got so much wrong in that first submission.. it seems my local notices are all significantly ahead of this patch set.

I've uploaded a fresh set of patches.. they should work for new installs and installs with an existing notice that closely resembles the original one installed.

Many thanks again,
Comment 12 David Nind 2021-11-08 11:34:28 UTC
Created attachment 127438 [details] [review]
Bug 28374: Convert printreceipt to use GetPreparedLetter

This patch converts the point of sale receipt printer controller to
using GetPreparedLetter instead of calling getletter directly.

Test plan
1. Setup for testing:
   i.   Enable EnablePointOfSale and UseCashRegisters system preferences
   ii.  Add a new cash register
        (Administration > Accounting > Cash registers)
   iii. Add one or two products for sale
        (Administration > Accounting > Debit types, make sure 'Can be sold?' is ticked)
2. Go to Home > Point of sale
3. Add some items for purchase to a sale
4. Click confirm
5. Click on 'Print receipt' and save the PDF file somewhere
6. Apply patches
7. Update database: updatedatabase
8. Repeat steps 2-5
9. Compare receipt PDFs - should be no changes
10. Check that the default RECEIPT is updated - for print, HTML should be ticked
11. Sign off!

Signed-off-by: David Nind <david@davidnind.com>
Comment 13 David Nind 2021-11-08 11:34:33 UTC
Created attachment 127439 [details] [review]
Bug 28374: Update existing notices

This patch updates the default slip to include KohaDates, Branches and
Price plugins as required and prepends the same to existing notices at
upgrade time.  We also, switch the HTML flag on.

Signed-off-by: David Nind <david@davidnind.com>
Comment 14 Tomás Cohen Arazi (tcohen) 2021-11-11 18:16:42 UTC
Created attachment 127549 [details] [review]
Bug 28374: Convert printreceipt to use GetPreparedLetter

This patch converts the point of sale receipt printer controller to
using GetPreparedLetter instead of calling getletter directly.

Test plan
1. Setup for testing:
   i.   Enable EnablePointOfSale and UseCashRegisters system preferences
   ii.  Add a new cash register
        (Administration > Accounting > Cash registers)
   iii. Add one or two products for sale
        (Administration > Accounting > Debit types, make sure 'Can be sold?' is ticked)
2. Go to Home > Point of sale
3. Add some items for purchase to a sale
4. Click confirm
5. Click on 'Print receipt' and save the PDF file somewhere
6. Apply patches
7. Update database: updatedatabase
8. Repeat steps 2-5
9. Compare receipt PDFs - should be no changes
10. Check that the default RECEIPT is updated - for print, HTML should be ticked
11. Sign off!

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 Tomás Cohen Arazi (tcohen) 2021-11-11 18:16:51 UTC
Created attachment 127550 [details] [review]
Bug 28374: Update existing notices

This patch updates the default slip to include KohaDates, Branches and
Price plugins as required and prepends the same to existing notices at
upgrade time.  We also, switch the HTML flag on.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Tomás Cohen Arazi (tcohen) 2021-11-11 18:30:04 UTC
Created attachment 127551 [details] [review]
Bug 28374: (QA follow-up) Avoid adding duplicate USE

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 17 Jonathan Druart 2021-11-15 11:00:18 UTC
I would have tried to limit a bit more the matches

* UPDATE letter SET content = REPLACE(content, 'payment.', 'credit.') WHERE code = 'RECEIPT';

Could have been payment\.

* UPDATE letter SET content = REPLACE(content, 'offset', 'debit') WHERE code = 'RECEIPT';

offset\. and debit.

* UPDATE letter SET content = REPLACE(content, 'offsets', 'credit.debits') WHERE code = 'RECEIPT';

IN offsets

Not considering blocker.
Comment 18 Jonathan Druart 2021-11-15 11:39:21 UTC
Pushed to master for 21.11, thanks to everybody involved!