Bug 29173 - Button "replace authority record via Z39/50/SRU" doesn't pre-fill
Summary: Button "replace authority record via Z39/50/SRU" doesn't pre-fill
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Thibaud Guillot
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks: 32906 33445
  Show dependency treegraph
 
Reported: 2021-10-05 12:14 UTC by Marion Durand
Modified: 2023-12-28 20:44 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes the behaviour of the replace an authority record via Z39.50/SRU buttons when editing an authority record. Both ways of doing this (Edit > Edit record > Replace record via Z39.50/SRU search and Edit > Replace record via Z39.50/SRU search) now pre-fill the search form with available data.
Version(s) released in:
23.05.00,22.11.03,22.05.10,21.11.18


Attachments
Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail) (3.25 KB, patch)
2021-12-09 15:18 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail) (3.31 KB, patch)
2021-12-11 09:49 UTC, David Nind
Details | Diff | Splinter Review
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail) (3.47 KB, patch)
2021-12-20 10:29 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail) (3.31 KB, patch)
2021-12-20 10:57 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 29173: (follow-up) add marcflavour preference in conditions (2.05 KB, patch)
2021-12-20 10:57 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 29173: (follow-up) add marcflavour preference in conditions (2.81 KB, patch)
2021-12-20 13:16 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail) (3.36 KB, patch)
2022-02-05 19:50 UTC, David Nind
Details | Diff | Splinter Review
Bug 29173: (follow-up) add marcflavour preference in conditions (2.85 KB, patch)
2022-02-05 19:50 UTC, David Nind
Details | Diff | Splinter Review
Bug 29173 : (follow-up) replace method to get the record from the authority (1.09 KB, patch)
2022-02-10 08:46 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail) (2.17 KB, patch)
2022-03-29 14:15 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail) (2.13 KB, patch)
2022-06-22 12:58 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail) (2.13 KB, patch)
2022-06-22 13:07 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail) (2.13 KB, patch)
2022-06-23 10:10 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail) (3.29 KB, patch)
2022-12-28 16:11 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail) (3.38 KB, patch)
2023-01-04 08:28 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail) (3.42 KB, patch)
2023-01-04 11:18 UTC, David Nind
Details | Diff | Splinter Review
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail) (3.49 KB, patch)
2023-02-08 06:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29173: (QA follow-up) Match pop-over sizes (1.50 KB, patch)
2023-02-08 06:57 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marion Durand 2021-10-05 12:14:22 UTC
To replace an authority record via Z39.50/SRU there are two buttons on differents pages but their behaviour is not the same. One pre-fill the search form but the other don't.

If I do:
1- go to the detail page of an authority record [Koha]/cgi-bin/koha/authorities/detail.pl?authid=[ID]
where Koha is your instance of koha and ID is your authority id
2- On the menu Edit click on the button "Replace record via Z39.50/SRU"
Then:
"Z39.50 authority search points" pop up but all field are empty.

But If I do:
1- go to the detail page of an authority record [Koha]/cgi-bin/koha/authorities/detail.pl?authid=[ID]
where Koha is your instance of koha and ID is your authority id
2- On the menu Edit click on the button "Edit record"
3- Click on the button "Replace record via Z39.50/SRU search"
Then:
"Z39.50 authority search points" pop up with usefull values already filled (for example the Author)

There is the same two button for bibliographic record and both of them pre-fill the form.
Comment 1 Marion Durand 2021-12-07 10:47:33 UTC
(be sur to have at least one Z39-50 or SRU serveur set for authority or the fisrt buton won't be displayed)
Comment 2 Fridolin Somers 2021-12-07 21:00:05 UTC
Indeed, this is a bug (not enhancement)
Comment 3 Thibaud Guillot 2021-12-09 15:18:01 UTC
Created attachment 128378 [details] [review]
Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail)

Now this action has the same pre-filling behaviour as the "Edit" -> "Edit Record" -> "Replace Record via Z39.50/SRU Search" action.
Author Personal - Author Meeting/Conf. - Uniform Title and Subject are filled in if the subfield contains data (like the "Edit->Replace" action).
Unimarc and Marc21 specificities are supported.

Test Plan :

