Bug 23354

Summary: Add a 'Point of sale' screen to allow anonymous payments
Product: Koha Reporter: Martin Renvoize <martin.renvoize>
Component: Point of SaleAssignee: Martin Renvoize <martin.renvoize>
Status: CLOSED FIXED QA Contact: Josef Moravec <josef.moravec>
Severity: new feature    
Priority: P5 - low CC: 1joynelson, gmcharlt, hayleypelham, jonathan.druart, josef.moravec, katrin.fischer, kyle, martin.renvoize, sally.healey, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
The new feature adds point of sale functionality to Koha. When enabled, a new "Point of sale" screen will be available from the staff client home page. From this screen, one can build up a transaction consisting of various items defined in the account debit types administration area and then process the transaction anonymously making a 'sale' to the end-user. The payment type, cash register and staff user id's are all stored for later auditing purposes. **New system preference**: `EnablePointOfSale` defaults to disabled.
Version(s) released in:
20.05.00
Bug Depends on: 23442    
Bug Blocks: 24491, 28599, 36029, 24477, 24478, 24479, 24481, 24482, 24490, 24525, 24775, 24790, 24791, 26160, 26161, 28181    
Attachments: Bug 23354: Initial work on payment screen
Bug 23354: Add a Point Of Sale 'pay' screen
Bug 23354: Add receipting to Pay page
Bug 23354: Add ability to remove items
Console errors
Bug 23354: Add a Point Of Sale 'pay' screen
Bug 23354: Add receipting to Pay page
Bug 23354: Add ability to remove items
Bug 23354: (follow-up) Responses to feedback
Bug 23354: Add permissions
Bug 23354: Add a Point Of Sale 'pay' screen
Bug 23354: Add receipting to Pay page
Bug 23354: Add ability to remove items
Bug 23354: (follow-up) Responses to feedback
Bug 23354: Add permissions
Bug 23354: Add tests for Koha::Charges::Sales
Bug 23354: Add a Point Of Sale 'pay' screen
Bug 23354: Add receipting to Pay page
Bug 23354: Add ability to remove items
Bug 23354: (follow-up) Responses to feedback
Bug 23354: Add permissions
Bug 23354: Add tests for Koha::Charges::Sales
Bug 23354: Add a Point Of Sale 'pay' screen
Bug 23354: Add receipting to Pay page
Bug 23354: Add ability to remove items
Bug 23354: (follow-up) Responses to feedback
Bug 23354: Add permissions
Bug 23354: Add tests for Koha::Charges::Sales
Bug 23354: (follow-up) Add till icon
Bug 23354: (follow-up) Input type on jEditable
Bug 23354: (follow-up) Use 'format_price' for totals
Bug 23354: Add a Point Of Sale 'pay' screen
Bug 23354: Add receipting to Pay page
Bug 23354: Add ability to remove items
Bug 23354: (follow-up) Responses to feedback
Bug 23354: Add permissions
Bug 23354: Add tests for Koha::Charges::Sales
Bug 23354: (follow-up) Add till icon
Bug 23354: (follow-up) Input type on jEditable
Bug 23354: (follow-up) Use 'format_price' for totals
Bug 23354: Add a Point Of Sale 'pay' screen
Bug 23354: Add receipting to Pay page
Bug 23354: Add ability to remove items
Bug 23354: (follow-up) Responses to feedback
Bug 23354: Add permissions
Bug 23354: Add tests for Koha::Charges::Sales
Bug 23354: (follow-up) Input type on jEditable
Bug 23354: (follow-up) Use 'format_price' for totals
Bug 23354: Add Icon for POS system
Bug 23354: Add a Point Of Sale 'pay' screen
Bug 23354: Add receipting to Pay page
Bug 23354: Add ability to remove items
Bug 23354: (follow-up) Responses to feedback
Bug 23354: Add permissions
Bug 23354: Add tests for Koha::Charges::Sales
Bug 23354: (follow-up) Input type on jEditable
Bug 23354: (follow-up) Use 'format_price' for totals
Bug 23354: Add Icon for POS system
Bug 23354: Update to account for bug 23049
Bug 23354: (follow-up) Make display of debit types configurable
Bug 23354: Add a Point Of Sale 'pay' screen
Bug 23354: Add receipting to Pay page
Bug 23354: Add ability to remove items
Bug 23354: (follow-up) Responses to feedback
Bug 23354: Add permissions
Bug 23354: Add tests for Koha::Charges::Sales
Bug 23354: (follow-up) Input type on jEditable
Bug 23354: (follow-up) Use 'format_price' for totals
Bug 23354: Add Icon for POS system
Bug 23354: Update to account for bug 23049
Bug 23354: (follow-up) Make display of debit types configurable
Bug 23354: (follow-up) Ensure 'CASH' payment type is present
Bug 23354: (follow-up) Update for bugs 23049 and 23805
Bug 23354: (follow-up) Fix for sending code instead of description
Bug 23354: Format price on item add
Bug 23354: Prevent negatives from being entered
Bug 23354: Add a Point Of Sale 'pay' screen
Bug 23354: Add a Point Of Sale 'pay' screen
Bug 23354: Add receipting to Pay page
Bug 23354: Add ability to remove items
Bug 23354: (follow-up) Responses to feedback
Bug 23354: Add permissions
Bug 23354: Add tests for Koha::Charges::Sales
Bug 23354: (follow-up) Input type on jEditable
Bug 23354: (follow-up) Use 'format_price' for totals
Bug 23354: Add Icon for POS system
Bug 23354: Update to account for bug 23049
Bug 23354: (follow-up) Make display of debit types configurable
Bug 23354: (follow-up) Ensure 'CASH' payment type is present
Bug 23354: (follow-up) Update for bugs 23049 and 23805
Bug 23354: (follow-up) Fix for sending code instead of description
Bug 23354: Format price on item add
Bug 23354: Prevent negatives from being entered
Bug 23354: (QA follow-up) Mark boolean columns in schema
Bug 23354: (QA follow-up) Add missing filters, remove tab chars
POS
Bug 23354: (QA follow-up) Allow manual receipt printing
Bug 23354: (QA follow-up) Rename test file
Bug 23354: Add a Point Of Sale 'pay' screen
Bug 23354: Add receipting to Pay page
Bug 23354: Add ability to remove items
Bug 23354: (follow-up) Responses to feedback
Bug 23354: Add permissions
Bug 23354: Add tests for Koha::Charges::Sales
Bug 23354: (follow-up) Input type on jEditable
Bug 23354: (follow-up) Use 'format_price' for totals
Bug 23354: Add Icon for POS system
Bug 23354: Update to account for bug 23049
Bug 23354: (follow-up) Make display of debit types configurable
Bug 23354: (follow-up) Ensure 'CASH' payment type is present
Bug 23354: (follow-up) Update for bugs 23049 and 23805
Bug 23354: (follow-up) Fix for sending code instead of description
Bug 23354: Format price on item add
Bug 23354: Prevent negatives from being entered
Bug 23354: (QA follow-up) Mark boolean columns in schema
Bug 23354: (QA follow-up) Add missing filters, remove tab chars
Bug 23354: (QA follow-up) Allow manual receipt printing
Bug 23354: (QA follow-up) Rename test file
Bug 23354: Add a Point Of Sale 'pay' screen
Bug 23354: Add receipting to Pay page
Bug 23354: Add ability to remove items
Bug 23354: (follow-up) Responses to feedback
Bug 23354: Add permissions
Bug 23354: Add tests for Koha::Charges::Sales
Bug 23354: (follow-up) Input type on jEditable
Bug 23354: (follow-up) Use 'format_price' for totals
Bug 23354: Add Icon for POS system
Bug 23354: Update to account for bug 23049
Bug 23354: (follow-up) Make display of debit types configurable
Bug 23354: (follow-up) Ensure 'CASH' payment type is present
Bug 23354: (follow-up) Update for bugs 23049 and 23805
Bug 23354: (follow-up) Fix for sending code instead of description
Bug 23354: Format price on item add
Bug 23354: Prevent negatives from being entered
Bug 23354: (QA follow-up) Mark boolean columns in schema
Bug 23354: (QA follow-up) Add missing filters, remove tab chars
Bug 23354: (QA follow-up) Allow manual receipt printing
Bug 23354: (QA follow-up) Rename test file
Bug 23354: (QA follow-up) Make db update idempotent
Bug 23354: (QA follow-up) Fix spelling in templates
Bug 23354: (QA follow-up) Add takepayment permission for new installs
Bug 23354: (RM follow-up) Change $q to $input for consistency
Bug 23354: (RM follow-up) Correction for cash register selection

