[Koha-bugs] [Bug 28567] Pick-up location is not saved correctly when creating a new library

[bugzilla-daemon at bugs.koha-community.org]

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

--- Comment #16 from Marcel de Rooy <m.de.rooy at rijksmuseum.nl> ---
This kind of thing in the template is confusing: [% IF !library ||
library.pickup_location == 1 %]
We should take care of such a thing elsewhere. This is 'hidden business logic'.
The html select does not make things easier in this case.
If our "business logic" or "data model" says that pickup location defaults to
True, I would say make that happen (otherwise NULL simply means zero or false).
Perhaps via Object->new already? We can't leave this to store here, since the
html select already forces us to make a decision.

TEXT fields which are set back to NULL, often generate lots of uninitialized
warnings all over. So I would not mind setting them to empty string.
Doing the opposite in this dbrev is arguable.

The principal discussion here exceeds the scope of this report. But It is not
clear where the division is between controller (caller script) and controlling
object module. What should we do in e.g. new and store, and what not?
In this case I would opt for the Koha::Objects instead of copying the same code
in controller scripts.

Object:new
        # Remove the arguments which exist, are not defined but NOT NULL to use
the default value
        my $columns_info = $schema->resultset( $class->_type
)->result_source->columns_info;
        for my $column_name ( keys %$attributes ) {
            my $c_info = $columns_info->{$column_name};
            next if $c_info->{is_nullable};
            next if not exists $attributes->{$column_name} or defined
$attributes->{$column_name};
            delete $attributes->{$column_name};
        }
Why not set $attributes->{$column_name} = $c_info->{default_value} if defined
and delete only if not?

Object:store
                } else {
                    # If cannot be null, get the default value
                    # What if cannot be null and does not have a default value?
Possible?
                    $self->_result()->set_column($col =>
$columns_info->{$col}->{default_value});
Note: I saw a lot of date fields NOT NULL but without a SQL default.

-- 
You are receiving this mail because:
You are watching all bug changes.