Bug 31036 - Cash management doesn't take SIP00 (Cash via SIP2) transactions into account
Summary: Cash management doesn't take SIP00 (Cash via SIP2) transactions into account
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Martin Renvoize (ashimema)
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks: 13985
  Show dependency tree / graph
 
Reported: 2022-06-24 07:09 UTC by Martin Renvoize (ashimema)
Modified: 2023-06-08 22:26 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fix adds the last missing piece for cash management when involving transactions via a SIP client. We now understand that a SIP00 coded transaction is equal to 'CASH' in other register environments. This means we treat it as such in the cashup system and also that we now require a register for cash transactions. WARNING: This makes register a required configuration field for SIP devices when cash registers are enabled on the system.
Version(s) released in:
22.11.00, 22.05.05, 21.11.12
Circulation function:

Attachments
Bug 31036: Treat SIP00 that same as CASH (7.86 KB, patch)
2022-06-24 08:22 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 31036: Treat SIP00 that same as CASH (8.48 KB, patch)
2022-06-24 08:31 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 31036: Treat SIP00 that same as CASH (8.61 KB, patch)
2022-07-08 15:30 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 31036: Treat SIP00 that same as CASH (8.66 KB, patch)
2022-07-15 13:37 UTC, Katrin Fischer 		Details | Diff | Splinter Review
Bug 31036: (QA follow-up) Add a warning to the system preference (1.44 KB, patch)
2022-07-15 14:22 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 31036: (QA follow-up) Add a warning to the system preference (1.39 KB, patch)
2022-07-15 14:23 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 2022-06-24 07:09:36 UTC 
Currently, all cash management summations are bound to the CASH payment type, but SIP2 devices can also take Cash payments and record them as SIP00.

Cash management should be updated to use 'CASH' OR 'SIP00' wherever we currently limit to 'CASH'.
Comment 1 Martin Renvoize (ashimema) 2022-06-24 08:22:35 UTC 
Created attachment 136476 [details] [review]
Bug 31036: Treat SIP00 that same as CASH

SIP00 is the code for CASH transactions that have taken place on a SIP
client.  Cash management treats CASH type transactions as a special case
for banking and register requirements. This patch brings SIP00 in line
with the CASH handling code of cash management.
Comment 2 Martin Renvoize (ashimema) 2022-06-24 08:31:26 UTC 
Created attachment 136477 [details] [review]
Bug 31036: Treat SIP00 that same as CASH

SIP00 is the code for CASH transactions that have taken place on a SIP
client.  Cash management treats CASH type transactions as a special case
for banking and register requirements. This patch brings SIP00 in line
with the CASH handling code of cash management.

Test plan
1) Enable cash management with 'EnablePointOfSale'
2) Enable cash registers with 'UseCashRegisters'
3) Add a cash register via 'Administration > Cash registers'
4) Make some payments against the ash register in the staff client
5) Attempt to make a payment via a SIP client without having associated
   a cash register to the SIP account config
5a) This should now fail
6) Attach the cash register to the SIP account used above
6a) Payment should now work again on SIP transactions
7) Look at the cash register details page for the cash register
7a) 'Bankale' should now include cash from both SIP and Staff client
sales
Comment 3 Martin Renvoize (ashimema) 2022-07-08 15:30:11 UTC 
Created attachment 137431 [details] [review]
Bug 31036: Treat SIP00 that same as CASH

SIP00 is the code for CASH transactions that have taken place on a SIP
client.  Cash management treats CASH type transactions as a special case
for banking and register requirements. This patch brings SIP00 in line
with the CASH handling code of cash management.

Test plan
1) Enable cash management with 'EnablePointOfSale'
2) Enable cash registers with 'UseCashRegisters'
3) Add a cash register via 'Administration > Cash registers'
4) Make some payments against the ash register in the staff client
5) Attempt to make a payment via a SIP client without having associated
   a cash register to the SIP account config
5a) This should now fail
6) Attach the cash register to the SIP account used above
6a) Payment should now work again on SIP transactions
7) Look at the cash register details page for the cash register
7a) 'Bankale' should now include cash from both SIP and Staff client
sales

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 4 Martin Renvoize (ashimema) 2022-07-08 15:31:32 UTC 
This has been deployed for a while now.. signing off
Comment 5 Katrin Fischer 2022-07-15 13:37:48 UTC 
Created attachment 137757 [details] [review]
Bug 31036: Treat SIP00 that same as CASH

SIP00 is the code for CASH transactions that have taken place on a SIP
client.  Cash management treats CASH type transactions as a special case
for banking and register requirements. This patch brings SIP00 in line
with the CASH handling code of cash management.

Test plan
1) Enable cash management with 'EnablePointOfSale'
2) Enable cash registers with 'UseCashRegisters'
3) Add a cash register via 'Administration > Cash registers'
4) Make some payments against the ash register in the staff client
5) Attempt to make a payment via a SIP client without having associated
   a cash register to the SIP account config
5a) This should now fail
6) Attach the cash register to the SIP account used above
6a) Payment should now work again on SIP transactions
7) Look at the cash register details page for the cash register
7a) 'Bankale' should now include cash from both SIP and Staff client
sales

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 6 Katrin Fischer 2022-07-15 13:39:22 UTC 
> 5) Attempt to make a payment via a SIP client without having associated
> a cash register to the SIP account config
> 5a) This should now fail

I think this would warrant a 'bold' note in the release notes as it has the potential to break self checks unexpectedly. 

Could it be an option to make it fail gracefully/with a good error message?
Comment 7 Martin Renvoize (ashimema) 2022-07-15 13:48:58 UTC 
It's a fair point.. I struggled to come up with a solution.. either a big bold warning next to the system preference that enables cash registers.. or perhaps a soft fallback to the first register we find on the system in SIP if there isn't one in the sip config..

Or.. both..  I feel like the soft fallback could lead to unexpected results at cashup too.

Any preference?
Comment 8 Katrin Fischer 2022-07-15 13:51:26 UTC 
I think the soft fallback is potentially more dangerous, so the note would be better. And I think this should probably not be backported.
Comment 9 Martin Renvoize (ashimema) 2022-07-15 14:22:13 UTC Comment hidden (obsolete)
Comment 10 Martin Renvoize (ashimema) 2022-07-15 14:23:36 UTC 
Created attachment 137763 [details] [review]
Bug 31036: (QA follow-up) Add a warning to the system preference

As suggested by Katrin, adding a warning to the system preference that
this has an effect on SIP configurations that needs to be understood.
Comment 11 Martin Renvoize (ashimema) 2022-07-15 14:24:39 UTC 
Added text for release notes and added a warning to the system preference in a QA follow-up as discussed :)
Comment 12 Tomás Cohen Arazi (tcohen) 2022-07-18 13:31:28 UTC 
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 13 Lucas Gass (lukeg) 2022-08-23 17:21:36 UTC 
Backported to 22.05.x for 22.05.05
Comment 14 Arthur Suzuki 2022-08-30 09:20:26 UTC 
thx pushed to 21.11.x for 21.11.12
Comment 15 Victor Grousset/tuxayo 2022-09-09 23:53:44 UTC 
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document, marking resolved.