Description Martin Renvoize 2019-07-22 07:15:22 UTC
Many libraries now also sell a small number of goods as part of their offering, therefore the library system needs to double as a simple POS (Point Of Sale) system.

This proposed feature will add a 'Pay' screen to the UI allowing for anonymous payments to be made against goods.
Comment 1 Martin Renvoize 2019-08-08 11:32:56 UTC Comment hidden (obsolete)
Comment 2 Martin Renvoize 2019-08-08 11:34:51 UTC
This is now ready for testing.. I'd love some feedback on the UX and things before adding the necessary tests and things to get it through QA.
Comment 3 Hayley Pelham 2019-08-13 21:43:30 UTC
(In reply to Martin Renvoize from comment #2)
> This is now ready for testing.. I'd love some feedback on the UX and things
> before adding the necessary tests and things to get it through QA.

Hi Martin,

Could you expand on the test plan a bit? Just want to make sure I don't miss anything :)
Comment 4 Martin Renvoize 2019-08-22 07:20:58 UTC Comment hidden (obsolete)
Comment 5 Martin Renvoize 2019-08-22 07:21:01 UTC Comment hidden (obsolete)
Comment 6 Martin Renvoize 2019-08-22 07:21:04 UTC Comment hidden (obsolete)
Comment 7 Martin Renvoize 2019-08-22 07:23:46 UTC
(In reply to Hayley Mapley from comment #3)
> (In reply to Martin Renvoize from comment #2)
> > This is now ready for testing.. I'd love some feedback on the UX and things
> > before adding the necessary tests and things to get it through QA.
> 
> Hi Martin,
> 
> Could you expand on the test plan a bit? Just want to make sure I don't miss
> anything :)

Hi Hayley, 

Thanks for showing interesting in this.  I think the included test plans in the patches should now go some way towards a decent test plan for this, but do feel free to ask questions.  Any feedback would be greatly appreciated.
Comment 8 Hayley Pelham 2019-08-30 01:36:40 UTC Comment hidden (obsolete)
Comment 9 Hayley Pelham 2019-08-30 01:38:30 UTC
Hi Martin,

Thanks for the test plans!
I get up to 7) on the first patch, and when I click the Add button it doesn't add the item to the This sale box. No plack-errors, but the console had some output which I've attached in the screenshot I attached.
Comment 10 Martin Renvoize 2019-09-16 11:59:24 UTC Comment hidden (obsolete)
Comment 11 Martin Renvoize 2019-09-16 11:59:27 UTC Comment hidden (obsolete)
Comment 12 Martin Renvoize 2019-09-16 11:59:30 UTC Comment hidden (obsolete)
Comment 13 Martin Renvoize 2019-09-16 11:59:33 UTC Comment hidden (obsolete)
Comment 14 Martin Renvoize 2019-09-16 11:59:36 UTC Comment hidden (obsolete)
Comment 15 Martin Renvoize 2019-09-16 12:01:48 UTC
Sorry Hayley, it just needed a re-base after the new datatables update recently pushed.. I've also added a followup bug that handles a few points brought up by the sponsors of the feature.
Comment 16 Martin Renvoize 2019-09-17 11:15:29 UTC Comment hidden (obsolete)
Comment 17 Martin Renvoize 2019-09-17 11:15:32 UTC Comment hidden (obsolete)
Comment 18 Martin Renvoize 2019-09-17 11:15:35 UTC Comment hidden (obsolete)
Comment 19 Martin Renvoize 2019-09-17 11:15:38 UTC Comment hidden (obsolete)
Comment 20 Martin Renvoize 2019-09-17 11:15:41 UTC Comment hidden (obsolete)
Comment 21 Martin Renvoize 2019-09-17 11:15:44 UTC Comment hidden (obsolete)
Comment 22 Martin Renvoize 2019-09-17 11:29:49 UTC Comment hidden (obsolete)
Comment 23 Martin Renvoize 2019-09-17 11:29:52 UTC Comment hidden (obsolete)
Comment 24 Martin Renvoize 2019-09-17 11:29:55 UTC Comment hidden (obsolete)
Comment 25 Martin Renvoize 2019-09-17 11:29:58 UTC Comment hidden (obsolete)
Comment 26 Martin Renvoize 2019-09-17 11:30:02 UTC Comment hidden (obsolete)
Comment 27 Martin Renvoize 2019-09-17 11:30:05 UTC Comment hidden (obsolete)
Comment 28 Tomás Cohen Arazi 2019-09-20 19:37:50 UTC
The feature works, and the code is pretty clean. I have a few remarks to be considered:

- We should really have a separate table and stop using MANUAL_INV. It feels really dirty to have a note field contain a decimal value to be used as a number. To start with. I think we agree on this, and there's even work from Josef that would do it.
- Things look misaligned to me (hidpi laptop).
- 'Collected from patron' should reuse some patron searching things already exist in the codebase. Thinking of the dialog to pick a guarantor.
- Currency symbol is the pound one, no matter what the system is configured to use.
- 'Click on the item cost' maybe instead of 'Click on the item'.
- Are you planning on having an inventory of things to sell? I think the use of authorised values is very limited, besides my criticism. I expect librarians wanting to just scan the barcode and get the price on the screen.
Comment 29 Martin Renvoize 2019-09-23 12:59:21 UTC
(In reply to Tomás Cohen Arazi from comment #28)
> The feature works, and the code is pretty clean. I have a few remarks to be
> considered:
> 
> - We should really have a separate table and stop using MANUAL_INV. It feels
> really dirty to have a note field contain a decimal value to be used as a
> number. To start with. I think we agree on this, and there's even work from
> Josef that would do it.

Totally agree here.. I was taking the pragmatic approach of not letting that work hold this up as we already have the MANUAL_INV code in place.. I intend to switch to our own table as a further bug down the line as we have more sponsorship in the pipeline for adding features which would require that distinction.

> - Things look misaligned to me (hidpi laptop).

Did you run the yarn build?  I've tried to fix alignment once already, but perhaps there are some bits still needing a little work.

> - 'Collected from patron' should reuse some patron searching things already
> exist in the codebase. Thinking of the dialog to pick a guarantor.

Not sure what you mean here? The 'Collected from patron' input is meant to be a monetary value. (Though I do also like the idea of enhancing the screen to allow a patron to be optionally linked to the transaction)

> - Currency symbol is the pound one, no matter what the system is configured
> to use.

Oops.. I'll post a followup for that now.

> - 'Click on the item cost' maybe instead of 'Click on the item'.

Good catch.

> - Are you planning on having an inventory of things to sell? I think the use
> of authorised values is very limited, besides my criticism. I expect
> librarians wanting to just scan the barcode and get the price on the screen.

I very much do want to add such functionality, including things like barcodes, tax rates etc.. but I see that as part of a bigger piece to migrate MANUAL_INC outside of authorized values.
Comment 30 Tomás Cohen Arazi 2019-09-23 13:07:18 UTC
(In reply to Martin Renvoize from comment #29)
> (In reply to Tomás Cohen Arazi from comment #28)
> > The feature works, and the code is pretty clean. I have a few remarks to be
> > considered:
> > 
> > - We should really have a separate table and stop using MANUAL_INV. It feels
> > really dirty to have a note field contain a decimal value to be used as a
> > number. To start with. I think we agree on this, and there's even work from
> > Josef that would do it.
> 
> Totally agree here.. I was taking the pragmatic approach of not letting that
> work hold this up as we already have the MANUAL_INV code in place.. I intend
> to switch to our own table as a further bug down the line as we have more
> sponsorship in the pipeline for adding features which would require that
> distinction.

Ok.

> > - Things look misaligned to me (hidpi laptop).
> 
> Did you run the yarn build?  I've tried to fix alignment once already, but
> perhaps there are some bits still needing a little work.

That must be it. Re-testing today.

> > - 'Collected from patron' should reuse some patron searching things already
> > exist in the codebase. Thinking of the dialog to pick a guarantor.
> 
> Not sure what you mean here? The 'Collected from patron' input is meant to
> be a monetary value. (Though I do also like the idea of enhancing the screen
> to allow a patron to be optionally linked to the transaction)

I got it wrong! Ok!

> > - Currency symbol is the pound one, no matter what the system is configured
> > to use.
> 
> Oops.. I'll post a followup for that now.
> 
> > - 'Click on the item cost' maybe instead of 'Click on the item'.
> 
> Good catch.
> 
> > - Are you planning on having an inventory of things to sell? I think the use
> > of authorised values is very limited, besides my criticism. I expect
> > librarians wanting to just scan the barcode and get the price on the screen.
> 
> I very much do want to add such functionality, including things like
> barcodes, tax rates etc.. but I see that as part of a bigger piece to
> migrate MANUAL_INC outside of authorized values.

I mentioned it only just in case we could spot something on the code preventing such enhancement being a smooth addition.
Comment 31 Martin Renvoize 2019-09-23 13:16:22 UTC Comment hidden (obsolete)
Comment 32 Martin Renvoize 2019-09-23 13:16:26 UTC Comment hidden (obsolete)
Comment 33 Martin Renvoize 2019-09-23 13:16:30 UTC Comment hidden (obsolete)
Comment 34 Martin Renvoize 2019-09-23 13:16:34 UTC Comment hidden (obsolete)
Comment 35 Martin Renvoize 2019-09-23 13:16:39 UTC Comment hidden (obsolete)
Comment 36 Martin Renvoize 2019-09-23 13:16:43 UTC Comment hidden (obsolete)
Comment 37 Martin Renvoize 2019-09-23 13:16:48 UTC Comment hidden (obsolete)
Comment 38 Martin Renvoize 2019-09-23 13:16:52 UTC Comment hidden (obsolete)
Comment 39 Martin Renvoize 2019-09-23 13:16:55 UTC Comment hidden (obsolete)
Comment 40 Martin Renvoize 2019-10-02 13:22:17 UTC Comment hidden (obsolete)
Comment 41 Martin Renvoize 2019-10-02 13:22:20 UTC Comment hidden (obsolete)
Comment 42 Martin Renvoize 2019-10-02 13:22:24 UTC Comment hidden (obsolete)
Comment 43 Martin Renvoize 2019-10-02 13:22:28 UTC Comment hidden (obsolete)
Comment 44 Martin Renvoize 2019-10-02 13:22:32 UTC Comment hidden (obsolete)
Comment 45 Martin Renvoize 2019-10-02 13:22:36 UTC Comment hidden (obsolete)
Comment 46 Martin Renvoize 2019-10-02 13:22:40 UTC Comment hidden (obsolete)
Comment 47 Martin Renvoize 2019-10-02 13:22:43 UTC Comment hidden (obsolete)
Comment 48 Martin Renvoize 2019-10-02 13:22:47 UTC Comment hidden (obsolete)
Comment 49 Martin Renvoize 2019-10-03 13:47:59 UTC Comment hidden (obsolete)
Comment 50 Martin Renvoize 2019-10-03 13:48:03 UTC Comment hidden (obsolete)
Comment 51 Martin Renvoize 2019-10-03 13:48:07 UTC Comment hidden (obsolete)
Comment 52 Martin Renvoize 2019-10-03 13:48:10 UTC Comment hidden (obsolete)
Comment 53 Martin Renvoize 2019-10-03 13:48:14 UTC Comment hidden (obsolete)
Comment 54 Martin Renvoize 2019-10-03 13:48:17 UTC Comment hidden (obsolete)
Comment 55 Martin Renvoize 2019-10-03 13:48:21 UTC Comment hidden (obsolete)
Comment 56 Martin Renvoize 2019-10-03 13:48:25 UTC Comment hidden (obsolete)
Comment 57 Martin Renvoize 2019-10-03 13:48:28 UTC Comment hidden (obsolete)
Comment 58 Martin Renvoize 2019-10-24 16:55:10 UTC
(In reply to Tomás Cohen Arazi from comment #28)
> - We should really have a separate table and stop using MANUAL_INV. It feels
> really dirty to have a note field contain a decimal value to be used as a
> number. To start with. I think we agree on this, and there's even work from
> Josef that would do it.

Done, I've rebased all this work on top of bug 23049 and bug 23805 (which are now pushed) :)

> - Are you planning on having an inventory of things to sell? I think the use
> of authorised values is very limited, besides my criticism. I expect
> librarians wanting to just scan the barcode and get the price on the screen.

This is my intention still, but at a later stage.
Comment 59 Martin Renvoize 2019-10-24 16:55:57 UTC Comment hidden (obsolete)
Comment 60 Martin Renvoize 2019-10-24 16:56:01 UTC Comment hidden (obsolete)
Comment 61 Martin Renvoize 2019-10-24 16:56:05 UTC Comment hidden (obsolete)
Comment 62 Martin Renvoize 2019-10-24 16:56:08 UTC Comment hidden (obsolete)
Comment 63 Martin Renvoize 2019-10-24 16:56:12 UTC Comment hidden (obsolete)
Comment 64 Martin Renvoize 2019-10-24 16:56:15 UTC Comment hidden (obsolete)
Comment 65 Martin Renvoize 2019-10-24 16:56:19 UTC Comment hidden (obsolete)
Comment 66 Martin Renvoize 2019-10-24 16:56:23 UTC Comment hidden (obsolete)
Comment 67 Martin Renvoize 2019-10-24 16:56:26 UTC Comment hidden (obsolete)
Comment 68 Martin Renvoize 2019-10-24 16:56:30 UTC Comment hidden (obsolete)
Comment 69 Martin Renvoize 2019-10-24 16:56:33 UTC Comment hidden (obsolete)
Comment 70 Martin Renvoize 2019-11-21 16:21:49 UTC Comment hidden (obsolete)
Comment 71 Martin Renvoize 2019-11-21 16:21:55 UTC Comment hidden (obsolete)
Comment 72 Martin Renvoize 2019-11-21 16:22:01 UTC Comment hidden (obsolete)
Comment 73 Martin Renvoize 2019-11-21 16:22:07 UTC Comment hidden (obsolete)
Comment 74 Martin Renvoize 2019-11-21 16:22:14 UTC Comment hidden (obsolete)
Comment 75 Martin Renvoize 2019-11-21 16:22:20 UTC Comment hidden (obsolete)
Comment 76 Martin Renvoize 2019-11-21 16:22:26 UTC Comment hidden (obsolete)
Comment 77 Martin Renvoize 2019-11-21 16:22:32 UTC Comment hidden (obsolete)
Comment 78 Martin Renvoize 2019-11-21 16:22:38 UTC Comment hidden (obsolete)
Comment 79 Martin Renvoize 2019-11-21 16:22:44 UTC Comment hidden (obsolete)
Comment 80 Martin Renvoize 2019-11-21 16:22:53 UTC Comment hidden (obsolete)
Comment 81 Martin Renvoize 2019-11-21 16:22:59 UTC Comment hidden (obsolete)
Comment 82 Martin Renvoize 2019-11-21 16:23:06 UTC Comment hidden (obsolete)
Comment 83 Martin Renvoize 2019-11-21 16:23:14 UTC Comment hidden (obsolete)
Comment 84 Martin Renvoize 2019-11-21 16:23:21 UTC Comment hidden (obsolete)
Comment 85 Martin Renvoize 2019-11-21 16:23:30 UTC Comment hidden (obsolete)
Comment 86 Kyle M Hall 2020-01-08 16:08:55 UTC Comment hidden (obsolete)
Comment 87 Kyle M Hall 2020-01-08 16:09:18 UTC Comment hidden (obsolete)
Comment 88 Kyle M Hall 2020-01-08 16:09:40 UTC Comment hidden (obsolete)
Comment 89 Kyle M Hall 2020-01-08 16:09:44 UTC Comment hidden (obsolete)
Comment 90 Kyle M Hall 2020-01-08 16:09:48 UTC Comment hidden (obsolete)
Comment 91 Kyle M Hall 2020-01-08 16:09:51 UTC Comment hidden (obsolete)
Comment 92 Kyle M Hall 2020-01-08 16:09:55 UTC Comment hidden (obsolete)
Comment 93 Kyle M Hall 2020-01-08 16:09:59 UTC Comment hidden (obsolete)
Comment 94 Kyle M Hall 2020-01-08 16:10:02 UTC Comment hidden (obsolete)
Comment 95 Kyle M Hall 2020-01-08 16:10:06 UTC Comment hidden (obsolete)
Comment 96 Kyle M Hall 2020-01-08 16:10:10 UTC Comment hidden (obsolete)
Comment 97 Kyle M Hall 2020-01-08 16:10:15 UTC Comment hidden (obsolete)
Comment 98 Kyle M Hall 2020-01-08 16:10:21 UTC Comment hidden (obsolete)
Comment 99 Kyle M Hall 2020-01-08 16:10:26 UTC Comment hidden (obsolete)
Comment 100 Kyle M Hall 2020-01-08 16:10:30 UTC Comment hidden (obsolete)
Comment 101 Kyle M Hall 2020-01-08 16:10:35 UTC Comment hidden (obsolete)
Comment 102 Kyle M Hall 2020-01-08 16:10:39 UTC Comment hidden (obsolete)
Comment 103 Josef Moravec 2020-01-10 09:25:28 UTC Comment hidden (obsolete)
Comment 104 Josef Moravec 2020-01-10 09:25:42 UTC Comment hidden (obsolete)
Comment 105 Josef Moravec 2020-01-10 09:30:50 UTC
Comment on attachment 97019 [details] [review]
Bug 23354: Add tests for Koha::Charges::Sales

Review of attachment 97019 [details] [review]:
-----------------------------------------------------------------

The file t/db_dependent/Koha/Charges/Sales.pm should be renamed to *.t

::: t/db_dependent/Koha/Charges/Sales.pm
@@ +139,5 @@
> +
> +    my $library2 = $builder->build_object( { class => 'Koha::Libraries' } );
> +    my $av = Koha::AuthorisedValue->new(
> +        {
> +            category         => 'MANUAL_INV',

Should be credit type object, not manual inv auth value
Comment 106 Josef Moravec 2020-01-10 09:36:17 UTC
Hi Martin,

looks good, I added to little follow-ups and spotted the problem in test (see my previous comment)

I have only one concern about printing receipt: I would like to see ability to print receipt manually (maybe using modal which is implemented to confirm change return). With current patches it is possible to print receipt only if automatic printing is enabled. But it also enables automatic printing in patrons accounting. I do think that there should be possibility to print receipt from POS manually too.
Comment 107 Josef Moravec 2020-01-10 09:38:32 UTC
Created attachment 97171 [details]
POS

Also I can see some visual glitches on POS page, see attachment
Comment 108 Martin Renvoize 2020-01-10 11:33:00 UTC
Thanks for taking a look and the followups Josef :)

Annoyed at myself for missing the filters.. this patchset was initially created long enough ago that they were not required... I aught to have updated it though by now :(

As for the display issues, did you run the yarn build step? (I may have forgotten to include it in the test plan :( ).

Finally, the reciept printing.. I'm not sure I can print it from the change modal (that doesn't always display depending on whether you actually need to receive change or not and also the transaction hasn't yet taken place at that point so we don't have any data to refer to.  I'll try to come up with an alternative though, as I agree it's not great having it only tied to the preference as it stands (You do however get the ability to print receipts in the next patch in the series which adds a 'cashup' page of historical transactions).

Followups incoming as I fix the receipting UX.
Comment 109 Martin Renvoize 2020-01-10 11:50:31 UTC Comment hidden (obsolete)
Comment 110 Martin Renvoize 2020-01-10 11:53:27 UTC Comment hidden (obsolete)
Comment 111 Martin Renvoize 2020-01-10 13:41:59 UTC Comment hidden (obsolete)
Comment 112 Martin Renvoize 2020-01-10 13:42:03 UTC Comment hidden (obsolete)
Comment 113 Martin Renvoize 2020-01-10 13:42:07 UTC Comment hidden (obsolete)
Comment 114 Martin Renvoize 2020-01-10 13:42:11 UTC Comment hidden (obsolete)
Comment 115 Martin Renvoize 2020-01-10 13:42:15 UTC Comment hidden (obsolete)
Comment 116 Martin Renvoize 2020-01-10 13:42:19 UTC Comment hidden (obsolete)
Comment 117 Martin Renvoize 2020-01-10 13:42:23 UTC Comment hidden (obsolete)
Comment 118 Martin Renvoize 2020-01-10 13:42:27 UTC Comment hidden (obsolete)
Comment 119 Martin Renvoize 2020-01-10 13:42:31 UTC Comment hidden (obsolete)
Comment 120 Martin Renvoize 2020-01-10 13:42:35 UTC Comment hidden (obsolete)
Comment 121 Martin Renvoize 2020-01-10 13:42:39 UTC Comment hidden (obsolete)
Comment 122 Martin Renvoize 2020-01-10 13:42:43 UTC Comment hidden (obsolete)
Comment 123 Martin Renvoize 2020-01-10 13:42:48 UTC Comment hidden (obsolete)
Comment 124 Martin Renvoize 2020-01-10 13:42:52 UTC Comment hidden (obsolete)
Comment 125 Martin Renvoize 2020-01-10 13:42:56 UTC Comment hidden (obsolete)
Comment 126 Martin Renvoize 2020-01-10 13:43:00 UTC Comment hidden (obsolete)
Comment 127 Martin Renvoize 2020-01-10 13:43:04 UTC Comment hidden (obsolete)
Comment 128 Martin Renvoize 2020-01-10 13:43:08 UTC Comment hidden (obsolete)
Comment 129 Martin Renvoize 2020-01-10 13:43:12 UTC Comment hidden (obsolete)
Comment 130 Martin Renvoize 2020-01-10 13:43:16 UTC Comment hidden (obsolete)
Comment 131 Martin Renvoize 2020-01-10 13:45:12 UTC
Ready for a second QA round now, hopefully, that's everything fixed. :)

Remember to `yarn build` to get the display issues corrected.

Thanks Josef :)
Comment 132 Josef Moravec 2020-01-12 20:03:36 UTC
Created attachment 97271 [details] [review]
Bug 23354: Add a Point Of Sale 'pay' screen

This patch adds a new Point Of Sale module to Koha's staff client front
page. The module button leads directly to a 'Pay' page giving the staff
user the ability to record anonymous payments for items that would not
normally require a patron to be registered at the library.

Test plan:
1) Enable `UseCashRegisters` via the system preferences.
2) Ensure your user has the 'manage_cash_registers' permission.
3) Add a cash register for your current branch.
4) Add at least one 'MANUAL_INV' authorized value.
5) Navigate to the new 'POS' pay page via the main menu.
6) Add an item to the 'sale' by clicking 'add' from the right side of
   the screen.
7) Note that said item was added to the table of items this sale on the
   left.