1) You must have a Z39.50/SRU server for authorities
2) Go to an authority detail page (Author - Uniform title - Subject)
3) First go to "Edit"-> "Edit record" -> "Replace record via..."
4) See that form is prefilled with form data
5) Go back and now click on "Replace Record.." directly
6) See that form is also prefilled with data now
Comment 4 David Nind 2021-12-11 09:49:01 UTC
Created attachment 128410 [details] [review]
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail)

Now this action has the same pre-filling behaviour as the "Edit" -> "Edit Record" -> "Replace Record via Z39.50/SRU Search" action.
Author Personal - Author Meeting/Conf. - Uniform Title and Subject are filled in if the subfield contains data (like the "Edit->Replace" action).
Unimarc and Marc21 specificities are supported.

Test Plan :

1) You must have a Z39.50/SRU server for authorities
2) Go to an authority detail page (Author - Uniform title - Subject)
3) First go to "Edit"-> "Edit record" -> "Replace record via..."
4) See that form is prefilled with form data
5) Go back and now click on "Replace Record.." directly
6) See that form is also prefilled with data now

https://bugs.koha-community.org/show_bug.cgi?id=29173

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2021-12-11 09:51:03 UTC
Comment on attachment 128378 [details] [review]
Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail)

Not sure what I did - obsoleting the patch. Note: I changed the title of the commit message so it includes the bug number.
Comment 6 Thibaud Guillot 2021-12-13 08:46:23 UTC
(In reply to David Nind from comment #5)
> Comment on attachment 128378 [details] [review] [review]
> Fixed pre-filling of the search form from the "Replace record with
> ZR39.50/SRU" (Authority page detail)
> 
> Not sure what I did - obsoleting the patch. Note: I changed the title of the
> commit message so it includes the bug number.

Okay, thanks for commit title, I'm new on this so I can made some mistakes :)
Comment 7 David Nind 2021-12-13 09:24:14 UTC
> Okay, thanks for commit title, I'm new on this so I can made some mistakes :)

8-) The QA Team are very helpful in helping out new contributors!

Here are the guidelines for commits:
https://wiki.koha-community.org/wiki/Commit_messages
Comment 8 Katrin Fischer 2021-12-18 22:50:28 UTC
Hi Marion, 

I believe this will work nicely for MARC21, but not work for UNIMARC as the fields are quite different.

Why in MARC21 100 is a person's name, in UNIMARC it seems to be used quite differently:

https://www.ifla.org/g/unimarc-rg/unimarc-authorities-3rd-edition-with-updates/
100 – General Processing Data (update 2018)

So I think we need at least a check on the marcflavor, but it would be even nicer if we could pick out the right fields for UNIMARC as well.

Also I wonder if we could avoid querying the record twice, but someone else might be better suited to comment on that:

my $record = GetAuthority($authid);
my $authobj = Koha::Authorities->find($authid);
Comment 9 Marion Durand 2021-12-20 08:26:04 UTC
(In reply to Katrin Fischer from comment #8)
> Hi Marion, 
> 
> I believe this will work nicely for MARC21, but not work for UNIMARC as the
> fields are quite different.
> 
> Why in MARC21 100 is a person's name, in UNIMARC it seems to be used quite
> differently:
> 
> https://www.ifla.org/g/unimarc-rg/unimarc-authorities-3rd-edition-with-
> updates/
> 100 – General Processing Data (update 2018)
> 
> So I think we need at least a check on the marcflavor, but it would be even
> nicer if we could pick out the right fields for UNIMARC as well.
> 
> Also I wonder if we could avoid querying the record twice, but someone else
> might be better suited to comment on that:
> 
> my $record = GetAuthority($authid);
> my $authobj = Koha::Authorities->find($authid);

Hi Katrin,

Thank you for looking at this bug.

I must point out that I did nothing on this bug (exept filing it). 
Thibaud is the one who wrote the patch :)
Comment 10 Katrin Fischer 2021-12-20 08:29:35 UTC
> Hi Katrin,
> 
> Thank you for looking at this bug.
> 
> I must point out that I did nothing on this bug (exept filing it). 
> Thibaud is the one who wrote the patch :)

I am sorry, misreading on my part.
Comment 11 Thibaud Guillot 2021-12-20 10:29:14 UTC Comment hidden (obsolete)
Comment 12 Thibaud Guillot 2021-12-20 10:57:19 UTC Comment hidden (obsolete)
Comment 13 Thibaud Guillot 2021-12-20 10:57:24 UTC
Created attachment 128731 [details] [review]
Bug 29173: (follow-up) add marcflavour preference in conditions
Comment 14 Thibaud Guillot 2021-12-20 11:04:54 UTC
Thanks to Katrin for the review, I have modified the patch to add the marcflavour preference in the conditions. 

