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

Hammat wele <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Failed QA                   |Needs Signoff

--- Comment #74 from Hammat wele <[email protected]> ---
(In reply to Marcel de Rooy from comment #67)

Hi Marcel thanks for your comments

> Did you see Nicks comment about URL manipulation. What did you do with that?

es, I saw Nick's comment about URL manipulation, and I've made the correction

> [% UNLESS ( my_branch_as_staff && my_branch_as_staff != c.branchcode ) %]
> I am seeing this construct a few times. An unless with an AND does not
> always read that easy.

I've changed it to an IF condition to make it easier to read

>
> +my $my_branch_as_staff =
> +    C4::Context->preference("IndependentBranchesAdditionalcontents")
> +    && !C4::Context->IsSuperLibrarian()
> +    ? C4::Context->userenv()->{'branch'}
> +    : undef;
> Variable name is unclear.

I've renamed my_branch_as_staff to is_restricted and added user_branch to make
the logic clearer.

> 
>      my $additional_content = Koha::AdditionalContents->find($id);
> +    $additional_content = undef
> +        if ( $my_branch_as_staff
> +        && $additional_content
> +        && C4::Context->userenv()->{'branch'} ne
> $additional_content->branchcode );
> This looks suboptimal. You probably could prevent a db hit here in some
> cases and adjust the search condition otherwise?
> 
> +    if ($my_branch_as_staff) {
> +        @ids = grep {
> +            my $id                 = $_;
> +            my $additional_content = Koha::AdditionalContents->find($id);
> +            defined $additional_content->branchcode
> +                && $additional_content->branchcode eq
> C4::Context->userenv()->{'branch'};
> +        } @ids;
> +    }
> +    if (@ids) {
> +        try {
> +            Koha::Database->new->schema->txn_do(
> +                sub {
> +                    my $contents = Koha::AdditionalContents->search( { id
> => \@ids } );
> Suboptimal too. Fetching records twice.

I've changed the find() to a search() with the branchcode condition directly,
so the branch restriction is handled by the database.

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list -- [email protected]
To unsubscribe send an email to [email protected]
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to