8) At this point you should be able to 'click to edit' the quantity or
   price of the item in the table on the left.
9) Enter an amount greater than the price of the item into the 'amount
   collected from patron' box.
10) Click 'Confirm'
11) Varify that the same change to give modal from the paycollect pages
    appears here.
12) Click 'Confirm'
13) Payment will have been recorded (check the database) and you will be
    back at a fresh 'Pay' page ready for the next transaction.
14) Signoff

Sponsored-by: PTFS Europe
Sponsored-by: Cheshire Libraries Shared Services

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 133 Josef Moravec 2020-01-12 20:03:43 UTC
Created attachment 97272 [details] [review]
Bug 23354: Add receipting to Pay page

This patch adds receipt printing to the new Point of Sale pay page.

Test plan:
1) Apply patch and run database update
2) Enable automatic receipt printing via the `` system preference.
3) Make a payment for an item via the new POS pay page.
4) Note that a receipt printing dialogue is shown automatically after
   payment.
5) Note that a new notice is available under tools where you can alter
   the content of the receipt.
6) Signoff

Sponsored-by: PTFS Europe
Sponsored-by: Cheshire Libraries Shared Services

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 134 Josef Moravec 2020-01-12 20:03:49 UTC
Created attachment 97273 [details] [review]
Bug 23354: Add ability to remove items