Regarding these two lines: 
"my $record = GetAuthority($authid);"
This is to get the contents of the fields in the "authfields" array
"my $authobj = Koha::Authorities->find($authid);"
This is to retrieve the authority as an object and extract the authority type.

If there is a simpler method I'm interested of course.
Comment 15 Jonathan Druart 2021-12-20 12:19:01 UTC
(In reply to Katrin Fischer from comment #8)
> Also I wonder if we could avoid querying the record twice, but someone else
> might be better suited to comment on that:
> 
> my $record = GetAuthority($authid);
> my $authobj = Koha::Authorities->find($authid);

Agreed, that certainly could be replaced with

my $authority = Koha::Authorities->find($authid);
my $record = $authority->record;
Comment 16 Thibaud Guillot 2021-12-20 13:16:44 UTC
Created attachment 128756 [details] [review]
Bug 29173: (follow-up) add marcflavour preference in conditions
Comment 17 David Nind 2022-02-05 19:50:52 UTC
Created attachment 130192 [details] [review]
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail)

Now this action has the same pre-filling behaviour as the "Edit" -> "Edit Record" -> "Replace Record via Z39.50/SRU Search" action.
Author Personal - Author Meeting/Conf. - Uniform Title and Subject are filled in if the subfield contains data (like the "Edit->Replace" action).
Unimarc and Marc21 specificities are supported.

Test Plan :

1) You must have a Z39.50/SRU server for authorities
2) Go to an authority detail page (Author - Uniform title - Subject)
3) First go to "Edit"-> "Edit record" -> "Replace record via..."
4) See that form is prefilled with form data
5) Go back and now click on "Replace Record.." directly
6) See that form is also prefilled with data now

https://bugs.koha-community.org/show_bug.cgi?id=29173

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: David Nind <david@davidnind.com>
Comment 18 David Nind 2022-02-05 19:50:57 UTC
Created attachment 130193 [details] [review]
Bug 29173: (follow-up) add marcflavour preference in conditions

Signed-off-by: David Nind <david@davidnind.com>
Comment 19 David Nind 2022-02-05 20:14:58 UTC
Testing notes (using koha-testing-docker):

- Tested using both a MARC21 and UNIMARC installation.

- For a UNIMARC installation there is only one way to replace (both before and after the patch is applied):
  . Select Edit > Edit record
  . Click on the 'Replace record via Z39.50/SRU search' button
  => There is no option in the dropdown list to change like a MARC21 installation (Edit > Replace record via Z39.50/SRU search)
