Bug 13327

Summary: OPACPopupAuthorsSearch doesn't work with XSLT views
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: OPACAssignee: Owen Leonard <oleonard>
Status: CLOSED FIXED QA Contact: Jonathan Druart <jonathan.druart>
Severity: normal    
Priority: P5 - low CC: 1joynelson, david, jonathan.druart, martin.renvoize
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This enhancement improves the OPACPopupAuthorsSearch feature so that it works in both the normal and XSLT views (OPACXSLTDetailsDisplay).
Version(s) released in:
20.05.00
Bug Depends on:    
Bug Blocks: 24690, 29462, 12561    
Attachments: Bug 13327: OPACPopupAuthorsSearch doesn't work with XSLT views
Bug 13327: Screenshots with different options
Bug 13327: OPACPopupAuthorsSearch doesn't work with XSLT views
Bug 13327: Screenshots - select items to search issue
Bug 13327: OPACPopupAuthorsSearch doesn't work with XSLT views
Bug 13327: OPACPopupAuthorsSearch doesn't work with XSLT views
Bug 13327: (follow-up) Correct CSS and XSL errors
Bug 13327: OPACPopupAuthorsSearch doesn't work with XSLT views
Bug 13327: (follow-up) Correct CSS and XSL errors
Bug 13327: OPACPopupAuthorsSearch doesn't work with XSLT views
Bug 13327: (follow-up) Correct CSS and XSL errors

Description Katrin Fischer 2014-11-22 10:06:06 UTC
The OPACPopupAuthorsSearch feature does only work with the normal view.
Comment 1 Owen Leonard 2019-07-15 18:09:35 UTC
It occurs to me that as long as we don't remove opac-detail's access to the MARCAUTHORS and MARCSUBJCTS information we could still have the corresponding modals be built in opac-detail.tt. That way we would only need to make sure the XSLT markup had the right classes to act as a hook for triggering the modals.
Comment 2 Owen Leonard 2020-01-07 17:56:00 UTC Comment hidden (obsolete)
Comment 3 David Nind 2020-01-25 02:11:50 UTC
Created attachment 97932 [details]
Bug 13327: Screenshots with different options

Results from my testing:

- For OPACPopupAuthorsSearch enabled and OPACXSLTDetailsDisplay default:
    • works for the author search (the new style for the modal is much nicer)
    • the modal for subject terms doesn't appear/work

- For OPACPopupAuthorsSearch enabled and OPACXSLTDetailsDisplay empty:
    • search icon different for additional authors
    • old search icon displays for ‘Subject(s):’
    • the modal for subject terms works, but uses 'old style'

- For OPACPopupAuthorsSearch disabled and OPACXSLTDetailsDisplay empty - the record displays the same as for OPACPopupAuthorsSearch enabled and OPACXSLTDetailsDisplay empty

I've attached a document with screenshots.

Tested use Firefox Developer Edition and Google Chrome on Ubuntu 18.04 - cleared browser history, cache, etc for different combination of options. Also used a fresh container for each combination using koha-testing-docker.
Comment 4 Owen Leonard 2020-01-25 15:46:13 UTC
Thanks for testing, David! I'll take a look.
Comment 5 Owen Leonard 2020-02-13 18:31:47 UTC Comment hidden (obsolete)
Comment 6 David Nind 2020-02-14 08:42:57 UTC
Created attachment 98904 [details]
Bug 13327: Screenshots - select items to search issue

I think the issues from the previous testing are now resolved.

A new issue is the pop-up select boxes - they aren't formatted correctly. See the attachment Bug 13327 - Select items to search issue.
Comment 7 Owen Leonard 2020-02-14 12:42:40 UTC
Created attachment 98939 [details] [review]
Bug 13327: OPACPopupAuthorsSearch doesn't work with XSLT views

This patch reimplements the OPACPopupAuthorsSearch feature so that it
will work in the XSLT view.