This patch adds the ability to remove items from a transactions on the
new Point of Sale pay page.

Test plan:
1) Populate a transaction with at least one item.
2) Note the new button to 'Remove item' on the items table.
3) Click the button and confirm the item is removed
4) Signoff

Sponsored-by: PTFS Europe
Sponsored-by: Cheshire Libraries Shared Services

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 135 Josef Moravec 2020-01-12 20:03:54 UTC
Created attachment 97274 [details] [review]
Bug 23354: (follow-up) Responses to feedback

This patch impliments a number of refinements requested after the first
round of feedback from the sponsors.

1) Fix padding on pay page
2) Switch 'items to add' from right to left on the page
3) Fix editable columns on pay page
4) Add explanitory text to pay page

Sponsored-by: PTFS Europe
Sponsored-by: Cheshire Libraries Shared Services

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 136 Josef Moravec 2020-01-12 20:04:00 UTC
Created attachment 97275 [details] [review]
Bug 23354: Add permissions

Sponsored-by: PTFS Europe
Sponsored-by: Cheshire Libraries Shared Services

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 137 Josef Moravec 2020-01-12 20:04:07 UTC
Created attachment 97276 [details] [review]
Bug 23354: Add tests for Koha::Charges::Sales

Sponsored-by: PTFS Europe
Sponsored-by: Cheshire Libraries Shared Services

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 138 Josef Moravec 2020-01-12 20:04:14 UTC
Created attachment 97277 [details] [review]
Bug 23354: (follow-up) Input type on jEditable

