https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 Pedro Amorim <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 Pedro Amorim <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |edith.speller@ukhsa.gov.uk, | |jeremy.evans@ukhsa.gov.uk -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 --- Comment #2 from Pedro Amorim <pedro.amorim@openfifth.co.uk> --- Created attachment 189603 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=189603&action=edit Bug 41247: Preparation: Cypress plugins 1) Add a way to access and manipulate koha-conf.xml (we need to ensure plugins_restricted is 0 for the actual tests) 2) Add a way to set sys prefs. This could be helpful as just an SQL query isn't enough, a flush_memcached is required for the sys pref change to be picked up -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 --- Comment #4 from Pedro Amorim <pedro.amorim@openfifth.co.uk> --- Created attachment 189607 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=189607&action=edit Bug 41247: Preparation: Cypress XML plugin Add a way to access and manipulate koha-conf.xml (we need to ensure plugins_restricted is 0 for the actual tests) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 Pedro Amorim <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #189603|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=41247 Pedro Amorim <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #189608|0 |1 is obsolete| | --- Comment #6 from Pedro Amorim <pedro.amorim@openfifth.co.uk> --- Created attachment 189609 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=189609&action=edit Bug 41247: Add cypress tests Apply just the 2 cypress tests patches, run: cypress run --spec t/cypress/integration/ILL/Batches_spec.ts Notice it fails. Apply the fix patch. Repeat the command above. Tests must pass. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Status|Needs Signoff |Failed QA --- Comment #8 from David Nind <david@davidnind.com> --- Everything works as per the test plan. However, I've failed QA as after the patches: 1. Do steps 1.1 to 1.3 or 2.1 to 2.5, an use the batch name 'Batch 1' 2. Create a new batch, and use the same batch number you used before, such as 'Batch 1' 3. You get a browser pop-up error message: "A batch named Batch 1 already exists" Testing notes (using KTD): 1. To run the tests, apply these two patches: - Bug 41247: Improve ILL batches modal resetability - Bug 41247: Add cypress tests 2. Run the tests, they fail: cypress run --spec t/cypress/integration/ILL/Batches_spec.ts 3. Apply the patch: Bug 41247: Preparation: Cypress XML plugin 4. Repeat step 2 - they now pass -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 Pedro Amorim <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #189572|0 |1 is obsolete| | --- Comment #10 from David Nind <david@davidnind.com> --- Created attachment 190903 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190903&action=edit Bug 41247: Improve ILL batches modal resetability Test plan, preparation: A) Enable ILLModule B) Install a metadata enrichment plugin, e.g: https://github.com/openfifth/koha-plugin-api-pubmed/releases/tag/1.1.1 C) Restart plack to pick up the newly installed plugin if needed. D) Create a new batch, visit: <staff_url>/cgi-bin/koha/ill/ill-requests.pl? Issue #1: 1.1) Click 'New ILL requests batch' 1.2) Enter a name, '42' on the cardnumber and any library. Click 'Continue'. 1.3) Click the 'Close' button (or click anywhere outside the modal) to close the modal. 1.4) Repeat 1.1). Notice the 'Continue' button is no longer there. Issue #2: 2.1) Click 'New ILL requests batch' 2.2) Enter a name, '42' on the cardnumber and any library. Click 'Continue'. 2.3) Enter '123' on the textarea, click 'Process identifiers'. 2.4) Notice a 'Add items to batch' buttons appears. Don't click that, as before, close the modal. 2.5) Repeat 2.1) Notice the progress bar shows right off the bat (it shouldn't) with NaN as its value as well as table elements (export button, 'No entries to show' message, etc, with no table. Apply patch, hard-refresh the browser and repeat Issues 1 and 2. Confirm the patch fixes them. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #189618|0 |1 is obsolete| | --- Comment #12 from David Nind <david@davidnind.com> --- Created attachment 190905 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190905&action=edit Bug 41247: Add cypress tests Apply just the 2 cypress tests patches, run: cypress run --spec t/cypress/integration/ILL/Batches_spec.ts Notice it fails. Apply the fix patch. Repeat the command above. Tests must pass. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 --- Comment #14 from Pedro Amorim <pedro.amorim@openfifth.co.uk> --- (In reply to Marcel de Rooy from comment #13)

I am wondering what happens here with koha-conf when the cypress test crashes?

I suppose there's a chance the koha-conf.xml may be left modified in relation to its default state e.g. if afterEach never gets to run for some reason. Having said that, all tests (cypress or otherwise) are meant run on a dev/test environment, the file path itself is hardcoded to the kohadev instance here. If any tests are executed in an non-dev/test environment then there are bigger problems at play. I'm not convinced this is a big enough reason to block, though I'm happy to discuss further of course. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 --- Comment #16 from Pedro Amorim <pedro.amorim@openfifth.co.uk> --- (In reply to Marcel de Rooy from comment #15)

Isnt there a more elegant way in cypress tests to mock koha-conf settings, preferences, etc just like we do in the regular perl tests?

There may be, I'm just not seeing it, not without baking it into Koha beforehand. Sys prefs are different as they come from the database, I believe cy.task("query") is changing the database directly and then reverting it back to what it was (setup+teardown). I've attempted to implement a similar mechanism here for koha-conf.xml.

Just an idea about koha-conf + let kohaconf = "/etc/koha/sites/kohadev/koha-conf.xml"; Why not pass a temporary modified koha-conf here? But still hoping that we could even find something better..

