Bug 20310

Summary: Article requests: Use details from the host record when submitting an article request on an analytic record without attached items
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: OPACAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: andrewfh, carnold, hagud, katrin.fischer, kyle, martin.renvoize, pasi.kallinen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11175
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27507
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29441
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This new feature add the `ArticleRequestsHostRedirection` system preference. When enabled, if a user attempts to place an article request from an analytic record the system will automatically populate some details in the request from using data from the host record.
Version(s) released in:
21.11.00
Bug Depends on:    
Bug Blocks: 29268    
Attachments: Bug 20310: Introduce Koha::Biblio->host_record
Bug 20310: Redirect article record without items for article requests
Bug 20310: Introduce Koha::Biblio->host_record
Bug 20310: Redirect article record without items for article requests
Bug 20310: (follow-up) Allow repeatable $w even without org code
Bug 20310: Introduce Koha::Biblio->host_record
Bug 20310: Redirect article record without items for article requests
Bug 20310: (follow-up) Allow repeatable $w even without org code
Bug 20310: Introduce Koha::Biblio->host_record
Bug 20310: Redirect article record without items for article requests
Bug 20310: (follow-up) Allow repeatable $w even without org code
Bug 20310: Introduce Koha::Biblio->host_record
Bug 20310: Redirect article record without items for article requests
Bug 20310: (follow-up) Add or remove orgcode in second try
Bug 20310: Adding a pref to control behavior
Bug 20310: Introduce Koha::Biblio->host_record
Bug 20310: Redirect article record without items for article requests
Bug 20310: (follow-up) Add or remove orgcode in second try
Bug 20310: Adding a pref to control behavior
Bug 20310: Introduce Koha::Biblio->host_record
Bug 20310: Redirect article record without items for article requests
Bug 20310: (follow-up) Add or remove orgcode in second try
Bug 20310: Adding a pref to control behavior
Bug 20310: (follow-up) Check if biblio exists; 755 host_record.t
Bug 20310: Introduce Koha::Biblio->host_record
Bug 20310: Redirect article record without items for article requests
Bug 20310: (follow-up) Add or remove orgcode in second try
Bug 20310: Adding a pref to control behavior
Bug 20310: (follow-up) Check if biblio exists; 755 host_record.t
Bug 20310: (follow-up) Move syspref check into controller
Bug 20310: (follow-up) Rename method to get_marc_host
Bug 20310: Introduce Koha::Biblio->host_record
Bug 20310: Redirect article record without items for article requests
Bug 20310: (follow-up) Add or remove orgcode in second try
Bug 20310: Adding a pref to control behavior
Bug 20310: (follow-up) Check if biblio exists; 755 host_record.t
Bug 20310: (follow-up) Move syspref check into controller
Bug 20310: (follow-up) Rename method to get_marc_host
Bug 20310: (QA follow-up) Add 'Opac' to pref and description

Description Marcel de Rooy 2018-02-28 15:21:29 UTC
If I have a biblio record (without items) for article XXX in serial Y and I included a MARC21 773 host record that includes title, page info and host record number, it would be very helpful if the OPAC form would redirect me to the serial Y host record and I could select the correct item over there. (Can we prefill the page info or even preselect the item?)

Probably we should assume here that the ART records have the 'Record only' rule and the SER records have 'Item only' rule, and the record number is referring to the local catalogue (check the marc org code?).
Comment 1 Katrin Fischer 2018-02-28 23:32:37 UTC
I think at the moment it's built so it's possible to order records without items - so not sure what you suggest would/should work in general. Some libraries might want to allow orders on articles without items.

I think if we do that, we should try to fill the form in for the user. Having found the article, they might not like to be forced to enter the information into a form, that was obvious on the record before. But it would probably not be easy to split up $g correctly.
Comment 2 Marcel de Rooy 2018-03-21 15:54:28 UTC
Created attachment 73142 [details] [review]
Bug 20310: Introduce Koha::Biblio->host_record

This is a more generic method that allows you to get to the host record
as defined in MARC21 field 773$w.

Test plan:
Run t/db_dependent/Koha/Biblio/host_record.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 3 Marcel de Rooy 2018-03-21 15:56:12 UTC
Created attachment 73143 [details] [review]
Bug 20310: Redirect article record without items for article requests

If an article record has been catalogued separately, has no items and
contains a reference to its host via MARC21 field 773 (host item entry),
this patch makes an article request redirect to the host record while
copying title, author and page info (from 773$g).

This is accomplished by using the new Koha::Biblio->host_record method.

Note: Subfield 773$g may contain additional information on volume and
issue number etc. It will be very hard or perhaps impossible to parse $g
and copy these details into the corresponding fields of the article
request form for all possible variations used in libraries. A similar
remark can be made for selecting the correct item (when item level is
used). We could try this on a future report, but will probably need at
least a preference to define the expected format.

Test plan:
[1] Enable article requests. Add rules for an ART and a SER itemtype.
[2] Create a SER host biblio record.
[3] Create an ART biblio record, no items. Include a 773w pointing to the
    SER record with '(MARCorgcode)[recno]' (keep the parentheses, remove
    the square brackets when inserting the biblionumber).
    Include text in 773$g too.
[4] Place an article request on the ART record. Verify that it redirects
    you to the SER record while copying title, author, page info.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 4 Maksim Sen 2018-04-06 22:18:18 UTC