Update the jEditable plugin to a maintained version and use the new
'number' type to enable increment arrows on click and remap onblur event
to submit.

Sponsored-by: PTFS Europe
Sponsored-by: Cheshire Libraries Shared Services

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 139 Josef Moravec 2020-01-12 20:04:23 UTC
Created attachment 97278 [details] [review]
Bug 23354: (follow-up) Use 'format_price' for totals

Sponsored-by: PTFS Europe
Sponsored-by: Cheshire Libraries Shared Services

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 140 Josef Moravec 2020-01-12 20:04:31 UTC
Created attachment 97279 [details] [review]
Bug 23354: Add Icon for POS system

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 141 Josef Moravec 2020-01-12 20:04:38 UTC
Created attachment 97280 [details] [review]
Bug 23354: Update to account for bug 23049

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 142 Josef Moravec 2020-01-12 20:04:45 UTC
Created attachment 97281 [details] [review]
Bug 23354: (follow-up) Make display of debit types configurable

This patch adds a the ability to define where a debit type will be
available as a option for use.

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 143 Josef Moravec 2020-01-12 20:04:57 UTC
Created attachment 97282 [details] [review]
Bug 23354: (follow-up) Ensure 'CASH' payment type is present

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 144 Josef Moravec 2020-01-12 20:05:19 UTC
Created attachment 97283 [details] [review]
Bug 23354: (follow-up) Update for bugs 23049 and 23805

