Bug 5979 - Add separate OPACISBD system preference
Summary: Add separate OPACISBD system preference
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P3 enhancement (vote)
Assignee: Alex Arnaud
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks: 5584
  Show dependency treegraph
 
Reported: 2011-03-28 21:02 UTC by Chris Cormack
Modified: 2016-12-05 21:27 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Proposed Patch (1.49 KB, patch)
2011-03-28 21:08 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 5979 - Follow up: using OPACISBD systempreference to display link (2.99 KB, patch)
2012-06-09 14:21 UTC, Alex Arnaud
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 5979 : OPAC : using OPACISBD systempreference to display link (1.55 KB, patch)
2012-06-17 06:06 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 5979 - Follow up: using OPACISBD systempreference to display link (3.24 KB, patch)
2012-06-17 06:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 5979 - Follow up: using OPACISBD systempreference to display link (10.23 KB, patch)
2015-10-28 14:23 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 5979 - OPAC ISBD view has its own syspref (2.96 KB, patch)
2015-10-28 14:50 UTC, Alex Arnaud
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 5979 : OPAC : using OPACISBD systempreference to display link (1.74 KB, patch)
2015-10-30 14:37 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 5979 - Follow up: using OPACISBD systempreference to display link (10.30 KB, patch)
2015-10-30 14:37 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 5979 - OPAC ISBD view has its own syspref (3.11 KB, patch)
2015-10-30 14:37 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 5979: follow to fix spelling (1.28 KB, patch)
2015-10-30 14:40 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 5979 - Follow up: Change ISBD syspref description and fix warning in t/Biblio/Isbd.t (1.63 KB, patch)
2016-02-22 13:46 UTC, Alex Arnaud
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 5979: follow to fix spelling (1.39 KB, patch)
2016-04-04 18:34 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 5979 - Follow up: Change ISBD syspref description and fix warning in t/Biblio/Isbd.t (1.74 KB, patch)
2016-04-04 18:34 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 5979 : OPAC : using OPACISBD systempreference to display link (1.80 KB, patch)
2016-04-05 08:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5979 - Follow up: using OPACISBD systempreference to display link (10.37 KB, patch)
2016-04-05 08:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5979 - OPAC ISBD view has its own syspref (3.17 KB, patch)
2016-04-05 08:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5979: follow to fix spelling (1.41 KB, patch)
2016-04-05 08:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Attachment to Bug 5979 - Add separate OPACISBD system preference (1.76 KB, patch)
2016-04-05 08:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5979: Update INSERT IGNORE INTO statement (1.19 KB, patch)
2016-04-05 08:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 5979: Follow up on systempreferences.sql (17.40 KB, patch)
2016-04-23 02:28 UTC, Mark Tompsett
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2011-03-28 21:02:42 UTC
The OPACISBD system preference is used in order to be able to have one ISBD for OPAC another one for intranet.
But the link ISBD at OPAC was depending on the presence or absence of value in ISBD which is for intranet.
This patch fixes the behaviour to depend on the correct system preference at OPAC
Comment 1 Chris Cormack 2011-03-28 21:08:06 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2011-03-29 15:17:07 UTC
This patch alters the scripts but lacks any change to preferences or the database.
Comment 3 MJ Ray (software.coop) 2011-04-01 09:43:10 UTC
This bug is mentioned in:
Bug 5979 : OPAC : using OPACISBD	systempreference to display link http://lists.koha-community.org/pipermail/koha-patches/2011-March/014296.html
Comment 4 Paul Poulain 2011-05-23 12:42:04 UTC
The idea behind this patch is correct: there are some problems with isbd when you do some advanced things, as links are supposed to be replaced on the fly depending on you're on opac or staff.
Also, one may want to have a different ISBD on staff and on opac.

This patch is quite incomplete though. Affecting it to hdl again
Comment 5 Alex Arnaud 2012-06-09 14:21:24 UTC Comment hidden (obsolete)
Comment 6 Alex Arnaud 2012-06-09 14:22:19 UTC
This follow up adds the OPACISBD syspref
Comment 7 Katrin Fischer 2012-06-17 06:06:58 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2012-06-17 06:07:15 UTC Comment hidden (obsolete)
Comment 9 Ian Walls 2012-06-21 18:43:26 UTC
OPACISBD exists in ru-RU and uk-UA language sysprefs only... very odd.  In that context, it's a text area, rather than a boolean.  It looks like this perference was added back around 3.00.05.001, but is not longer part of the default installation.

The first patch is fine; it changes the logic to use OPACISBD in the opac.  But getting that preference properly installed where it may not already be is something else.  If OPACISBD syspref is not present in newer installs, we should create one that is congruent with the original intent.  A YesNo type is not appropriate in this context

