[Koha-bugs] [Bug 19889] LocalHoldsPriority needs exclusions

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

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

--- Comment #21 from Tomás Cohen Arazi <tomascohen at gmail.com> ---
(In reply to Christopher Brannon from comment #20)
> * This system preference would take a list of patron categories
> * Alter code to ignore local holds priority for these
> 
> Talk to me about the reasoning for the placement of the patron exclusion in
> the patron category rather than a central preference that was outlined here.
> I just want to understand before you do anything.

The original idea was to add a syspref, as you outline. I'm the one that
decided it was better to make it a patron category attribute. This, because the
syspref is basically a way to say something about the category, but in a place
that is not tied to categories (and thus unnatural).

This is the reasoning:

>From the coding point of view, we might need/want to ask if a patron belongs to
a category that is excluded from localholdspriority, and we would do it like
this:

    # Find the patron
    my $patron = Koha::Patrons->find($request->{borrowernumber});
    # skip the patron if its category is excluded
    next if $patron->category->exclude_from_local_holds_priority; 

if we go the sysprefs way it would read like:

    my @excluded_categories = split(/\|/,
C4::Context->preference("CategoriesToExcludeFromLocalHoldsPriority")) // ();
    my $patron = Koha::Patrons->find($request->{borrowernumber});
    next if any { $_ eq $patron->categorycode } @excluded_categories;

So the result is similar, but ugly-ish. And involves an error prone free-text
field with pipe-separated values. So I don't see any advantage on it. If we
were to expose this condition on the API also, we would need to add methods to
extract the boolean with this kind of comparison, instead of just reading
what's on the category table.

That said, we can absolutely roll back to the syspref approach and come back to
refactor this later. Once we have a more clear interface for setting all things
related to local holds priorities in a single place. As I understand your point
of the whole setting being sparse in different places being counter-intuitive.

> My comment about the priority for the item not showing as set still stands. 
> That is confusing.

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