Comment 20 Jonathan Druart 2022-02-08 17:35:19 UTC
(In reply to Jonathan Druart from comment #15)
> (In reply to Katrin Fischer from comment #8)
> > Also I wonder if we could avoid querying the record twice, but someone else
> > might be better suited to comment on that:
> > 
> > my $record = GetAuthority($authid);
> > my $authobj = Koha::Authorities->find($authid);
> 
> Agreed, that certainly could be replaced with
> 
> my $authority = Koha::Authorities->find($authid);
> my $record = $authority->record;

What about this, Thibaud?
Comment 21 Thibaud Guillot 2022-02-10 08:46:51 UTC
Created attachment 130422 [details] [review]
Bug 29173 : (follow-up) replace method to get the record from the authority
Comment 22 Thibaud Guillot 2022-02-10 08:49:33 UTC
(In reply to Jonathan Druart from comment #20)
> (In reply to Jonathan Druart from comment #15)
> > (In reply to Katrin Fischer from comment #8)
> > > Also I wonder if we could avoid querying the record twice, but someone else
> > > might be better suited to comment on that:
> > > 
> > > my $record = GetAuthority($authid);
> > > my $authobj = Koha::Authorities->find($authid);
> > 
> > Agreed, that certainly could be replaced with
> > 
> > my $authority = Koha::Authorities->find($authid);
> > my $record = $authority->record;
> 
> What about this, Thibaud?

Yes it's simpler.. I don't know why the first time that I tested I found that it doesn't work.. its my mistake, sorry.

Changed and tested, it seems to be ok now
Comment 23 Thibaud Guillot 2022-02-10 09:36:28 UTC
(In reply to David Nind from comment #19)
> Testing notes (using koha-testing-docker):
> 
> - Tested using both a MARC21 and UNIMARC installation.
> 
> - For a UNIMARC installation there is only one way to replace (both before
> and after the patch is applied):
>   . Select Edit > Edit record
>   . Click on the 'Replace record via Z39.50/SRU search' button
>   => There is no option in the dropdown list to change like a MARC21
> installation (Edit > Replace record via Z39.50/SRU search)

Oh ok, I see what you mean, but maybe this dropdown list is about another issue or enhancement and not directly on this BZ. Its my opinion but I can make a mistake, I'm just a newbie on Koha :)
Comment 24 David Nind 2022-02-10 10:06:20 UTC
(In reply to Thibaud Guillot from comment #23)
 
> Oh ok, I see what you mean, but maybe this dropdown list is about another
> issue or enhancement and not directly on this BZ. Its my opinion but I can
> make a mistake, I'm just a newbie on Koha :)

It was more just an observation, so not something that needs to be "fixed" if someone wants to.
Comment 25 Jonathan Druart 2022-02-18 09:41:48 UTC
Could not compile /kohadevbox/koha/cataloguing/z3950_auth_search.pl: Global symbol "$authobj" requires explicit package name (did you forget to declare "my $authobj"?) at /kohadevbox/koha/cataloguing/z3950_auth_search.pl line 39.
Comment 26 Jonathan Druart 2022-02-18 09:43:09 UTC
Also the C4::AuthoritiesMarc import is not longer needed.
Comment 27 Thibaud Guillot 2022-03-29 14:15:49 UTC
Created attachment 132443 [details] [review]
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail)

Now this action has the same pre-filling behaviour as the "Edit" -> "Edit Record" -> "Replace Record via Z39.50/SRU Search" action.
Author Personal - Author Meeting/Conf. - Uniform Title and Subject are filled in if the subfield contains data (like the "Edit->Replace" action).
Unimarc and Marc21 specificities are supported.

Test Plan :

1) You must have a Z39.50/SRU server for authorities
2) Go to an authority detail page (Author - Uniform title - Subject)
3) First go to "Edit"-> "Edit record" -> "Replace record via..."
4) See that form is prefilled with form data
5) Go back and now click on "Replace Record.." directly
6) See that form is also prefilled with data now
Comment 28 Thibaud Guillot 2022-06-22 12:58:20 UTC
Created attachment 136410 [details] [review]
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail)

Now this action has the same pre-filling behaviour as the "Edit" -> "Edit Record" -> "Replace Record via Z39.50/SRU Search" action.
Author Personal - Author Meeting/Conf. - Uniform Title and Subject are filled in if the subfield contains data (like the "Edit->Replace" action).
Unimarc and Marc21 specificities are supported.

Test Plan :

1) You must have a Z39.50/SRU server for authorities
2) Go to an authority detail page (Author - Uniform title - Subject)
3) First go to "Edit"-> "Edit record" -> "Replace record via..."
4) See that form is prefilled with form data
5) Go back and now click on "Replace Record.." directly
6) See that form is also prefilled with data now
Comment 29 Thibaud Guillot 2022-06-22 13:07:28 UTC
Created attachment 136411 [details] [review]
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail)

Now this action has the same pre-filling behaviour as the "Edit" -> "Edit Record" -> "Replace Record via Z39.50/SRU Search" action.
Author Personal - Author Meeting/Conf. - Uniform Title and Subject are filled in if the subfield contains data (like the "Edit->Replace" action).
Unimarc and Marc21 specificities are supported.

Test Plan :

1) You must have a Z39.50/SRU server for authorities
2) Go to an authority detail page (Author - Uniform title - Subject)
3) First go to "Edit"-> "Edit record" -> "Replace record via..."
4) See that form is prefilled with form data
5) Go back and now click on "Replace Record.." directly
6) See that form is also prefilled with data now
Comment 30 David Nind 2022-06-22 21:27:50 UTC
I'm getting this error after applying the patch:

The method Koha::Authority->record is not covered by tests!

