https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20644
--- Comment #13 from Nick Clemens (kidclamp) <n...@bywatersolutions.com> ---
(In reply to Matthias Le Gac from comment #12)
>
> But for the point where in the tests it's marked NULL and not "inherit" I
> don't know what you're talking about, because for me it's already marked
> "inherit" as requested.
t/db_dependent/Koha/ItemTypes.t:
180 'Two item types with \'checkprevcheckout\' set to \'undef\' have been
added'
It's just the comment there
>
> For the "should fix" point, could you be more precise? I don't know which
> part you're talking about.
When editing an itemtype, it the system preference is 'soft' the options are:
Yes and try to override system preference
No and try to override system preference
Inherit from system preference
From what I understand it does not 'try' to override, it always does
I think we should also display the system preference when set to 'Hard'
otherwise this can be a somewhat hidden feature
> The same goes for the "would fix" part. Could you be more specific about how
> to improve test comprehension?
For me, 4 nested 'map' calls is a bit obfuscated. Clearer would be something
like:
foreach my $syspref_setting ('hardno','hardyes','softno','softyes'){
foreach my $itemtype_setting( 'inherit','no','yes'){
...
The output from your tests is clear, the matrix in the comments is clear, if
large, it's a matter of preference but not a blocker
>
> For the "could fix" part, I've tried to change using the builder, but with
> the builder you can't create an "ItemType" with the value
> "checkprevcheckout" at "undef" because this field can't be empty, so the
> builder throws an error.
> I tried this:
> my $itemType1 = $builder->build_object({
> class => 'Koha::ItemTypes',
> value => {
> checkprevcheckout => undef,
> }
> });
> and I get this error:
> "Null value for checkprevcheckout in Itemtype not allowed at
> /inlibro/git/koha-master-dev-matthias/t/lib/TestBuilder.pm line 430."
> Tell me if there is another solution with the builder or if we leave it as
> it is now?
t/db_dependent/Patron/Borrower_PrevCheckout.t
49 itemtype => 'YESIT',
59 itemtype => 'NOIT',
69 itemtype => 'INHERITIT'
I just meant you don't need to specify the codes here, you use them later, but
could save the generated codes as variables in the hash and reference them -
this just ensures the tests won't have a conflict with existing data - rare,
but slightly possible if tests die and rollback fails, etc. Again, not a
blocker
> Finally, for the last point about the script that adds the column, there's
> already something that tests whether the column exists, isn't there? with :
> "ALTER TABLE itemtypes ADD IF NOT EXISTS checkprevcheckout"
> Does "ADD IF NOT EXISTS" do this?
It does, but it is inconsistent with how we handle this in Koha:
if( !column_exists( 'borrowers', 'middle_name' ) ) {
I am not sure if your construction was blocked for being a MySQL-ism, but I on
second glance it seems we should stick with the established pattern
git grep column_exists
git grep "ADD IF NOT EXISTS"
--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
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/