Created attachment 73805 [details] [review]
Bug 20310: Introduce Koha::Biblio->host_record

This is a more generic method that allows you to get to the host record
as defined in MARC21 field 773$w.

Test plan:
Run t/db_dependent/Koha/Biblio/host_record.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Maksim Sen <maksim.sen@inlibro.com>
Comment 5 paxed 2018-04-09 08:53:38 UTC
Comment on attachment 73805 [details] [review]
Bug 20310: Introduce Koha::Biblio->host_record

Shouldn't this also check that the 003 fields match? AFAIUI, 001+003 is unique, 001 is not guaranteed to be - because you could import data from different organization.
Comment 6 Marcel de Rooy 2018-04-09 09:29:55 UTC
(In reply to paxed from comment #5)
> Comment on attachment 73805 [details] [review] [review]
> Bug 20310: Introduce Koha::Biblio->host_record
> 
> Shouldn't this also check that the 003 fields match? AFAIUI, 001+003 is
> unique, 001 is not guaranteed to be - because you could import data from
> different organization.

Not sure if this is done consistently in Koha, but sounds good.
Comment 7 Marcel de Rooy 2018-04-09 09:33:26 UTC
(In reply to Maksim Sen from comment #4)
> Created attachment 73805 [details] [review] [review]
> Bug 20310: Introduce Koha::Biblio->host_record
> 
> This is a more generic method that allows you to get to the host record
> as defined in MARC21 field 773$w.
> 
> Test plan:
> Run t/db_dependent/Koha/Biblio/host_record.t
> 
> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
> Signed-off-by: Maksim Sen <maksim.sen@inlibro.com>

Maksim: You only signed off one record ?
Comment 8 paxed 2018-04-10 11:54:29 UTC
Comment on attachment 73805 [details] [review]
Bug 20310: Introduce Koha::Biblio->host_record

The MARC21 documentation from the Finnish National Library says the 773$w is just the RCN, nothing else - so the regex will fail for us.

Even the loc.gov MARC21 spec just says "$w - Record control number", so I don't know why their examples have the issuing org listed in the field too.
Comment 9 Katrin Fischer 2018-04-10 11:58:46 UTC
The problem is that $w is repeatable. You use the (MARCOrgCode) to identify which kind of ID it is. That said Koha can't handle repeated $w well at the moment, the XSLT (I think) assume there is only one. Handling both, with and without the prefix would be good for now I think.
Comment 10 Katrin Fischer 2018-04-10 12:00:47 UTC
But the prefix is correct, found the documentation:
http://www.loc.gov/marc/bibliographic/ecbdcntf.html
Comment 11 Marcel de Rooy 2018-04-11 11:30:06 UTC
Created attachment 73998 [details] [review]
Bug 20310: Redirect article record without items for article requests

If an article record has been catalogued separately, has no items and
contains a reference to its host via MARC21 field 773 (host item entry),
this patch makes an article request redirect to the host record while
copying title, author and page info (from 773$g).

This is accomplished by using the new Koha::Biblio->host_record method.

Note: Subfield 773$g may contain additional information on volume and
issue number etc. It will be very hard or perhaps impossible to parse $g
and copy these details into the corresponding fields of the article
request form for all possible variations used in libraries. A similar
remark can be made for selecting the correct item (when item level is
used). We could try this on a future report, but will probably need at
least a preference to define the expected format.

Test plan:
[1] Enable article requests. Add rules for an ART and a SER itemtype.
[2] Create a SER host biblio record.
[3] Create an ART biblio record, no items. Include a 773w pointing to the
    SER record with '(MARCorgcode)[recno]' (keep the parentheses, remove
    the square brackets when inserting the biblionumber).
    Include text in 773$g too.
[4] Place an article request on the ART record. Verify that it redirects
    you to the SER record while copying title, author, page info.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2018-04-11 11:30:36 UTC
Restoring order of original patch still hoping for a response from Maksim.
Comment 13 Marcel de Rooy 2018-04-11 12:54:39 UTC
Created attachment 74000 [details] [review]
Bug 20310: (follow-up) Allow repeatable $w even without org code

As mentioned in the BZ comments, we might expect people to add $w
subfields without a MARC organization code. And we should keep in
mind that 773 is repeatable, and both $g as well as $w is also
repeatable.

Added these cases into sub host_record, including tests.

Test plan:
Run t/db_dependent/Koha/Biblio/host_record.t again

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Marcel de Rooy 2018-04-11 12:55:32 UTC
(In reply to paxed from comment #8)
> Comment on attachment 73805 [details] [review] [review]
> Bug 20310: Introduce Koha::Biblio->host_record
> 
> The MARC21 documentation from the Finnish National Library says the 773$w is
> just the RCN, nothing else - so the regex will fail for us.

It should no longer. Allows this case now.
Comment 15 Marcel de Rooy 2018-04-11 12:55:51 UTC
(In reply to Katrin Fischer from comment #9)
> The problem is that $w is repeatable. You use the (MARCOrgCode) to identify
> which kind of ID it is. That said Koha can't handle repeated $w well at the
> moment, the XSLT (I think) assume there is only one. Handling both, with and
> without the prefix would be good for now I think.

Fixed
Comment 16 Marcel de Rooy 2018-04-11 13:00:40 UTC
(In reply to paxed from comment #5)
> Comment on attachment 73805 [details] [review] [review]
> Bug 20310: Introduce Koha::Biblio->host_record
> 
> Shouldn't this also check that the 003 fields match? AFAIUI, 001+003 is
> unique, 001 is not guaranteed to be - because you could import data from
> different organization.

Added a check with pref MARCOrgCode
Comment 17 paxed 2018-04-12 05:00:10 UTC
(In reply to Marcel de Rooy from comment #16)
> (In reply to paxed from comment #5)
> > Comment on attachment 73805 [details] [review] [review] [review]
> > Bug 20310: Introduce Koha::Biblio->host_record
> > 
> > Shouldn't this also check that the 003 fields match? AFAIUI, 001+003 is
> > unique, 001 is not guaranteed to be - because you could import data from
> > different organization.
> 
> Added a check with pref MARCOrgCode

But if you import data from another org then the orgcode (field 003) for those records won't be your org, but something else. You should match the 773$w orgcode to the host biblio 003
Comment 18 Marcel de Rooy 2018-04-12 08:20:03 UTC
(In reply to paxed from comment #17)
> (In reply to Marcel de Rooy from comment #16)
> > (In reply to paxed from comment #5)
> > > Comment on attachment 73805 [details] [review] [review] [review] [review]
> > > Bug 20310: Introduce Koha::Biblio->host_record
> > > 
> > > Shouldn't this also check that the 003 fields match? AFAIUI, 001+003 is
> > > unique, 001 is not guaranteed to be - because you could import data from
> > > different organization.
> > 
> > Added a check with pref MARCOrgCode
> 
> But if you import data from another org then the orgcode (field 003) for
> those records won't be your org, but something else. You should match the
> 773$w orgcode to the host biblio 003

Hi paxed,
I understand your point. But I am not so sure if we do these checks somewhere in Koha. In my proposal I ignore (Remote)x and only look at z or (My)z.
If record z from (My)z is found, I do not check if 001==z and 003==My.
Similarly, if record x from (Remote)x would be in our system (which I don't know) we also don't know if 001==x and 003==Remote.
Practical question would be too: Do we really want to check all 773 contents and pull all 001, 003s?

Since this logic is all about record control number handling in Koha and imo outside the scope of this report, we c/should move further refinements to a new report. But I have doubts about what we should check. This is a discussion on its own..

Thx for your feedback though.
Comment 19 paxed 2018-04-24 09:30:29 UTC
in Koha/Biblio.pm, you have:

    my $marc = C4::Biblio::GetMarcBiblio($self->biblionumber);

but in C4/Biblio.pm, GetMarcBiblio does this:

    my $biblionumber = $params->{biblionumber};

causing:

Can't use string ("11") as a HASH ref while "strict refs" in use at /usr/share/koha/lib/C4/Biblio.pm line 1131.
Comment 20 Marcel de Rooy 2018-05-03 06:38:49 UTC
(In reply to paxed from comment #19)
> in Koha/Biblio.pm, you have:
> 
>     my $marc = C4::Biblio::GetMarcBiblio($self->biblionumber);
> 
> but in C4/Biblio.pm, GetMarcBiblio does this:
> 
>     my $biblionumber = $params->{biblionumber};
> 
> causing:
> 
> Can't use string ("11") as a HASH ref while "strict refs" in use at
> /usr/share/koha/lib/C4/Biblio.pm line 1131.

Yeah, that was changed recently. I wrote the patch originally on 16.11..
And the associated test mocked GetMarcBiblio. So it did not fail.
Thx for reporting.
Comment 21 Marcel de Rooy 2018-05-03 06:46:20 UTC
Created attachment 74992 [details] [review]
Bug 20310: Introduce Koha::Biblio->host_record

This is a more generic method that allows you to get to the host record
as defined in MARC21 field 773$w.

Test plan:
Run t/db_dependent/Koha/Biblio/host_record.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Maksim Sen <maksim.sen@inlibro.com>
Comment 22 Marcel de Rooy 2018-05-03 06:46:24 UTC
Created attachment 74993 [details] [review]
Bug 20310: Redirect article record without items for article requests

If an article record has been catalogued separately, has no items and
contains a reference to its host via MARC21 field 773 (host item entry),
this patch makes an article request redirect to the host record while
copying title, author and page info (from 773$g).

This is accomplished by using the new Koha::Biblio->host_record method.

Note: Subfield 773$g may contain additional information on volume and
issue number etc. It will be very hard or perhaps impossible to parse $g
and copy these details into the corresponding fields of the article
request form for all possible variations used in libraries. A similar
remark can be made for selecting the correct item (when item level is
used). We could try this on a future report, but will probably need at
least a preference to define the expected format.

Test plan:
[1] Enable article requests. Add rules for an ART and a SER itemtype.
[2] Create a SER host biblio record.
[3] Create an ART biblio record, no items. Include a 773w pointing to the
    SER record with '(MARCorgcode)[recno]' (keep the parentheses, remove
    the square brackets when inserting the biblionumber).
    Include text in 773$g too.
[4] Place an article request on the ART record. Verify that it redirects
    you to the SER record while copying title, author, page info.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 23 Marcel de Rooy 2018-05-03 06:46:28 UTC
Created attachment 74994 [details] [review]
Bug 20310: (follow-up) Allow repeatable $w even without org code

As mentioned in the BZ comments, we might expect people to add $w
subfields without a MARC organization code. And we should keep in
mind that 773 is repeatable, and both $g as well as $w is also
repeatable.

Added these cases into sub host_record, including tests.

Test plan:
Run t/db_dependent/Koha/Biblio/host_record.t again

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 24 Hugo Agud 2019-04-26 10:47:52 UTC
Created attachment 88887 [details] [review]
Bug 20310: Introduce Koha::Biblio->host_record

This is a more generic method that allows you to get to the host record
as defined in MARC21 field 773$w.

Test plan:
Run t/db_dependent/Koha/Biblio/host_record.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Maksim Sen <maksim.sen@inlibro.com>
Signed-off-by: Hugo Agud <hagud@orex.es>
Comment 25 Hugo Agud 2019-04-26 10:47:55 UTC
Created attachment 88888 [details] [review]
Bug 20310: Redirect article record without items for article requests

If an article record has been catalogued separately, has no items and
contains a reference to its host via MARC21 field 773 (host item entry),
this patch makes an article request redirect to the host record while
copying title, author and page info (from 773$g).

This is accomplished by using the new Koha::Biblio->host_record method.

Note: Subfield 773$g may contain additional information on volume and
issue number etc. It will be very hard or perhaps impossible to parse $g
and copy these details into the corresponding fields of the article
request form for all possible variations used in libraries. A similar
remark can be made for selecting the correct item (when item level is
used). We could try this on a future report, but will probably need at
least a preference to define the expected format.

Test plan:
[1] Enable article requests. Add rules for an ART and a SER itemtype.
[2] Create a SER host biblio record.
[3] Create an ART biblio record, no items. Include a 773w pointing to the
    SER record with '(MARCorgcode)[recno]' (keep the parentheses, remove
    the square brackets when inserting the biblionumber).
    Include text in 773$g too.
[4] Place an article request on the ART record. Verify that it redirects
    you to the SER record while copying title, author, page info.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Hugo Agud <hagud@orex.es>
Comment 26 Hugo Agud 2019-04-26 10:47:59 UTC
Created attachment 88889 [details] [review]
Bug 20310: (follow-up) Allow repeatable $w even without org code

As mentioned in the BZ comments, we might expect people to add $w
subfields without a MARC organization code. And we should keep in
mind that 773 is repeatable, and both $g as well as $w is also
repeatable.

Added these cases into sub host_record, including tests.

Test plan:
Run t/db_dependent/Koha/Biblio/host_record.t again

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Hugo Agud <hagud@orex.es>
Comment 27 Martin Renvoize 2019-05-01 07:40:17 UTC
I'm starting to look at this with a view to QAing... but, as the conversation has gone towards how we deal with multi-part records and 773,001,003 handling it might be helpful to get some peoples thoughts and any updates over on the Wiki: https://wiki.koha-community.org/wiki/Multipart_Bibliographics.  Some time ago I tried hard to understand it all with a view to trying to get it 'right' in Koha eventually.  It would be lovely for that effort to be picked up again.
Comment 28 Martin Renvoize 2019-05-01 08:47:15 UTC
I'm not entirely happy that this is built to only work with biblionumbers in reality.

I think it would be significantly more adaptable in the future if we add a field to the database that maps the combination of 003 + 001 into an indexed unique field and did the find against that.  That would mean that if your library happens to maintain 001 = biblionumber (as is the assumption here I believe) it'll all still work, but it will also work if your library uses local 001's or remote 001 and 003's.  It'll likely need tweaking as we learn of other schemes but that feels the most forward thinking approach I can come up with.

Thoughts?
Comment 29 Katrin Fischer 2019-05-01 09:10:07 UTC
If I understand the code correctly, it assumes the the first $w contains a valid biblionumber, with or without proper prefix. 

I think this assumption can't hold up.

At the moment, we expect in $w a number, that is also in 001 of a matching record. Ideally with a 003 that has the matching prefix, but this is not checked. So:

Analytic:
$w(MARCOrgCode)rcn

Host:
001 rcn
003 MARCOrgCode

Actually... we probably should also check 035, but that is a different issue:

035 (MARCOrgCode)rcn

In Koha there is no way to automatically enforce 001 = biblionumber. And if you want to do it, it's tedious, as you have to save first to create the biblionumber before you can enter it in 001. There is even a new feature for Rancor, that allows you to create custom 001, that are not relating to biblionumber at all.

The risk I see with this feature as is, is that the number from $w might accidentally match a biblionumber. So we would create unwanted redirects. You can't turn it off.
Comment 30 Martin Renvoize 2019-05-01 09:11:20 UTC
(In reply to Martin Renvoize from comment #28)
> I'm not entirely happy that this is built to only work with biblionumbers in
> reality.
> 
> I think it would be significantly more adaptable in the future if we add a
> field to the database that maps the combination of 003 + 001 into an indexed
> unique field and did the find against that.  That would mean that if your
> library happens to maintain 001 = biblionumber (as is the assumption here I
> believe) it'll all still work, but it will also work if your library uses
> local 001's or remote 001 and 003's.  It'll likely need tweaking as we learn
> of other schemes but that feels the most forward thinking approach I can
> come up with.
> 
> Thoughts?

That, or use the search engine to find hosts which is how the xslt's currently do it I believe
Comment 31 Katrin Fischer 2019-05-01 09:14:01 UTC
Note: If you use the 'addd child record' feature in Koha, it will copy 001 to $w. While it might not be super fast, maybe we could change the host_record feature to look for a matching 001 instead of just biblionumber? It would make this work along the current assumptions.

Also: We should add UNIMARC support.
Comment 32 Marcel de Rooy 2019-05-01 10:58:44 UTC
OK Thx for your feedback. This might need more thought about the right direction for a solution. Will leave it for now, hopefully picking it up later..
Comment 33 Marcel de Rooy 2020-09-08 14:06:07 UTC
Created attachment 109750 [details] [review]
Bug 20310: Introduce Koha::Biblio->host_record

This is a more generic method that allows you to get to the host record
as defined in MARC21 field 773$w. This control number is looked up in the
search engine (MARC21 field 001).

Note: The current implementation is based on MARC21 and does not change
behavior for UNIMARC.
This includes the follow-up originally submitted separately on Bugzilla,
dealing with multiple 773 fields.

Test plan:
Run t/db_dependent/Koha/Biblio/host_record.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 34 Marcel de Rooy 2020-09-08 14:06:13 UTC
Created attachment 109751 [details] [review]
Bug 20310: Redirect article record without items for article requests

If an article record has been catalogued separately, has no items and
contains a reference to its host via MARC21 field 773 (host item entry),
this patch makes an article request redirect to the host record while
copying title, author and page info (from 773$g).

This is accomplished by using the new Koha::Biblio->host_record method.

Note: Subfield 773$g may contain additional information on volume and
issue number etc. It will be very hard or perhaps impossible to parse $g
and copy these details into the corresponding fields of the article
request form for all possible variations used in libraries. A similar
remark can be made for selecting the correct item (when item level is
used). We could try this on a future report, but will probably need at
least a preference to define the expected format.

Test plan:
[1] Enable article requests. Add rules for an ART and a SER itemtype.
[2] Create a SER host biblio record.
[3] Create an ART biblio record, no items. Include a 773w pointing to the
    SER record with '(MARCorgcode)[recno]' (keep the parentheses, remove
    the square brackets when inserting the biblionumber).
    Include text in 773$g too.
[4] Place an article request on the ART record. Verify that it redirects
    you to the SER record while copying title, author, page info.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Hugo Agud <hagud@orex.es>
Comment 35 Marcel de Rooy 2020-09-08 14:06:18 UTC
Created attachment 109752 [details] [review]
Bug 20310: (follow-up) Add or remove orgcode in second try

We did remove an orgcode, but we could also do the other way
around.

Note: Hard to include in the unit test since it mocks the whole
search engine part.

Test plan:
[1] Run t/db_dependent/Koha/Biblio/host_record.t
[2] Test host 001 = number and child 773 = (org)number
[3] Test host 001 = (org)number and child 773 = number

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 36 Marcel de Rooy 2020-09-08 14:06:23 UTC
Created attachment 109753 [details] [review]
Bug 20310: Adding a pref to control behavior

If people want to control the redirection, we could add a pref
like ArticleRequestsHostRedirection. If it is not enabled,
the party is over.

Test plan:
Test if redirection stops on a child and host when you disable.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 37 Marcel de Rooy 2020-09-08 14:13:28 UTC
(In reply to Marcel de Rooy from comment #32)
> OK Thx for your feedback. This might need more thought about the right
> direction for a solution. Will leave it for now, hopefully picking it up
> later..

Well, this is a second try then.
First patch is a combination and rebase of the former 1st and 3rd patch.
The second patch is untouched. I therefore kept the signoff line from Hugo.
Third patch is new. And makes it possible that host or child has or has no orgcode.
Fourth patch is new. Adds a pref to enable/disable.

Former feedback was about the interpretation of 773$w as a biblionumber. The current patch set uses $w to query the search index on the index Control-number. No direct interpretation anymore.

Control by pref hopefully makes it even easier to accept the current code.
Comment 38 Marcel de Rooy 2020-09-08 14:36:46 UTC
(In reply to Katrin Fischer from comment #31)
> Also: We should add UNIMARC support.

This can be done later (when there is a need). The code in Biblio can be easily extended. We're starting with MARC21 now. Not all features come in all flavors rightaway ;)
Comment 39 Marcel de Rooy 2020-10-02 09:49:01 UTC Comment hidden (obsolete)
Comment 40 Marcel de Rooy 2020-10-12 06:52:38 UTC
Created attachment 111467 [details] [review]
Bug 20310: Introduce Koha::Biblio->host_record

This is a more generic method that allows you to get to the host record
as defined in MARC21 field 773$w. This control number is looked up in the
search engine (MARC21 field 001).

Note: The current implementation is based on MARC21 and does not change
behavior for UNIMARC.
This includes the follow-up originally submitted separately on Bugzilla,
dealing with multiple 773 fields.

Test plan:
Run t/db_dependent/Koha/Biblio/host_record.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 41 Marcel de Rooy 2020-10-12 06:52:45 UTC
Created attachment 111468 [details] [review]
Bug 20310: Redirect article record without items for article requests

If an article record has been catalogued separately, has no items and
contains a reference to its host via MARC21 field 773 (host item entry),
this patch makes an article request redirect to the host record while
copying title, author and page info (from 773$g).

This is accomplished by using the new Koha::Biblio->host_record method.

Note: Subfield 773$g may contain additional information on volume and
issue number etc. It will be very hard or perhaps impossible to parse $g
and copy these details into the corresponding fields of the article
request form for all possible variations used in libraries. A similar
remark can be made for selecting the correct item (when item level is
used). We could try this on a future report, but will probably need at
least a preference to define the expected format.

Test plan:
[1] Enable article requests. Add rules for an ART and a SER itemtype.
[2] Create a SER host biblio record.
[3] Create an ART biblio record, no items. Include a 773w pointing to the
    SER record with '(MARCorgcode)[recno]' (keep the parentheses, remove
    the square brackets when inserting the biblionumber).
    Include text in 773$g too.
[4] Place an article request on the ART record. Verify that it redirects
    you to the SER record while copying title, author, page info.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Hugo Agud <hagud@orex.es>
Comment 42 Marcel de Rooy 2020-10-12 06:52:51 UTC
Created attachment 111469 [details] [review]
Bug 20310: (follow-up) Add or remove orgcode in second try

We did remove an orgcode, but we could also do the other way
around.

Note: Hard to include in the unit test since it mocks the whole
search engine part.

Test plan:
[1] Run t/db_dependent/Koha/Biblio/host_record.t
[2] Test host 001 = number and child 773 = (org)number
[3] Test host 001 = (org)number and child 773 = number

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 43 Marcel de Rooy 2020-10-12 06:52:59 UTC
Created attachment 111470 [details] [review]
Bug 20310: Adding a pref to control behavior

If people want to control the redirection, we could add a pref
like ArticleRequestsHostRedirection. If it is not enabled,
the party is over.

Test plan:
Test if redirection stops on a child and host when you disable.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 44 Marcel de Rooy 2020-10-12 07:47:21 UTC
Rebased. Please test !
Comment 45 Nick Clemens 2021-03-29 18:42:42 UTC
Created attachment 118976 [details] [review]
Bug 20310: Introduce Koha::Biblio->host_record

This is a more generic method that allows you to get to the host record
as defined in MARC21 field 773$w. This control number is looked up in the
search engine (MARC21 field 001).

Note: The current implementation is based on MARC21 and does not change
behavior for UNIMARC.
This includes the follow-up originally submitted separately on Bugzilla,
dealing with multiple 773 fields.

Test plan:
Run t/db_dependent/Koha/Biblio/host_record.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

https://bugs.koha-community.org/show_bug.cgi?id=20130
Comment 46 Nick Clemens 2021-03-29 18:42:45 UTC
Created attachment 118977 [details] [review]
Bug 20310: Redirect article record without items for article requests

If an article record has been catalogued separately, has no items and
contains a reference to its host via MARC21 field 773 (host item entry),
this patch makes an article request redirect to the host record while
copying title, author and page info (from 773$g).

This is accomplished by using the new Koha::Biblio->host_record method.

Note: Subfield 773$g may contain additional information on volume and
issue number etc. It will be very hard or perhaps impossible to parse $g
and copy these details into the corresponding fields of the article
request form for all possible variations used in libraries. A similar
remark can be made for selecting the correct item (when item level is
used). We could try this on a future report, but will probably need at
least a preference to define the expected format.

Test plan:
[1] Enable article requests. Add rules for an ART and a SER itemtype.
[2] Create a SER host biblio record.
[3] Create an ART biblio record, no items. Include a 773w pointing to the
    SER record with '(MARCorgcode)[recno]' (keep the parentheses, remove
    the square brackets when inserting the biblionumber).
    Include text in 773$g too.
[4] Place an article request on the ART record. Verify that it redirects
    you to the SER record while copying title, author, page info.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Hugo Agud <hagud@orex.es>

https://bugs.koha-community.org/show_bug.cgi?id=20130
Comment 47 Nick Clemens 2021-03-29 18:42:49 UTC
Created attachment 118978 [details] [review]
Bug 20310: (follow-up) Add or remove orgcode in second try

We did remove an orgcode, but we could also do the other way
around.

Note: Hard to include in the unit test since it mocks the whole
search engine part.

Test plan:
[1] Run t/db_dependent/Koha/Biblio/host_record.t
[2] Test host 001 = number and child 773 = (org)number
[3] Test host 001 = (org)number and child 773 = number

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

https://bugs.koha-community.org/show_bug.cgi?id=20130
Comment 48 Nick Clemens 2021-03-29 18:42:52 UTC
Created attachment 118979 [details] [review]
Bug 20310: Adding a pref to control behavior

If people want to control the redirection, we could add a pref
like ArticleRequestsHostRedirection. If it is not enabled,
the party is over.

Test plan:
Test if redirection stops on a child and host when you disable.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

https://bugs.koha-community.org/show_bug.cgi?id=20130
Comment 49 Marcel de Rooy 2021-04-09 07:45:33 UTC
Created attachment 119346 [details] [review]
Bug 20310: (follow-up) Check if biblio exists; 755 host_record.t

In order to prevent an error like:
Can't call method "host_record" on an undefined value at opac/opac-request-article.pl

Also chmod's host_record.t to 755 (File must have the exec flag)

Test plan:
Change the URL so that the biblionumber does not exist.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 50 Marcel de Rooy 2021-06-24 09:45:57 UTC
HELP Who wants to assist in getting this fURTHER ?
Comment 51 Martin Renvoize 2021-06-28 08:52:07 UTC
Oops, I started looking at this on Friday but got pulled away into other things.. I hope to get back to it later this week.

One comment so far however.. I think the 'host_record' relationship accessor could be generalised a little more to make it less specific to this feature and thus adoptable by other uses in the future.  I'm happy to undertake this as a QA follow-up. I think we should move the item count test and syspref check out of the module method and into the controllers instead.. as much as this goes against the instinct to make sure it's testable I think this will lead to a much more flexible low level object method... I wonder if we could move the test into the integration tests so it's still caught but at the controller level rather than the object?
Comment 52 Marcel de Rooy 2021-06-28 09:46:55 UTC
(In reply to Martin Renvoize from comment #51)
> Oops, I started looking at this on Friday but got pulled away into other
> things.. I hope to get back to it later this week.
> 
> One comment so far however.. I think the 'host_record' relationship accessor
> could be generalised a little more to make it less specific to this feature
> and thus adoptable by other uses in the future.  I'm happy to undertake this
> as a QA follow-up. I think we should move the item count test and syspref
> check out of the module method and into the controllers instead.. as much as
> this goes against the instinct to make sure it's testable I think this will
> lead to a much more flexible low level object method... I wonder if we could
> move the test into the integration tests so it's still caught but at the
> controller level rather than the object?

Great, Martin.
Please note that this patch is already dating from 2018 and unless there are specific developments underway that could directly be tied in, I would prefer to get this further instead of making it more generic for some future development not yet identified.
In short, please add some more details or references to other bugs that you have in mind.
Thanks for your support.
Comment 53 Martin Renvoize 2021-06-28 13:10:08 UTC
Created attachment 122469 [details] [review]
Bug 20310: Introduce Koha::Biblio->host_record

This is a more generic method that allows you to get to the host record
as defined in MARC21 field 773$w. This control number is looked up in the
search engine (MARC21 field 001).

Note: The current implementation is based on MARC21 and does not change
behavior for UNIMARC.
This includes the follow-up originally submitted separately on Bugzilla,
dealing with multiple 773 fields.

Test plan:
Run t/db_dependent/Koha/Biblio/host_record.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 54 Martin Renvoize 2021-06-28 13:10:12 UTC
Created attachment 122470 [details] [review]
Bug 20310: Redirect article record without items for article requests

If an article record has been catalogued separately, has no items and
contains a reference to its host via MARC21 field 773 (host item entry),
this patch makes an article request redirect to the host record while
copying title, author and page info (from 773$g).

This is accomplished by using the new Koha::Biblio->host_record method.

Note: Subfield 773$g may contain additional information on volume and
issue number etc. It will be very hard or perhaps impossible to parse $g
and copy these details into the corresponding fields of the article
request form for all possible variations used in libraries. A similar
remark can be made for selecting the correct item (when item level is
used). We could try this on a future report, but will probably need at
least a preference to define the expected format.

Test plan:
[1] Enable article requests. Add rules for an ART and a SER itemtype.
[2] Create a SER host biblio record.
[3] Create an ART biblio record, no items. Include a 773w pointing to the
    SER record with '(MARCorgcode)[recno]' (keep the parentheses, remove
    the square brackets when inserting the biblionumber).
    Include text in 773$g too.
[4] Place an article request on the ART record. Verify that it redirects
    you to the SER record while copying title, author, page info.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 55 Martin Renvoize 2021-06-28 13:10:16 UTC
Created attachment 122471 [details] [review]
Bug 20310: (follow-up) Add or remove orgcode in second try

We did remove an orgcode, but we could also do the other way
around.

Note: Hard to include in the unit test since it mocks the whole
search engine part.

Test plan:
[1] Run t/db_dependent/Koha/Biblio/host_record.t
[2] Test host 001 = number and child 773 = (org)number
[3] Test host 001 = (org)number and child 773 = number

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 56 Martin Renvoize 2021-06-28 13:10:20 UTC
Created attachment 122472 [details] [review]
Bug 20310: Adding a pref to control behavior

If people want to control the redirection, we could add a pref
like ArticleRequestsHostRedirection. If it is not enabled,
the party is over.

Test plan:
Test if redirection stops on a child and host when you disable.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 57 Martin Renvoize 2021-06-28 13:10:24 UTC
Created attachment 122473 [details] [review]
Bug 20310: (follow-up) Check if biblio exists; 755 host_record.t

In order to prevent an error like:
Can't call method "host_record" on an undefined value at opac/opac-request-article.pl

Also chmod's host_record.t to 755 (File must have the exec flag)

Test plan:
Change the URL so that the biblionumber does not exist.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 58 Martin Renvoize 2021-06-28 13:10:28 UTC
Created attachment 122474 [details] [review]
Bug 20310: (follow-up) Move syspref check into controller

I believe the syspref check is better placed in the controller. This way
the host_record method is not so closely tied to the articel requests
feature and may subsequently prove useful to future use cases.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 59 Martin Renvoize 2021-06-28 13:14:54 UTC
Created attachment 122475 [details] [review]
Bug 20310: (follow-up) Rename method to get_marc_host

This method is a mix of database relationship accessor and marc field
accessor. We have get_marc_notes already and other patches in the queue
are also starting to introduce get_marc_* named methods. This patch
updates the method name to conform with that scheme.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 60 Martin Renvoize 2021-06-28 13:34:07 UTC
This all appears to work reasonably well.. I've not dug deeply into how these patches relate to bug 11175, so that may be worth considering during QA.
Comment 61 Marcel de Rooy 2021-06-28 14:14:12 UTC
(In reply to Martin Renvoize from comment #60)
> This all appears to work reasonably well.. I've not dug deeply into how
> these patches relate to bug 11175, so that may be worth considering during
> QA.

Thx for signing off.
That is an interesting reference indeed. It adds $biblio->components, actually going the other way around from host to children. Looking for records that contain a 773 with that biblionumber or associated control number.
Comment 62 Nick Clemens 2021-07-09 12:03:46 UTC
Created attachment 122725 [details] [review]
Bug 20310: Introduce Koha::Biblio->host_record

This is a more generic method that allows you to get to the host record
as defined in MARC21 field 773$w. This control number is looked up in the
search engine (MARC21 field 001).

Note: The current implementation is based on MARC21 and does not change
behavior for UNIMARC.
This includes the follow-up originally submitted separately on Bugzilla,
dealing with multiple 773 fields.

Test plan:
Run t/db_dependent/Koha/Biblio/host_record.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 63 Nick Clemens 2021-07-09 12:03:50 UTC
Created attachment 122726 [details] [review]
Bug 20310: Redirect article record without items for article requests

If an article record has been catalogued separately, has no items and
contains a reference to its host via MARC21 field 773 (host item entry),
this patch makes an article request redirect to the host record while
copying title, author and page info (from 773$g).

This is accomplished by using the new Koha::Biblio->host_record method.

Note: Subfield 773$g may contain additional information on volume and
issue number etc. It will be very hard or perhaps impossible to parse $g
and copy these details into the corresponding fields of the article
request form for all possible variations used in libraries. A similar
remark can be made for selecting the correct item (when item level is
used). We could try this on a future report, but will probably need at
least a preference to define the expected format.

Test plan:
[1] Enable article requests. Add rules for an ART and a SER itemtype.
[2] Create a SER host biblio record.
[3] Create an ART biblio record, no items. Include a 773w pointing to the
    SER record with '(MARCorgcode)[recno]' (keep the parentheses, remove
    the square brackets when inserting the biblionumber).
    Include text in 773$g too.
[4] Place an article request on the ART record. Verify that it redirects
    you to the SER record while copying title, author, page info.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 64 Nick Clemens 2021-07-09 12:03:54 UTC
Created attachment 122727 [details] [review]
Bug 20310: (follow-up) Add or remove orgcode in second try

We did remove an orgcode, but we could also do the other way
around.

Note: Hard to include in the unit test since it mocks the whole
search engine part.

Test plan:
[1] Run t/db_dependent/Koha/Biblio/host_record.t
[2] Test host 001 = number and child 773 = (org)number
[3] Test host 001 = (org)number and child 773 = number

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 65 Nick Clemens 2021-07-09 12:03:58 UTC
Created attachment 122728 [details] [review]
Bug 20310: Adding a pref to control behavior

If people want to control the redirection, we could add a pref
like ArticleRequestsHostRedirection. If it is not enabled,
the party is over.

Test plan:
Test if redirection stops on a child and host when you disable.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 66 Nick Clemens 2021-07-09 12:04:01 UTC
Created attachment 122729 [details] [review]
Bug 20310: (follow-up) Check if biblio exists; 755 host_record.t

In order to prevent an error like:
Can't call method "host_record" on an undefined value at opac/opac-request-article.pl

Also chmod's host_record.t to 755 (File must have the exec flag)

Test plan:
Change the URL so that the biblionumber does not exist.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 67 Nick Clemens 2021-07-09 12:04:06 UTC
Created attachment 122730 [details] [review]
Bug 20310: (follow-up) Move syspref check into controller

I believe the syspref check is better placed in the controller. This way
the host_record method is not so closely tied to the articel requests
feature and may subsequently prove useful to future use cases.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 68 Nick Clemens 2021-07-09 12:04:14 UTC
Created attachment 122731 [details] [review]
Bug 20310: (follow-up) Rename method to get_marc_host

This method is a mix of database relationship accessor and marc field
accessor. We have get_marc_notes already and other patches in the queue
are also starting to introduce get_marc_* named methods. This patch
updates the method name to conform with that scheme.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 69 Nick Clemens 2021-07-09 12:04:20 UTC
Created attachment 122732 [details] [review]
Bug 20310: (QA follow-up) Add 'Opac' to pref and description

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 70 Marcel de Rooy 2021-07-09 12:07:08 UTC
(In reply to Nick Clemens from comment #69)
> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Cool. Thanks
Comment 71 Jonathan Druart 2021-07-12 13:48:24 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 72 Kyle M Hall 2021-07-16 13:24:26 UTC
Not backported for 21.05
Comment 73 Martin Renvoize 2021-08-09 10:17:09 UTC
Could you add release notes for this Marcel?
Comment 74 Martin Renvoize 2021-08-09 10:22:11 UTC
Scratch that.. can you just varify my description is correct ;)
Comment 75 Marcel de Rooy 2021-08-09 11:36:46 UTC
(In reply to Martin Renvoize from comment #74)
> Scratch that.. can you just varify my description is correct ;)

It reads fine, Martin. Given that I am not so familiar with the word analytic, and it does not intuitively put my mind in the right direction ;)