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

--- Comment #6 from Jonathan Druart <[email protected]> ---
(In reply to David Cook from comment #5)
> Comment on attachment 194409 [details] [review]
> Bug 41834: Set systempreferences's options, explanation and type to NULL
> 
> Review of attachment 194409 [details] [review]:
> -----------------------------------------------------------------
> 
> Unfortunately, I've run out of time today, but I'll take another look at
> this one next week, as I'm pretty excited about it.
> 
> ::: C4/Installer.pm
> @@ +460,5 @@
> >      # marc_cleaned finds the marcflavour, without the variant.
> >      my $marc_cleaned = 'MARC21';
> >      $marc_cleaned = 'UNIMARC' if $marcflavour =~ /unimarc/i;
> > +    my $request = $self->{'dbh'}
> > +        ->prepare("INSERT IGNORE INTO `systempreferences` (variable, 
> > value) VALUES('marcflavour', '$marc_cleaned')");
> 
> At some point, someone should make this use a prepared statement, but all
> good for now.
> 
> ::: installer/data/mysql/atomicupdate/bug_41834.pl
> @@ +8,5 @@
> > +    up          => sub {
> > +        my ($args) = @_;
> > +        my ( $dbh, $out ) = @$args{qw(dbh out)};
> > +
> > +        # First fix some discrepancies
> 
> Hmm I don't know that we should be fixing discrepancies here?

I've embedded what I've done on
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192826 and that was
still valid after the deletion of the 3 rows.
We can move them to a dedicate bug report if you insist.
IIRC the tests (check_sysprefs.t) fail if we don't.

> @@ +36,5 @@
> > +            DELETE FROM systempreferences WHERE 
> > variable='OPACMySummaryNote'
> > +        }
> > +        );
> > +
> > +        # Then remove NULL the 3 columns for sysprefs listed in 
> > sysprefs.sql
> 
> Hmm that's an interesting point. I suppose local sysprefs will still need to
> retain this data in the database... and there's no clear separation of local
> and built-in sysprefs...

Exactly. We should add a "is_system" flag to this table but that's for another
bug: it's not mandatory as we can guess it reading the yml files.

> :::
> koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
> @@ +696,3 @@
> >                multiple:
> >                  intransit: In-transit
> >                  checkedout: Checked out
> 
> Are all the circulation.pref changes just tidy-ups unrelated to the overall
> bug here?

It's related, the code in get_all_from_yml does not build the resulting
structure if the yml contains incorrect/empty entries

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
https://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