Trace begun at /kohadevbox/koha/Koha/Object.pm line 875
Koha::Object::AUTOLOAD('Koha::Authority=HASH(0x55aa079a8cf0)') called at /kohadevbox/koha/cataloguing/z3950_auth_search.pl line 37
eval {...} at /kohadevbox/koha/cataloguing/z3950_auth_search.pl line 2
CGI::Compile::ROOT::kohadevbox_koha_cataloguing_z3950_auth_search_2epl::__ANON__('CGI::Compile=HASH(0x55aa07855ba0)', '', '/kohadevbox/koha/cataloguing/z3950_auth_search.pl', '/kohadevbox/koha/cataloguing', 'ARRAY(0x55aa07853d88)') called at /usr/share/perl5/CGI/Compile.pm line 151
CGI::Compile::ROOT::kohadevbox_koha_cataloguing_z3950_auth_search_2epl::z3950_auth_search_2epl at /usr/share/perl5/CGI/Emulate/PSGI.pm line 30
CGI::Emulate::PSGI::__ANON__('HASH(0x55aa07ea9998)') called at /usr/share/perl5/Plack/App/WrapCGI.pm line 95
Plack::App::WrapCGI::call('Plack::App::WrapCGI=HASH(0x55aa07884ad0)', 'HASH(0x55aa07ea9998)') called at /usr/share/perl5/Plack/Component.pm line 50
Plack::Component::__ANON__('HASH(0x55aa07ea9998)') called at /usr/share/perl5/Plack/App/CGIBin.pm line 50
Plack::App::CGIBin::serve_path('Plack::App::CGIBin=HASH(0x55a9fcf2d7b8)', 'HASH(0x55aa07ea9998)', '/kohadevbox/koha/cataloguing/z3950_auth_search.pl') called at /usr/share/perl5/Plack/App/File.pm line 34
Plack::App::File::call('Plack::App::CGIBin=HASH(0x55a9fcf2d7b8)', 'HASH(0x55aa07ea9998)') called at /usr/share/perl5/Plack/Component.pm line 50
Plack::Component::__ANON__('HASH(0x55aa07ea9998)') called at /usr/share/perl5/Plack/Middleware/LogWarn.pm line 22
Plack::Middleware::LogWarn::call('Plack::Middleware::LogWarn=HASH(0x55aa0666c2e0)', 'HASH(0x55aa07ea9998)') called at /usr/share/perl5/Plack/Component.pm line 50
Plack::Component::__ANON__('HASH(0x55aa07ea9998)') called at /usr/share/perl5/Plack/Middleware/Log4perl.pm line 30
Plack::Middleware::Log4perl::call('Plack::Middleware::Log4perl=HASH(0x55aa0666c670)', 'HASH(0x55aa07ea9998)') called at /usr/share/perl5/Plack/Component.pm line 50
Plack::Component::__ANON__('HASH(0x55aa07ea9998)') called at /usr/share/perl5/Plack/Middleware/HTTPExceptions.pm line 20
Plack::Middleware::HTTPExceptions::try {...}  at /usr/share/perl5/Try/Tiny.pm line 100
.......
Comment 31 Thibaud Guillot 2022-06-23 10:10:10 UTC
Created attachment 136440 [details] [review]
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail)

Now this action has the same pre-filling behaviour as the "Edit" -> "Edit Record" -> "Replace Record via Z39.50/SRU Search" action.
Author Personal - Author Meeting/Conf. - Uniform Title and Subject are filled in if the subfield contains data (like the "Edit->Replace" action).
Unimarc and Marc21 specificities are supported.

Test Plan :

