[OPEN-ILS-DEV] opt-in patch

[James Fournie]

Ok here we go again :)

> A
> |
> |--- A-A
> |   |--- A-A-A
> |   |--- A-A-B

Yes, kinda confused but our tree is:

Cons (D0)
|
|--- Federation (depth 1 - no users/items)
   |--- System (depth 2 - no users/items)
       |--- Branch (depth 3)
       |--- Branch (depth 3)
   |--- Library (ie: single branch library) (depth 2)


> +   my $opt_depth = 0;
> +   if( user_opt_in_enabled($self) ){
> +       my $orgid = check_user_perms3($self, $client, $auth,
> $e->requestor->id, "patron.opt_in");
>>>>>>>>>
>
> So, the staffs "patron.opt_in" permission depth defines which foreign
> libraries they can search for patrons, even though the patrons have
> opted in to a local library? (see the section of the patch starting at
> @@ -576,14 +577,17 @@)

Yes, the reason is, if I have a multibranch site, patrons won't be
opted-in by default.  Thus, if a new patron is created at a
multibranch site, they will be opted-in at all the branches (or rather
the opting is ignored, they are treated as if they are at their home
org).

The main problem is that if we force them to opt and they go to the
other branch and forget their card, staff at another branch of the
same system won't be able to search for this patron unless the patron
brings their card and opts-in.

While I agree it should be possible to accomodate if they *did* need
to opt-in in case of a complex org tree, I would think it's important
to be able to ignore opting as many large multibranch public libraries
would want their patrons to have full access to all branches upon
registration without having to bother with opting.

An alternative to ignoring opting would be to create the opt-in
entries upon patron creation, but I didn't want to add codeand I
thought it would be easier to add it all to the opt-check method since
that's how it was done originally

>     $opt_in->org_unit($org_id);
> Index: Open-ILS/src/perlmods/OpenILS/Application/Storage/Publisher/actor.pm
> ===================================================================
> --- Open-ILS/src/perlmods/OpenILS/Application/Storage/Publisher/actor.pm        (revision
> 13335)
> JSAN.use('util.functional');
>         util.widgets.remove_children(obj.controller.view.search_depth);
>         var ml = util.widgets.make_menulist(
> -            util.functional.map_list( obj.OpenILS.data.list.aout,
> +            util.functional.map_list( obj.OpenILS.data.list.my_aou,
>>>>>>>>
>
> I haven't tested this yet, but does this display ancestors as well as
> descendants (or vice versa_?  Just want to be sure -- I don't recall
> if my_aou is grabbing full_path or not.

Yes.  And it displays all descendants, just means the variable el is
now an org unit not an org unit type.

> would be your case)   [UPDATE: side-conversation in IRC brought up
> security.  In 1.4 and beyond, org settings each have their own magical
> permission check.  Add a permission called
> UPDATE_ORG_UNIT_SETTING.$name (where name is the org setting) and give
> that to users that should be allowed to adjust it.]

Yeah I did't know about that, the security was the main reason I
dismissed that idea, but if that's available and permission restricted
then it makes more sense.

>  * when opting a user in, instead of creating an opt-in for $ws_ou,
> set it for actor.org_unit_anscestor_at_depth($ws_ou,
> $opt_in_org_depth)

I believe how I wrote it open-ils.actor.user.org_unit_opt_in.create
should check for passed-in org unit first, failing that it should use
check_user_perms3 to get the highest org unit for the user's perm, and
failing that the workstation.  Maybe i'm looking at the wrong spot?

>  * change the search SQL to use
> actor.org_unit_full_path($ws_ou,$opt_in_org_depth) instead of just
> ancestors.

> I think you want that to be: {org_unit=>\@orgunits, usr=>$user_id},{idlist=>1});
> Note that \@orgunits is an arrayref instead of a bare array.

> -   return $e->event unless $e->allowed('VIEW_USER');
> +   return $e->event unless $e->allowed('VIEW_USER', $search_ou);
>>>>>>>>>
> I think I'm missing the purpose of this change.

Yes I agree with all of these those.

~James
BC SITKA