There are 2 ways in Koha to manage analytical records - one is EasyAnalytics, which I think you are referring to. The other is using the "New child record" option and is based on the bibliographic record only, without involving the items. If you are using EasyAnalytics you could probably just hide the link with CSS as it should not be needed. 

The basic problem is, that the parent record doesn't know in the MARC if it has children... so we can't solve that only with XSLT, we need another way of checking for children.

In unumarc is possible to use Leader pos. 8 [Hierarchical code].
The value '1' means 'highest level record', so it is sure it has children.
In MARC21 I think we need a proprietary subfield.
I suggest one from tag 942.

The 942 won't work in our use case for the same reason that we can't use EasyAnalatics. For libraries, where the bibliographic record is created outside of Koha and automatically kept in sync with the external master record, there is no way to have 'local' fields, because they will always be overwritten/deleted with an update.

We can change the wording.
From "Analytics: Show analytics" to "Analytics: Search if present".

We're doing a silly change in xslt in order to fix this

we use local marc tag to indicate koha that this record has analytics and then modifiy the xlst according to this.

There is two ways of fixing this.

1st --> manual one, using one tag to indicate koha that this record has analytics and show the link

2nd --> some kind of indexes that looks for 773$0 or 773$9 and activate it for the selected records

3rd. move to the OPAC the current functionality that indicates that some items includes analytics like the intranet does .

Created attachment 66593 [details] [review]
Bug 15851: Only display 'show analytics' link if there are

As I am not sure to understand the need correctly, I have provided a quick (and dirty) patch. Let me know if it fixes the needs and I will improve it later if it is what you are waiting for.

Hi Jonathan, I don't see how the flag would be added, can you explain?

We have a lot of libraries with analytics which is why we added the featuer in the first place. The unsolved problem was and is that the MARC parent record doesn't know about it's children. And it's not easy to find out if there is one. If the library uses UseControlNumber for linking, you can check if the 001 of the parent appears in a 773$w with the rcn-Index. If the library doesn't link using numbers, you'd have to do a phrase search for 773$t(?) and Title.

A purely manual solution where you have to add a flag to the record would not work for us as we don't catlaog in Koha but in a cooperative catalouging environment (union catalog). So this should be optional then. 

Having a clean soluton that works without manual interference would be really nice as a lot of libraries have asked for the link to be smarter.

The flag ($analytics_flag) is set from detail.pl:289

286     #count if item is used in analytical bibliorecords
287     my $countanalytics= GetAnalyticsCount($item->{itemnumber});
288     if ($countanalytics > 0){
289         $analytics_flag=1;
290         $item->{countanalytics} = $countanalytics;
291     }

It looks like this could work for EasyAnalytics - but it would break the workflow for people using UseControlNumber - linking without any items involved on bibliographic record level (MARC21 standard)

Hi,

in fact Katrin underline the problematic point: in MARC21 standard catalouging nothing in parent biblio record of a serial  says if it has analitics.
Probably a good option is:
1)A new Sys pref "AnaliticsWithControlNumbers" and say: "Set it 'ON' only if you use control numbers in 77x$w and don't use  EasyAnalytics".
2)A new function like "GetAnalyticsCount" but based on controlnumber and rcn-Index.

At the what we need is to know if a bib record has analytics, we do not need silly xslt links based on bib-leve:s (journals) and on the other hand we have forgotten about chapters not only serial (bib-level:b)

