Bug 37963 - Improve error handling and testing of ERM eUsage SUSHI
Summary: Improve error handling and testing of ERM eUsage SUSHI
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Pedro Amorim
QA Contact: Testopia
URL:
Keywords: rel_23_11_candidate, rel_24_05_candidate
Depends on:
Blocks: 37856
  Show dependency treegraph
 
Reported: 2024-09-19 14:10 UTC by Pedro Amorim
Modified: 2024-09-27 17:41 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00
Circulation function:


Attachments
Bug 37963: Case 1 - Add tests for invalid api key use case (3.35 KB, patch)
2024-09-19 14:20 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 37963: Case 1 - Return if sushi_errors even if response code >= 400 (2.51 KB, patch)
2024-09-19 14:20 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 37963: Case 2 - Add tests for different case error message (2.83 KB, patch)
2024-09-19 14:20 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 37963: Case 2 - Consider providers returning error messages lower cased (1.71 KB, patch)
2024-09-19 14:20 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 37963: Case 1 - Add tests for invalid api key use case (3.39 KB, patch)
2024-09-23 19:33 UTC, David Nind
Details | Diff | Splinter Review
Bug 37963: Case 1 - Return if sushi_errors even if response code >= 400 (2.55 KB, patch)
2024-09-23 19:33 UTC, David Nind
Details | Diff | Splinter Review
Bug 37963: Case 2 - Add tests for different case error message (2.87 KB, patch)
2024-09-23 19:33 UTC, David Nind
Details | Diff | Splinter Review
Bug 37963: Case 2 - Consider providers returning error messages lower cased (1.76 KB, patch)
2024-09-23 19:33 UTC, David Nind
Details | Diff | Splinter Review
Bug 37963: Case 1 - Add tests for invalid api key use case (3.40 KB, patch)
2024-09-26 18:37 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 37963: Case 1 - Return if sushi_errors even if response code >= 400 (2.56 KB, patch)
2024-09-26 18:37 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 37963: Case 2 - Add tests for different case error message (2.88 KB, patch)
2024-09-26 18:38 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 37963: Case 2 - Consider providers returning error messages lower cased (1.76 KB, patch)
2024-09-26 18:38 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 37963: (QA follow-up) Tidy (8.36 KB, patch)
2024-09-26 18:38 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 Pedro Amorim 2024-09-19 14:10:55 UTC
There a few problems currently with ERM SUSHI:
1) The harvest backgroud job is designed to show the user any SUSHI errors with the SUSHI error message, but this is not happening in some cases.
2) Often times it's hard to provide test plans for SUSHI related patches as these require SUSHI credentials to replicate the problem.
Comment 1 Pedro Amorim 2024-09-19 14:20:38 UTC
Created attachment 171769 [details] [review]
Bug 37963: Case 1 - Add tests for invalid api key use case
Comment 2 Pedro Amorim 2024-09-19 14:20:40 UTC
Created attachment 171770 [details] [review]
Bug 37963: Case 1 - Return if sushi_errors even if response code >= 400

This ensures that SUSHI errors, if present, are shown on the UI even if
response->code >= 400.
This means that if, for example, SUSHI provider returns 401 with SUSHI
error like so:
{"Code": 2020, "Severity": "Error", "Message": "API Key Invalid"}

Then, the 'API Key Invalid' will be shown to the user, without having to
check logs.
Comment 3 Pedro Amorim 2024-09-19 14:20:43 UTC
Created attachment 171771 [details] [review]
Bug 37963: Case 2 - Add tests for different case error message
Comment 4 Pedro Amorim 2024-09-19 14:20:45 UTC
Created attachment 171772 [details] [review]
Bug 37963: Case 2 - Consider providers returning error messages lower cased

Test plan:
1) Apply only test patch for case 1. Run tests:
prove t/db_dependent/Koha/BackgroundJob/ErmSushiHarvester.t
2) Verify tests fail. Apply the other patch for case 1.
3) Run tests again. Verify they now pass.
4) Repeat the test plan but using patches for case 2
Comment 5 David Nind 2024-09-23 19:33:43 UTC
Created attachment 171887 [details] [review]
Bug 37963: Case 1 - Add tests for invalid api key use case

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2024-09-23 19:33:45 UTC
Created attachment 171888 [details] [review]
Bug 37963: Case 1 - Return if sushi_errors even if response code >= 400

This ensures that SUSHI errors, if present, are shown on the UI even if
response->code >= 400.
This means that if, for example, SUSHI provider returns 401 with SUSHI
error like so:
{"Code": 2020, "Severity": "Error", "Message": "API Key Invalid"}

Then, the 'API Key Invalid' will be shown to the user, without having to
check logs.

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2024-09-23 19:33:48 UTC
Created attachment 171889 [details] [review]
Bug 37963: Case 2 - Add tests for different case error message

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2024-09-23 19:33:51 UTC
Created attachment 171890 [details] [review]
Bug 37963: Case 2 - Consider providers returning error messages lower cased

Test plan:
1) Apply only test patch for case 1. Run tests:
prove t/db_dependent/Koha/BackgroundJob/ErmSushiHarvester.t
2) Verify tests fail. Apply the other patch for case 1.
3) Run tests again. Verify they now pass.
4) Repeat the test plan but using patches for case 2

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 Nick Clemens (kidclamp) 2024-09-26 18:37:56 UTC
Created attachment 172007 [details] [review]
Bug 37963: Case 1 - Add tests for invalid api key use case

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 Nick Clemens (kidclamp) 2024-09-26 18:37:59 UTC
Created attachment 172008 [details] [review]
Bug 37963: Case 1 - Return if sushi_errors even if response code >= 400

This ensures that SUSHI errors, if present, are shown on the UI even if
response->code >= 400.
This means that if, for example, SUSHI provider returns 401 with SUSHI
error like so:
{"Code": 2020, "Severity": "Error", "Message": "API Key Invalid"}

Then, the 'API Key Invalid' will be shown to the user, without having to
check logs.

Signed-off-by: David Nind <david@davidnind.com>
Comment 11 Nick Clemens (kidclamp) 2024-09-26 18:38:01 UTC
Created attachment 172009 [details] [review]
Bug 37963: Case 2 - Add tests for different case error message

Signed-off-by: David Nind <david@davidnind.com>
Comment 12 Nick Clemens (kidclamp) 2024-09-26 18:38:04 UTC
Created attachment 172010 [details] [review]
Bug 37963: Case 2 - Consider providers returning error messages lower cased

Test plan:
1) Apply only test patch for case 1. Run tests:
prove t/db_dependent/Koha/BackgroundJob/ErmSushiHarvester.t
2) Verify tests fail. Apply the other patch for case 1.
3) Run tests again. Verify they now pass.
4) Repeat the test plan but using patches for case 2

Signed-off-by: David Nind <david@davidnind.com>
Comment 13 Nick Clemens (kidclamp) 2024-09-26 18:38:06 UTC
Created attachment 172011 [details] [review]
Bug 37963: (QA follow-up) Tidy
Comment 14 Pedro Amorim 2024-09-27 09:57:54 UTC
Thanks for QA, Nick.
Adding 24.05 and 23.11 release candidate keywords as this improves how the SUSHI response is handled widening the pool of providers Koha is able to work with.
Comment 15 Katrin Fischer 2024-09-27 17:41:34 UTC
Pushed for 24.11!

Well done everyone, thank you!