For building different UI elements based on whether the patron is logged or not in the OPAC, it would be handy to add such information to the variables that are added to the MARCXML so the XSLT can take advantage of it.
Created attachment 104533 [details] [review] Bug 25416: Sample usage for testing purposes [DO NOT PUSH]
Created attachment 104534 [details] [review] Bug 25416: Let OPAC XSLTs know if the context is an anonymous session This patch makes use of the 'variables' parameter in XSLTParse4Display method in the different places that it is used in the OPAC. It does by passing this parameter with anonymous_session => 1|0 The value will depend on the output from get_template_and_user (i.e. if there's a returned borrowernumber). A special case takes place in search results, as the call to XSLTParse4Display happens in C4::Search::searchResults. So a new parameter 'xslt_variables' is added to it. To test: 1. Apply the [DO NOT PUSH] patch 2. Open the OPAC in your browser 3. Try detail pages, search results, tags and shelves pages with or without an active session => FAIL: It always says (somewhere) 'Anonymous session: Yes' 4. Apply this patch, restart_all 5. Repeat 3 => SUCCESS: It will tell the Yes/No correctly regarding anonymous sessions! 6. Sign off :-D Sponsored-by: Universidad ORT Uruguay
I've been curious about this one as I have been wondering about a similar feature that would require some configuration information to the the XSLT. We have some similar features for passing XSLT variables (system preferences, authorised value descriptions) to MARCXML in XSLT.pm, but this patch doesn't touch the file. Is there a reason? Also missing tests :)
(In reply to Katrin Fischer from comment #3) > I've been curious about this one as I have been wondering about a similar > feature that would require some configuration information to the the XSLT. > We have some similar features for passing XSLT variables (system > preferences, authorised value descriptions) to MARCXML in XSLT.pm, but this > patch doesn't touch the file. Is there a reason? Also missing tests :) This uses a feature already present (that $variables parameter in XSLTTransform...
(In reply to Katrin Fischer from comment #3) > I've been curious about this one as I have been wondering about a similar > feature that would require some configuration information to the the XSLT. > We have some similar features for passing XSLT variables (system > preferences, authorised value descriptions) to MARCXML in XSLT.pm, but this > patch doesn't touch the file. Is there a reason? Also missing tests :) It is totally related to injecting sysprefs, as it happens in the same method. It is just an extra variable Julian seems to have added a while ago, and this patch is just making use of it.
(In reply to Katrin Fischer from comment #3) > I've been curious about this one as I have been wondering about a similar > feature that would require some configuration information to the the XSLT. > We have some similar features for passing XSLT variables (system > preferences, authorised value descriptions) to MARCXML in XSLT.pm, but this > patch doesn't touch the file. Is there a reason? Also missing tests :) I will add tests. I wanted your opinion on the feature first, as the tests are the most difficult bit here. Specially on this area
Created attachment 104689 [details] [review] Bug 25416: Unit tests This patch adds tests for the new 'feature': i.e. searchResults passes the added $xslt_variables to the XSLTParse4Display method. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(In reply to Katrin Fischer from comment #3) > Also missing tests :) Done :-D
Created attachment 104694 [details] [review] Bug 24458: Regression tests This patch introduces tests for searchResults. The idea is to set a subfield as hidden in OPAC and another on the staff interface, and see if searchResults honours this on the XSLTResultsRecord contents. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Search.t => FAIL: This is not really implemented Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 104695 [details] [review] Bug 24458: Use the ViewPolicy filter on search results This patch implements the use of the ViewPolicy record processor filter inside C4::Search::searchResults. The idea is that the $record_processor is instantiated once and reused inside the loop. This leaves options for further optimizations I will do on a follow-up bug. The filter is applied to the MARC data before it is passed to the XSLT processor. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/Search.t => FAIL: This is not implemented, tests fail 3. Apply this patch 4. Repeat 2 5. Feel the joy in your body from a long standing bug being solved 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 104718 [details] [review] Bug 25416: Sample usage for testing purposes [DO NOT PUSH] Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 104719 [details] [review] Bug 25416: Let OPAC XSLTs know if the context is an anonymous session This patch makes use of the 'variables' parameter in XSLTParse4Display method in the different places that it is used in the OPAC. It does by passing this parameter with anonymous_session => 1|0 The value will depend on the output from get_template_and_user (i.e. if there's a returned borrowernumber). A special case takes place in search results, as the call to XSLTParse4Display happens in C4::Search::searchResults. So a new parameter 'xslt_variables' is added to it. To test: 1. Apply the [DO NOT PUSH] patch 2. Open the OPAC in your browser 3. Try detail pages, search results, tags and lists/shelves pages with or without an active session => FAIL: It always says (somewhere) 'Anonymous session: Yes' 4. Apply this patch, restart_all 5. Repeat 3 => SUCCESS: It will tell the Yes/No correctly regarding anonymous sessions! 6. Sign off :-D Sponsored-by: Universidad ORT Uruguay Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 104720 [details] [review] Bug 25416: Unit tests This patch adds tests for the new 'feature': i.e. searchResults passes the added $xslt_variables to the XSLTParse4Display method. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 104764 [details] [review] Bug 25416: (follow-up) Remove useless use statement Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 104767 [details] [review] Bug 25416: Sample usage for testing purposes [DO NOT PUSH] Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> https://bugs.koha-community.org/show_bug.cgi?id=24516
Created attachment 104768 [details] [review] Bug 25416: Let OPAC XSLTs know if the context is an anonymous session This patch makes use of the 'variables' parameter in XSLTParse4Display method in the different places that it is used in the OPAC. It does by passing this parameter with anonymous_session => 1|0 The value will depend on the output from get_template_and_user (i.e. if there's a returned borrowernumber). A special case takes place in search results, as the call to XSLTParse4Display happens in C4::Search::searchResults. So a new parameter 'xslt_variables' is added to it. To test: 1. Apply the [DO NOT PUSH] patch 2. Open the OPAC in your browser 3. Try detail pages, search results, tags and lists/shelves pages with or without an active session => FAIL: It always says (somewhere) 'Anonymous session: Yes' 4. Apply this patch, restart_all 5. Repeat 3 => SUCCESS: It will tell the Yes/No correctly regarding anonymous sessions! 6. Sign off :-D Sponsored-by: Universidad ORT Uruguay Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> https://bugs.koha-community.org/show_bug.cgi?id=24516
Created attachment 104769 [details] [review] Bug 25416: Unit tests This patch adds tests for the new 'feature': i.e. searchResults passes the added $xslt_variables to the XSLTParse4Display method. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> https://bugs.koha-community.org/show_bug.cgi?id=24516
Created attachment 104770 [details] [review] Bug 25416: (follow-up) Remove useless use statement Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> https://bugs.koha-community.org/show_bug.cgi?id=24516
the others were faster :)
Nice work everyone! Pushed to master for 20.05
enhancement not backported to 19.11.x
(In reply to Joy Nelson from comment #21) > enhancement not backported to 19.11.x Why not? :-D
Created attachment 105160 [details] [review] [19.11.x] Bug 25416: Let OPAC XSLTs know if the context is an anonymous session This patch makes use of the 'variables' parameter in XSLTParse4Display method in the different places that it is used in the OPAC. It does by passing this parameter with anonymous_session => 1|0 The value will depend on the output from get_template_and_user (i.e. if there's a returned borrowernumber). A special case takes place in search results, as the call to XSLTParse4Display happens in C4::Search::searchResults. So a new parameter 'xslt_variables' is added to it. To test: 1. Apply the [DO NOT PUSH] patch 2. Open the OPAC in your browser 3. Try detail pages, search results, tags and lists/shelves pages with or without an active session => FAIL: It always says (somewhere) 'Anonymous session: Yes' 4. Apply this patch, restart_all 5. Repeat 3 => SUCCESS: It will tell the Yes/No correctly regarding anonymous sessions! 6. Sign off :-D Sponsored-by: Universidad ORT Uruguay Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 105161 [details] [review] [19.11.x] Bug 25416: Unit tests This patch adds tests for the new 'feature': i.e. searchResults passes the added $xslt_variables to the XSLTParse4Display method. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(In reply to Joy Nelson from comment #21) > enhancement not backported to 19.11.x Just in case you would reconsider it, I submitted the 19.11 version, which is just adjusting the name of the XSLT class (Koha::XSLT_handler vs Koha::XSLT::Base) and the tests count (removal of QueryParser in master).
Pushed to 19.11.x for 19.11.06
Enhancement not backported to 19.05.x (oldoldstable) It could eventually be backported if there is demand. Or if in the future it would help another patch to be backported. Because of this patch doing various change scattered in a file, not having it could cause conflicts. But until then, leaning on the conservative side by default for oldoldstable.