Remove all mentions of accounttype and replace with debit_type_code and
credit_type_code as appropriate.

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 145 Josef Moravec 2020-01-12 20:05:30 UTC
Created attachment 97284 [details] [review]
Bug 23354: (follow-up) Fix for sending code instead of description

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 146 Josef Moravec 2020-01-12 20:05:37 UTC
Created attachment 97285 [details] [review]
Bug 23354: Format price on item add

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 147 Josef Moravec 2020-01-12 20:05:45 UTC
Created attachment 97286 [details] [review]
Bug 23354: Prevent negatives from being entered

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 148 Josef Moravec 2020-01-12 20:05:57 UTC
Created attachment 97287 [details] [review]
Bug 23354: (QA follow-up) Mark boolean columns in schema

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 149 Josef Moravec 2020-01-12 20:06:05 UTC
Created attachment 97288 [details] [review]
Bug 23354: (QA follow-up) Add missing filters, remove tab chars

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 150 Josef Moravec 2020-01-12 20:06:13 UTC
Created attachment 97289 [details] [review]
Bug 23354: (QA follow-up) Allow manual receipt printing

This patch adds an alert message that allows receipt printing for
transactions when FinePaymentAutoPopup is not enabled.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 151 Josef Moravec 2020-01-12 20:06:29 UTC
Created attachment 97290 [details] [review]
Bug 23354: (QA follow-up) Rename test file