Failed QA.
Comment 10 Fridolin Somers 2012-11-26 14:09:17 UTC
Seems like OPACISBD was a syspref to configure an OPAC specific ISBD view, that's why it is a textarea.

Why not using 2 syspref in OPAC like in intranet : 
ISBD and OPACISBD : ISBD config for intranet/OPAC
viewISBD and OPACviewISBD : allow ISBD view in intranet/OPAC
Comment 11 Mark Tompsett 2015-10-19 22:52:15 UTC
Can someone clarify the point of this bug? I'm not sure I understand, and it has been sitting Failed QA for nearly 3 years.
Comment 12 Fridolin Somers 2015-10-20 07:50:41 UTC
Is the ISBD view even used today ?
Since the XSLT are largely used for a customised view.
Comment 13 Zeno Tajoli 2015-10-20 10:13:26 UTC
I confirm that ISBD view (opac and intranet) is wide used here in Italy.
The main user are librarians.
Comment 14 Alex Arnaud 2015-10-28 14:23:06 UTC Comment hidden (obsolete)
Comment 15 Alex Arnaud 2015-10-28 14:50:39 UTC Comment hidden (obsolete)
Comment 16 Alex Arnaud 2015-10-28 15:03:00 UTC
Syspref OPACISBD is now a textarea so it's possible to have a different template in staff and opac.

I didn't add a "OPACviewISBD", empty OPACISBD is enough for disable ISBD view in opac.
Comment 17 Bernardo Gonzalez Kriegel 2015-10-30 14:37:30 UTC Comment hidden (obsolete)
Comment 18 Bernardo Gonzalez Kriegel 2015-10-30 14:37:38 UTC Comment hidden (obsolete)
Comment 19 Bernardo Gonzalez Kriegel 2015-10-30 14:37:47 UTC Comment hidden (obsolete)
Comment 20 Bernardo Gonzalez Kriegel 2015-10-30 14:40:18 UTC Comment hidden (obsolete)
Comment 21 Katrin Fischer 2015-11-16 22:13:13 UTC
1) Please copy the current ISBD into OPACISBD to keep the same behaviour as before the patch for an installation with a modified ISBD view.

2) Please amend the ISBD preference description to include "staff", so it's more clear about the new difference.

3) The tests output some warnings, please take a look to see if they can be fixed:
ok 1 - use C4::Biblio;
Use of uninitialized value $template in string eq at /home/katrin/kohaclone/C4/Biblio.pm line 926.
Use of uninitialized value $template in string eq at /home/katrin/kohaclone/C4/Biblio.pm line 998.
Use of uninitialized value $template in string eq at /home/katrin/kohaclone/C4/Biblio.pm line 1010.
Use of uninitialized value $template in string eq at /home/katrin/kohaclone/C4/Biblio.pm line 998.
Use of uninitialized value $template in string eq at /home/katrin/kohaclone/C4/Biblio.pm line 1010.
Comment 22 Katrin Fischer 2015-11-16 22:15:29 UTC
Hm, also a question: if we have separate preferences, do we really still need 4 preferences? Could we have only 2 preferences and deactivate the feature by emptying them out?
Comment 23 Alex Arnaud 2015-11-17 08:24:14 UTC
(In reply to Katrin Fischer from comment #22)
> Hm, also a question: if we have separate preferences, do we really still
> need 4 preferences? Could we have only 2 preferences and deactivate the
> feature by emptying them out?

Hello Katrin,

We have 3 preferences. 2 for staff (ISBD, viewISBD) and 1 for OPAC (OPACISBD). I agree that we don't need 2 preferences for staff as we do the same job with 1 on opac.
Comment 24 Katrin Fischer 2015-11-17 14:19:29 UTC
Hi Alex, you are right, sorry - I think I misread last night. I think deduplicating the 2 staff preferences would be good, but on a separate bug :)
Comment 25 Alex Arnaud 2016-02-22 09:14:40 UTC
Does anyone known why this bug failed QA ?
Comment 26 Katrin Fischer 2016-02-22 09:21:44 UTC
comment#21
Comment 27 Alex Arnaud 2016-02-22 13:18:10 UTC
(In reply to Katrin Fischer from comment #21)
> 1) Please copy the current ISBD into OPACISBD to keep the same behaviour as
> before the patch for an installation with a modified ISBD view.
Katrin, i think this is done in installer/data/mysql/atomicupdate/bug_5979-add_opacisbd_syspref.sql
Are you ok with that ?
Comment 28 Katrin Fischer 2016-02-22 13:20:10 UTC
Looks ok!
Comment 29 Alex Arnaud 2016-02-22 13:46:16 UTC Comment hidden (obsolete)
Comment 30 Mark Tompsett 2016-03-18 19:16:12 UTC
Comment on attachment 44210 [details] [review]
[SIGNED-OFF] Bug 5979 - OPAC ISBD view has its own syspref

