Description
Phil Ringnalda
2023-04-07 15:31:34 UTC
Created attachment 149504 [details] [review] Revert "Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail)" This reverts commit 26c26164ba78029cb99fa99662f9e0a8ce9557df. Test plan: 1. Without the patch, edit a personal name authority record, replacing the value in 100 subfield a (200 subfield a for UNIMARC) with your own name. 2. Click "Replace record via Z39.50/SRU search" and note that the prefilled search field contains the original value, not your name. 3. Replace the Author (personal) field with your name again, choose a search target, and click Search. 4. Note that the results of the search are for the original value, not for your name. 5. Apply patch, repeat, noting that the search form is prefilled with your change made in the editor, and any changes you make in the search form are reflected in what the search results return. Created attachment 149624 [details] [review] Revert "Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail)" This reverts commit 26c26164ba78029cb99fa99662f9e0a8ce9557df. Test plan: 1. Without the patch, edit a personal name authority record, replacing the value in 100 subfield a (200 subfield a for UNIMARC) with your own name. 2. Click "Replace record via Z39.50/SRU search" and note that the prefilled search field contains the original value, not your name. 3. Replace the Author (personal) field with your name again, choose a search target, and click Search. 4. Note that the results of the search are for the original value, not for your name. 5. Apply patch, repeat, noting that the search form is prefilled with your change made in the editor, and any changes you make in the search form are reflected in what the search results return. https://bugs.koha-community.org/show_bug.cgi?id=33445 Signed-off-by: David Nind <david@davidnind.com> Testing notes (using KTD): 1. I have signed off based on what is in the test plan working. I'm not familiar enough with authorities, and how it should work, to comment on that. 2. I think the bug title/description needs fixing - see https://wiki.koha-community.org/wiki/Commit_messages#Subject_line Hi Phil, if you are working on a patch, please remember to make yourself Assignee of the bug. It helps to avoid doubled up work among other things. Created attachment 150367 [details] [review] Bug 33445: [alternate] Pass through form info if present and use record values otherwise This patch updates the form to pass through values formthe form when searching, and to use record values if nothing is passed in. TO test: 1. Without the patch, edit a personal name authority record, replacing the value in 100 subfield a (200 subfield a for UNIMARC) with your own name. 2. Click "Replace record via Z39.50/SRU search" and note that the prefilled search field contains the original value, not your name. 3. Replace the Author (personal) field with your name again, choose a search target, and click Search. 4. Note that the results of the search are for the original value, not for your name. 5. Apply patch, repeat, noting that the search form is prefilled with your change made in the editor, and any changes you make in the search form are reflected in what the search results return. Created attachment 150368 [details] [review] Bug 33445: [alternate] Pass through form info if present and use record values otherwise This patch updates the form to pass through values formthe form when searching, and to use record values if nothing is passed in. TO test: 1 - Without the patch, edit a personal name authority record 2 - Replace 100 subfield a (200 subfield a for UNIMARC) with your own name 3 - Click "Replace record via Z39.50/SRU search" 4 - Note that prefilled search field contains the original value, not your name. 5 - Replace the Author (personal) field with your name in the search form 6 - choose a search target, and click Search. 7 - Note that the results of the search are for the original value, not your name 8 - From the details page of a record, click 'Edit -> Replace record via Z3950' 9 - Note form is prefilled 10 - Change value as in 5 and search 11 - Note original term was searched 12 - Apply patch, restart all 13 - Repeat searches as before 14 - Confirm that form is prefilled with record or the updated value 15 - Confirm your entered search terms are always searched Rather than revert, I think we need to ensure we handle record values when needed, and the form values when searching. I attached an alternate That does still retain one behavior change, altering the meaning of the "Try another search" button on search results from the historic "Try a different search starting from scratch with an empty form" to "Try another search starting with a form prefilled from the saved record" but I can't object too much to that, since I have always avoided the another search from scratch button, preferring to use the browser's back button if I'm only a page or two forward, or just closing and reopening the popup if I'm several pages deep. According to my concept of the meaning of "stable" we should still revert back down to oldoldstable and then land this on master, but obviously my concept of stable is very different from Koha's. Created attachment 150419 [details] [review] Bug 33445: [alternate] Pass through form info if present and use record values otherwise This patch updates the form to pass through values formthe form when searching, and to use record values if nothing is passed in. TO test: 1 - Without the patch, edit a personal name authority record 2 - Replace 100 subfield a (200 subfield a for UNIMARC) with your own name 3 - Click "Replace record via Z39.50/SRU search" 4 - Note that prefilled search field contains the original value, not your name. 5 - Replace the Author (personal) field with your name in the search form 6 - choose a search target, and click Search. 7 - Note that the results of the search are for the original value, not your name 8 - From the details page of a record, click 'Edit -> Replace record via Z3950' 9 - Note form is prefilled 10 - Change value as in 5 and search 11 - Note original term was searched 12 - Apply patch, restart all 13 - Repeat searches as before 14 - Confirm that form is prefilled with record or the updated value 15 - Confirm your entered search terms are always searched Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> I was testing at the same time as Phil (using KTD): 1. Everything works as per the test plan. 2. However, after applying the patch and repeating the searches: - for Step 6, after getting no results using my name and clicking on "Try another search", the name in the field reverts to the original record name and not my name - for Step 10, the same as step 6 - which is probably correct (but I'm not sure...) - feel free to change the status to Failed QA if this is not the expected behavour (I would probably have failed QA if there wasn't already a sign off) 3. I tested using MARC21 and UNIMARC instances. (In reply to David Nind from comment #10) > I was testing at the same time as Phil (using KTD): > > 1. Everything works as per the test plan. > > 2. However, after applying the patch and repeating the searches: > - for Step 6, after getting no results using my name and clicking on "Try > another search", the name in the field reverts to the original record name > and not my name > - for Step 10, the same as step 6 - which is probably correct (but I'm > not sure...) > - feel free to change the status to Failed QA if this is not the expected > behavour (I would probably have failed QA if there wasn't already a sign off) > > 3. I tested using MARC21 and UNIMARC instances. Hi Nick, could you please have a look at David's notes here? I believe we'd always want the new name, so that would be one last glitch to fix. Created attachment 150466 [details] [review] Bug 33445: Pass through form info if present and use record values otherwise This patch updates the form to pass through values formthe form when searching, and to use record values if nothing is passed in. TO test: 1 - Without the patch, edit a personal name authority record 2 - Replace 100 subfield a (200 subfield a for UNIMARC) with your own name 3 - Click "Replace record via Z39.50/SRU search" 4 - Note that prefilled search field contains the original value, not your name. 5 - Replace the Author (personal) field with your name in the search form 6 - choose a search target, and click Search. 7 - Note that the results of the search are for the original value, not your name 8 - From the details page of a record, click 'Edit -> Replace record via Z3950' 9 - Note form is prefilled 10 - Change value as in 5 and search 11 - Note original term was searched 12 - Apply patch, restart all 13 - Repeat searches as before 14 - Confirm that form is prefilled with record or the updated value 15 - Confirm your entered search terms are always searched Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Created attachment 150467 [details] [review] Bug 33445: (follow-up) Pass form values through when clicking 'Try another search' We didn't include anything except the authid id in the try again form, this patch extends to cover all the fields Created attachment 150468 [details] [review] Bug 33445: (follow-up) Allow all search fields to be passed through and returned when trying another search It seems the form didn't populate all fields form data passed in. This patch corrects that Comment on attachment 150468 [details] [review] Bug 33445: (follow-up) Allow all search fields to be passed through and returned when trying another search >-[% USE raw %] >+nt[% USE raw %] Haven't looked at whether that nt belonged somewhere else, but it doesn't look right there. (In reply to Phil Ringnalda from comment #15) > Comment on attachment 150468 [details] [review] [review] > Bug 33445: (follow-up) Allow all search fields to be passed through and > returned when trying another search > > >-[% USE raw %] > >+nt[% USE raw %] > > Haven't looked at whether that nt belonged somewhere else, but it doesn't > look right there. Was just retesting - the nt appears at the top left of the Z3950 search window. Created attachment 150469 [details] [review] Bug 33445: (follow-up) Allow all search fields to be passed through and returned when trying another search It seems the form didn't populate all fields form data passed in. This patch corrects that (In reply to David Nind from comment #16) > (In reply to Phil Ringnalda from comment #15) > > Was just retesting - the nt appears at the top left of the Z3950 search > window. That was a slip, new patch attached Created attachment 150470 [details] [review] Bug 33445: Pass through form info if present and use record values otherwise This patch updates the form to pass through values formthe form when searching, and to use record values if nothing is passed in. TO test: 1 - Without the patch, edit a personal name authority record 2 - Replace 100 subfield a (200 subfield a for UNIMARC) with your own name 3 - Click "Replace record via Z39.50/SRU search" 4 - Note that prefilled search field contains the original value, not your name. 5 - Replace the Author (personal) field with your name in the search form 6 - choose a search target, and click Search. 7 - Note that the results of the search are for the original value, not your name 8 - From the details page of a record, click 'Edit -> Replace record via Z3950' 9 - Note form is prefilled 10 - Change value as in 5 and search 11 - Note original term was searched 12 - Apply patch, restart all 13 - Repeat searches as before 14 - Confirm that form is prefilled with record or the updated value 15 - Confirm your entered search terms are always searched Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: David Nind <david@davidnind.com> Created attachment 150471 [details] [review] Bug 33445: (follow-up) Pass form values through when clicking 'Try another search' We didn't include anything except the authid id in the try again form, this patch extends to cover all the fields Signed-off-by: David Nind <david@davidnind.com> Created attachment 150472 [details] [review] Bug 33445: (follow-up) Allow all search fields to be passed through and returned when trying another search It seems the form didn't populate all fields form data passed in. This patch corrects that Signed-off-by: David Nind <david@davidnind.com> (In reply to Nick Clemens from comment #18) > (In reply to David Nind from comment #16) > > (In reply to Phil Ringnalda from comment #15) > > > > Was just retesting - the nt appears at the top left of the Z3950 search > > window. > > That was a slip, new patch attached Thanks Nick! Now signed off. I added some values to the keyword input field, and these were retained (including 'Try another search'. David Created attachment 150473 [details] [review] Bug 33445: Pass through form info if present and use record values otherwise This patch updates the form to pass through values formthe form when searching, and to use record values if nothing is passed in. TO test: 1 - Without the patch, edit a personal name authority record 2 - Replace 100 subfield a (200 subfield a for UNIMARC) with your own name 3 - Click "Replace record via Z39.50/SRU search" 4 - Note that prefilled search field contains the original value, not your name. 5 - Replace the Author (personal) field with your name in the search form 6 - choose a search target, and click Search. 7 - Note that the results of the search are for the original value, not your name 8 - From the details page of a record, click 'Edit -> Replace record via Z3950' 9 - Note form is prefilled 10 - Change value as in 5 and search 11 - Note original term was searched 12 - Apply patch, restart all 13 - Repeat searches as before 14 - Confirm that form is prefilled with record or the updated value 15 - Confirm your entered search terms are always searched Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: David Nind <david@davidnind.com> Created attachment 150474 [details] [review] Bug 33445: (follow-up) Pass form values through when clicking 'Try another search' We didn't include anything except the authid id in the try again form, this patch extends to cover all the fields Signed-off-by: David Nind <david@davidnind.com> Created attachment 150475 [details] [review] Bug 33445: (follow-up) Allow all search fields to be passed through and returned when trying another search It seems the form didn't populate all fields form data passed in. This patch corrects that Signed-off-by: David Nind <david@davidnind.com> Apologies for the noise - somehow managed to add the patch with the 'nt' when I signed off. Amended the patch, so should now be correct! Created attachment 151125 [details] [review] Bug 33445: Pass through form info if present and use record values otherwise This patch updates the form to pass through values formthe form when searching, and to use record values if nothing is passed in. TO test: 1 - Without the patch, edit a personal name authority record 2 - Replace 100 subfield a (200 subfield a for UNIMARC) with your own name 3 - Click "Replace record via Z39.50/SRU search" 4 - Note that prefilled search field contains the original value, not your name. 5 - Replace the Author (personal) field with your name in the search form 6 - choose a search target, and click Search. 7 - Note that the results of the search are for the original value, not your name 8 - From the details page of a record, click 'Edit -> Replace record via Z3950' 9 - Note form is prefilled 10 - Change value as in 5 and search 11 - Note original term was searched 12 - Apply patch, restart all 13 - Repeat searches as before 14 - Confirm that form is prefilled with record or the updated value 15 - Confirm your entered search terms are always searched Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 151126 [details] [review] Bug 33445: (follow-up) Pass form values through when clicking 'Try another search' We didn't include anything except the authid id in the try again form, this patch extends to cover all the fields Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 151127 [details] [review] Bug 33445: (follow-up) Allow all search fields to be passed through and returned when trying another search It seems the form didn't populate all fields form data passed in. This patch corrects that Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Pushed to master for 23.05. Nice work everyone, thanks! Nice work everyone! Pushed to stable for 22.11.x Merge conflicts in 22.05.x, no backport. Please rebase if needed. |