Note to RM: Squash on push

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 152 Josef Moravec 2020-01-12 20:06:36 UTC
Created attachment 97291 [details] [review]
Bug 23354: (QA follow-up) Make db update idempotent

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 153 Josef Moravec 2020-01-12 20:06:50 UTC
Created attachment 97292 [details] [review]
Bug 23354: (QA follow-up) Fix spelling in templates

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 154 Josef Moravec 2020-01-12 20:09:27 UTC
(In reply to Martin Renvoize from comment #131)
> Ready for a second QA round now, hopefully, that's everything fixed. :)
> 
> Remember to `yarn build` to get the display issues corrected.
> 
> Thanks Josef :)


I thought I did run 'yarn build', but maybe not...

Anyway I just added two more small follow-ups and passed QA, great job!
Comment 155 Martin Renvoize 2020-01-13 14:07:10 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 156 Jonathan Druart 2020-01-13 15:57:17 UTC
Notice templates are missing in .sql files

xt/sample_notices.t .. 1/?
#   Failed test 'No sample notice to add'
#   at xt/sample_notices.t line 83.
# Sample notices to add in fr-FR/1-Obligatoire/sample_notices.sql: RECEIPT

#   Failed test 'No sample notice to add'
#   at xt/sample_notices.t line 83.
# Sample notices to add in fr-CA/obligatoire/sample_notices.sql: RECEIPT

