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.
Created attachment 171769 [details] [review] Bug 37963: Case 1 - Add tests for invalid api key use case
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.
Created attachment 171771 [details] [review] Bug 37963: Case 2 - Add tests for different case error message
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
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>
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>
Created attachment 171889 [details] [review] Bug 37963: Case 2 - Add tests for different case error message Signed-off-by: David Nind <david@davidnind.com>
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>
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>
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>
Created attachment 172009 [details] [review] Bug 37963: Case 2 - Add tests for different case error message Signed-off-by: David Nind <david@davidnind.com>
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>
Created attachment 172011 [details] [review] Bug 37963: (QA follow-up) Tidy
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.
Pushed for 24.11! Well done everyone, thank you!