https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 146343 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146343&action=edit Bug 32898: Fix Cypress tests -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 --- Comment #3 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 146384 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146384&action=edit Bug 32898: ERM - Improve fetch ... and fix cypress tests The main problem was that there were no more checkError call for fetchLocalTitleCount and the error was not displayed on the UI. This commit is doing several things: - Refactor fetch.js to call the same fetch method (myFetch) in order to deal with the error in a single place - Have a new myFetchTotal JS function to get the X-Total-Count header from the response and get the benefit of the previous refactoring - Rename fetchCountLocalPackages with fetchLocalPackagesCount to match fetchLocalTitleCount - Use fetchLocalPackagesCount on the Package list view - Use checkError from components using fetch directly. Ideally the could should be moved to fetch.js and myFetch should be used. - checkError get a new flag in parameter to ask for the response instead of the result (ie. the JSON of the response). That's useful when we need the response status or the headers - Make checkError throw a proper exception to stop propation. This modify the error we display on the UI, it's why we have the following changes in cypress tests: - "Something went wrong: Internal Server Error" + "Something went wrong: Error: Internal Server Error" That's not ideal, we should have our own JS exception to avoid that "Error" (coming from the toString call on the error) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #146384|0 |1 is obsolete| | --- Comment #5 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 146385 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146385&action=edit Bug 32898: ERM - Improve fetch ... and fix cypress tests The main problem was that there were no more checkError call for fetchLocalTitleCount and the error was not displayed on the UI. This commit is doing several things: - Refactor fetch.js to call the same fetch method (myFetch) in order to deal with the error in a single place - Have a new myFetchTotal JS function to get the X-Total-Count header from the response and get the benefit of the previous refactoring - Rename fetchCountLocalPackages with fetchLocalPackagesCount to match fetchLocalTitleCount - Use fetchLocalPackagesCount on the Package list view - Use checkError from components using fetch directly. Ideally the could should be moved to fetch.js and myFetch should be used. - checkError get a new flag in parameter to ask for the response instead of the result (ie. the JSON of the response). That's useful when we need the response status or the headers - Make checkError throw a proper exception to stop propation. This modify the error we display on the UI, it's why we have the following changes in cypress tests: - "Something went wrong: Internal Server Error" + "Something went wrong: Error: Internal Server Error" That's not ideal, we should have our own JS exception to avoid that "Error" (coming from the toString call on the error) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #146385|0 |1 is obsolete| | --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 146399 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146399&action=edit Bug 32898: ERM - Improve fetch ... and fix cypress tests The main problem was that there were no more checkError call for fetchLocalTitleCount and the error was not displayed on the UI. This commit is doing several things: - Refactor fetch.js to call the same fetch method (myFetch) in order to deal with the error in a single place - Have a new myFetchTotal JS function to get the X-Total-Count header from the response and get the benefit of the previous refactoring - Rename fetchCountLocalPackages with fetchLocalPackagesCount to match fetchLocalTitleCount - Use fetchLocalPackagesCount on the Package list view - Use checkError from components using fetch directly. Ideally the could should be moved to fetch.js and myFetch should be used. - checkError get a new flag in parameter to ask for the response instead of the result (ie. the JSON of the response). That's useful when we need the response status or the headers - Make checkError throw a proper exception to stop propation. This modify the error we display on the UI, it's why we have the following changes in cypress tests: - "Something went wrong: Internal Server Error" + "Something went wrong: Error: Internal Server Error" That's not ideal, we should have our own JS exception to avoid that "Error" (coming from the toString call on the error) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 --- Comment #7 from Paul Derscheid <paul.derscheid@lmscloud.de> --- A more idiomatic way to write response && response.status === 2XX would be response?.status === 2XX ?? false I think this is easier to read when scanning through the code. As this is a matter of taste, I'll let you be the judge. Additionally I don't think you need the return await. You await already in the myFetch function so you can just return. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 --- Comment #9 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Paul Derscheid from comment #7)

A more idiomatic way to write

response && response.status === 2XX