#   Failed test 'No sample notice to add'
#   at xt/sample_notices.t line 83.
# Sample notices to add in de-DE/mandatory/sample_notices.sql: RECEIPT

#   Failed test 'No sample notice to add'
#   at xt/sample_notices.t line 83.
# Sample notices to add in es-ES/mandatory/sample_notices.sql: RECEIPT

#   Failed test 'No sample notice to add'
#   at xt/sample_notices.t line 83.
# Sample notices to add in it-IT/necessari/notices.sql: RECEIPT

#   Failed test 'No sample notice to add'
#   at xt/sample_notices.t line 83.
# Sample notices to add in nb-NO/1-Obligatorisk/sample_notices.sql: RECEIPT

#   Failed test 'No sample notice to add'
#   at xt/sample_notices.t line 83.
# Sample notices to add in pl-PL/mandatory/sample_notices.sql: RECEIPT

#   Failed test 'No sample notice to add'
#   at xt/sample_notices.t line 83.
# Sample notices to add in ru-RU/mandatory/sample_notices.sql: RECEIPT

#   Failed test 'No sample notice to add'
#   at xt/sample_notices.t line 83.
# Sample notices to add in uk-UA/mandatory/sample_notices.sql: RECEIPT
# Looks like you failed 9 tests of 38.
Comment 157 Jonathan Druart 2020-01-13 16:02:49 UTC Comment hidden (obsolete)
Comment 158 Jonathan Druart 2020-01-13 16:07:12 UTC
The column account_debit_types.can_be_added_manually has been removed:

-  `can_be_added_manually` tinyint(4) NOT NULL DEFAULT 1,

Is that expected?

Test is failing because of that:
t/db_dependent/Koha/Account/DebitType.t .. DBIx::Class::Row::store_column(): No such column 'can_be_added_manually' on Koha::Schema::Result::AccountDebitType at /kohadevbox/koha/Koha/Object.pm line 77
Comment 159 Martin Renvoize 2020-01-13 16:47:14 UTC
Two followups pushed
Comment 160 Joy Nelson 2020-01-14 00:17:12 UTC
Not backported to 19.11.x
Comment 161 Jonathan Druart 2020-01-22 09:49:41 UTC
my $q         = CGI->new();

We already have $input and $cgi :)
Comment 162 Martin Renvoize 2020-01-22 12:16:01 UTC
(In reply to Jonathan Druart from comment #161)
> my $q         = CGI->new();
> 
> We already have $input and $cgi :)

Fixed in RM followup.
Comment 163 Nick Clemens 2020-01-22 13:45:49 UTC
Created attachment 97725 [details] [review]
Bug 23354: (QA follow-up) Add takepayment permission for new installs
Comment 164 Martin Renvoize 2020-01-22 13:48:23 UTC
Created attachment 97726 [details] [review]
Bug 23354: (RM follow-up) Change $q to $input for consistency

As pointed out by Jonathan, this script introduced a third form for the
CGI variable. This patch updates the script to use the more common
$input variable name.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 165 Martin Renvoize 2020-03-05 08:54:20 UTC
Created attachment 100167 [details] [review]
Bug 23354: (RM follow-up) Correction for cash register selection

The form element for selectin cash register override at point of sale was
misnamed and as such one could not actually override the cash register.

This patch corrects the form element name and updates the logic very
slighlty to ensure we still fall back to the defualt no the subsequent
page load.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>