1) You must have a Z39.50/SRU server for authorities
2) Go to an authority detail page (Author - Uniform title - Subject)
3) First go to "Edit"-> "Edit record" -> "Replace record via..."
4) See that form is prefilled with form data
5) Go back and now click on "Replace Record.." directly
6) See that form is also prefilled with data now
Comment 32 Thibaud Guillot 2022-06-23 10:11:29 UTC
(In reply to David Nind from comment #30)
> I'm getting this error after applying the patch:
> 
> The method Koha::Authority->record is not covered by tests!
> 
> Trace begun at /kohadevbox/koha/Koha/Object.pm line 875
> Koha::Object::AUTOLOAD('Koha::Authority=HASH(0x55aa079a8cf0)') called at
> /kohadevbox/koha/cataloguing/z3950_auth_search.pl line 37
> eval {...} at /kohadevbox/koha/cataloguing/z3950_auth_search.pl line 2
> CGI::Compile::ROOT::kohadevbox_koha_cataloguing_z3950_auth_search_2epl::
> __ANON__('CGI::Compile=HASH(0x55aa07855ba0)', '',
> '/kohadevbox/koha/cataloguing/z3950_auth_search.pl',
> '/kohadevbox/koha/cataloguing', 'ARRAY(0x55aa07853d88)') called at
> /usr/share/perl5/CGI/Compile.pm line 151
> CGI::Compile::ROOT::kohadevbox_koha_cataloguing_z3950_auth_search_2epl::
> z3950_auth_search_2epl at /usr/share/perl5/CGI/Emulate/PSGI.pm line 30
> CGI::Emulate::PSGI::__ANON__('HASH(0x55aa07ea9998)') called at
> /usr/share/perl5/Plack/App/WrapCGI.pm line 95
> Plack::App::WrapCGI::call('Plack::App::WrapCGI=HASH(0x55aa07884ad0)',
> 'HASH(0x55aa07ea9998)') called at /usr/share/perl5/Plack/Component.pm line 50
> Plack::Component::__ANON__('HASH(0x55aa07ea9998)') called at
> /usr/share/perl5/Plack/App/CGIBin.pm line 50
> Plack::App::CGIBin::serve_path('Plack::App::CGIBin=HASH(0x55a9fcf2d7b8)',
> 'HASH(0x55aa07ea9998)', '/kohadevbox/koha/cataloguing/z3950_auth_search.pl')
> called at /usr/share/perl5/Plack/App/File.pm line 34
> Plack::App::File::call('Plack::App::CGIBin=HASH(0x55a9fcf2d7b8)',
> 'HASH(0x55aa07ea9998)') called at /usr/share/perl5/Plack/Component.pm line 50
> Plack::Component::__ANON__('HASH(0x55aa07ea9998)') called at
> /usr/share/perl5/Plack/Middleware/LogWarn.pm line 22
> Plack::Middleware::LogWarn::call('Plack::Middleware::
> LogWarn=HASH(0x55aa0666c2e0)', 'HASH(0x55aa07ea9998)') called at
> /usr/share/perl5/Plack/Component.pm line 50
> Plack::Component::__ANON__('HASH(0x55aa07ea9998)') called at
> /usr/share/perl5/Plack/Middleware/Log4perl.pm line 30
> Plack::Middleware::Log4perl::call('Plack::Middleware::
> Log4perl=HASH(0x55aa0666c670)', 'HASH(0x55aa07ea9998)') called at
> /usr/share/perl5/Plack/Component.pm line 50
> Plack::Component::__ANON__('HASH(0x55aa07ea9998)') called at
> /usr/share/perl5/Plack/Middleware/HTTPExceptions.pm line 20
> Plack::Middleware::HTTPExceptions::try {...}  at
> /usr/share/perl5/Try/Tiny.pm line 100
> .......

Maybe I miss the rebase, can you test it now and tell me if you have the same issue ?
Comment 33 David Nind 2022-06-23 12:00:53 UTC
(In reply to Thibaud Guillot from comment #32)

> Maybe I miss the rebase, can you test it now and tell me if you have the
> same issue ?

I still have the same issue.

Testing using koha-testing-docker, current master, flush_memcached, restart_all, and cleared browser cache.

For what it's worth: I was getting a similar error with some recalls related bugs, such as bug 30971.
Comment 34 Thibault Keromnès 2022-06-30 15:44:13 UTC
I have the same issue as David (from a sandbox). 
The patch applies but the error happens when trying to replace authorities (even by the method that works without the patch)
Comment 35 Thibaud Guillot 2022-12-28 16:11:51 UTC
Created attachment 144867 [details] [review]
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail)

Test Plan :

1) You must have a Z39.50/SRU server for authorities
2) Go to an authority detail page (Author - Uniform title - Subject)
3)After go to "Edit"-> "Edit record" -> "Replace record via..." or
choose "Replace Record.." directly
4) See that form is also prefilled with data now
Comment 36 Thibaud Guillot 2022-12-28 16:16:06 UTC
I change the logical this time, in a "easier" way ? With a direct mapping with data's fields and sufields..

I hope this work now for you :)

I also changed my test plan cause now I had the same behavior when I click on "Edit" then "Replace with.." than directly choose 'Replace with.."