The link between bib and analytics is 773 field... here the big issue is most of fields are text 773$t and the one to one link are 773$0 773$9 or even 773$x(issn (journals) or 773$w(controlnumber) depends on every library, how they have created the analytics...they will have more or less filled tags

If you're using koha and have created the analytics with koha you will have a nice 773 to work create links ;)

Now in the staff cients, in the analytics tab you have info about if that item has anaytics attached and how many... because the 773$9 (one to one link and it works fine)

I think there is not a 100% solution, because there will be plenty of different situations and one solution won't match all possible situations

I like the idea of the syspref 'GetAnalyticsCount' but we must be clear enought with the customers explaining how the links works

The idea it should be, that the library must  check their current 773 tags and fix it. (they have a nice batch record modification tool)

There is 3 potential ways:
for current biblionumber --> appear the link if there is any 773$0 that matchs the biblio
or

for current itemnumbers --> appear a link if there is any 773$9 that matchs the itemnumber

or
for current issn --> apprear a link if there is any 773$x that matches 022$a

I don not like the control-number:
the use of control number I dislike because koha is not using 001 control-number

(In reply to Hugo Agud from comment #13)
> At the what we need is to know if a bib record has analytics, we do not need
> silly xslt links based on bib-leve:s (journals) and on the other hand we
> have forgotten about chapters not only serial (bib-level:b)

The links have been created so you can hide them very easily with CSS if you don't want or need them.

> The link between bib and analytics is 773 field... here the big issue is
> most of fields are text 773$t and the one to one link are 773$0 773$9 or
> even 773$x(issn (journals) or 773$w(controlnumber) depends on every library,
> how they have created the analytics...they will have more or less filled tags
> 
> If you're using koha and have created the analytics with koha you will have
> a nice 773 to work create links ;)

Not all libraries catalog solely in Koha. But: you can create analytics within Koha in 2 different ways already. One is EasyAnalytics which is tied to the items, the other is from the detail view available as well and supports Controlnumber linking: New > New child record. If a 001 exists, it will be used.

The different linking methods/options are the reson that there is a fallback from $w to a Title search. More logic could be added there to include other possible good search parameters like ISBN/ISSN.

> Now in the staff cients, in the analytics tab you have info about if that
> item has anaytics attached and how many... because the 773$9 (one to one
> link and it works fine)

Please keep in mind that this is only available for libraries using EasyAnalytics.

> I think there is not a 100% solution, because there will be plenty of
> different situations and one solution won't match all possible situations
> 
> I like the idea of the syspref 'GetAnalyticsCount' but we must be clear
> enought with the customers explaining how the links works
> 
> The idea it should be, that the library must  check their current 773 tags
> and fix it. (they have a nice batch record modification tool)

> 
> There is 3 potential ways:
> for current biblionumber --> appear the link if there is any 773$0 that
> matchs the biblio
> or

There is no $0 defined for 773 in MARC standard?

> 
> for current itemnumbers --> appear a link if there is any 773$9 that matchs
> the itemnumber
> 
> or
> for current issn --> apprear a link if there is any 773$x that matches 022$a

Please don't forget $w here - it's actually one of the best possible links, as the matching should be exact based on ID.


> I don not like the control-number:
> the use of control number I dislike because koha is not using 001
> control-number

Please keep other workflows in mind. Our records are linked outside of Koha by ID in an external cataloguing environment, everything is held together by 001. One central cataloguing environment, data delivered to lots of different ILS. Also: 001/$w IS MARC standard.

Cait-
I can appreciate that your libraries have workflows related to analytics here.  However, the vast majority of other libraries using MARC21 standard do not intend the Leader position 7 to indicate that there are analytics present.  I believe that Koha should be displaying the bibliographic data in a way that is accurate to the data that is entered.  In this case not display an analytic tag just because leader position 7 =s.  

I have a number of libraries who would love to see this 'feature' removed and have Koha catalog and display in the correct fashion (according to marc21 standards).

joy

(In reply to Joy Nelson from comment #15)
> Cait-
> I can appreciate that your libraries have workflows related to analytics
> here.  However, the vast majority of other libraries using MARC21 standard
> do not intend the Leader position 7 to indicate that there are analytics
> present.  I believe that Koha should be displaying the bibliographic data in
> a way that is accurate to the data that is entered.  In this case not
> display an analytic tag just because leader position 7 =s.  
> 
> I have a number of libraries who would love to see this 'feature' removed
> and have Koha catalog and display in the correct fashion (according to
> marc21 standards).
> 
> joy

I am not sure how to hide it by default and make it display on demand. At the moment you can hide it with one line of CSS. Would hiding it in the default CSS and unhiding in OpacUserCSS work?

We also hide it for a lot of libraries as most don't have analytics - and then they don't have analytics on all records. I just wanted to say: don't kill it by assuming everyone uses EasyAnalytics. I am open to suggestions on how to make this work better - Ideally it should only display when there are linked records.

Cait-
I agree that this could be a responsive feature in Koha.  However, to most librarians it appears that Koha is 'doing something' (displaying analytic links) that is not supported by the correct cataloging of records.  Catalogers put a lot of effort into correctly cataloging their collections.  And when this cataloging results in behavior that is not related to the industry standards, it causes frustration, confusion and some lack of confidence in the tool.  

Yes, it could be hidden, but by doing so we are expecting the bulk of Marc21 libraries to hide something that should never be there in the first place.

Joy, I understand. Let's try to figure out something that works for us both. What about my suggestion to turn the CSS around from the last comment? (hide by default, make it possible to unhide)

I'd be happiest with a solution where we fixed the idea from Jonathan so that it works for both use cases - EasyAnalytics and simple 773 linking. Looking at the patch it seems to be missing the code to actually calculate the $analytics_flag?

I think we could all agree that this would be a nice enhancement, showing a link only when there are analytics. Or just never show a link if EasyAnalytics is used?

I am just not sure how to do it, which is the whole reason it was not implemented like that from the beginning :( With some guidance I would be happy to give it a try.

(In reply to Joy Nelson from comment #17)
> Cait-
> I agree that this could be a responsive feature in Koha.  However, to most
> librarians it appears that Koha is 'doing something' (displaying analytic
> links) that is not supported by the correct cataloging of records. 
> Catalogers put a lot of effort into correctly cataloging their collections. 
> And when this cataloging results in behavior that is not related to the
> industry standards, it causes frustration, confusion and some lack of
> confidence in the tool.  
> 
> Yes, it could be hidden, but by doing so we are expecting the bulk of Marc21
> libraries to hide something that should never be there in the first place.

Sounds reasonable to me. 'Can you hide it' should not be the approach imo..

Can we all start working on a solution instead of chiming in that a feature added 2006 is bad? ;)

Sorry, 2011.

Has any more progress been made on Bug 15851?

Carnegie Staff

Katrin,
Currently this analytic feature only works where the leader7=s.  I'd like to propose the following changes to this feature to allow it be used by your library and others in a more standard method.

1. Allow the link to appear when leader7 =s or leader7=m.  Both monographs and serial records can have component parts and as such have children that should be displayed by this feature.  The Show analytics link is searching for bib-level:a (Monographic component part) and bib-level:b (Serial component part).

2. Add a search function to this display page that checks the index (Host-item) to see if any children are present. If so, then display the "Show Analytics Link".  This search would search for control number or title depending on the value of usecontrolnumber sys pref.  

3. Add a syspref for this method of analytic cataloging, much like we have for Easy Analytics and 'use control number'.  Allow libraries the ability to turn off the analytic cataloging if they don't use it.  Call it "ShowLinktoChildfromParent"?  

Your thoughts?
joy

Hi Joy, 

sorry, I missed getting back to this :(

(In reply to Joy Nelson from comment #24)
> Katrin,
> Currently this analytic feature only works where the leader7=s.  I'd like to
> propose the following changes to this feature to allow it be used by your
> library and others in a more standard method.
> 
> 1. Allow the link to appear when leader7 =s or leader7=m.  Both monographs
> and serial records can have component parts and as such have children that
> should be displayed by this feature.  The Show analytics link is searching
> for bib-level:a (Monographic component part) and bib-level:b (Serial
> component part).

Yes, that makes sense. Actually we have that link in some custom XSLT, but use a different class to allow changing visibility independently for m and s.

> 
> 2. Add a search function to this display page that checks the index
> (Host-item) to see if any children are present. If so, then display the
> "Show Analytics Link".  This search would search for control number or title
> depending on the value of usecontrolnumber sys pref.  

I would be totally ok with that. I just never figured out how to do this myself. Some help here would be great. I am not sure Host-item will work as an index or if a new index for $w in 773 would be needed for the controlnumber part.

> 3. Add a syspref for this method of analytic cataloging, much like we have
> for Easy Analytics and 'use control number'.  Allow libraries the ability to
> turn off the analytic cataloging if they don't use it.  Call it
> "ShowLinktoChildfromParent"?  

I am not sure about adding more and more prefs when CSS would work, but if we add the function in 2) it makes sense to have a real switch to avoid unnecessary processing.

I had some hope that [Bug 11175] Show the parent record's component parts in the detailed views would help resolve the issues or replace the current way, but it hasn't worked out.

Sponsor propousal

We need a solution for show links to analaytics only if there are analytics and not just at journal level, there is also book chapter as analytics.

CASE 1.

If analytics are created via koha, we have all 773 fields filled and we can apply the used in as it is in staff gui

CASE 2
If the analytics have been propertly created using

773$w <--check match 773w vs 001-->
773$x <-- check match 773x vs 020 and 022-->

I know that some records came from other catalogues, but if they do not fill 773$w or 773$x they're not marc21 and then they do not desserve havings show analytics publishing ;)

If this is the case then they just need use batch record modification to improve the quality of their records.

I suggest the creation of cronjob with options how to check if there are analyrtics


count_analytics --match w --match 0 --match 9 --match x (the cron will check all 77309wx and compare with 001, 020, 022, 999c and 9520 and if there is a match add a counter at the record (this records has 9 analytics) and publish on the bib level for example.

Trying to summarize and epxlain... sorry it got long:

I think we need to be careful here in general as we have multiple ways of handling analytics in Koha right now, that cannot be mixed (afaik). The best write-up about this we have is: 

  https://wiki.koha-community.org/wiki/Multipart_Bibliographics

1) Links between records using $w (7xx $w => 001)

The links here are only on record level. There is no effect on item visibility.

2) Links between records and items using EasyAnalytics feature (773 => $9 $0 $w)

$0 = biblionumber of parent record
§9 = itemnumber of parent item
$w = (003)001 of parent record 

The links here involve records and items. One existing physical item can be displayed on multiple records. This makes it really interesting for bound items. I've not been able to use this feature in our union catalog setting as it adds 773 entries to the records and we'd kill them with our regular updates :(

--

Joubu's attached patch suggests using the analytics_flag, but this will only give results for 2) and never display the link for libraries using 1). You can also see here, that it relies on an itemnumber. The link here is more between a host item and related records, than between records.

opac-detail.pl:
my $countanalytics = C4::Context->preference('EasyAnalyticalRecords') ? GetAnalyticsCount($item->{itemnumber}) : 0;
if ($countanalytics > 0){
  $analytics_flag=1;
  $item->{countanalytics} = $countanalytics;
}

Maybe we could improve Joubu's patch by also checking the EasyAnalyticalRecords pref - then it could work for case 2). To my knowledge there were issues using 1) and 2) together - maybe libraries can tell if that's still the case (or testing could). The wiki lists UseControlNumbers and EasyAnalyticalRecords as mutually exclusive.

What Hugo suggests could work, if I understand correctly, it would do about this:

- Find all records with 773
- Look for matching linked records using the available standard identifiers:
  - If EasyAnalyticalrecords is used:
    - $9 = biblionumber
    - $0 = itemnumber
  - Also:
    - $w = 001 Control number
    - $x = 020 ISBN
    - $z = 022 ISSN

Deduplicate bilbionumbers and add their count (or maybe the full relationship information? 773, host biblionumber, linked biblionumber) to a table.

Then we could look up that information and push it to the XML like Joubus patch does with the flag. Even more: We could show the exact number of linked records.

I'd like to make following suggestions:

- Use a separate table for storing information about record relationships (count of records linked)
- Make it generic enough it could be expanded for other relationships beyond 773 (volumes in a set, volumnes in a series.. etc.) 
- Have a way to update the counter when adding/deleting/editing a record in the GUI. People might get irritated if the link doesn't show when saving their records.
- Give the script the option to run incrementally, either for all records stored in a database (starting out) or only looking at records added or changed since x. ago. It could be too 'expensive' to fully run it on a big database over and over.

100% agree katrin, perfect summary!

However I have been thinking on it and I suggest to go one step further,  add to the scrip the chance of filling 001 with 999$c

In some migrated libraries they came to koha using type2 (773w-->001), the issue  is that 001 is mandatory for linking and at koha is empty by default, I suggest to add an option to the script to copy 999c to 001 and helps librarian to create anañytics using 773$w--001

If you add multiple mappings, there is a risk of creating false links.

$w = 001 or 999$c 

$w is actually a 001 value, but the same value exists as biblionumber.
or
$w is a biblionumber, but there also exists a 001 with the value.

At the end what we wish display the link propertly .... the only issue (it is big) it could came with counter, not with the link.

I mean if we have one bib record and one analytic with all fields filled (773W0 --> 001 + 773$0 or even 773$9)

The worth scenario is couning two times, but even then we have a valid link with but wrong count number... ok, 100% agree but evern is better to have no links ;)

On the other hand it is just a matter of dedup linking I mean, host bib couldn't link to same bib analyic record more than one...then if the count take care of that, issue fixed

Almost all migrsted libraries have analytics using 773$w and 773$0, and perhpas some of them will migrate from 773w to 7730 because is stronger and better as you can add analytics to issue level, if we manage that, the solution is stronger

Using $0 is non standard and not an option for all libraries as not all libraries maintain their records locally in Koha. It's good to keep different use cases in mind. 

If the script was adding biblionumber to 001, I guess it would not matter as much: 001 = 999$c so $w = $0?

I've started thinking on a solution to this, due to my recent works on XSLTs and plugins. I'll keep you posted.

Truth is catalogues need to be assumed heterogeneous, and that's starting point on any solution to this:
- Advanced analytics
- EasyAnalytics
- Hand-made 773 fields (and whatever happens in UNIMARC)+

They can all happen in the same catalog (importing external records, different geological eras on the biblio database, etc)

I see two options:
1) Implement the same logic of searching linked records that is used on creating the link, but on the *detail.pl files. This would be similar to Jonathan's approach, but it would consider more use cases. Pros: the implementation is very simple and straight-forward, if it works on the XSLT we could consider it valid on the .pl and we could just pass the variable to the XSLT for proper rendering, independent on the leader value. Cons: It is not exactly correct (if we assume the catalog is heterogeneous, then some research needs to be done on those searches, maybe there should be a search for both use cases in a single query, but also the one Jonathan used (for the EasyAnalytics case). This would certainly add latency on rendering a record.

2) Add a 'has_child_biblios' boolean column, and introduce sysprefs to control what strategies to use for finding children records. This could be hooked on saving records, and have an on/off switch. This way we would have something like:

- BibliosKeepChildrenInfo (on/off switch)
- BibliosKeepChildrenStrategy: controlnumber|easyanalytics|freesearch

BibliosKeepChildrenStrategy should be sortable and you should be able to choose which options to use.

A maintenance script should be added for populating this boolean the first time.
The current XSLT behaviour could be kept with a deprecation notice, but I think the constraint to do it only for serials is wrong (a bug).

I understand the disagreement, but we have added lots of other information on the base that it can be hidden with CSS if unwanted - serials are more likely to have analytics than monographs, so that's why the limit was introduced in the first place. In our own XSLT we also have a version for monographs (same but different class) and control both of them with CSS. But I know our libraries would love it too to only see it when there actually are child records.

I am not sure I understand how 1) would look like, can you explain a bit more?

With 2) we also need to account for every way a record can be added or edited in Koha - including the imports.

