https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11897 --- Comment #184 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Comment on attachment 70524 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70524 Bug 11897: Add bulk of Stockrotation module. Review of attachment 70524: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=11897&attachment=70524) ----------------------------------------------------------------- ::: api/v1/swagger/paths.json @@ +26,5 @@

"/illrequests": { "$ref": "paths/illrequests.json#/~1illrequests" + }, + "/rotas/{rota_id}/stages/{stage_id}/position": { + "$ref": "paths/rotas.json#/~1rotas~1{rota_id}~1stages~1{stage_id}~1position"

8) Not commenting on the API specifically, but I think this will need more work as there has been a lot of changes since this part was written. ::: installer/data/mysql/userflags.sql @@ +19,4 @@

(19, 'plugins', 'Koha plugins', '0'), (20, 'lists', 'Lists', 0), (21, 'clubs', 'Patron clubs', '0'), +(22,'ill','The Interlibrary Loans Module',0),

9) Not related to these patches - maybe a leftover from a rebase/merge? ::: koha-tmpl/intranet-tmpl/prog/en/includes/biblio-view-menu.inc @@ +40,4 @@

[% IF ( issuehistoryview ) %]<li class="active">[% ELSE %]<li>[% END %] <a href="/cgi-bin/koha/catalogue/issuehistory.pl?biblionumber=[% biblio_object_id | url %]" >Checkout history</a></li> [% IF ( CAN_user_tools_view_system_logs ) %][% IF ( logview ) %]<li class="active">[% ELSE %]<li>[% END %]<a href="/cgi-bin/koha/tools/viewlog.pl?do_it=1&modules=CATALOGUING&action=MODIFY&object=[% biblio_object_id | url %]">Modification log</a> </li>[% END %] +[% IF ( CAN_user_stockrotation_can_add_items_rotas && Koha.Preference('StockRotation') ) %][% IF ( stockrotationview ) %]<li class="active">[% ELSE %]<li>[% END %]<a href="/cgi-bin/koha/catalogue/stockrotation.pl?biblionumber=[% biblio_object_id %]">Rota</a> </li>[% END %]

9) Here user_stockrotation_can_add_items_rotas is asked, but stockrotation.pl asks for catalogue => 1 ? ::: koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc @@ +110,5 @@

[%- CASE 'edit_templates' -%]<span>Create and edit club templates</span> [%- CASE 'enroll' -%]<span>Enroll patrons in clubs</span> [%- CASE 'edi_manage' -%]<span>Manage EDIFACT transmissions</span> + [%- CASE 'can_add_items_rotas' -%]<span>Add and remove items from rotas</span> + [%- CASE 'can_edit_rotas' -%]<span>Create, edit and delete rotas</span>

10) Ha! Fixes 5) ::: koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc @@ +4,5 @@

+ $(document).ready(function() { + var path = location.pathname.substring(1); + if (path.indexOf("labels") >= 0 && path.indexOf("spine") < 0 ) { + $('#navmenulist a[href$="/cgi-bin/koha/labels/label-home.pl"]').css('font-weight','bold'); + } else if (path.indexOf("patroncards") >= 0 ) {

11) This JavaScript block appears unrelated to the stock rotation feature? ::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ +868,4 @@

yes: Enable no: Disable - "housebound module" + Stock Rotation module:

12) Capitalization: Stock rotation ::: koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt @@ +4,5 @@

+<title>Koha › Catalog › Stock rotation details for [% biblio.title %]</title> +[% INCLUDE 'doc-head-close.inc' %] +[% INCLUDE 'browser-strings.inc' %] +<!--[if lt IE 9]> +<script type="text/javascript" src="[% interface %]/lib/shims/json2.min.js"></script>

13) All .js files referenced need version variable added (see bug 12904) @@ +37,5 @@

+ <table class="items_table dataTable no-footer" role="grid"> + <thead> + <tr> + <th>Barcode</th> + <th>Shelfmark</th>

14) Shelfmark is not a term used in Koha, should be callnumber. @@ +58,5 @@

+ [% IF item.rota %] + [% IF !item.rota.active %] + <span class="highlighted-row"> + [% END %] + [% item.rota.active ? 'Active' : 'Inactive' %]

15) Please check if this construct is translatable! ::: koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt @@ +3,5 @@

+[% USE KohaDates %] +[% INCLUDE 'doc-head-open.inc' %] +<title>Koha › Stock rotation</title> +[% INCLUDE 'doc-head-close.inc' %] +<link rel="stylesheet" type="text/css" href="[% interface %]/[% theme %]/css/datatables.css" />

16) Same as for JavaScript files - needs version added (see bug 12904) @@ +247,5 @@

+ <form id="stage_form" method="post" enctype="multipart/form-data" class="validated"> + <fieldset class="rows"> + <ol> + <li> + <label class="required" for="branch">Branch:</label>

17) Please always prefer 'library' to 'branch' (see terminology page on the wiki) @@ +349,5 @@

+ <thead> + <th>Barcode</th> + <th>Title</th> + <th>Author</th> + <th>Shelfmark</th>

18) See above - use callnumber instead. ::: misc/cronjobs/stockrotation.pl @@ +217,5 @@

+ my $header = ""; + my $body = ""; + # Summary + $header .= sprintf " +STOCKROTATION REPORT

19) This hardcoded report is not translatable- appears to apply to the email as well? Please consider moving this into a proper notice template. ::: tools/stockrotation.pl @@ +50,5 @@

+ { + template_name => 'tools/stockrotation.tt', + query => $input, + type => 'intranet', + authnotrequired => 0

20) No permission checks? -- You are receiving this mail because: You are watching all bug changes.