Bug 33482

Summary: Errors from EBSCO's ws are not reported to the UI
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: ERMAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: jonathan.druart, jonathan.field, martin.renvoize, pedro.amorim
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.06
Circulation function:
Attachments: Bug 33482: Propagate errors from EBSCO's ws to the UI
Bug 33482: Add Unsupported Media Type exception
Bug 33482: Unsupported Media Type exception in packages patch method
Bug 33482: Propagate errors from EBSCO's ws to the UI
Bug 33408: We need only one this.loaded call
Bug 33482: Propagate errors from EBSCO's ws to the UI

Description Jonathan Druart 2023-04-11 11:46:33 UTC

    
Comment 1 Jonathan Druart 2023-04-11 11:49:28 UTC
Created attachment 149435 [details] [review]
Bug 33482: Propagate errors from EBSCO's ws to the UI

When adding or removing eholdings from EBSCO, if an error happens it is
lost and not propagated to the UI.

We were missing a return in the logic, and always returning 200

Test plan:
Do not apply (or revert) bug 33481
Configure EBSCO for ERM and add a package to your holdings
With this patch applied you will get a 500 that is propagated to the UI
Without this patch nothing happens and the user is not aware that
something bad happened
Comment 2 Pedro Amorim 2023-04-11 12:47:02 UTC
Created attachment 149439 [details] [review]
Bug 33482: Add Unsupported Media Type exception
Comment 3 Pedro Amorim 2023-04-11 12:47:05 UTC
Created attachment 149440 [details] [review]
Bug 33482: Unsupported Media Type exception in packages patch method

This adds the Unsupported Media Type exception error code
and error message to the UI
Comment 4 Pedro Amorim 2023-04-11 12:49:38 UTC
The error code and message are now showing when reproducing the error for packages and titles, however you'll notice the patch only contains changes to erm_eholdings_packages.yaml, this is because "adding titles to holdings" is using the same endpoint. @Joubu just to confirm, is this intended?
Comment 5 Pedro Amorim 2023-04-11 15:16:38 UTC
Created attachment 149450 [details] [review]
Bug 33482: Propagate errors from EBSCO's ws to the UI

When adding or removing eholdings from EBSCO, if an error happens it is
lost and not propagated to the UI.

We were missing a return in the logic, and always returning 200

Test plan:
Do not apply (or revert) bug 33481
Configure EBSCO for ERM and add a package to your holdings
With this patch applied you will get a 500 that is propagated to the UI
Without this patch nothing happens and the user is not aware that
something bad happened

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 6 Pedro Amorim 2023-04-12 08:23:36 UTC
Created attachment 149508 [details] [review]
Bug 33408: We need only one this.loaded call

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 7 Martin Renvoize (ashimema) 2023-04-27 12:13:35 UTC
Created attachment 150292 [details] [review]
Bug 33482: Propagate errors from EBSCO's ws to the UI

When adding or removing eholdings from EBSCO, if an error happens it is
lost and not propagated to the UI.

We were missing a return in the logic, and always returning 200

Test plan:
Do not apply (or revert) bug 33481
Configure EBSCO for ERM and add a package to your holdings
With this patch applied you will get a 500 that is propagated to the UI
Without this patch nothing happens and the user is not aware that
something bad happened

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize (ashimema) 2023-04-27 12:14:10 UTC
All working as expected, no QA failures.

Passing QA
Comment 9 Tomás Cohen Arazi (tcohen) 2023-05-05 12:17:31 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 10 Martin Renvoize (ashimema) 2023-05-11 11:12:52 UTC
Many hands makes light work, thankyou everyone!

Pushed to 22.11.x for the next release