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

--- Comment #14 from Katrin Fischer <katrin.fisc...@bsz-bw.de> ---
Comment on attachment 23639
  --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23639
Bug 10263 - Add ability to limit which branch can edit a bibliographic record
(IndependentBranchesMarcEditing)

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

Doing some code review here too... 

1) I think instead of using a new separate authorised value entry to have an
empty option, this could work like the item form. If you mark the subfield
mandatory, it will not offer an empty option. If it's not mandatory, it will.
But it looks like the rule does not apply to the cataloguing form yet, as 942$c
(itemtype) offers an
empty option and is mandatory. Hm.

2) HasIndependentGroupModificationRightsFor needs some unit tests and
documentation. Why is $self ignored? 
Why not use GetIndependentGroupModificationRights( { for => $branchcode } );?

3) Spotted another 'old' superlibrarian permission check.

Failing for 2), 3) might be resolved by one of the other dependent patches.

::: C4/Auth.pm
@@ +197,3 @@
>          # to create the template's parameters that will indicate
>          # which menus the user can access.
>          if ( $flags && $flags->{superlibrarian}==1 ) {

Old check for superlibrarian permission.

::: Koha/Template/Plugin/Koha.pm
@@ +44,4 @@
>      return C4::Context->preference( $pref );
>  }
>  
> +sub HasIndependentGroupModificationRightsFor {

POD, UT

::: admin/marc_subfields_structure.pl
@@ +133,4 @@
>          push @authorised_values, $category;
>      }
>      push( @authorised_values, "branches" );
> +    push( @authorised_values, "branches_optional" );

See 1) for an idea to handle this a bit differently.

::: installer/data/mysql/kohastructure.sql
@@ +127,4 @@
>    `timestamp` timestamp NOT NULL default CURRENT_TIMESTAMP on update 
> CURRENT_TIMESTAMP, -- date and time this record was last touched
>    `datecreated` DATE NOT NULL, -- the date this record was added to Koha
>    `abstract` mediumtext, -- summary from the MARC record (520$a in MARC21)
> +  `branchcode` VARCHAR( 10 ) NULL DEFAULT NULL, -- branchcode for the 
> opationl 'owner' of this record.

Small documentation typo.

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://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/

Reply via email to