would be

response?.status === 2XX ?? false

I think this is easier to read when scanning through the code. As this is a matter of taste, I'll let you be the judge.

I didn't know about that syntax. I am not sure I am ready yet :D

Additionally I don't think you need the return await. You await already in the myFetch function so you can just return.

I noticed that but was lazy to try if removing them was ok. Thanks for confirming! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #146399|0 |1 is obsolete| | --- Comment #11 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 146724 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146724&action=edit Bug 32898: ERM - Improve fetch ... and fix cypress tests The main problem was that there were no more checkError call for fetchLocalTitleCount and the error was not displayed on the UI. This commit is doing several things: - Refactor fetch.js to call the same fetch method (myFetch) in order to deal with the error in a single place - Have a new myFetchTotal JS function to get the X-Total-Count header from the response and get the benefit of the previous refactoring - Rename fetchCountLocalPackages with fetchLocalPackagesCount to match fetchLocalTitleCount - Use fetchLocalPackagesCount on the Package list view - Use checkError from components using fetch directly. Ideally the could should be moved to fetch.js and myFetch should be used. - checkError get a new flag in parameter to ask for the response instead of the result (ie. the JSON of the response). That's useful when we need the response status or the headers - Make checkError throw a proper exception to stop propation. This modify the error we display on the UI, it's why we have the following changes in cypress tests: - "Something went wrong: Internal Server Error" + "Something went wrong: Error: Internal Server Error" That's not ideal, we should have our own JS exception to avoid that "Error" (coming from the toString call on the error) Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 --- Comment #13 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Tests were failing, applied patch, tests are now passing. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 --- Comment #15 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- There is one test failing, GET 500 /api/v1/erm/eholdings/local/titles?_page=1&_per_page=1 no alias 5 get main div[class='dialog alert'] 0 AssertionError Timed out retrying after 4000ms: Expected to find element: main div[class='dialog alert'], but never found it. The error is not raised to the user when we list titles. It's because fetchLocalTitleCount does not call checkError. We are dealing with that correctly in follow-up bugs, I am tempted to comment the test for now and add it back later. What do you think Pedro? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #146957|0 |1 is obsolete| | --- Comment #17 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 146960 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146960&action=edit Bug 32898: Avoid hardcoding user/pass in tests This patch makes the authentication parameters be configurable through env. Before this, it was hardcoded to koha/koha, which only works if the ktd defaults haven't been tweaked. If no env is defined, it fallsback to koha/koha. In order to pass the ktd configured params, the tests need to be run like: $ yarn cypress run --env KOHA_USER=$KOHA_USER,KOHA_PASS=$KOHA_PASS Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #146960|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #146724|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 --- Comment #18 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 146961 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146961&action=edit Bug 32898: Fix Cypress tests Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 --- Comment #20 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 146963 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146963&action=edit Bug 32898: Remove unecessary async/await myFetch awaits already Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 --- Comment #22 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #15)

There is one test failing,

GET 500 /api/v1/erm/eholdings/local/titles?_page=1&_per_page=1 no alias 5 get main div[class='dialog alert'] 0 AssertionError Timed out retrying after 4000ms: Expected to find element: main div[class='dialog alert'], but never found it.

The error is not raised to the user when we list titles. It's because fetchLocalTitleCount does not call checkError. We are dealing with that correctly in follow-up bugs, I am tempted to comment the test for now and add it back later. What do you think Pedro?

Forget that, I was using an old version of dist file. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 Jacob O'Mara <jacob.omara@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.05.00 |23.05.00,22.11.04 released in| | Status|Pushed to master |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 Jacob O'Mara <jacob.omara@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.05.00,22.11.04 |23.05.00,22.11.04,22.11.04 released in| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 Jacob O'Mara <jacob.omara@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jacob.omara@ptfs-europe.com Version(s)|23.05.00,22.11.04,22.11.04 |23.05.00,22.11.04 released in| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=33282 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=33861 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32898 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36012 -- You are receiving this mail because: You are watching all bug changes.