(In reply to Katrin Fischer from comment #34)
> I understand the disagreement, but we have added lots of other information
> on the base that it can be hidden with CSS if unwanted - serials are more
> likely to have analytics than monographs, so that's why the limit was
> introduced in the first place. In our own XSLT we also have a version for
> monographs (same but different class) and control both of them with CSS. But
> I know our libraries would love it too to only see it when there actually
> are child records.

I don't see a disagreement yet. I'm pondering options. My idea is that we could add different classes for each leader value (if you require that) and even hide them all by default to keep the current behaviour. No proposal from me will break your workflow, Katrina.

> I am not sure I understand how 1) would look like, can you explain a bit
> more?

Jonathan's code uses a method to search for EasyAnalytics related biblios. We need to have that AND a Zebra/Elasticsearch query for Host-item and rcn. If any of those returns something, then there's analytics and something similar-ish to Jonathan's approach can be used. I will produce a patch for this option tomorrow (my) morning so we check how bad it is for performance.

> With 2) we also need to account for every way a record can be added or
> edited in Koha - including the imports.

Yes, they all ultimately call ModBiblio so I'm not *that* worried about it.

So the struggle here is: fix the data? assume heterogeneous database?

Created attachment 106260 [details] [review]
Bug 15851: Display analytics links for more cases

