Bug 27204

Summary: SIP patron information request with fee line items returns incorrect data
Product: Koha Reporter: Kyle M Hall <kyle>
Component: SIP2Assignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: andrewfh, caroline.cyr-la-rose, colin.campbell, fridolin.somers, martin.renvoize, victor
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.03,20.05.09
Attachments: Bug 27204: Add unit tests for fine_items
Bug 27204: Fix end boundary index, never return results for non-existant accountlines
Bug 27204: Add unit tests for fine_items
Bug 27204: Fix end boundary index, never return results for non-existant accountlines
Bug 27204: Add unit tests for fine_items
Bug 27204: Fix end boundary index, never return results for non-existant accountlines

Description Kyle M Hall 2020-12-11 12:08:26 UTC
There are two primary issues I've identified with requesting line items for fees via SIP:
1) The end boundary is incorrect. For example, if send a request with a BP ( starting item) of 1, and a BQ (end item) of 1, I should get just the first item. Instead I will get two items

2) Our SIP server does not check bounds. For example, if I have 3 fines, but I send a BP of 1 and a BQ of 5, I will get back 5 AVs, two of them being "empty" because the patron only has 3 accountlines!
Comment 1 Kyle M Hall 2020-12-11 12:16:42 UTC
Created attachment 114338 [details] [review]
Bug 27204: Add unit tests for fine_items
Comment 2 Kyle M Hall 2020-12-11 12:16:51 UTC
Created attachment 114339 [details] [review]
Bug 27204: Fix end boundary index, never return results for non-existant accountlines

There are two primary issues I've identified with requesting line items
for fees via SIP:

1) The end boundary is incorrect. For example, if send a request with a
BP ( starting item) of 1, and a BQ (end item) of 1, I should get just
the first item. Instead I will get two items

2) Our SIP server does not check bounds. For example, if I have 3 fines,
but I send a BP of 1 and a BQ of 5, I will get back 5 AVs, two of
them being "empty" because the patron only has 3 accountlines!

Test Plan:
1) Apply the unit test patch
2) prove t/db_dependent/SIP/Patron.t
3) Note the failures
4) Apply the second patch
5) prove t/db_dependent/SIP/Patron.t
6) All tests should pass!
Comment 3 Martin Renvoize 2021-01-26 16:26:44 UTC
Created attachment 115824 [details] [review]
Bug 27204: Add unit tests for fine_items

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Martin Renvoize 2021-01-26 16:26:47 UTC
Created attachment 115825 [details] [review]
Bug 27204: Fix end boundary index, never return results for non-existant accountlines

There are two primary issues I've identified with requesting line items
for fees via SIP:

1) The end boundary is incorrect. For example, if send a request with a
BP ( starting item) of 1, and a BQ (end item) of 1, I should get just
the first item. Instead I will get two items

2) Our SIP server does not check bounds. For example, if I have 3 fines,
but I send a BP of 1 and a BQ of 5, I will get back 5 AVs, two of
them being "empty" because the patron only has 3 accountlines!

Test Plan:
1) Apply the unit test patch
2) prove t/db_dependent/SIP/Patron.t
3) Note the failures
4) Apply the second patch
5) prove t/db_dependent/SIP/Patron.t
6) All tests should pass!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2021-01-26 16:27:34 UTC
Nice catch, 

Signing off
Comment 6 Katrin Fischer 2021-01-31 14:40:55 UTC
Created attachment 116119 [details] [review]
Bug 27204: Add unit tests for fine_items

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Katrin Fischer 2021-01-31 14:40:59 UTC
Created attachment 116120 [details] [review]
Bug 27204: Fix end boundary index, never return results for non-existant accountlines

There are two primary issues I've identified with requesting line items
for fees via SIP:

1) The end boundary is incorrect. For example, if send a request with a
BP ( starting item) of 1, and a BQ (end item) of 1, I should get just
the first item. Instead I will get two items

2) Our SIP server does not check bounds. For example, if I have 3 fines,
but I send a BP of 1 and a BQ of 5, I will get back 5 AVs, two of
them being "empty" because the patron only has 3 accountlines!

Test Plan:
1) Apply the unit test patch
2) prove t/db_dependent/SIP/Patron.t
3) Note the failures
4) Apply the second patch
5) prove t/db_dependent/SIP/Patron.t
6) All tests should pass!

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Katrin Fischer 2021-01-31 14:41:39 UTC
Test makes sense and passes tests. For functionality testing I trust Kyle and Martin here, they are the SIP experts.
Comment 9 Jonathan Druart 2021-02-01 16:26:32 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 10 Fridolin Somers 2021-02-05 15:42:07 UTC
Pushed to 20.11.x for 20.11.03
Comment 11 Andrew Fuerste-Henry 2021-02-09 12:44:54 UTC
Pushed to 20.05.x for 20.05.09
Comment 12 Victor Grousset/tuxayo 2021-02-11 01:47:44 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.