Bug 28586 - Cannot resolve a claim
Summary: Cannot resolve a claim
Status: Pushed to oldoldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Kyle M Hall
URL:
Keywords: rel_21_05_candidate
Depends on:
Blocks: 28271
  Show dependency treegraph
 
Reported: 2021-06-17 14:44 UTC by Jonathan Druart
Modified: 2021-07-12 01:27 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
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


Attachments
Bug 28586: Pass the right parameter to resolve claim (998 bytes, patch)
2021-06-17 15:51 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 28586: (follow-up) updated_by should be set (2.35 KB, patch)
2021-06-17 19:07 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 28586: Pass the right parameter to resolve claim (1.02 KB, patch)
2021-06-18 09:17 UTC, David Nind
Details | Diff | Splinter Review
Bug 28586: (follow-up) updated_by should be set (2.40 KB, patch)
2021-06-18 09:17 UTC, David Nind
Details | Diff | Splinter Review
Bug 28586: Pass the right parameter to resolve claim (1.09 KB, patch)
2021-06-18 11:51 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 28586: (follow-up) updated_by should be set (2.47 KB, patch)
2021-06-18 11:51 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 28586: Pass the right parameter to resolve claim (1.09 KB, patch)
2021-06-18 12:50 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28586: (follow-up) updated_by should be set (2.47 KB, patch)
2021-06-18 12:50 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28586: Pass the right parameter to resolve claim (1.14 KB, patch)
2021-06-18 13:04 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28586: (follow-up) updated_by should be set (2.52 KB, patch)
2021-06-18 13:04 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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