The current XSLT displays the link to linked analytics only for serials.
This patch makes it show for all the relevant position 7 on the leader
values. I left out a and b as recommended by expert librarians on my
team, but I can revert that if required.

The current implementation adds a new CSS class for each case, so
libraries willing to keep the current behaviour or just have more
granular control on the cases they want the link to display, just can.

This patch makes sense with the follow-up one, which will display the
link only if there are really related records.

To test:
1. Open a non-serial record, notice there's no link to analytics
2. Apply this patch and reload
=> SUCCESS: There's an 'Analytics' link
3. Inspect the produced HTML
=> SUCCESS: A special class with analytic_* value has been added, and
thus we now can control its display through CSS
4. Sign off :-D

Sponsored-by: Orex Digital

Created attachment 106261 [details] [review]
Bug 15851: Only display the analytics link when required

This patch makes opac-detail.pl query for analytics the same way it
would do with the generated link (i.e. based on UseControlNumber) and
passes a flag to the XSLT so it displays (or not) the 'Analytics' link.

To test:
1. Apply the first patch
2. Have a known record without analytics
3. Open the record in the OPAC
=> FAIL: It shows the 'Analytics' link
4. Have a record known to have analytics and open in OPAC, on a separate
   tab
=> SUCCESS: It shows the 'Analytics' link
5. Apply this patch and restart_all
6. Reload the tabs
=> SUCCESS: It shows the link where it has to, and hides it where it
shouldn't be displayed
7. Sign off :-D

Sponsored-by: Orex Digital

Ok, this is my attempt. The basic idea is split in two:
- Display analytic links for more cases (i.e. not just serial)
- If the result for showing the link is considered (today) good enough, just use the same query in the code to make the decision on the .pl+XSLT combination

If this approach to 'hiding when unnecessary' is good enough (I think it is) we could file a new but for adding new conditions (like ISBN, etc).

Feedback welcome!

I hae tested the patch and it works fine! however the code applied to opac is not applied to staff xslt and it shows the links always, even when there is not analytics.