I'm waiting your feedbacks, thanks
Comment 37 David Nind 2023-01-03 23:39:34 UTC
Hi 

I'm getting this error when trying to apply the patch:

root@kohadevbox:koha(bz29173)$ git bz apply 29173

Bug 29173 - Button "replace authority record via Z39/50/SRU" doesn't pre-fill

144867 - Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail)

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail)
error: sha1 information is lacking or useless (cataloguing/z3950_auth_search.pl).
error: could not build fake ancestor
Patch failed at 0001 Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail)

David
Comment 38 Thibaud Guillot 2023-01-04 08:28:03 UTC
Created attachment 144972 [details] [review]
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail)

Now this action has the same pre-filling behaviour as the "Edit" -> "Edit Record" -> "Replace Record via Z39.50/SRU Search" action.
Author Personal - Author Meeting/Conf. - Uniform Title and Subject are filled in if the subfield contains data (like the "Edit->Replace" action).
Unimarc and Marc21 specificities are supported.

Test Plan :

1) You must have a Z39.50/SRU server for authorities
2) Go to an authority detail page (Author - Uniform title - Subject)
3)After go to "Edit"-> "Edit record" -> "Replace record via..." or
choose "Replace Record.." directly
4) See that form is also prefilled with data now
Comment 39 Thibaud Guillot 2023-01-04 08:58:32 UTC
(In reply to David Nind from comment #37)
> Hi 
> 
> I'm getting this error when trying to apply the patch:
> 
> root@kohadevbox:koha(bz29173)$ git bz apply 29173
> 
> Bug 29173 - Button "replace authority record via Z39/50/SRU" doesn't pre-fill
> 
> 144867 - Bug 29173: Fixed pre-filling of the search form from the "Replace
> record with ZR39.50/SRU" (Authority page detail)
> 
> Apply? [(y)es, (n)o, (i)nteractive] y
> Applying: Bug 29173: Fixed pre-filling of the search form from the "Replace
> record with ZR39.50/SRU" (Authority page detail)
> error: sha1 information is lacking or useless
> (cataloguing/z3950_auth_search.pl).
> error: could not build fake ancestor
> Patch failed at 0001 Bug 29173: Fixed pre-filling of the search form from
> the "Replace record with ZR39.50/SRU" (Authority page detail)
> 
> David

Hi David, 

Thanks for your feedback :) and best wishes for 2023 by the way ;) 

I squash my last commit and I test it in kohadevbox, it seems ok to me, can you re-test it ?
Comment 40 David Nind 2023-01-04 10:40:57 UTC
(In reply to Thibaud Guillot from comment #39)

> Thanks for your feedback :) and best wishes for 2023 by the way ;) 
> 
> I squash my last commit and I test it in kohadevbox, it seems ok to me, can
> you re-test it ?

Best wishes to you for 2023 as well!

The patch now applies, I am now testing...

David
Comment 41 David Nind 2023-01-04 11:18:13 UTC
Created attachment 144983 [details] [review]
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail)

Now this action has the same pre-filling behaviour as the "Edit" -> "Edit Record" -> "Replace Record via Z39.50/SRU Search" action.
Author Personal - Author Meeting/Conf. - Uniform Title and Subject are filled in if the subfield contains data (like the "Edit->Replace" action).
Unimarc and Marc21 specificities are supported.

Test Plan :

1) You must have a Z39.50/SRU server for authorities
2) Go to an authority detail page (Author - Uniform title - Subject)
3)After go to "Edit"-> "Edit record" -> "Replace record via..." or
choose "Replace Record.." directly
4) See that form is also prefilled with data now

Signed-off-by: David Nind <david@davidnind.com>
Comment 42 David Nind 2023-01-04 11:22:59 UTC
It would be good if both pop up windows were the same size for the search. At the moment the pop up window is slightly narrower for Edit > Edit record > Replace record via Z39.40/SRU search. This causes the labels to be above the input fields, rather than nicely aligned to the left.

I tried to work out where the window size is set, but had no luck...

Maybe this should be on a separate bug.
Comment 43 Martin Renvoize 2023-02-08 06:57:53 UTC
Created attachment 146380 [details] [review]
Bug 29173: Fixed pre-filling of the search form from the "Replace record with ZR39.50/SRU" (Authority page detail)

