Bug 26274 - Expose cash register cashup summaries via an API route.
Summary: Expose cash register cashup summaries via an API route.
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 26172
Blocks: 26272 26273 26676 26677
  Show dependency treegraph
 
Reported: 2020-08-21 14:01 UTC by Martin Renvoize
Modified: 2021-06-14 21:34 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
This enhancement adds the `/cash_registers/{cash_register_id}/cashups` and `/cashups/{cashup_id}` API endpoints. This opens up the possibility to display the cashup summaries more easily on a wider range of Koha pages.
Version(s) released in:
21.05.00


Attachments
Bug 26274: Add Cashups subclass and API classes (16.84 KB, patch)
2021-01-26 12:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26274: Allow embedding summary (2.38 KB, patch)
2021-01-26 12:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26274: Add cashups api routes (46.14 KB, patch)
2021-01-28 21:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26274: Update register.tt to use the API (10.04 KB, patch)
2021-01-28 21:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26274: Add cashups api routes (46.14 KB, patch)
2021-02-01 08:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26274: Update register.tt to use the API (10.04 KB, patch)
2021-02-01 08:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26274: Add cashups api routes (46.21 KB, patch)
2021-02-01 09:56 UTC, David Nind
Details | Diff | Splinter Review
Bug 26274: Update register.tt to use the API (10.11 KB, patch)
2021-02-01 09:56 UTC, David Nind
Details | Diff | Splinter Review
Bug 26274: Add cashups api routes (52.25 KB, patch)
2021-02-01 11:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26274: Update register.tt to use the API (10.09 KB, patch)
2021-02-01 11:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26274: Add cashups api routes (57.56 KB, patch)
2021-02-01 16:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26274: Update register.tt to use the API (10.09 KB, patch)
2021-02-01 16:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26274: Add cashups api routes (57.64 KB, patch)
2021-02-04 14:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 26274: Update register.tt to use the API (10.15 KB, patch)
2021-02-04 14:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 26274: (QA follow-up) Minor fixes (1.52 KB, patch)
2021-02-04 14:12 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 26274: (QA follow-up) Minor fixes (2.37 KB, patch)
2021-02-04 14:20 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 26274: (QA follow-up) Fix the order of summary groups (1.34 KB, patch)
2021-02-17 15:02 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2020-08-21 14:01:52 UTC
It would be useful to expose cashup summaries via an API endpoint to allow for displaying this data in various area's around Koha on demand.
Comment 1 Martin Renvoize 2021-01-26 12:01:19 UTC
Created attachment 115803 [details] [review]
Bug 26274: Add Cashups subclass and API classes
Comment 2 Martin Renvoize 2021-01-26 12:01:22 UTC
Created attachment 115804 [details] [review]
Bug 26274: Allow embedding summary
Comment 3 David Nind 2021-01-28 19:59:19 UTC
Is there a test plan, or some way to test? 8-)
Comment 4 Martin Renvoize 2021-01-28 21:07:33 UTC
Created attachment 115991 [details] [review]
Bug 26274: Add cashups api routes

This patch creates a Koha::Cash::Register::Cashup(s) class pair which
subclass Koha::Cash::Register::Action(s) and moves the cashup specific
code into these new classes to improve code separation.

We then introduce API routes based on these classes to allow fetching
a list of cashups associated to a cash register and a full cashup with
emeddable summary for individual cashups.

Test plan
1/ Run the updated unit tests.
   t/db_dependent/Koha/Cash/Register/Action.t
   t/db_dependent/Koha/Cash/Register/Cashup.t
2/ Run the incuded api tests.
   t/
Comment 5 Martin Renvoize 2021-01-28 21:07:36 UTC
Created attachment 115992 [details] [review]
Bug 26274: Update register.tt to use the API

This patch updates the existing register details page to utilise the new
api routes to gather the summary details on demand.

Test plan
1/ Enable cash registers
2/ Add some transactions
3/ Perform a cashup
4/ Click 'Summary' next to the last cashup date
5/ Note the modal appears as it did prior to the patch being applied.
6/ Check the print option still works
7/ Signoff
Comment 6 Martin Renvoize 2021-01-29 10:51:15 UTC
Sorry David, Once I dug more into this there was a little more to do.. the current patches now have attached test plans :)
Comment 7 David Nind 2021-01-29 15:50:07 UTC
Thanks Martin!