(In reply to Hugo Agud from comment #39)
> I hae tested the patch and it works fine! however the code applied to opac
> is not applied to staff xslt and it shows the links always, even when there
> is not analytics.

My idea was to have feedback on the approach, and then move to other areas. The 'how to find analytics' discussion (i.e. the queries that are done) should be moved to another bug, for example.

Thanks for testing, and I will add the intranet counterpart for you to SO ASAP.

Created attachment 106821 [details] [review]
Bug 15851: Display analytics links for more cases

The current XSLT displays the link to linked analytics only for serials.
This patch makes it show for all the relevant position 7 on the leader
values. I left out a and b as recommended by expert librarians on my
team, but I can revert that if required.

The current implementation adds a new CSS class for each case, so
libraries willing to keep the current behaviour or just have more
granular control on the cases they want the link to display, just can.

This patch makes sense with the follow-up one, which will display the
link only if there are really related records.

To test:
1. Open a non-serial record, notice there's no link to analytics
2. Apply this patch and reload
=> SUCCESS: There's an 'Analytics' link
3. Inspect the produced HTML
=> SUCCESS: A special class with analytic_* value has been added, and
thus we now can control its display through CSS
4. Sign off :-D

Sponsored-by: Orex Digital

Signed-off-by: Hugo Agud <hagud@orex.es>

Created attachment 106822 [details] [review]
Bug 15851: Only display the analytics link when required

This patch makes opac-detail.pl query for analytics the same way it
would do with the generated link (i.e. based on UseControlNumber) and
passes a flag to the XSLT so it displays (or not) the 'Analytics' link.

To test:
1. Apply the first patch
2. Have a known record without analytics
3. Open the record in the OPAC
=> FAIL: It shows the 'Analytics' link
4. Have a record known to have analytics and open in OPAC, on a separate
   tab
=> SUCCESS: It shows the 'Analytics' link
5. Apply this patch and restart_all
6. Reload the tabs
=> SUCCESS: It shows the link where it has to, and hides it where it
shouldn't be displayed
7. Sign off :-D

Sponsored-by: Orex Digital

Signed-off-by: Your Full Name <your_email>

Created attachment 106988 [details] [review]
Bug 15851: Display analytics links for more cases

The current XSLT displays the link to linked analytics only for serials.
This patch makes it show for all the relevant position 7 on the leader
values. I left out a and b as recommended by expert librarians on my
team, but I can revert that if required.

The current implementation adds a new CSS class for each case, so
libraries willing to keep the current behaviour or just have more
granular control on the cases they want the link to display, just can.

This patch makes sense with the follow-up one, which will display the
link only if there are really related records.

To test:
1. Open a non-serial record, notice there's no link to analytics
2. Apply this patch and reload
=> SUCCESS: There's an 'Analytics' link
3. Inspect the produced HTML
=> SUCCESS: A special class with analytic_* value has been added, and
thus we now can control its display through CSS
4. Sign off :-D

Sponsored-by: Orex Digital

Signed-off-by: Hugo Agud <hagud@orex.es>

Created attachment 106989 [details] [review]
Bug 15851: Only display the analytics link when required

This patch makes opac-detail.pl query for analytics the same way it
would do with the generated link (i.e. based on UseControlNumber) and
passes a flag to the XSLT so it displays (or not) the 'Analytics' link.

To test:
1. Apply the first patch
2. Have a known record without analytics
3. Open the record in the OPAC
=> FAIL: It shows the 'Analytics' link
4. Have a record known to have analytics and open in OPAC, on a separate
   tab
=> SUCCESS: It shows the 'Analytics' link
5. Apply this patch and restart_all
6. Reload the tabs
=> SUCCESS: It shows the link where it has to, and hides it where it
shouldn't be displayed
7. Sign off :-D

Sponsored-by: Orex Digital

Signed-off-by: Hugo Agud <hagud@orex.es>

Created attachment 106990 [details] [review]
Bug 15851: (follow-up) Only display the analytics link when required (staff)

This patch replicates the introduced behaviour, for the admin interface.
To test, follow the test plan from the OPAC, but on the intranet.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

I was wondering what kind of performance impacts this would have on a large database with lots of analytics, but a code review shows that it looks good enough (depending on your views).

Looking at C4::Search::SimpleSearch, we're passing an offset of 0 and max_results of 0, which generates a range of 1..0 which is nothing, so no records are actually fetched from Zebra. 

It's not very elegant, but it should be functional, even for a large database. 

I tested an unrelated search using yaz-client on a very large database (over 1 million bib records), and for a query with 17626 hits, it took .432 seconds. 

For some people, adding .432 seconds to web page load time is unacceptable, but I imagine most of us might not care too much.

I'd argue that this lookup should be performed asynchronously after the HTML page has loaded.

The MARC data could trigger whether to show an "Analytics" label I suppose, and then the result of the async lookup would say something like "Show Analytics" or "None" or something like that. 

Anyway, that's just a thought. I'm not interested in doing any work on this. I'm just always interested in performance. 

The current patches are fine, so I'm not being critical, but I think an optimization would be to do an async lookup.

(In reply to David Cook from comment #46)
> I was wondering what kind of performance impacts this would have on a large
> database with lots of analytics, but a code review shows that it looks good
> enough (depending on your views).
> 
> Looking at C4::Search::SimpleSearch, we're passing an offset of 0 and
> max_results of 0, which generates a range of 1..0 which is nothing, so no
> records are actually fetched from Zebra. 
> 
> It's not very elegant, but it should be functional, even for a large
> database.

I take it as a compliment-ish comment, thanks :-P
 
> I tested an unrelated search using yaz-client on a very large database (over
> 1 million bib records), and for a query with 17626 hits, it took .432
> seconds. 

Can you give me the query so I can try on my databases as well?

> For some people, adding .432 seconds to web page load time is unacceptable,
> but I imagine most of us might not care too much.

I'd agree. We could make it an opt-in feature? PreciseAnalyticsLink or smth.

> I'd argue that this lookup should be performed asynchronously after the HTML
> page has loaded.
> 
> The MARC data could trigger whether to show an "Analytics" label I suppose,
> and then the result of the async lookup would say something like "Show
> Analytics" or "None" or something like that. 
> 
> Anyway, that's just a thought. I'm not interested in doing any work on this.
> I'm just always interested in performance. 
> 
> The current patches are fine, so I'm not being critical, but I think an
> optimization would be to do an async lookup.

I like the idea. Will draft an endpoint for fetching such information using the exact queries we use here.

(In reply to Tomás Cohen Arazi from comment #47)
> I take it as a compliment-ish comment, thanks :-P
>

I'm sure that you know that I think the world of you, Tomás <3. Heh.

> Can you give me the query so I can try on my databases as well?
> 

Actually, now that I look at it, I gave it a bad query. I tried "local-number=1000" thinking I had it set up for CCL2RPN, but it actually tried that as a PQF query. It got 0 hits, but it searched it as "local and number and 1000" with subquery results of 250,000, 125,000 and 20,000 respectively in 0.435 seconds.

I just tried searching "the" and it took 5.16 seconds to return a result with 1,125,000 hits.

I just tried "local" and got 250,000 hits in 0.193 seconds. 

Note that this is all using yaz-client and a unix socket (rather than a TCP socket).

I wonder if doing a CCL2RPN conversion adds much overhead... I'll try that and add another separate comment, as this one is growing too large.

> > For some people, adding .432 seconds to web page load time is unacceptable,
> > but I imagine most of us might not care too much.
> 
> I'd agree. We could make it an opt-in feature? PreciseAnalyticsLink or smth.
> 
> > I'd argue that this lookup should be performed asynchronously after the HTML
> > page has loaded.
> > 
> > The MARC data could trigger whether to show an "Analytics" label I suppose,
> > and then the result of the async lookup would say something like "Show
> > Analytics" or "None" or something like that. 
> > 
> > Anyway, that's just a thought. I'm not interested in doing any work on this.
> > I'm just always interested in performance. 
> > 
> > The current patches are fine, so I'm not being critical, but I think an
> > optimization would be to do an async lookup.
> 
> I like the idea. Will draft an endpoint for fetching such information using
> the exact queries we use here.

If we're doing it async, I don't think we'd need it to be an opt-in feature, as it would already be optimized. 

And thanks for drafting that!

Ok using .yazclientrc to set the following:

1. querytype ccl2rpn
2. set_cclfile /path/to/etc/zebradb/ccl.properties

Trying the following:

1. Host-item="This is a test"
2. rcn=1000 and (bib-level=a or bib-level=b)

Both of these returned results in 0.002 seconds with 0 hits.

The search performance is interesting.

Going back to these:

1. "find e" 
2. "find the"

They're both using the "Any:w" index, but "e" takes .09 seconds to return 37500 hits while "the" takes 3.39 seconds to return 1125000 hits.

Zebra has to take the same amount of time to scan the index, so the time spent in yaz-client isn't on Zebra finding the data. It's spent on either Zebra/yaz-client/both processing the data.

The time taken seems to be linear/O(n) not for searching per se but rather in terms of returning a search result. 

Now it's just unclear whether it's Zebra or yaz-client (or both) causing the time delay. 

I might try writing a quick little Perl script to see how it fairs when using Zoom with Zebra...

yaz-client: local-number=10000
1 hit
0.002227 seconds

Koha code*: local-number=10000
1 hit
2.093 seconds

At this point, it's worth noting that approximately 2 seconds is spent just loading Modern::Perl, C4::Context, and C4::Search, so it's not a fair comparison.

I'm going to do another comparison using "e" and "the"... 


*
#!/usr/bin/perl
use Modern::Perl;
use C4::Context;
use C4::Search;
my @tmpresults;
my @zconns;
$zconns[0] = C4::Context->Zconn( 'biblioserver', 1 );
my $query = "Host-Item=this is a test";
$query = "local-number=10000";
my $zoom_query = new ZOOM::Query::CCL2RPN($query, $zconns[0]);
$tmpresults[0] = $zconns[0]->search($zoom_query);
my $total_hits = 0;
C4::Search::_ZOOM_event_loop(
        \@zconns,
        \@tmpresults,
        sub {
                my ($i,$size) = @_;
                $total_hits += $tmpresults[$i - 1] ->size();

        },
);
warn $total_hits;

(In reply to David Cook from comment #49)
> Ok using .yazclientrc to set the following:
> 
> 1. querytype ccl2rpn
> 2. set_cclfile /path/to/etc/zebradb/ccl.properties
> 
> Trying the following:
> 
> 1. Host-item="This is a test"
> 2. rcn=1000 and (bib-level=a or bib-level=b)
> 
> Both of these returned results in 0.002 seconds with 0 hits.
> 
> The search performance is interesting.
> 
> Going back to these:
> 
> 1. "find e" 
> 2. "find the"
> 
> They're both using the "Any:w" index, but "e" takes .09 seconds to return
> 37500 hits while "the" takes 3.39 seconds to return 1125000 hits.
> 
> Zebra has to take the same amount of time to scan the index, so the time
> spent in yaz-client isn't on Zebra finding the data. It's spent on either
> Zebra/yaz-client/both processing the data.
> 
> The time taken seems to be linear/O(n) not for searching per se but rather
> in terms of returning a search result. 
> 
> Now it's just unclear whether it's Zebra or yaz-client (or both) causing the
> time delay. 
> 
> I might try writing a quick little Perl script to see how it fairs when
> using Zoom with Zebra...

Can you try zoomsh? Is what IndexData recommends.

(In reply to Tomás Cohen Arazi from comment #51)
> (In reply to David Cook from comment #49)
> > Ok using .yazclientrc to set the following:
> > 
> > 1. querytype ccl2rpn
> > 2. set_cclfile /path/to/etc/zebradb/ccl.properties
> > 
> > Trying the following:
> > 
> > 1. Host-item="This is a test"
> > 2. rcn=1000 and (bib-level=a or bib-level=b)
> > 
> > Both of these returned results in 0.002 seconds with 0 hits.
> > 
> > The search performance is interesting.
> > 
> > Going back to these:
> > 
> > 1. "find e" 
> > 2. "find the"
> > 
> > They're both using the "Any:w" index, but "e" takes .09 seconds to return
> > 37500 hits while "the" takes 3.39 seconds to return 1125000 hits.
> > 
> > Zebra has to take the same amount of time to scan the index, so the time
> > spent in yaz-client isn't on Zebra finding the data. It's spent on either
> > Zebra/yaz-client/both processing the data.
> > 
> > The time taken seems to be linear/O(n) not for searching per se but rather
> > in terms of returning a search result. 
> > 
> > Now it's just unclear whether it's Zebra or yaz-client (or both) causing the
> > time delay. 
> > 
> > I might try writing a quick little Perl script to see how it fairs when
> > using Zoom with Zebra...
> 
> Can you try zoomsh? Is what IndexData recommends.

And also the plain PQF query while you're on it

yaz-client: e
37500 hits
.09 seconds

Koha: e
37500 hits
2.110 seconds (2 seconds being loading Perl modules, so .110 sec)

yaz-client: the
1125000 hits
3.39 seconds

Koha: the
1125000 hits
5.315 seconds (2 seconds being loading Perl modules, so 3.315 sec)

If we convert from async code to sync code, we can get the following:
1125000 hits
3.78 seconds (2 seconds being loading Perl modules, so 1.78 sec)

If we comment out the code after the search(), we can see that it still takes 3.7-4 seconds to run, so the time delay appears to be on the Zebra side. I see high CPU usage but not high memory usage, so it looks like it's processing something. Of course, Zebra is a bit of a black box for us. 

For a Plack enabled Koha, my guess is that this search should have a minimal impact, unless a bib record has hundreds of thousands/millions of articles referencing it. 

The highest hit is probably going to be < 100, and since the time is dependent on the size of the search result rather than the size of the index, database size should be irrelevant. 

The biggest impact in terms of performance will probably be loading Perl modules via "use Koha::SearchEngine::Search". For Plack, this will be a once off for a worker that hasn't loaded opac-detail.pl before. For CGI, it could add 1-2 seconds of latency each time opac-detail.pl is loaded.

(In reply to Tomás Cohen Arazi from comment #51)
> Can you try zoomsh? Is what IndexData recommends.

I'd never heard of that. Very cool.

(In reply to Tomás Cohen Arazi from comment #52)
> And also the plain PQF query while you're on it

So the "e" and "the" queries are plain PQF queries.

So it looks like Zebra response times increase as the number of hits increased (even without fetching records).

And loading C4::Search takes 1-2 seconds.

Ultimately, this is unlikely to have a negative impact on Plack Koha, but it probably will cause opac-detail.pl to load 1-2 seconds slower in CGI Koha.

Can you please rebase?

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 15851: Display analytics links for more cases
Applying: Bug 15851: Only display the analytics link when required
error: sha1 information is lacking or useless (koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slim2OPACDetail.xsl).
error: could not build fake ancestor
Patch failed at 0001 Bug 15851: Only display the analytics link when required
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-15851-Only-display-the-analytics-link-when-req-6AcS8l.patch

Created attachment 108049 [details] [review]
Bug 15851: Display analytics links for more cases

The current XSLT displays the link to linked analytics only for serials.
This patch makes it show for all the relevant position 7 on the leader
values. I left out a and b as recommended by expert librarians on my
team, but I can revert that if required.

The current implementation adds a new CSS class for each case, so
libraries willing to keep the current behaviour or just have more
granular control on the cases they want the link to display, just can.

This patch makes sense with the follow-up one, which will display the
link only if there are really related records.

To test:
1. Open a non-serial record, notice there's no link to analytics
2. Apply this patch and reload
=> SUCCESS: There's an 'Analytics' link
3. Inspect the produced HTML
=> SUCCESS: A special class with analytic_* value has been added, and
thus we now can control its display through CSS
4. Sign off :-D

Sponsored-by: Orex Digital

Signed-off-by: Hugo Agud <hagud@orex.es>

Created attachment 108050 [details] [review]
Bug 15851: Only display the analytics link when required

This patch makes opac-detail.pl query for analytics the same way it
would do with the generated link (i.e. based on UseControlNumber) and
passes a flag to the XSLT so it displays (or not) the 'Analytics' link.

To test:
1. Apply the first patch
2. Have a known record without analytics
3. Open the record in the OPAC
=> FAIL: It shows the 'Analytics' link
4. Have a record known to have analytics and open in OPAC, on a separate
   tab
=> SUCCESS: It shows the 'Analytics' link
5. Apply this patch and restart_all
6. Reload the tabs
=> SUCCESS: It shows the link where it has to, and hides it where it
shouldn't be displayed
7. Sign off :-D

Sponsored-by: Orex Digital

Signed-off-by: Hugo Agud <hagud@orex.es>

Created attachment 108051 [details] [review]
Bug 15851: (follow-up) Only display the analytics link when required (staff)

This patch replicates the introduced behaviour, for the admin interface.
To test, follow the test plan from the OPAC, but on the intranet.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

This works well without UseControlNumber so far, but when I turn it on, the link that was visible before, disappears:

Parent record:
001 15661423

Child record:
LDR 00395    a2200121   4500 = monographic component part
773 	0 		_dCambridge, Mass. : Belknap Press of Harvard University Press, 2009.
_aWilson, Peter H.
_z9780674036345 (hardcover : alk. paper)
_w15661423
_tThe Thirty Years War : 

Can you please check?

(In reply to Katrin Fischer from comment #61)
> This works well without UseControlNumber so far, but when I turn it on, the
> link that was visible before, disappears:
> 
> Parent record:
> 001 15661423
> 
> Child record:
> LDR 00395    a2200121   4500 = monographic component part
> 773 	0 		_dCambridge, Mass. : Belknap Press of Harvard University Press,
> 2009.
> _aWilson, Peter H.
> _z9780674036345 (hardcover : alk. paper)
> _w15661423
> _tThe Thirty Years War : 
> 
> Can you please check?

Will check. If you use master, and the link shows. Does it give you any results? It now shows based on those results.

I repeated the test with a serial, as they show the link on master:

Zebra, UseControlNumber = Don't.

On master:
1) Search for Gairm in test data
2) New > Add new child record
3) Check 773$w is set
4) Add 245 and other mandatory fields, save
5) Verify link back and forth between Analytics: and In: works.

Switch UseControlnumber = Do

6) Links back and forth work.

Switch to Elasticsearch, UseControlnumber = Don't

7) Links back and forth work.

Switch to UseControlnumber = Do

8) In: link works, analytics link doesn't (NOT OK)
   rcn:11388674 and (bib-level:a or bib-level:b)
   rcn: alone works.
   bib-level:a alone works.
   Something wrong with the search syntax for Elasticsearch?

Repeating the test on the analytics branch:

9) Zebra, UseControlNumber = Don't: Works
10) Zebra, UseControlNumber = Do: Works (now)
11) Elasticsearch, UseControlNumber = Don't: Works
12) Elasticsearch, UseControlnumber = Do: Link disappears (NOT OK)

