https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28567
--- Comment #15 from Jonathan Druart <[email protected]> --- (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. _______________________________________________ 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/