Also, the patch no longer apply! 8-(
Comment 8 Martin Renvoize 2021-02-01 08:42:18 UTC
Created attachment 116136 [details] [review]
Bug 26274: Add cashups api routes

This patch creates a Koha::Cash::Register::Cashup(s) class pair which
subclass Koha::Cash::Register::Action(s) and moves the cashup specific
code into these new classes to improve code separation.

We then introduce API routes based on these classes to allow fetching
a list of cashups associated to a cash register and a full cashup with
emeddable summary for individual cashups.

Test plan
1/ Run the updated unit tests.
   t/db_dependent/Koha/Cash/Register/Action.t
   t/db_dependent/Koha/Cash/Register/Cashup.t
2/ Run the incuded api tests.
   t/
Comment 9 Martin Renvoize 2021-02-01 08:42:22 UTC
Created attachment 116137 [details] [review]
Bug 26274: Update register.tt to use the API

This patch updates the existing register details page to utilise the new
api routes to gather the summary details on demand.

Test plan
1/ Enable cash registers
2/ Add some transactions
3/ Perform a cashup
4/ Click 'Summary' next to the last cashup date
5/ Note the modal appears as it did prior to the patch being applied.
6/ Check the print option still works
7/ Signoff
Comment 10 David Nind 2021-02-01 09:56:05 UTC
Created attachment 116142 [details] [review]
Bug 26274: Add cashups api routes

This patch creates a Koha::Cash::Register::Cashup(s) class pair which
subclass Koha::Cash::Register::Action(s) and moves the cashup specific
code into these new classes to improve code separation.

We then introduce API routes based on these classes to allow fetching
a list of cashups associated to a cash register and a full cashup with
emeddable summary for individual cashups.

Test plan
1/ Run the updated unit tests.
   t/db_dependent/Koha/Cash/Register/Action.t
   t/db_dependent/Koha/Cash/Register/Cashup.t
2/ Run the incuded api tests.
   t/

Signed-off-by: David Nind <david@davidnind.com>
Comment 11 David Nind 2021-02-01 09:56:08 UTC
Created attachment 116143 [details] [review]
Bug 26274: Update register.tt to use the API

This patch updates the existing register details page to utilise the new
api routes to gather the summary details on demand.

Test plan
1/ Enable cash registers
2/ Add some transactions
3/ Perform a cashup
4/ Click 'Summary' next to the last cashup date
5/ Note the modal appears as it did prior to the patch being applied.
6/ Check the print option still works
7/ Signoff

Signed-off-by: David Nind <david@davidnind.com>
Comment 12 David Nind 2021-02-01 09:57:48 UTC
I've signed off, but just checking about this step in the test plab:

2/ Run the incuded api tests.
   t/

I ran prove t/, so I hope that is what was required here.
Comment 13 Martin Renvoize 2021-02-01 11:34:28 UTC
Created attachment 116145 [details] [review]
Bug 26274: Add cashups api routes

This patch creates a Koha::Cash::Register::Cashup(s) class pair which
subclass Koha::Cash::Register::Action(s) and moves the cashup specific
code into these new classes to improve code separation.

We then introduce API routes based on these classes to allow fetching
a list of cashups associated to a cash register and a full cashup with
emeddable summary for individual cashups.

Test plan
1/ Run the updated unit tests.
   t/db_dependent/Koha/Cash/Register/Action.t
   t/db_dependent/Koha/Cash/Register/Cashup.t
2/ Run the incuded api tests.
   t/db_dependent/api/v1/cashups.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 14 Martin Renvoize 2021-02-01 11:34:31 UTC
Created attachment 116146 [details] [review]
Bug 26274: Update register.tt to use the API

This patch updates the existing register details page to utilise the new
api routes to gather the summary details on demand.

Test plan
1/ Enable cash registers
2/ Add some transactions
3/ Perform a cashup
4/ Click 'Summary' next to the last cashup date
5/ Note the modal appears as it did prior to the patch being applied.
6/ Check the print option still works
7/ Signoff

Signed-off-by: David Nind <david@davidnind.com>
Comment 15 Martin Renvoize 2021-02-01 11:35:33 UTC
(In reply to David Nind from comment #12)
> I've signed off, but just checking about this step in the test plab:
> 
> 2/ Run the incuded api tests.
>    t/
> 
> I ran prove t/, so I hope that is what was required here.

Oops, seems I missed writing the actual test name in that second point.. however you will have inadvertently run it by running t/

I've fixed the commit message to include it now :)

Thanks David :)
Comment 16 Martin Renvoize 2021-02-01 16:44:17 UTC
Created attachment 116174 [details] [review]
Bug 26274: Add cashups api routes

This patch creates a Koha::Cash::Register::Cashup(s) class pair which
subclass Koha::Cash::Register::Action(s) and moves the cashup specific
code into these new classes to improve code separation.

We then introduce API routes based on these classes to allow fetching
a list of cashups associated to a cash register and a full cashup with
emeddable summary for individual cashups.

Test plan
1/ Run the updated unit tests.
   t/db_dependent/Koha/Cash/Register/Action.t
   t/db_dependent/Koha/Cash/Register/Cashup.t
2/ Run the incuded api tests.
   t/db_dependent/api/v1/cashups.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 17 Martin Renvoize 2021-02-01 16:44:21 UTC
Created attachment 116175 [details] [review]
Bug 26274: Update register.tt to use the API

This patch updates the existing register details page to utilise the new
api routes to gather the summary details on demand.

Test plan
1/ Enable cash registers
2/ Add some transactions
3/ Perform a cashup
4/ Click 'Summary' next to the last cashup date
5/ Note the modal appears as it did prior to the patch being applied.
6/ Check the print option still works
7/ Signoff

Signed-off-by: David Nind <david@davidnind.com>
Comment 18 David Nind 2021-02-01 17:41:13 UTC
> Oops, seems I missed writing the actual test name in that second point..
> however you will have inadvertently run it by running t/
> 
> I've fixed the commit message to include it now :)

Thanks Martin!

Just to confirm that the tests for t/db_dependent/api/v1/cashups.t pass.

David
Comment 19 Tomás Cohen Arazi 2021-02-04 14:11:52 UTC
Created attachment 116324 [details] [review]
Bug 26274: Add cashups api routes

This patch creates a Koha::Cash::Register::Cashup(s) class pair which
subclass Koha::Cash::Register::Action(s) and moves the cashup specific
code into these new classes to improve code separation.

We then introduce API routes based on these classes to allow fetching
a list of cashups associated to a cash register and a full cashup with
emeddable summary for individual cashups.

Test plan
1/ Run the updated unit tests.
   t/db_dependent/Koha/Cash/Register/Action.t
   t/db_dependent/Koha/Cash/Register/Cashup.t
2/ Run the incuded api tests.
   t/db_dependent/api/v1/cashups.t

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 20 Tomás Cohen Arazi 2021-02-04 14:11:57 UTC
Created attachment 116325 [details] [review]
Bug 26274: Update register.tt to use the API

This patch updates the existing register details page to utilise the new
api routes to gather the summary details on demand.

Test plan
1/ Enable cash registers
2/ Add some transactions
3/ Perform a cashup
4/ Click 'Summary' next to the last cashup date
5/ Note the modal appears as it did prior to the patch being applied.
6/ Check the print option still works
7/ Signoff

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 21 Tomás Cohen Arazi 2021-02-04 14:12:01 UTC
Created attachment 116326 [details] [review]
Bug 26274: (QA follow-up) Minor fixes

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 22 Tomás Cohen Arazi 2021-02-04 14:20:17 UTC
Created attachment 116327 [details] [review]
Bug 26274: (QA follow-up) Minor fixes

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 23 Jonathan Druart 2021-02-09 11:02:46 UTC
Please fill the release notes.
Comment 24 Jonathan Druart 2021-02-12 12:11:48 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 25 Jonathan Druart 2021-02-15 14:15:14 UTC
There is a failing test on My8

    #   Failed test 'income_grouped arrayref is correct'
    #   at t/db_dependent/Koha/Cash/Register/Cashup.t line 223.
    #     Structures begin differing at:
    #          $got->[0]{debit_type}{description} = 'Overdue fine'
    #     $expected->[0]{debit_type}{description} = 'Account creation fee'
    # Looks like you failed 1 test of 29.

The order is different that the one we expect, I think we should order by created_on.
Comment 26 Martin Renvoize 2021-02-17 15:02:41 UTC
Created attachment 116939 [details] [review]
Bug 26274: (QA follow-up) Fix the order of summary groups

The grouped income/outgoing summaries were not being explicitly
ordered and it appears the default ordering for MySQL 8 is different to
that of MariaDB and previous versions of MySQL and as such was causing
test failures for that version. This patch adds an explicit sort on the
'description' for each debit_type and credit_type grouping.
Comment 27 Martin Renvoize 2021-02-17 15:04:47 UTC
(In reply to Jonathan Druart from comment #25)
> There is a failing test on My8
> 
>     #   Failed test 'income_grouped arrayref is correct'
>     #   at t/db_dependent/Koha/Cash/Register/Cashup.t line 223.
>     #     Structures begin differing at:
>     #          $got->[0]{debit_type}{description} = 'Overdue fine'
>     #     $expected->[0]{debit_type}{description} = 'Account creation fee'
>     # Looks like you failed 1 test of 29.
> 
> The order is different that the one we expect, I think we should order by
> created_on.

Actually.. I opted for an explicit sort on the description. I think this is more useful to the end user and also it prevents us hitting up against issues with the created_on field not being part of the grouping.

We could, alternatively, sort on the amounts. This is a SUM, however, and I'm not sure how nicely that plays either.. and I personally think the alpha sort is more useful to end users who may want to compare one summary to another.
Comment 28 Jonathan Druart 2021-02-17 15:31:35 UTC
Patch pushed to master, thanks Martin!
Comment 29 Fridolin Somers 2021-02-19 14:59:58 UTC
Enhancement not pushed to 20.11.x