https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37392
Marcel de Rooy <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #38 from Marcel de Rooy <[email protected]> --- In general, the code is not very clear although this is kind of a new feature.. Just some thoughts about the POD of involved methods (apart from their ugly names..): my $can_see = $patron->can_see_things_from( $branchcode ); Return true if the I<Koha::Patron> can perform some action on the given thing => Can see is imho different from "performing actions on" ? Same for libraries_where_can_see_things POD "an aribitarary [...] action is allowed to be taken" => is that seeing? "the thing can see thing's infos" => really? how clear is that? The else branch is not intuitive. Because the empty list return of libraries_where_can_see_things is confusing. } elsif ( my @branches = $self->libraries_where_can_see_things($params) ) { $can = ( any { $_ eq $branchcode } @branches ) ? 1 : 0; } else { # This should be the case of not finding any limits above, so we can $can = 1; POD libraries_where_can_see_things "An empty array means no restriction" ! But if there is no userenv, it also returns empty list. So you can see all. The name of the routine together with an empty list as return is kind of confusing! If you do have the specified permission, the @restricted_branchcodes is EMPTY too. So you can see.. If you dont have the permission, we are checking the library groups. We are calling get_root_ancestor: if the group has no parent, the ancestor is the group itself. If it has the feature enabled, we are calling ->parent again (no result) and we CRASH on ->all_libraries. If we have another ancestor but no root has the feature enabled, we are again getting empty list. So can see all. If some root ancestor has the specified feature enabled say ft_hide_patron_info, then all libraries under the parent group are added to @restricted. Ultimately, if the branchcode is found in this list in can_see_things_from, you can 'see things''. (Side note: If the group with that feature has no parent, undef is returned and it looks like calling ->all_libraries will CRASH again?) Suppose patron library L1 is part of group A with ft enabled. And the branchcode L2 we are looking for is not in those groups/subgroups. But L2 is part of group B that has not enabled the feature. A patron from a library under A (or below) can see L1, but cannot see L2. (Why not: B did not enable ft) A patron from a library under B and no other groups, can see L1 and can see L2. A patron from a library that has no group, can see L1 and L2. Note that the description "Hide patron's info for librarians outside of this group" is confusing/misleading. Librarians btw? Actually you are hiding L2 from an A patron. And L1 can be seen by all above. So isnt it: Hide patron info outside "this group tree or something"? What about L3 if it is part of A with ft and part of B without ft btw? Etc. Maybe I am missing something here. Please clarify and prevent the code crashing on no parent. -- 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/
