The OPACPopupAuthorsSearch feature does only work with the normal view.
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.
Created attachment 96919 [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 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. Test records which have both multiple contributors and which have only one additional author. When OPACPopupAuthorsSearch is turned on, verify that the author 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.
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.
Thanks for testing, David! I'll take a look.
Created attachment 98885 [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 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.
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.
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.
(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!
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>
I get a 404 Request URL:http://catalogue.kohadev.org/opac-tmpl/bootstrap/images/z.png
(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
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).
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.
(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?
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
(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.
Thx! I'll try to come back to this one in my next QA session (not mad if someone beats me to it :) )
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.
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>
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>
I wanted to get back to this.
> 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?
(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).
(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.
Sorry, working on the older bugs by bug number - missed your QA contact (also sometimes it's not reliable :) )
(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! :)
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.
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>
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>
(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.
Thanks guys, really pleased to see this one having made it through :)
Nice work everyone! Pushed to master for 20.05
enhancement not backported to 19.11.x