Bug 33482 - Errors from EBSCO's ws are not reported to the UI
Summary: Errors from EBSCO's ws are not reported to the UI
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: ERM (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major
Assignee: Jonathan Druart
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-04-11 11:46 UTC by Jonathan Druart
Modified: 2023-12-28 20:44 UTC (History)
4 users (show)

See Also:
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 (2.05 KB, patch)
2023-04-11 11:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33482: Add Unsupported Media Type exception (919 bytes, patch)
2023-04-11 12:47 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33482: Unsupported Media Type exception in packages patch method (2.50 KB, patch)
2023-04-11 12:47 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33482: Propagate errors from EBSCO's ws to the UI (2.11 KB, patch)
2023-04-11 15:16 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33408: We need only one this.loaded call (2.10 KB, patch)
2023-04-12 08:23 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33482: Propagate errors from EBSCO's ws to the UI (2.17 KB, patch)
2023-04-27 12:13 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

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