Bug 27363 - Restore temporary selection of Z39.50 targets throughout multiple searches
Summary: Restore temporary selection of Z39.50 targets throughout multiple searches
Status: Pushed to oldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Phil Ringnalda
QA Contact: Testopia
URL:
Keywords: regression
Depends on: 17515
Blocks:
  Show dependency treegraph
 
Reported: 2021-01-07 17:33 UTC by Barbara Johnson
Modified: 2024-11-15 00:41 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.06,23.05.12
Circulation function:


Attachments
Bug 27363: (bug 17515 follow-up) Restore temporary selection of Z39.50 targets throughout multiple searches (2.48 KB, patch)
2021-01-20 14:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
MARC bib record from Advanced Cataloging Editor (62.14 KB, image/png)
2021-01-20 15:46 UTC, Barbara Johnson
Details
Z39.50 results showing preselected servers (85.31 KB, image/png)
2021-01-20 15:46 UTC, Barbara Johnson
Details
Z39.50 results showing new server checked (88.73 KB, image/png)
2021-01-20 15:47 UTC, Barbara Johnson
Details
New Z39.50 search showing added server did not get retained (80.60 KB, image/png)
2021-01-20 15:49 UTC, Barbara Johnson
Details
Bug 27363: Restore temporary selection of Z39.50 targets throughout multiple searches (4.71 KB, patch)
2024-05-02 23:39 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 27363: Restore temporary selection of Z39.50 targets throughout multiple searches (4.77 KB, patch)
2024-05-03 15:12 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 27363: Restore temporary selection of Z39.50 targets throughout multiple searches (4.83 KB, patch)
2024-05-09 12:44 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Barbara Johnson 2021-01-07 17:33:27 UTC
Prior to 20.05.06, when using Z39.50 in the Advanced Cataloging Editor, servers that were selected each time a new Z39.50 search was done would be retained throughout multiple searches.  This is no longer the case.  

It would be helpful if that behavior was restored.  We have preselected servers which persist for all searches.  As we catalog we add some additional servers that would be retained for a short period of time until we decided we no longer needed them. For instance, we may be cataloging Spanish and we could check a server that could be used for several bibs but it wasn't a server that we 
wanted to be a preselected server.  

Now that a newly checked server is not retained in the short term we have to check it again for every search which is much less efficient.  We could go into Admin and make it a preselected search for the short duration we plan to use it and then go back into Admin to uncheck it when done but that is also not efficient.  

It would be so much more efficient to be able to check a new server, have it retained, and then uncheck it when done.
Comment 1 Katrin Fischer 2021-01-16 20:18:16 UTC
Hi Barbara, as you said this worked before I have updated this to be a bug in stead of an enhancement.
Comment 2 Jonathan Druart 2021-01-20 10:34:41 UTC
"Prior to 20.05.06"
=> Was it working on 20.05.05?
Comment 3 Barbara Johnson 2021-01-20 12:59:12 UTC
Yes, it worked on 20.05.05.
Comment 4 Jonathan Druart 2021-01-20 14:37:45 UTC
May be caused by

  commit 17571f824d391989247465c7ae18c92c744464c2
  Bug 17515: Order Z3950 server by rank and preserve ordering
Comment 5 Jonathan Druart 2021-01-20 14:40:03 UTC
Created attachment 115437 [details] [review]
Bug 27363: (bug 17515 follow-up) Restore temporary selection of Z39.50 targets throughout multiple searches

Not sure it will fix the problem (not sure I recreated the problem
correctly).
So this patch is just a guess, it seems that
  commit 17571f824d391989247465c7ae18c92c744464c2
  Bug 17515: Order Z3950 server by rank and preserve ordering
forgot to update two occurrences of server.name with server.servername
Comment 6 Nick Clemens (kidclamp) 2021-01-20 14:51:24 UTC
Hi Barbara,

I am not sure I can recreate the problem, can you provide step by step instructions or screenshots to highlight the issue?

Thanks,
Nick
Comment 7 Jonathan Druart 2021-01-20 15:05:24 UTC
The patch I attached is wrong.
Comment 8 Barbara Johnson 2021-01-20 15:44:01 UTC
To reproduce the problem:

1. Go to the Advanced Cataloging Editor (addbooks.pl)
2. Search for a bib record
3. Select and open a bib record from your own library collection 
4. Copy the ISBN or enter title, author, keyword, etc. in the Search box on the left side of the screen to perform a Z39.50 search (bib.png)
5. View the Z39.50 results – notice a few servers at the top of the list are checked because they are defined as preselected servers (preselected.png)
6. None of the results from the preselected servers have the desired MARC record.
7. Select a new server from the list (such as Dallas) (new_server.png)
8. Select any of the records and click on the Substitute button to import it into Koha.  
9. Edit the bib and save.
10. Edit/add the item record and save.
11. From additem.pl search for the next title to catalog by entering it in the search box at the top of the screen. 
12. Repeat steps 3-5.
13. View the results of the Z39.50 search – notice that the server for Dallas is no longer checked – it does not get retained from search to search like it used to. (server_no_longer_checked.png)
Comment 9 Barbara Johnson 2021-01-20 15:46:13 UTC
Created attachment 115439 [details]
MARC bib record from Advanced Cataloging Editor
Comment 10 Barbara Johnson 2021-01-20 15:46:55 UTC
Created attachment 115440 [details]
Z39.50 results showing preselected servers
Comment 11 Barbara Johnson 2021-01-20 15:47:34 UTC
Created attachment 115441 [details]
Z39.50 results showing new server checked
Comment 12 Barbara Johnson 2021-01-20 15:49:29 UTC
Created attachment 115442 [details]
New Z39.50 search showing added server did not get retained
Comment 13 Barbara Johnson 2021-03-18 16:13:44 UTC
Just checking in on the status of this bug.  We're really missing the old behavior because it is negatively affecting our cataloging efficiency.
Comment 14 Jonathan Druart 2021-03-23 10:26:53 UTC
Nick, can you have a look at this one please?
Comment 15 Phil Ringnalda 2024-04-26 20:50:11 UTC
So confusing. At least I finally almost understand what I hacked around by editing localstorage to delete the wrong values I had after the change :)

When you check or uncheck a server in the advanced search, https://git.koha-community.org/Koha-community/Koha/src/commit/0363d08a850f2d59cc9dd9b8a54d6bea1f7d7f83/koha-tmpl/intranet-tmpl/prog/en/includes/cateditor-ui.inc#L719 saves "false" or "true" to localstorage for the closest data-server-id.

https://git.koha-community.org/Koha-community/Koha/src/commit/e3c138c02951852166ec9258afcf1d426f955902/koha-tmpl/intranet-tmpl/prog/en/includes/cateditor-ui.inc#L448 creates data-server-id with the index, not with the server_id that it used before bug 17515 landed.

https://git.koha-community.org/Koha-community/Koha/src/commit/0363d08a850f2d59cc9dd9b8a54d6bea1f7d7f83/koha-tmpl/intranet-tmpl/prog/en/includes/cateditor-ui.inc#L685 reads the values as though the key was still server_id, not index.

Steps to reproduce:
1. Set  EnableAdvancedCatalogingEditor to enable
2. Z39.50 servers, for LIBRARY OF CONGRESS uncheck preselected, and set rank to
   2, and for NATIONAL LIBRARY OF FRANCE uncheck preselected, and set rank to 1
3. Open Firefox's Web Developer Tools, and in the Storage tab open Local Storage   
   and select your URL
4. In the row for cateditor_preferences_{your user id}, if there are any values
   for selected_search_targets, delete them so you have
   "selected_search_targets":{}
5. Open the advanced editor, reopen Local Storage, then click Advanced » in the
   search box
6. Nothing is selected because you have no preselected servers. Click the box
   for Library of Congress, and close the advanced search popup, you now have
   "2": true in Local Storage
7. Open a new browser tab, open the advanced editor in it, and click Advanced »
   and note that you have nothing selected, because reading your "2": true as
   a server_id, server_id 2 is an authority server, so it's not there to select
8. Check and then uncheck the box for Library of Congress, and check the box
   for National Library of France. Local Storage now has
   "1":true,"2":false
9. Open a new browser tab, open the advanced editor in it, and click Advanced »
   and note that instead of having National Library of France selected, you have
   Library of Congress selected, because Library of Congress is index 2 but
   server_id 1.

The trivial patch to read them as index instead of server_id works, but I'm not sure it's the right fix, since while testing it I would then swap my server rank around to have Library of Congress as 1 and National Library of France as 2, and as a result have the wrong one selected until I changed both checkboxes. Or without ranks, I would change the names so they sorted alphabetically the other way, and again have the wrong one selected. Is it worth the extra effort to maintain a map from index to server_id, so we can read and write server_id and map that to index?
Comment 16 Phil Ringnalda 2024-05-02 23:18:21 UTC
Yes, I wish I hadn't said "maintain a map from index to server_id" when obviously the array z3950Servers is a map from its own index to the server_id that's a property of each of its elements.
Comment 17 Phil Ringnalda 2024-05-02 23:39:15 UTC
Created attachment 166092 [details] [review]
Bug 27363: Restore temporary selection of Z39.50 targets throughout multiple searches

The advanced editor currently saves which Z39.50 servers are selected in
local storage using the index in the sorted array of servers, and then misreads
them back as though they were server_ids. We want them to be server_ids, since
that's immutable and the index can change with changes in rank or server name.

We use the data-server-id property in the HTML more often as a server_id than
as the index that's currently stored there, so this patches switches it back
to be the server_id, and in the one place we would use the index, instead uses
Array.find to just get the server object instead of referencing it by index.

Test plan:
 1. Set the preference EnableAdvancedCatalogingEditor to enable
 2. Administration - Z39.50 Servers, for NATIONAL LIBRARY OF FRANCE uncheck
    "Preselected"
 3. To be sure you don't have confusing saved preferences, open Firefox's
    Developer Tools, in the Storage tab open Local Storage and select your URL
 4. In the row for cateditor_preferences_{your Koha user id}, if there are any
    values for selected_search_targets, delete them so you have
    "selected_search_targets":{}
 5. Open the advanced editor, and click "Advanced »" in the search box
 6. You should have Local catalog unchecked, LIBRARY OF CONGRESS checked because
    it is preselected, and NATIONAL LIBRARY OF FRANCE unchecked
 7. Toggle each checkbox so Local catalog and France are selected, and LoC is
    unselected.
 8. In a new broser tab, open the advanced editor, and click "Advanced »"
 9. You will see Local catalog unchecked, LoC checked, and France unchecked.
    That's the bug.
