Bug 28586

Summary: Cannot resolve a claim
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: REST APIAssignee: Tomás Cohen Arazi <tomascohen>
Status: Pushed to oldoldoldstable --- QA Contact: Kyle M Hall <kyle>
Severity: critical    
Priority: P5 - low CC: andrewfh, david, fridolin.somers, kyle, nick, tomascohen, victor, wainuiwitikapark
Version: masterKeywords: rel_21_05_candidate
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
This fixes an issue with the 'Returned claims' feature (enabled by setting a value for ClaimReturnedLostValue)- resolving returned claims now works as expected. Before this fix, an attempt to resolve a claim resulted in the page hanging and the claim not being able to be resolved.
Version(s) released in:
21.11.00,21.05.01,20.11.08,20.05.14,19.11.20
Bug Depends on:    
Bug Blocks: 28271    
Attachments: Bug 28586: Pass the right parameter to resolve claim
Bug 28586: (follow-up) updated_by should be set
Bug 28586: Pass the right parameter to resolve claim
Bug 28586: (follow-up) updated_by should be set
Bug 28586: Pass the right parameter to resolve claim
Bug 28586: (follow-up) updated_by should be set
Bug 28586: Pass the right parameter to resolve claim
Bug 28586: (follow-up) updated_by should be set
Bug 28586: Pass the right parameter to resolve claim
Bug 28586: (follow-up) updated_by should be set

Description Jonathan Druart 2021-06-17 14:44:31 UTC
[2021-06-17 14:42:51.00970] [909] [warn] OpenAPI <<< PUT api/v1/return_claims/1/resolve [{"message":"Properties not allowed: updated_by.","path":"\/body"}]

To recreate:
Checkout
Claim it
Go to claims
Action > resolve > resolve claim
Comment 1 Tomás Cohen Arazi 2021-06-17 15:51:08 UTC
Created attachment 122083 [details] [review]
Bug 28586: Pass the right parameter to resolve claim

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi 2021-06-17 15:55:30 UTC
This is yet another case highlighted by adding additionalProperties: false.
Comment 3 Tomás Cohen Arazi 2021-06-17 19:07:30 UTC
Created attachment 122092 [details] [review]
Bug 28586: (follow-up) updated_by should be set

This patch makes the route set the 'updated_by' attribute as well, when
resolving a return claim through the API.

Tests are added for this behavior.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/return_claims.t
=> SUCCESS: Tests pass! updated_by is set correctly!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 David Nind 2021-06-18 09:17:40 UTC
Created attachment 122110 [details] [review]
Bug 28586: Pass the right parameter to resolve claim

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2021-06-18 09:17:43 UTC
Created attachment 122111 [details] [review]
Bug 28586: (follow-up) updated_by should be set

This patch makes the route set the 'updated_by' attribute as well, when
resolving a return claim through the API.

Tests are added for this behavior.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/return_claims.t
=> SUCCESS: Tests pass! updated_by is set correctly!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2021-06-18 10:23:38 UTC
Testing notes (koha-testing-docker):
- To reproduce the issue:
  . Set a value for ClaimReturnedLostValue, for example: 4
  . Check out an item to a patron
  . Go to the patron's check out page > show check outs
  . In the 'Return claims' column select 'Claim returned', enter a note and click 'Make claim"
  . Go to to the Claim(s) tab and select Actions > Resolve > select a resolution reason from the drop down list > Click on 'Resolve claim'
  . Select a reason and enter some text
- Before the patch is applied the page just hangs and there is the [warn] message (see bug description) in /var/log/koha/kohadev/plack-error.log
- After the patch is applied (and browser cache cleared) the browser no longer hangs and there is no warning message in the log
Comment 7 Andrew Fuerste-Henry 2021-06-18 11:51:17 UTC
Created attachment 122124 [details] [review]
Bug 28586: Pass the right parameter to resolve claim

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 8 Andrew Fuerste-Henry 2021-06-18 11:51:21 UTC
Created attachment 122125 [details] [review]
Bug 28586: (follow-up) updated_by should be set

This patch makes the route set the 'updated_by' attribute as well, when
resolving a return claim through the API.

Tests are added for this behavior.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/return_claims.t
=> SUCCESS: Tests pass! updated_by is set correctly!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 9 Nick Clemens 2021-06-18 12:50:47 UTC
Created attachment 122126 [details] [review]
Bug 28586: Pass the right parameter to resolve claim

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 10 Nick Clemens 2021-06-18 12:50:52 UTC
Created attachment 122127 [details] [review]
Bug 28586: (follow-up) updated_by should be set

This patch makes the route set the 'updated_by' attribute as well, when
resolving a return claim through the API.

Tests are added for this behavior.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/return_claims.t
=> SUCCESS: Tests pass! updated_by is set correctly!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 11 Nick Clemens 2021-06-18 13:04:51 UTC
Created attachment 122129 [details] [review]
Bug 28586: Pass the right parameter to resolve claim

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 12 Nick Clemens 2021-06-18 13:04:55 UTC
Created attachment 122130 [details] [review]
Bug 28586: (follow-up) updated_by should be set

This patch makes the route set the 'updated_by' attribute as well, when
resolving a return claim through the API.

Tests are added for this behavior.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/return_claims.t
=> SUCCESS: Tests pass! updated_by is set correctly!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 13 Jonathan Druart 2021-06-21 10:07:05 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 14 Kyle M Hall 2021-06-25 13:32:05 UTC
(In reply to Jonathan Druart from comment #13)
> Pushed to master for 21.11, thanks to everybody involved!

Pushed to 21.05.x for 21.05.01
Comment 15 Fridolin Somers 2021-07-10 01:29:49 UTC
Pushed to 20.11.x for 20.11.08
Comment 16 Victor Grousset/tuxayo 2021-07-10 23:31:11 UTC
Backported: Pushed to 20.05.x branch for 20.05.14
Comment 17 wainuiwitikapark 2021-07-12 01:27:30 UTC
Backported to 19.11.x for 19.11.20