Bug 32728 - ERM - Search header should change to match the section you are in
Summary: ERM - Search header should change to match the section you are in
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: ERM (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks: 33422
  Show dependency treegraph
 
Reported: 2023-01-25 17:51 UTC by David Nind
Modified: 2023-12-28 20:42 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.04


Attachments
Bug 32728: ERM - Update search header depending on navigation (1.27 KB, patch)
2023-01-31 16:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32728: ERM - Update search header depending on navigation (1.33 KB, patch)
2023-02-06 12:52 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 32728: ERM - Update search header depending on navigation (2.04 KB, patch)
2023-02-24 14:41 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32728: Fix warning (1.93 KB, patch)
2023-02-28 10:07 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Nind 2023-01-25 17:51:23 UTC
The ERM module search header has three options: Search agreements, Search packages, and Search titles.

If you click through the sidebar options (Agreements, Packages, and Titles) the search header doesn't change to reflect the area you are in.

If you change the header to another option, for example 'Search titles', then perform a search, the header goes back to 'Agreement search'.

To replicate:
1. Enable the ERM module: ERMModule system preference.
2. Go to the ERM module: Home > E-resource management.
3. Select the options from the sidebar menu (Agreements, Packages and Titles), and note that the ERM search header options don't change.
4. Add a minimal agreement, package and title.
5. Change the search header to another option, for example 'Search titles', then perform a search. Note that:
   - the appropriate sidebar menu option is highlighted, for example: 'Titles'
   - the search header changes back to 'Agreement search'

This was noted as a potential issue when testing bug 32683 - Convert header search tabs to Bootstrap.
Comment 1 Jonathan Druart 2023-01-31 16:21:34 UTC
Created attachment 145897 [details] [review]
Bug 32728: ERM - Update search header depending on navigation

Search header should change to match the section you are in.

Default is "agreement". If you are in "local titles" or "local
packages", the header will be positionned on the relevant tab.
Comment 2 Jonathan Druart 2023-01-31 16:22:06 UTC
I am not convinced by this patch. Ideally we should embed the search header into the Vue app, but that is much more work!
Comment 3 PTFS Europe Sandboxes 2023-02-06 12:52:19 UTC
Created attachment 146264 [details] [review]
Bug 32728: ERM - Update search header depending on navigation

Search header should change to match the section you are in.

Default is "agreement". If you are in "local titles" or "local
packages", the header will be positionned on the relevant tab.

Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com>
Comment 4 Katrin Fischer 2023-02-24 14:04:25 UTC
Please remember to include the test plans in the commit messages :)
Comment 5 Katrin Fischer 2023-02-24 14:41:29 UTC
Created attachment 147334 [details] [review]
Bug 32728: ERM - Update search header depending on navigation

Search header should change to match the section you are in.

Default is "agreement". If you are in "local titles" or "local
packages", the header will be positionned on the relevant tab.

To replicate:

1. Enable the ERM module: ERMModule system preference.
2. Go to the ERM module: Home > E-resource management.
3. Select the options from the sidebar menu (Agreements, Packages and Titles),
   and note that the ERM search header options don't change.
4. Add a minimal agreement, package and title.
5. Change the search header to another option, for example 'Search titles',
   then perform a search. Note that:
   - the appropriate sidebar menu option is highlighted, for example: 'Titles'
   - the search header changes back to 'Agreement search'
6. Apply patch, run yarn build
7. Verify the search options are now switched when moving between pages

Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 6 Tomás Cohen Arazi 2023-02-24 19:56:33 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 7 Martin Renvoize 2023-02-27 13:38:38 UTC
Many hands makes light work, thankyou everyone!

Pushed to 22.11.x for the next release
Comment 8 Jonathan Druart 2023-02-28 10:07:13 UTC
Created attachment 147482 [details] [review]
Bug 32728: Fix warning

There is a couple of wrong things from the previous patch:

* There is a warning in the console
Firefox
[Vue Router warn]: Unexpected error when starting the router: TypeError: document.getElementById(...) is null
Chrome
[Vue Router warn]: Unexpected error when starting the router: TypeError: Cannot read properties of null (reading 'click')
That's because the tabs are not initialized yet, and so the tab is not
correctly selected when the app is loaded.

* "Search titles" is selected when on packages, and "Search packages" is
selected when on titles

This patch remove the warning and fix the second problem.

However the tab is still wrong when the app is loaded. Will see later if
that can be fixed, but it's not trivial at first glance.
Comment 9 Martin Renvoize 2023-02-28 10:15:48 UTC
Simple enough patch to clean up the warning and get us display correctly most of the time.. we can deal with the initial load issue as a distinct thing in my opinion.

PQA on that follow-up from me.
Comment 10 Lucas Gass 2023-03-08 13:18:54 UTC
No ERM in 22.05.x, no backport.
Comment 11 Tomás Cohen Arazi 2023-03-20 12:21:48 UTC
Follow-up in master. Please check it is in 22.11