To test, apply the patch and rebuild the OPAC CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

 - View the detail page for a bibliographic record in the OPAC.
   Test under the following conditions:

 - OPACPopupAuthorsSearch ENABLED, OPACXSLTDetailsDisplay "default"
 - OPACPopupAuthorsSearch ENABLED, OPACXSLTDetailsDisplay empty
 - OPACPopupAuthorsSearch DISABLED, OPACXSLTDetailsDisplay "default"
 - OPACPopupAuthorsSearch DISABLED, OPACXSLTDetailsDisplay empty

In each of these cases, test the detail page's listing of additional
authors/contributors and subjects. Test records which have both multiple
contributors or subjects and which have only one author or subject.

When OPACPopupAuthorsSearch is turned on, verify that the author/subject
selection modal appears and that your selections are correctly combined
to build a search string.

Note that OPACPopupAuthorsSearch feature has a problem with terms
containing parentheses. The issue predates this patch.

Also note that the QA tools will give a false positive about Bootstrap
button styles. This rule only applies to the staff client.
Comment 8 Owen Leonard 2020-02-14 12:43:11 UTC
(In reply to David Nind from comment #6)
> A new issue is the pop-up select boxes - they aren't formatted correctly.

I forgot to update the test plan with my revised patch: The OPAC CSS needs to be rebuilt!
Comment 9 Liz Rea 2020-02-14 18:31:30 UTC
Created attachment 99002 [details] [review]
Bug 13327: OPACPopupAuthorsSearch doesn't work with XSLT views

This patch reimplements the OPACPopupAuthorsSearch feature so that it
will work in the XSLT view.

To test, apply the patch and rebuild the OPAC CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

 - View the detail page for a bibliographic record in the OPAC.
   Test under the following conditions:

 - OPACPopupAuthorsSearch ENABLED, OPACXSLTDetailsDisplay "default"
 - OPACPopupAuthorsSearch ENABLED, OPACXSLTDetailsDisplay empty
 - OPACPopupAuthorsSearch DISABLED, OPACXSLTDetailsDisplay "default"
 - OPACPopupAuthorsSearch DISABLED, OPACXSLTDetailsDisplay empty

In each of these cases, test the detail page's listing of additional
authors/contributors and subjects. Test records which have both multiple
contributors or subjects and which have only one author or subject.

When OPACPopupAuthorsSearch is turned on, verify that the author/subject
selection modal appears and that your selections are correctly combined
to build a search string.

Note that OPACPopupAuthorsSearch feature has a problem with terms
containing parentheses. The issue predates this patch.

Also note that the QA tools will give a false positive about Bootstrap
button styles. This rule only applies to the staff client.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 10 Jonathan Druart 2020-02-18 16:11:49 UTC
I get a 404
Request URL:http://catalogue.kohadev.org/opac-tmpl/bootstrap/images/z.png
Comment 11 Jonathan Druart 2020-02-18 16:13:23 UTC
(In reply to Jonathan Druart from comment #10)
> I get a 404
> Request URL:http://catalogue.kohadev.org/opac-tmpl/bootstrap/images/z.png

https://snipboard.io/8BbRWf.jpg
Comment 12 Jonathan Druart 2020-02-18 16:23:40 UTC
Also, the style looks weird (I did yarn build --view opac): https://snipboard.io/ob6UZq.jpg

And the popup does not display for the non XSLT view (new search is launched instead).
Comment 13 Owen Leonard 2020-02-19 13:52:40 UTC
Created attachment 99250 [details] [review]
Bug 13327: (follow-up) Correct CSS and XSL errors

This patch adds back the CSS changes which seem to have been lost in a
rebase somewhere. The patch also corrects a bad copy/paste in the XSL
file which led to a 404 error.
Comment 14 Owen Leonard 2020-02-19 13:56:23 UTC
(In reply to Jonathan Druart from comment #10)
> Request URL:http://catalogue.kohadev.org/opac-tmpl/bootstrap/images/z.png
> Also, the style looks weird (I did yarn build --view opac):

My follow-up should correct these mistakes.

> And the popup does not display for the non XSLT view (new search is launched
> instead).

I don't see this problem. I believe the popup displays in all cases where it did previously, which excepts the "magnifying glass" link which links directly to an authority search. Can you re-test?
Comment 15 Katrin Fischer 2020-02-19 21:58:47 UTC
Hi Owen, can you please check?

 FAIL	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
   OK	  filters
   FAIL	  forbidden patterns
		forbidden pattern: Contains old bootstrap style for buttons (see bug 16239) (line 1347)
   OK	  git manipulation
   OK	  js_in_body
   OK	  spelling
   OK	  tt_valid
   OK	  valid_template
Comment 16 Owen Leonard 2020-02-19 23:15:38 UTC
(In reply to Katrin Fischer from comment #15)
> 		forbidden pattern: Contains old bootstrap style for buttons (see bug

It's a false positive. That check doesn't apply to OPAC templates.
Comment 17 Katrin Fischer 2020-02-19 23:16:25 UTC
Thx! I'll try to come back to this one in my next QA session (not mad if someone beats me to it :) )
Comment 18 Katrin Fischer 2020-02-20 07:02:13 UTC
hi Owen, sorry, just realized it was even in the test plan:

    Also note that the QA tools will give a false positive about Bootstrap
    button styles. This rule only applies to the staff client.
Comment 19 Katrin Fischer 2020-02-20 07:07:14 UTC
Created attachment 99296 [details] [review]
Bug 13327: OPACPopupAuthorsSearch doesn't work with XSLT views

This patch reimplements the OPACPopupAuthorsSearch feature so that it
will work in the XSLT view.

To test, apply the patch and rebuild the OPAC CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

 - View the detail page for a bibliographic record in the OPAC.
   Test under the following conditions:

 - OPACPopupAuthorsSearch ENABLED, OPACXSLTDetailsDisplay "default"
 - OPACPopupAuthorsSearch ENABLED, OPACXSLTDetailsDisplay empty
 - OPACPopupAuthorsSearch DISABLED, OPACXSLTDetailsDisplay "default"
 - OPACPopupAuthorsSearch DISABLED, OPACXSLTDetailsDisplay empty

In each of these cases, test the detail page's listing of additional
authors/contributors and subjects. Test records which have both multiple
contributors or subjects and which have only one author or subject.

When OPACPopupAuthorsSearch is turned on, verify that the author/subject
selection modal appears and that your selections are correctly combined
to build a search string.

Note that OPACPopupAuthorsSearch feature has a problem with terms
containing parentheses. The issue predates this patch.

Also note that the QA tools will give a false positive about Bootstrap
button styles. This rule only applies to the staff client.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 20 Katrin Fischer 2020-02-20 07:07:18 UTC
Created attachment 99297 [details] [review]
Bug 13327: (follow-up) Correct CSS and XSL errors

This patch adds back the CSS changes which seem to have been lost in a
rebase somewhere. The patch also corrects a bad copy/paste in the XSL
file which led to a 404 error.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 21 Jonathan Druart 2020-02-20 07:48:21 UTC
I wanted to get back to this.
Comment 22 Jonathan Druart 2020-02-20 08:07:40 UTC
> Note that OPACPopupAuthorsSearch feature has a problem with terms
> containing parentheses. The issue predates this patch.

To avoid that, should not we search using the id instead of the string?
Comment 23 Katrin Fischer 2020-02-20 08:31:33 UTC
(In reply to Jonathan Druart from comment #22)
> > Note that OPACPopupAuthorsSearch feature has a problem with terms
> > containing parentheses. The issue predates this patch.
> 
> To avoid that, should not we search using the id instead of the string?

We could, but we still need to fix it, as not all entries will be linked to authorities - some libraries don't use them at all and there can also be a mix. For example 653 (uncontrolled term) will show as subject but is, if i am not mistaken, usually unlinked (uncontrolled).
Comment 24 Katrin Fischer 2020-02-20 08:32:05 UTC
(In reply to Katrin Fischer from comment #23)
> (In reply to Jonathan Druart from comment #22)
> > > Note that OPACPopupAuthorsSearch feature has a problem with terms
> > > containing parentheses. The issue predates this patch.
> > 
> > To avoid that, should not we search using the id instead of the string?
> 
> We could, but we still need to fix it, as not all entries will be linked to
> authorities - some libraries don't use them at all and there can also be a
> mix. For example 653 (uncontrolled term) will show as subject but is, if i
> am not mistaken, usually unlinked (uncontrolled).

Oh, see bug 24690 - I filed it for follow-ups.
Comment 25 Katrin Fischer 2020-02-20 08:33:06 UTC
Sorry, working on the older bugs by bug number - missed your QA contact (also sometimes it's not reliable :) )
Comment 26 Jonathan Druart 2020-02-21 11:32:53 UTC
(In reply to Katrin Fischer from comment #25)
> Sorry, working on the older bugs by bug number - missed your QA contact
> (also sometimes it's not reliable :) )

No worries I was not, I assigned to me after you PQAed it! :)
Comment 27 Jonathan Druart 2020-02-21 11:34:42 UTC
Owen, sorry but I found a regression.

With OPACPopupAuthorsSearch and NOT OPACXSLTDetailsDisplay
Hit /cgi-bin/koha/opac-detail.pl?biblionumber=15
Without the patch, the link on "Additional authors: Bradbury, Nicola." opens a popup. It does not with your patch.
Comment 28 Jonathan Druart 2020-02-28 15:13:26 UTC
Created attachment 99770 [details] [review]
Bug 13327: OPACPopupAuthorsSearch doesn't work with XSLT views

This patch reimplements the OPACPopupAuthorsSearch feature so that it
will work in the XSLT view.

To test, apply the patch and rebuild the OPAC CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

 - View the detail page for a bibliographic record in the OPAC.
   Test under the following conditions:

 - OPACPopupAuthorsSearch ENABLED, OPACXSLTDetailsDisplay "default"
 - OPACPopupAuthorsSearch ENABLED, OPACXSLTDetailsDisplay empty
 - OPACPopupAuthorsSearch DISABLED, OPACXSLTDetailsDisplay "default"
 - OPACPopupAuthorsSearch DISABLED, OPACXSLTDetailsDisplay empty

In each of these cases, test the detail page's listing of additional
authors/contributors and subjects. Test records which have both multiple
contributors or subjects and which have only one author or subject.

When OPACPopupAuthorsSearch is turned on, verify that the author/subject
selection modal appears and that your selections are correctly combined
to build a search string.

Note that OPACPopupAuthorsSearch feature has a problem with terms
containing parentheses. The issue predates this patch.

Also note that the QA tools will give a false positive about Bootstrap
button styles. This rule only applies to the staff client.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 29 Jonathan Druart 2020-02-28 15:13:30 UTC
Created attachment 99771 [details] [review]
Bug 13327: (follow-up) Correct CSS and XSL errors

This patch adds back the CSS changes which seem to have been lost in a
rebase somewhere. The patch also corrects a bad copy/paste in the XSL
file which led to a 404 error.

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 30 Jonathan Druart 2020-02-28 15:14:23 UTC
(In reply to Jonathan Druart from comment #27)
> Owen, sorry but I found a regression.
> 
> With OPACPopupAuthorsSearch and NOT OPACXSLTDetailsDisplay
> Hit /cgi-bin/koha/opac-detail.pl?biblionumber=15
> Without the patch, the link on "Additional authors: Bradbury, Nicola." opens
> a popup. It does not with your patch.

Saw with Owen on IRC. There was only 1 checkbox on the popup, so assuming it's a bug fix and not a regression.
Comment 31 Martin Renvoize 2020-03-02 09:28:24 UTC
Thanks guys, really pleased to see this one having made it through :)
Comment 32 Martin Renvoize 2020-03-02 09:47:08 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 33 Joy Nelson 2020-03-31 22:42:45 UTC
enhancement not backported to 19.11.x