| Summary: | ERM should be able to display error messages coming from the API | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Pedro Amorim <pedro.amorim> |
| Component: | ERM | Assignee: | Pedro Amorim <pedro.amorim> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | fridolin.somers, jonathan.druart, jonathan.field, martin.renvoize, matt.blenkinsop, pedro.amorim |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
23.11.00,23.05.04,22.11.10
|
|
| Circulation function: | |||
| Bug Depends on: | |||
| Bug Blocks: | 34587, 35199 | ||
| Attachments: |
Bug 34448: Update the way we handle response in http-client.js
Bug 34448: Update cypress tests Bug 34448: Update cypress tests Bug 34448: Update cypress tests Bug 34448: Update the way we handle response in http-client.js Bug 34448: Update cypress tests Bug 34448: Update the way we handle response in http-client.js |
||
|
Description
Pedro Amorim
2023-07-31 14:54:31 UTC
Created attachment 154097 [details] [review] Bug 34448: Update the way we handle response in http-client.js Test plan: Before patch: - Visit a non-existent ID i.e. /cgi-bin/koha/erm/agreements/999 - Visit a char ID i.e. /cgi-bin/koha/erm/agreements/abc Apply patch: Repeat above steps. Run cypress tests Created attachment 154098 [details] [review] Bug 34448: Update cypress tests Created attachment 154099 [details] [review] Bug 34448: Update cypress tests I would keep the console.log with the whole 'response' object. Looking at js/datatables.js, https://git.koha-community.org/Koha-community/Koha/src/branch/master/koha-tmpl/intranet-tmpl/prog/js/datatables.js#L493 And wondering if we should not split on several lines instead of separating with a space. Created attachment 154123 [details] [review] Bug 34448: Update cypress tests Created attachment 154124 [details] [review] Bug 34448: Update the way we handle response in http-client.js Test plan: Before patch: - Visit a non-existent ID i.e. /cgi-bin/koha/erm/agreements/999 - Visit a char ID i.e. /cgi-bin/koha/erm/agreements/abc Apply patch: Repeat above steps. Run cypress tests (In reply to Jonathan Druart from comment #4) > I would keep the console.log with the whole 'response' object. > > Looking at js/datatables.js, > > https://git.koha-community.org/Koha-community/Koha/src/branch/master/koha- > tmpl/intranet-tmpl/prog/js/datatables.js#L493 > > And wondering if we should not split on several lines instead of separating > with a space. Thank you very much Jonathan, I've squashed that into the patch! Before: Something went wrong: Error: Error: Internal Server Error (500) After: Something went wrong: Error: Error: Something went wrong, check Koha logs for details. Before: Something went wrong: Error: Not Found (404) After: Something went wrong: Error: Agreement not found Before: Something went wrong: Error: Bad Request (400) After: Something went wrong: Error: Expected integer - got string. Note for myself: """The ok read-only property of the Response interface contains a Boolean stating whether the response was successful (status in the range 200-299) or not.""" Only wondering if we should not keep the HTTP status code. Created attachment 154259 [details] [review] Bug 34448: Update cypress tests Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 154260 [details] [review] Bug 34448: Update the way we handle response in http-client.js Test plan: Before patch: - Visit a non-existent ID i.e. /cgi-bin/koha/erm/agreements/999 - Visit a char ID i.e. /cgi-bin/koha/erm/agreements/abc Apply patch: Repeat above steps. Run cypress tests Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to master for 23.11. Nice work everyone, thanks! Can we backport to 23.05.x ? (In reply to Fridolin Somers from comment #12) > Can we backport to 23.05.x ? I believe so, if it applies and the test plan checks out? It's technically an enhancement, though. Up to you really! Pushed to 23.05.x for 23.05.04 Nice work everyone! Pushed to oldstable for 22.11.x 386 // Submit the form, get 500
387 cy.intercept("POST", "/api/v1/erm/agreements", {
388 statusCode: 500,
389 error: "Something went wrong",
390 });
391 cy.get("#agreements_add").contains("Submit").click();
392 cy.get("main div[class='dialog alert']").contains(
393 "Something went wrong: SyntaxError: Unexpected end of JSON input"
394 );
This is totally wrong. We are now showing a JS error instead of the expected 500: internal server error!
|