10. Apply patch, open the advanced editor in a new tab, click "Advanced »"
11. Toggle the checkboxes so Local catalog and France are selected, and LoC
    is not selected.
12. Open the advanced editor in another new tab, click "Advanced »"
13. You should see what you just set, Local catalog and France checked, and
    LoC unchecked, That's the correct behavior.
Comment 18 Owen Leonard 2024-05-03 15:12:27 UTC
Created attachment 166148 [details] [review]
Bug 27363: Restore temporary selection of Z39.50 targets throughout multiple searches

The advanced editor currently saves which Z39.50 servers are selected
in local storage using the index in the sorted array of servers, and
then misreads them back as though they were server_ids. We want them to
be server_ids, since that's immutable and the index can change with
changes in rank or server name.

We use the data-server-id property in the HTML more often as a server_id
than as the index that's currently stored there, so this patches
switches it back to be the server_id, and in the one place we would use
the index, instead uses Array.find to just get the server object instead
of referencing it by index.

Test plan:
 1. Set the preference EnableAdvancedCatalogingEditor to enable
 2. Administration - Z39.50 Servers, for NATIONAL LIBRARY OF FRANCE
    uncheck "Preselected"
 3. To be sure you don't have confusing saved preferences, open
    Firefox's Developer Tools, in the Storage tab open Local Storage and
    select your URL
 4. In the row for cateditor_preferences_{your Koha user id}, if there
    are any values for selected_search_targets, delete them so you
    have "selected_search_targets":{}
 5. Open the advanced editor, and click "Advanced »" in the search box
 6. You should have Local catalog unchecked, LIBRARY OF CONGRESS checked
    because it is preselected, and NATIONAL LIBRARY OF FRANCE unchecked
 7. Toggle each checkbox so Local catalog and France are selected, and
    LoC is unselected.
 8. In a new broser tab, open the advanced editor, and click "Advanced »"
 9. You will see Local catalog unchecked, LoC checked, and France
    unchecked.
    That's the bug.
10. Apply patch, open the advanced editor in a new tab, click "Advanced »"
11. Toggle the checkboxes so Local catalog and France are selected, and
    LoC is not selected.
12. Open the advanced editor in another new tab, click "Advanced »"
13. You should see what you just set, Local catalog and France checked,
    and LoC unchecked, That's the correct behavior.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 19 Nick Clemens (kidclamp) 2024-05-09 12:44:34 UTC
Created attachment 166454 [details] [review]
Bug 27363: Restore temporary selection of Z39.50 targets throughout multiple searches

The advanced editor currently saves which Z39.50 servers are selected
in local storage using the index in the sorted array of servers, and
then misreads them back as though they were server_ids. We want them to
be server_ids, since that's immutable and the index can change with
changes in rank or server name.

We use the data-server-id property in the HTML more often as a server_id
than as the index that's currently stored there, so this patches
switches it back to be the server_id, and in the one place we would use
the index, instead uses Array.find to just get the server object instead
of referencing it by index.

Test plan:
 1. Set the preference EnableAdvancedCatalogingEditor to enable
 2. Administration - Z39.50 Servers, for NATIONAL LIBRARY OF FRANCE
    uncheck "Preselected"
 3. To be sure you don't have confusing saved preferences, open
    Firefox's Developer Tools, in the Storage tab open Local Storage and
    select your URL
 4. In the row for cateditor_preferences_{your Koha user id}, if there
    are any values for selected_search_targets, delete them so you
    have "selected_search_targets":{}
 5. Open the advanced editor, and click "Advanced »" in the search box
 6. You should have Local catalog unchecked, LIBRARY OF CONGRESS checked
    because it is preselected, and NATIONAL LIBRARY OF FRANCE unchecked
 7. Toggle each checkbox so Local catalog and France are selected, and
    LoC is unselected.
 8. In a new broser tab, open the advanced editor, and click "Advanced »"
 9. You will see Local catalog unchecked, LoC checked, and France
    unchecked.
    That's the bug.
10. Apply patch, open the advanced editor in a new tab, click "Advanced »"
11. Toggle the checkboxes so Local catalog and France are selected, and
    LoC is not selected.
12. Open the advanced editor in another new tab, click "Advanced »"
13. You should see what you just set, Local catalog and France checked,
    and LoC unchecked, That's the correct behavior.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 20 Katrin Fischer 2024-05-10 12:12:48 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 21 Fridolin Somers 2024-05-24 15:22:32 UTC
Pushed to 23.11.x for 23.11.06
Comment 22 Lucas Gass (lukeg) 2024-05-28 21:59:34 UTC
Backported to 23.05.x for upcoming 23.05.12