[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 #15 from Jonathan Druart <jonathan.druart+koha at gmail.com> ---
(In reply to Marcel de Rooy from comment #14)
> (In reply to Jonathan Druart from comment #13)
> > > Should this be a change done by the store sub of Library instead?
> > 
> > No, that's controller job.
> 
> Well, that is a clear expression but not an answer ;)
> Why not?
> 
> Have a look at Koha::Object->store and look whats happening there for
> instance?
> Or what are we doing in Koha::Patron->store?
> Probably the list could be longer..

Because they have a TEXT datatype, that can be NULL OR "".
IMO we should not force NULL, a text can be "". However here, the controller
assumes that if the user didn't fill in the inputs then the values are
undef/NULL.

pickup_location is a boolean and store must explode if we are passing undef.
Either it's not passed and it fallback to the default value in DB, or it's a
boolean, or it's an invalid value and we expect the insert to fail.

What we are doing in some of the modules is wrong, because of legacy code that
has been moved.

>From Koha::Patron->store:
 205             $self->relationship(undef) # We do not want to store an empty
string in this field
 206               if defined $self->relationship
 207                      and $self->relationship eq "";
Typically this is wrong :)


 215                 # Add expiration date if it isn't already there
 216                 unless ( $self->dateexpiry ) {
 217                     $self->dateexpiry( $self->category->get_expiry_date );
 218                 }
This is correct, if the value does not exist we calculate it.

Do you agree with that?

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