Now this action has the same pre-filling behaviour as the "Edit" -> "Edit Record" -> "Replace Record via Z39.50/SRU Search" action.
Author Personal - Author Meeting/Conf. - Uniform Title and Subject are filled in if the subfield contains data (like the "Edit->Replace" action).
Unimarc and Marc21 specificities are supported.

Test Plan :

1) You must have a Z39.50/SRU server for authorities
2) Go to an authority detail page (Author - Uniform title - Subject)
3)After go to "Edit"-> "Edit record" -> "Replace record via..." or
choose "Replace Record.." directly
4) See that form is also prefilled with data now

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 44 Martin Renvoize 2023-02-08 06:57:56 UTC
Created attachment 146381 [details] [review]
Bug 29173: (QA follow-up) Match pop-over sizes

As suggested by David Nind, this patch updates the call to z3950 search
on the authority edit page to match the sizing of the one from the
details page.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 45 Martin Renvoize 2023-02-08 07:01:50 UTC
This all works well and the QA script is happy.

I would have loved to see some consistency in the implementation between the two buttons however..  At first I thought this approach made the most logical sense, at the controller level, but as I worked through the code to understand how it works on the edit page I concluded that I think that's the cleaner approach pre-populating the search fields using the data already on display in the page rather than having to query for it again in the search controller.

Anyway.. I don't think that should hold this up.. but it might be nice to clean up as a follow-up bug.

Passing QA, thanks Thibaud, and welcome!
Comment 46 Thibaud Guillot 2023-02-08 08:11:31 UTC
(In reply to Martin Renvoize from comment #45)
> This all works well and the QA script is happy.
> 
> I would have loved to see some consistency in the implementation between the
> two buttons however..  At first I thought this approach made the most
> logical sense, at the controller level, but as I worked through the code to
> understand how it works on the edit page I concluded that I think that's the
> cleaner approach pre-populating the search fields using the data already on
> display in the page rather than having to query for it again in the search
> controller.
> 
> Anyway.. I don't think that should hold this up.. but it might be nice to
> clean up as a follow-up bug.
> 
> Passing QA, thanks Thibaud, and welcome!

Thanks to you Martin :)
Comment 47 Tomás Cohen Arazi 2023-02-09 13:01:23 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 48 Jacob O'Mara 2023-02-13 09:45:48 UTC
Nice work, thanks everyone!

Pushed to 22.11.x for the next release.
Comment 49 Lucas Gass 2023-02-15 21:57:20 UTC
Backported to 22.05.x for upcoming 22.05.10
Comment 50 Arthur Suzuki 2023-02-17 13:09:26 UTC
applied to 21.11.x for 21.11.18
Comment 51 wainuiwitikapark 2023-03-15 02:19:15 UTC
Not backported to 21.05.x
Comment 52 Jonathan Druart 2023-04-25 14:04:48 UTC
Please have a look at bug 33445 that is suggesting to revert these patches!
Comment 53 Jonathan Druart 2023-05-03 08:57:31 UTC
(In reply to Jonathan Druart from comment #52)
> Please have a look at bug 33445 that is suggesting to revert these patches!

ping Thibaud?
Comment 54 Thibaud Guillot 2023-05-03 11:45:14 UTC
(In reply to Jonathan Druart from comment #53)
> (In reply to Jonathan Druart from comment #52)
> > Please have a look at bug 33445 that is suggesting to revert these patches!
> 
> ping Thibaud?

Hello Jonathan,

Sorry for the delay, I saw and read the bug 33445 apparently other patches have been proposed to replace the revert. But I understand the problematic behavior of this one.. Do you think there is a need for me to do something?
Comment 55 Jonathan Druart 2023-05-03 14:59:01 UTC
(In reply to Thibaud Guillot from comment #54)
> (In reply to Jonathan Druart from comment #53)
> > (In reply to Jonathan Druart from comment #52)
> > > Please have a look at bug 33445 that is suggesting to revert these patches!
> > 
> > ping Thibaud?
> 
> Hello Jonathan,
> 
> Sorry for the delay, I saw and read the bug 33445 apparently other patches
> have been proposed to replace the revert. But I understand the problematic
> behavior of this one.. Do you think there is a need for me to do something?

If you agree with the changes there it's all good. I hadn't noticed Nick submitted a new version of the patches!
Comment 56 Thibaud Guillot 2023-05-03 15:12:40 UTC
ok great, thanks for the alert by the way