[Koha-bugs] [Bug 28031] Koha::Patron::Attribute->_check_repeatable doesn't exclude the object's ID

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

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

--- Comment #15 from Tomás Cohen Arazi <tomascohen at gmail.com> ---
(In reply to Jonathan Druart from comment #14)
> 1. 
> Patches applied on top of the whole tree
> 
>     #   Failed test 'globally mandatory attributes tests'
>     #   at t/db_dependent/Koha/Patron.t line 661.
>     # Looks like you failed 1 test of 15.
> t/db_dependent/Koha/Patron.t .. 7/7 
> #   Failed test 'extended_attributes'
> #   at t/db_dependent/Koha/Patron.t line 664.
> Can't locate object method "value" via package
> "Koha::Exceptions::Patron::MissingMandatoryExtendedAttribute" at
> t/db_dependent/Koha/Patron.t line 650.

Will check now.

> 2.
> Why Koha::Patron::Attribute->type does not look like
> 
>   return Koha::Patron::Attribute::Type->_new_from_dbic( $self->_result->code
> );

I didn't intend to change the method inline, I was tempted to, though. I
thought of doing it on a separate bug, also adding the 'type' relationship so
it can be followed through on embedding (API).

> 3. Additionally I don't agree with the "cache", simply because the following
> code will be wrong:
> 
>   $attr->type($new_code)->store;
>   my $type = $attr->type; # will get the old type

I agree caching is not as trivial. I changed the methods signature to accept
the type to avoid fetching things twice, but then Martin found some uses of the
methods in a controller so it felt like changing the signature would required
changes outside.

Maybe an alternate patch making the param optional is a better approach?

my ($self, $type) = @_;
my $THE_type = $type // $self->type;

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