Originally fixed in bug 32728 but it's not working again, we need to fix it, and provide tests for it to make sure this doesn't happen again.
Created attachment 149220 [details] [review] Bug 33422: ERM search header should change This patch removes reference to a non-existent id ("ui-id-X") and replaces it with new ids specific to agreements, titles and packages. Test plan: 1) Apply patch 2) Navigate between Agreements, Packages, Titles using the sidebar. The search bar should change each time to match the section 3) Add some agreemens, packages, titles 4) Perform a search using the different search bar options. Each time it should take you to the correct page with the search activated in the KohaTable. The navbar should also be highlighted to match the correct section and the top search bar should also show the correct section
Created attachment 149221 [details] [review] Bug 33422: ERM search header should change This patch removes reference to a non-existent id ("ui-id-X") and replaces it with new ids specific to agreements, titles and packages. Test plan: 1) Apply patch 2) Navigate between Agreements, Packages, Titles using the sidebar. The search bar should change each time to match the section 3) Add some agreemens, packages, titles 4) Perform a search using the different search bar options. Each time it should take you to the correct page with the search activated in the KohaTable. The navbar should also be highlighted to match the correct section and the top search bar should also show the correct section Signed-off-by: David Nind <david@davidnind.com>
Testing notes (using KTD): 1. After applying the patch (step 1), run: yarn build
Bug 32683 certainly caused this regression.
Created attachment 149224 [details] [review] Bug 33422: (bug 32728 follow-up) ERM - Update search header depending on navigation This patch removes reference to a non-existent id ("ui-id-X") and replaces it with new ids specific to agreements, titles and packages. Test plan: 1) Apply patch 2) Navigate between Agreements, Packages, Titles using the sidebar. The search bar should change each time to match the section 3) Add some agreemens, packages, titles 4) Perform a search using the different search bar options. Each time it should take you to the correct page with the search activated in the KohaTable. The navbar should also be highlighted to match the correct section and the top search bar should also show the correct section Signed-off-by: David Nind <david@davidnind.com> JD Amended commit message: link with 32728 Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Not blocker - we should add tests for this.
I need to get used to Cypress so I can have a look at writing some tests
Created attachment 149227 [details] [review] Bug 33422: Add cypress test This patch adds a cypress test to check whether the "active" class is correctly updating when navigating
These look good, some points for discussion: 1) Should we make this bug depend on bug 33408 and have these tests be written in that fashion? i.e. mock ERMModule sys pref. 2) Should we move Dialog_spec.ts and this new Searchbar_spec.ts into a single ERM/Common_spec.ts just for organization sake.
(In reply to Pedro Amorim from comment #9) > These look good, some points for discussion: > 1) Should we make this bug depend on bug 33408 and have these tests be > written in that fashion? i.e. mock ERMModule sys pref. > 2) Should we move Dialog_spec.ts and this new Searchbar_spec.ts into a > single ERM/Common_spec.ts just for organization sake. This (33422) should go first, as it's a bugfix. We will need to adjust the other bug on top of it. I would suggest ERM/Regressions_spec.ts, we have one "regressions" for selenium and it's good to group those small tests together IMO.
Pushed to master for 23.05. Nice work everyone, thanks!
Nice work, thanks everyone! Pushed to 22.11.x for the next release.