Bug 24132

Summary: search_with_library_limits vs filter_by_branch_limitations
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Bugs List <koha-bugs>
Status: NEW --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: tomascohen
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 20443    
Bug Blocks:    

Description Jonathan Druart 2019-11-28 09:49:05 UTC
Aside from the naming of these two subroutines (implemented at different times), Koha::Patron::Attribute::Types->search_with_library_limits and Koha::Patron::Attributes->filter_by_branch_limitations looks similar and that the code must be simplified.

The join table is between borrower_attribute_types and branches (borrower_attribute_types_branches). So Koha::Patron::Attribute::Types can inherit directly from Koha::Objects::Limit::Library, not Koha::Patron::Attributes

There are a couple of ways to deal with that:
1. Improve Koha::Objects::Limit::Library => Sounds wrong as there is no join table between borrower_attributes and branches
2. Remove Koha::Patron::Attributes->filter_by_branch_limitations and replace it by something like: 
  my $attribute_types = Koha::Patron::Attribute::Types
                         ->search_with_library_limits;
  my $attribute = $patron->extended_attributes
                         ->search({ code => { -in => attribute_types->column('code') } });
Which sounds more correct.
Comment 1 Tomás Cohen Arazi 2021-04-16 18:35:50 UTC
(In reply to Jonathan Druart from comment #0)
> Aside from the naming of these two subroutines (implemented at different
> times), Koha::Patron::Attribute::Types->search_with_library_limits and
> Koha::Patron::Attributes->filter_by_branch_limitations looks similar and
> that the code must be simplified.
> 
> The join table is between borrower_attribute_types and branches
> (borrower_attribute_types_branches). So Koha::Patron::Attribute::Types can
> inherit directly from Koha::Objects::Limit::Library, not
> Koha::Patron::Attributes
> 
> There are a couple of ways to deal with that:
> 1. Improve Koha::Objects::Limit::Library => Sounds wrong as there is no join
> table between borrower_attributes and branches
> 2. Remove Koha::Patron::Attributes->filter_by_branch_limitations and replace
> it by something like: 
>   my $attribute_types = Koha::Patron::Attribute::Types
>                          ->search_with_library_limits;
>   my $attribute = $patron->extended_attributes
>                          ->search({ code => { -in =>
> attribute_types->column('code') } });
> Which sounds more correct.

I agree with the latter, Jonathan. Let me know if we can sort this one for 21.05, to finish this consistency rewrite.