Bug 39823 - SUSHI harvest fails to display error if the provider's response does not contain Severity
Summary: SUSHI harvest fails to display error if the provider's response does not cont...
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: ERM (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Pedro Amorim
QA Contact: Nick Clemens (kidclamp)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-05-03 12:53 UTC by Nick Clemens (kidclamp)
Modified: 2025-06-30 22:34 UTC (History)
8 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes SUSHI harvests for ERM. It now displays an error message when running a report/harvest, where there is no severity marker returned for the error. Previously (even if the test connect passed), only a red bar with no error message was shown.
Version(s) released in:
25.11.00,25.05.01,24.11.06
Circulation function:


Attachments
Bug 39823: Clean-up: Mock LWP response instead of class method being tested (3.00 KB, patch)
2025-05-05 10:34 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 39823: Add tests (2.96 KB, patch)
2025-05-05 10:34 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 39823: Identify error messages based on code (2.67 KB, patch)
2025-05-05 10:34 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 39823: Clean-up: Mock LWP response instead of class method being tested (3.04 KB, patch)
2025-05-08 21:24 UTC, David Nind
Details | Diff | Splinter Review
Bug 39823: Add tests (3.00 KB, patch)
2025-05-08 21:24 UTC, David Nind
Details | Diff | Splinter Review
Bug 39823: Identify error messages based on code (2.72 KB, patch)
2025-05-08 21:24 UTC, David Nind
Details | Diff | Splinter Review
Bug 39823: Clean-up: Mock LWP response instead of class method being tested (3.10 KB, patch)
2025-05-28 12:12 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 39823: Add tests (3.06 KB, patch)
2025-05-28 12:12 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 39823: Identify error messages based on code (2.78 KB, patch)
2025-05-28 12:12 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2025-05-03 12:53:42 UTC
A library was having issues with Sushi harvests, the test connection passed, however, the harvest failed with no info.

Dumping the response on the command line:
'_content' => '{"Code": 2010, "Message": "Requestor is Not Authorized to Access Usage for Institution"}'

It seems _sushi_errors doesn't handle the case of a code and message but no severity marker

This was Gale Cengage
Comment 1 Pedro Amorim 2025-05-05 10:34:26 UTC
Created attachment 181904 [details] [review]
Bug 39823: Clean-up: Mock LWP response instead of class method being tested
Comment 2 Pedro Amorim 2025-05-05 10:34:29 UTC
Created attachment 181905 [details] [review]
Bug 39823: Add tests
Comment 3 Pedro Amorim 2025-05-05 10:34:32 UTC
Created attachment 181906 [details] [review]
Bug 39823: Identify error messages based on code

Even if severity does not exist. Additionally, only add severity to message if it exists.

Test plan:
1) Apply tests patches. Run tests (they should fail):
prove t/db_dependent/Koha/BackgroundJobs/ErmSushiHarvester.t
2) Apply this patch and run tests again (they should pass this time).

UI test:
1) Create a new data provider linked to Gale Cengage, service url:
https://api.siqcloud.online/counterapi/r51
2) Add report release 5, customer id: '123', requestor id: '123', report types: 'TR_J1'
3) Run a report for this data provider, from Feb 2025 to Apr 2025. Notice that before applying patches, the job report results in a red bar with no error message.
4) Apply patches, repeat 3. Notice it now results in a nice error message.
Comment 4 David Nind 2025-05-08 21:24:44 UTC
Created attachment 182121 [details] [review]
Bug 39823: Clean-up: Mock LWP response instead of class method being tested

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2025-05-08 21:24:46 UTC
Created attachment 182122 [details] [review]
Bug 39823: Add tests

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2025-05-08 21:24:49 UTC
Created attachment 182123 [details] [review]
Bug 39823: Identify error messages based on code

Even if severity does not exist. Additionally, only add severity to message if it exists.

Test plan:
1) Apply tests patches. Run tests (they should fail):
prove t/db_dependent/Koha/BackgroundJobs/ErmSushiHarvester.t
2) Apply this patch and run tests again (they should pass this time).

UI test:
1) Create a new data provider linked to Gale Cengage, service url:
https://api.siqcloud.online/counterapi/r51
2) Add report release 5, customer id: '123', requestor id: '123', report types: 'TR_J1'
3) Run a report for this data provider, from Feb 2025 to Apr 2025. Notice that before applying patches, the job report results in a red bar with no error message.
4) Apply patches, repeat 3. Notice it now results in a nice error message.

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2025-05-08 21:27:44 UTC
Testing notes (using KTD):
1. Set the Gale Cengage data provider up manually (it does not come up if searching using the registry).
2. Run the report from data providers - the "Run now" action.
Comment 8 Nick Clemens (kidclamp) 2025-05-28 12:12:10 UTC
Created attachment 182816 [details] [review]
Bug 39823: Clean-up: Mock LWP response instead of class method being tested

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 9 Nick Clemens (kidclamp) 2025-05-28 12:12:13 UTC
Created attachment 182817 [details] [review]
Bug 39823: Add tests

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 10 Nick Clemens (kidclamp) 2025-05-28 12:12:15 UTC
Created attachment 182818 [details] [review]
Bug 39823: Identify error messages based on code

Even if severity does not exist. Additionally, only add severity to message if it exists.

Test plan:
1) Apply tests patches. Run tests (they should fail):
prove t/db_dependent/Koha/BackgroundJobs/ErmSushiHarvester.t
2) Apply this patch and run tests again (they should pass this time).

UI test:
1) Create a new data provider linked to Gale Cengage, service url:
https://api.siqcloud.online/counterapi/r51
2) Add report release 5, customer id: '123', requestor id: '123', report types: 'TR_J1'
3) Run a report for this data provider, from Feb 2025 to Apr 2025. Notice that before applying patches, the job report results in a red bar with no error message.
4) Apply patches, repeat 3. Notice it now results in a nice error message.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Lucas Gass (lukeg) 2025-06-02 21:27:38 UTC
Nice work everyone!

Pushed to main for 25.11
Comment 12 Fridolin Somers 2025-06-19 07:38:37 UTC
I see in 25.05.x
Comment 13 Fridolin Somers 2025-06-19 07:39:28 UTC
Pushed to 24.11.x for 24.11.06
Comment 14 Paul Derscheid 2025-06-19 09:01:22 UTC
Nice work everyone!

Pushed to 25.05.x for 25.05.03
Comment 15 David Nind 2025-06-30 22:34:39 UTC
I've had a go at a release note, but not sure that I've got it right.

Please improve it!