[Koha-bugs] [Bug 10276] Extend IndependentBranches to support groups of libraries

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

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10276

--- Comment #22 from Katrin Fischer <katrin.fischer at bsz-bw.de> ---
Comment on attachment 23641
  --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23641
Bug 10276 - Extend IndependentBranches to support groups of libraries

Review of attachment 23641:
 --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=10276&attachment=23641)
-----------------------------------------------------------------

Patch doesn't apply, but doing a quick code review:

1) Commit message could be updated, dependencies noted are resolved and example
is slightly wrong (should be group 1 and 3).

2) Patch touches a lot of routines in different modules, but none of this is
backed up by unit tests.
   This is a big feature with a lot of possible consequences if we break
something, I think unit tests
   are a requirement, at least for the new routine. (UT)

3) Patch has bug 10277 notes as dependency, but is not using the new function
consequently (C4::Context->IsSuperLibrarian())

4) Patch seems to remove 'properties' from the interface. Currently libraries
can add properties library groups.
   I think we need to discuss this before removing the feature. Also we need to
think about databases with property groups
   defined and how to migrate them. Or keep the option. (properties)

::: C4/Branch.pm
@@ +43,4 @@
>  		&GetBranchCategories
>  		&GetBranchesInCategory
>  		&ModBranchCategoryInfo
> +        &GetIndependentGroupModificationRights

UT

@@ +376,4 @@
>  
>  	#TODO  manage category types.  rename possibly to 'agency domains' ? as borrowergroups are called categories.
>  sub GetCategoryTypes {
> + return ( 'searchdomain','independent_groups');

properties

@@ +446,5 @@
> +
> +    If 'branch' is not provided, it will be looked up via
> +    C4::Context->userenv->{branch}.
> +
> +    If 'for' is provided, the lookup is limited to that branch.

Could you explain the behaviour/use of 'for' a bit more?

@@ +452,5 @@
> +    If called in a list context, returns a list of
> +    branchcodes ( including $this_branch ).
> +
> +    If called in a scalar context, it returns
> +    a count of matching branchcodes. Returns 1 if

...? :)

::: C4/Serials.pm
@@ +238,5 @@
> +               subscription.subscriptionid as subsid
> +    |;
> +    if (   C4::Context->preference('IndependentBranches')
> +        && C4::Context->userenv
> +        && C4::Context->userenv->{'flags'} % 2 != 1

C4::Context->IsSuperLibrarian()

@@ +355,5 @@
> +    |;
> +
> +    if (   C4::Context->preference('IndependentBranches')
> +        && C4::Context->userenv
> +        && C4::Context->userenv->{'flags'} % 2 != 1

C4::Context->IsSuperLibrarian()

@@ +413,5 @@
> +    |;
> +
> +    if (   C4::Context->preference('IndependentBranches')
> +        && C4::Context->userenv
> +        && C4::Context->userenv->{'flags'} % 2 != 1

C4::Context->IsSuperLibrarian()

@@ +594,5 @@
> +    |;
> +
> +    if (   C4::Context->preference('IndependentBranches')
> +        && C4::Context->userenv
> +        && C4::Context->userenv->{'flags'} != 1

C4::Context->IsSuperLibrarian()

::: catalogue/moredetail.pl
@@ +177,3 @@
>          my $userenv = C4::Context->userenv();
> +        unless (
> +            $userenv->{'flags'} % 2 != 1

C4::Context->IsSuperLibrarian()

::: cataloguing/additem.pl
@@ +702,4 @@
>              #verifying rights
>              my $userenv = C4::Context->userenv();
> +            unless (
> +                $userenv->{'flags'} % 2 == 1

C4::Context->IsSuperLibrarian()f

::: circ/circulation-home.pl
@@ +39,5 @@
>  $template->param( fast_cataloging => 1 ) if (defined $fa);
>  
>  # Checking if the transfer page needs to be displayed
> +$template->param( display_transfer => 1 )
> +  if ( $flags->{'superlibrarian'} == 1

C4::Context->IsSuperLibrarian()

::: installer/data/mysql/kohastructure.sql
@@ +365,4 @@
>    `categorycode` varchar(10) NOT NULL default '', -- unique identifier for the library/branch group
>    `categoryname` varchar(32), -- name of the library/branch group
>    `codedescription` mediumtext, -- longer description of the library/branch group
> +  `categorytype` ENUM(  'searchdomain',  'independent_group' ) NULL DEFAULT NULL, -- says whether this is a search group or an independent group

properties

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