Review of attachment 44210 [details] [review]:
-----------------------------------------------------------------

::: t/Biblio/Isbd.t
@@ +26,5 @@
> +        use_ok('C4::Biblio');
> +}
> +
> +my $dbh = C4::Context->dbh;
> +# Start transaction

Perhaps this is better suited for t/db_dependent?
Comment 31 Mark Tompsett 2016-03-18 19:18:22 UTC
I was thinking perhaps INSERT IGNORE INTO ... not just
INSERT INTO `systempreferences` (variable,value,options,explanation,type)  SELECT 'OPACISBD', value, '70|10', 'Allows to define ISBD view in OPAC', 'Textarea' FROM `systempreferences` WHERE variable = 'ISBD';
Comment 32 Owen Leonard 2016-04-04 18:34:46 UTC Comment hidden (obsolete)
Comment 33 Owen Leonard 2016-04-04 18:34:58 UTC Comment hidden (obsolete)
Comment 34 Jonathan Druart 2016-04-05 08:55:55 UTC
Created attachment 49898 [details] [review]
Bug 5979 : OPAC : using OPACISBD systempreference to display link

The OPACISBD system preference is used in order to be able to have one ISBD for OPAC another one for intranet.
But the link ISBD at OPAC was depending on the presence or absence of value in ISBD which is for intranet.
This patch fixes the behaviour to depend on the correct system preference at OPAC

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Verified that new system preference is added cleanly and
ISBD view can be configured independently for staff and OPAC
using OPACISBD and viewISBD system preferences.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 35 Jonathan Druart 2016-04-05 08:56:00 UTC
Created attachment 49899 [details] [review]
Bug 5979 - Follow up: using OPACISBD systempreference to display link

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 36 Jonathan Druart 2016-04-05 08:56:04 UTC
Created attachment 49900 [details] [review]
Bug 5979 - OPAC ISBD view has its own syspref

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Works well, empty OPACISBD removes link.
Test pass.
Small koha-qa error fixed in followup

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 37 Jonathan Druart 2016-04-05 08:56:09 UTC
Created attachment 49901 [details] [review]
Bug 5979: follow to fix spelling

Changed Montains -> Mountains

koha-qa complains!!

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 38 Jonathan Druart 2016-04-05 08:56:14 UTC
Created attachment 49902 [details] [review]
Attachment to Bug 5979 - Add separate OPACISBD system preference

Bug 5979 - Follow up: Change ISBD syspref description and fix warning in t/Biblio/Isbd.t

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 39 Jonathan Druart 2016-04-05 08:56:19 UTC
Created attachment 49903 [details] [review]
Bug 5979: Update INSERT IGNORE INTO statement

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 40 Katrin Fischer 2016-04-05 13:35:17 UTC
Maybe this should be an enh?
Comment 41 Mark Tompsett 2016-04-05 14:03:47 UTC
(In reply to Katrin Fischer from comment #40)
> Maybe this should be an enh?

I agree, because it adds a new system preference.
Comment 42 Brendan Gallagher 2016-04-22 03:21:02 UTC
Pushed to Master - Should be in the May 2016 Release.  Thanks!
Comment 43 Julian Maurice 2016-04-22 09:28:08 UTC
(In reply to M. Tompsett from comment #41)
> (In reply to Katrin Fischer from comment #40)
> > Maybe this should be an enh?
> 
> I agree, because it adds a new system preference.

So be it
Comment 44 Brendan Gallagher 2016-04-22 23:50:38 UTC
ERROR 1064 (42000) at line 1: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '('OPACISBD','#100||{ 100a }{ 100b }{ 100c }{ 100d }{ 110a }{ 110b }{ 110c }{ 110' at line 536
Comment 45 Brendan Gallagher 2016-04-23 01:41:31 UTC
(In reply to Brendan Gallagher from comment #44)
> ERROR 1064 (42000) at line 1: You have an error in your SQL syntax; check
> the manual that corresponds to your MySQL server version for the right
> syntax to use near '('OPACISBD','#100||{ 100a }{ 100b }{ 100c }{ 100d }{
> 110a }{ 110b }{ 110c }{ 110' at line 536

That's doing a fresh install... If anyone spots the problem before I do - please let me know - This is a candidate to get reverted...  

Thanks,
Brendan
Comment 46 Mark Tompsett 2016-04-23 02:28:47 UTC
Created attachment 50591 [details] [review]
Bug 5979: Follow up on systempreferences.sql

Missing comma, extra comma, and bad placement
Comment 47 Brendan Gallagher 2016-04-23 02:35:43 UTC
Pushed last patch from Mtompsett (thanks!)