Bug 28374 - Convert pos/printreceipt.pl to use GetPreparedLetter
Summary: Convert pos/printreceipt.pl to use GetPreparedLetter
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: David Nind
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 28368
Blocks: 13985
  Show dependency treegraph
 
Reported: 2021-05-18 15:08 UTC by Martin Renvoize
Modified: 2021-11-19 15:48 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
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


Attachments
Bug 28373: Convert printreceipt to use GetPreparedLetter (3.57 KB, patch)
2021-05-18 15:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28374: Convert printreceipt to use GetPreparedLetter (3.57 KB, patch)
2021-05-18 15:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28374: Convert printreceipt to use GetPreparedLetter (3.76 KB, patch)
2021-07-06 12:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28374: Convert printreceipt to use GetPreparedLetter (3.76 KB, patch)
2021-11-05 14:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28374: Update existing notices (2.11 KB, patch)
2021-11-05 14:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28374 - Print receipt is blank after patches applied (16.10 KB, application/pdf)
2021-11-07 19:59 UTC, David Nind
Details
Bug 28374: Convert printreceipt to use GetPreparedLetter (4.27 KB, patch)
2021-11-08 10:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28374: Update existing notices (5.16 KB, patch)
2021-11-08 10:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28374: Convert printreceipt to use GetPreparedLetter (4.32 KB, patch)
2021-11-08 11:34 UTC, David Nind
Details | Diff | Splinter Review
Bug 28374: Update existing notices (5.20 KB, patch)
2021-11-08 11:34 UTC, David Nind
Details | Diff | Splinter Review
Bug 28374: Convert printreceipt to use GetPreparedLetter (4.37 KB, patch)
2021-11-11 18:16 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 28374: Update existing notices (5.25 KB, patch)
2021-11-11 18:16 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 28374: (QA follow-up) Avoid adding duplicate USE (1.25 KB, patch)
2021-11-11 18:30 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 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 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 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 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 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 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 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 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 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 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 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 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 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 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!