https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18421

--- Comment #11 from Katrin Fischer <[email protected]> ---
Comment on attachment 73401
  --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73401
Bug18421 - Add Coce to the staff intranet

Review of attachment 73401:
 --> 
(https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=18421&attachment=73401)
-----------------------------------------------------------------

Hi,

I've added comments to the splinter review.

The database update doesn't look clean to me, please revise and take a look
here:
https://wiki.koha-community.org/wiki/Database_updates

::: C4/Auth.pm
@@ +504,5 @@
> +            useDischarge                                                     
>           => C4::Context->preference('useDischarge'),
> +            KOHA_VERSION                                                     
>           => C4::Context->preference('Version'),
> +            CoceIntranet                                                     
>           => C4::Context->preference('CoceIntranet'),
> +            CoceProviders                                                    
>           => C4::Context->preference('CoceProviders'),
> +            CoceHost                                                         
>           => C4::Context->preference('CoceHost'),

Please use the TT plugin for dealing with the preferences in the templates
instead, like it's already done for the OPAC.

::: installer/data/mysql/atomicupdate/bug_18421_add_coce_intranet.perl
@@ +1,4 @@
> +#! /usr/bin/perl
> +
> +use strict;
> +use warnings;

Please always use Modern::Perl instead.

@@ +2,5 @@
> +
> +use strict;
> +use warnings;
> +use C4::Context;
> +use Data::Dumper;

Should not be here.

@@ +20,5 @@
> +    $current_coce_pref = $rows->{Coce}->{value};
> +}
> +
> +# add two new systempreferences in order to have distinct behavior between 
> intranet and OPAC
> +$dbh->do("INSERT INTO systempreferences 
> (variable,value,options,explanation,type) VALUES

Please always use INSERT IGNORE when adding system preferences.

@@ +26,5 @@
> +        ('CoceOPAC','$current_coce_pref', NULL, 'If on, enables cover 
> retrieval from the configured Coce server in the OPAC', 'YesNo')
> +        ;") or die "Impossible d\'executer cam5446_ajouter_coce_intranet: 
> erreur lors de l'ajout des nouvelles prefs: ". $dbh->errstr . "\n";
> +$dbh->do("DELETE FROM systempreferences WHERE variable = 'Coce';")
> +    or die "Impossible d\'executer cam5446_ajouter_coce_intranet: erreur 
> lors de la suppression de la préférence 'Coce': ". $dbh->errstr . "\n";
> +print "cam5446: Add Coce image cache to the staff intranet.\n";

Please use bugzilla bug numbers in your update message.

::: installer/data/mysql/sysprefs.sql
@@ +108,5 @@
>  
> ('CircControl','ItemHomeLibrary','PickupLibrary|PatronLibrary|ItemHomeLibrary','Specify
>  the agency that controls the circulation and fines policy','Choice'),
>  ('CircSidebar','0',NULL,'Activate or deactivate the navigation sidebar on 
> all Circulation pages','YesNo'),
>  ('ClaimsBccCopy','0','','Bcc the ClaimAcquisition and ClaimIssues 
> alerts','YesNo'),
> +('CoceIntranet','0', NULL, 'If on, enables cover retrieval from the 
> configured Coce server in the staff intranet', 'YesNo'),
> +('CoceOPAC','0', NULL, 'If on, enables cover retrieval from the configured 
> Coce server in the OPAC', 'YesNo'),

Please always sort prefs alphabetically in this file.
Also to be consistent with other prefs, it would be good to switch that:
OpacCoce, IntranetCoce (see other prefs for examples: *AmazonCoverImages,
*NumbersPreferPhrase, ..)

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to