After retesting I think the problem last night was either Elasticsearch being active or the record not being properly reindexed when I tried.

Result: UseControlNumber doesn't work with Elasticsearch

This works, and only highlights UseControlNumber doesn't work correctly in ES, which should have it's own bug reported.

(In reply to Tomás Cohen Arazi from comment #64)
> This works, and only highlights UseControlNumber doesn't work correctly in
> ES, which should have it's own bug reported.

But with this applied the links all disappear for ES users - I think we should fix the other issue quickly because this will shine a bad light on a nice feature that we want to boldly advertise in release notes.

(In reply to Katrin Fischer from comment #65)
> (In reply to Tomás Cohen Arazi from comment #64)
> > This works, and only highlights UseControlNumber doesn't work correctly in
> > ES, which should have it's own bug reported.
> 
> But with this applied the links all disappear for ES users - I think we
> should fix the other issue quickly because this will shine a bad light on a
> nice feature that we want to boldly advertise in release notes.

I agree. Can you file a bug for ES with your findings and make this one dependent on it? They are really unrelated. And ES it's showing non-working links anyway without this patch set

I've filed bug 26313. I suspect changes to this will be needed as the same syntax for the search is used here and in the XSLT we will need to fix there.

Created attachment 109267 [details] [review]
Bug 15851: Display analytics links for more cases

The current XSLT displays the link to linked analytics only for serials.
This patch makes it show for all the relevant position 7 on the leader
values. I left out a and b as recommended by expert librarians on my
team, but I can revert that if required.

The current implementation adds a new CSS class for each case, so
libraries willing to keep the current behaviour or just have more
granular control on the cases they want the link to display, just can.

This patch makes sense with the follow-up one, which will display the
link only if there are really related records.

To test:
1. Open a non-serial record, notice there's no link to analytics
2. Apply this patch and reload
=> SUCCESS: There's an 'Analytics' link
3. Inspect the produced HTML
=> SUCCESS: A special class with analytic_* value has been added, and
thus we now can control its display through CSS
4. Sign off :-D

Sponsored-by: Orex Digital

Signed-off-by: Hugo Agud <hagud@orex.es>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 109268 [details] [review]
Bug 15851: Only display the analytics link when required

This patch makes opac-detail.pl query for analytics the same way it
would do with the generated link (i.e. based on UseControlNumber) and
passes a flag to the XSLT so it displays (or not) the 'Analytics' link.

To test:
1. Apply the first patch
2. Have a known record without analytics
3. Open the record in the OPAC
=> FAIL: It shows the 'Analytics' link
4. Have a record known to have analytics and open in OPAC, on a separate
   tab
=> SUCCESS: It shows the 'Analytics' link
5. Apply this patch and restart_all
6. Reload the tabs
=> SUCCESS: It shows the link where it has to, and hides it where it
shouldn't be displayed
7. Sign off :-D

Sponsored-by: Orex Digital

Signed-off-by: Hugo Agud <hagud@orex.es>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 109269 [details] [review]
Bug 15851: (follow-up) Only display the analytics link when required (staff)

This patch replicates the introduced behaviour, for the admin interface.
To test, follow the test plan from the OPAC, but on the intranet.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Testing on a fresh ktd:
* Turn on EasyAnalyticalRecords
* /cgi-bin/koha/catalogue/detail.pl?biblionumber=25
Edit > link to host
use barcode from itemnumber 1
=> https://snipboard.io/4TXWVU.jpg
I don't see the link

What am I missing?

This is not for EasyAnalytics, but for UseControlnumber. Please follow my test plan from comment#63

Created attachment 109459 [details] [review]
Bug 15851: Display analytics links for more cases

The current XSLT displays the link to linked analytics only for serials.
This patch makes it show for all the relevant position 7 on the leader
values. I left out a and b as recommended by expert librarians on my
team, but I can revert that if required.

The current implementation adds a new CSS class for each case, so
libraries willing to keep the current behaviour or just have more
granular control on the cases they want the link to display, just can.

This patch makes sense with the follow-up one, which will display the
link only if there are really related records.

To test:
1. Open a non-serial record, notice there's no link to analytics
2. Apply this patch and reload
=> SUCCESS: There's an 'Analytics' link
3. Inspect the produced HTML
=> SUCCESS: A special class with analytic_* value has been added, and
thus we now can control its display through CSS
4. Sign off :-D

Sponsored-by: Orex Digital

Signed-off-by: Hugo Agud <hagud@orex.es>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 109460 [details] [review]
Bug 15851: Only display the analytics link when required

This patch makes opac-detail.pl query for analytics the same way it
would do with the generated link (i.e. based on UseControlNumber) and
passes a flag to the XSLT so it displays (or not) the 'Analytics' link.

To test:
1. Apply the first patch
2. Have a known record without analytics
3. Open the record in the OPAC
=> FAIL: It shows the 'Analytics' link
4. Have a record known to have analytics and open in OPAC, on a separate
   tab
=> SUCCESS: It shows the 'Analytics' link
5. Apply this patch and restart_all
6. Reload the tabs
=> SUCCESS: It shows the link where it has to, and hides it where it
shouldn't be displayed
7. Sign off :-D

Sponsored-by: Orex Digital

Signed-off-by: Hugo Agud <hagud@orex.es>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 109461 [details] [review]
Bug 15851: (follow-up) Only display the analytics link when required (staff)

This patch replicates the introduced behaviour, for the admin interface.
To test, follow the test plan from the OPAC, but on the intranet.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 109462 [details] [review]
Bug 15851: (QA follow-up) Fix booleans to uppercase to make this work for Elasticsearch and UseControlNumber

Same test plan as before, but with UseControlNumber = Use and
Elasticsearch.

Pushed to master for 20.11, thanks to everybody involved!

enhancement will not be backported to 20.05.x series