Could do, the problems: 1) Koha server side (perl) reads that file (kohadev/koha-conf.xml). 2) plugins_restricted (default) is 1 3) Installation of the plugin through the UI is not possible with plugins_restricted = 1 4) Tests will fail as they require a particular type of plugin to be installed in order for UI things e.g. buttons to appear. As always, I'm open to discuss further, the issue here I believe boils down to: "How to mock C4:Context->config from a cypress test" -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190904|0 |1 is obsolete| | --- Comment #19 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 194282 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=194282&action=edit Bug 41247: Preparation: Cypress XML plugin Add a way to access and manipulate koha-conf.xml (we need to ensure plugins_restricted is 0 for the actual tests) Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com --- Comment #22 from Jonathan Druart <jonathan.druart@gmail.com> --- + cy.task("query", { + sql: "DELETE from illbatches", + }); + //Clean-up installed plugin(s) + cy.task("query", { + sql: "DELETE from plugin_data", + }); + cy.task("query", { + sql: "DELETE from plugin_methods", + }); Really? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@openfifth.c | |o.uk Text to go in the| |The "New ILL requests release notes| |batch" modal was not | |resetting its state | |correctly after being | |closed, causing unexpected | |behaviour when it was | |reopened. | | | |The modal now resets its | |internal state fully when | |closed, so that each new | |batch creation session | |starts from a clean initial | |state regardless of how far | |through the workflow the | |previous session | |progressed. | |Testing | |Cypress tests have been | |added to cover both reset | |scenarios, ensuring | |regressions are caught | |automatically going | |forward. | | | |Feel free to adjust the | |tone or level of technical | |detail — for example, if | |these notes are aimed at | |end users I can simplify | |the language further, or if | |they're for a technical | |changelog I can add more | |specifics about the Vue | |component state management | |involved. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 --- Comment #24 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Thanks for all the hard work! Pushed to main for the next 26.05.00 release as RM Assistant -- You are receiving this mail because: You are watching all bug changes.

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

+ cy.task("query", { + sql: "DELETE from illbatches", + }); + //Clean-up installed plugin(s) + cy.task("query", { + sql: "DELETE from plugin_data", + }); + cy.task("query", { + sql: "DELETE from plugin_methods", + });

Really?

Also this should have been answered before the push. The tests are removing all the plugins! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 --- Comment #27 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Just spoke to Pedro, I'm going to revert this one.. I missed the final comment before push.. too many tabs open. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|The "New ILL requests |The "New ILL requests release notes|batch" modal was not |batch" modal was not |resetting its state |resetting its state |correctly after being |correctly after being |closed, causing unexpected |closed, causing unexpected |behaviour when it was |behaviour when it was |reopened. |reopened. | | |The modal now resets its |The modal now resets its |internal state fully when |internal state fully when |closed, so that each new |closed, so that each new |batch creation session |batch creation session |starts from a clean initial |starts from a clean initial |state regardless of how far |state regardless of how far |through the workflow the |through the workflow the |previous session |previous session |progressed. |progressed. |Testing | |Cypress tests have been | |added to cover both reset | |scenarios, ensuring | |regressions are caught | |automatically going | |forward. | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 --- Comment #28 from Pedro Amorim <pedro.amorim@openfifth.co.uk> --- Created attachment 196588 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196588&action=edit Bug 41247: (QA follow-up): Update cypress test This cleans up the now removed XML changes and improves the clean-up process to only delete the illbatches created during testing as well as uninstalling the plugin through the UI instead of all plugins through a database query The original XML based implementation is no longer relevant as plugins_restricted is now default 0 in ktd. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 --- Comment #29 from Pedro Amorim <pedro.amorim@openfifth.co.uk> --- Hi all appreciate the patience here. I've dropped the XML based implementation to update koha-conf.xml. It was met with resistance + it's no longer needed as ktd now ships with plugins_restricted = 0. I've provided a follow-up to the cypress test to cleanup by deleting only the ILL batches created during testing as well as uninstalling the plugin through the UI instead of removing all plugins through a db query as was before. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #30 from Jonathan Druart <jonathan.druart@gmail.com> --- The Cypress tests are not passing CypressError: `cy.click()` can only be called on a single element. Your subject contained 4 elements. Pass `{ multiple: true }` if you want to serially click each element. Also this task does not longer exist: + cy.task("readXmlElementValue", { -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 --- Comment #31 from Pedro Amorim <pedro.amorim@openfifth.co.uk> --- Created attachment 197201 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197201&action=edit Bug 41247: (QA follow-up): Improve uninstalling plugin Ensure uninstalling works for the correct plugin if multiple plugins are installed cypress run --spec t/cypress/integration/ILL/Batches_spec.ts -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #194283|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=41247 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197201|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=41247 --- Comment #34 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 197260 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197260&action=edit Bug 41247: Add cypress tests Apply just the 2 cypress tests patches, run: cypress run --spec t/cypress/integration/ILL/Batches_spec.ts Notice it fails. Apply the fix patch. Repeat the command above. Tests must pass. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 --- Comment #36 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 197262 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197262&action=edit Bug 41247: (QA follow-up): Improve uninstalling plugin Ensure uninstalling works for the correct plugin if multiple plugins are installed cypress run --spec t/cypress/integration/ILL/Batches_spec.ts Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 --- Comment #38 from Pedro Amorim <pedro.amorim@openfifth.co.uk> --- (In reply to Jonathan Druart from comment #37)

I've only reviewed the Cypress tests. IMO we should do proper E2E testing instead of mocking the whole response, but not considering blocker for now...

The plugin queries a third-party service though (external API) to fetch metadata. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41247 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|26.05.00 |26.05.00,25.11.05 released in| | -- You are receiving this mail because: You are watching all bug changes.