Created attachment 25887 [details]
Technical Spec

I've uploaded a detailed spec which from which we are basing the work.

We will shortly be able to finally upload a complete patch set for this feature.

The basic idea with this feature is that:
- librarians can create rotas, that specify a route that stock should automatically take along 'stages' (using branchtransfers)
- for now, stages are considered completed if an item has spent X items at the branch associated with the stage (this could later be enhanced with other triggers, e.g. X number of check outs at the given branch)
- Items are associated with rotas, and are automatically added to the first stage.

Once an item arrives at the first stage, a nightly cron script will make sure it is put into branchtransfer to the next stage when the item has exceeded the number of days specified in the stage at the current branch.

This feature is subtly different from rotating collections, as that feature is to do with the management of a 'group of items', that can be moved on together to the next library.

This feature is about the automatic rotation of individual items attached to rotas along the rota.

Alex

Created attachment 58207 [details] [review]
Bug 11897: Add Stock Rotation atomic update schema.

* installer/data/mysql/atomicupdate/stockrot_tables.sql: New file.
* installer/data/mysql/kohastructure.sql (stockrotationrotas)
  (stockrotationstages, stockrotationitems): New tables.
* installer/data/mysql/sysprefs.sql: Add Stockrotation sysprefs.
* installer/data/mysql/userflags.sql: Add Stockrotaiton userflag.
* installer/data/mysql/userpermissions.sql: Add Stockrotation
  userpermissions.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:
  Add Stockrotation menu.

Created attachment 58208 [details] [review]
Bug 11897: DO NOT PUSH: Add stock rotation schema files.

* Koha/Schema/Result/Branch.pm: New file.
* Koha/Schema/Result/Item.pm: New file.
* Koha/Schema/Result/Stockrotationitem.pm: New file.
* Koha/Schema/Result/Stockrotationrota.pm: New file.
* Koha/Schema/Result/Stockrotationstage.pm: New file.

Created attachment 58209 [details] [review]
Bug 11897: DBIX::Class::Ordered in Stockrotationstage.

* Koha/Schema/Result/Stockrotationstage.pm: Load `Ordered`, add
  grouping_column.

Created attachment 58210 [details] [review]
Bug 11897: Add Koha::Objects for Stockrotation module.

* Koha/Stockrotationitem.pm: New file.
* Koha/Stockrotationitems.pm: New file.
* Koha/Stockrotationrota.pm: New file.
* Koha/Stockrotationrotas.pm: New file.
* Koha/Stockrotationstage.pm: New file.
* Koha/Stockrotationstages.pm: New file.
* t/db_dependent/Stockrotationitems.t: New file.
* t/db_dependent/Stockrotationrotas.t: New file.
* t/db_dependent/Stockrotationstages.t: New file.
* Koha/Item.pm (stockrotationitem, add_to_rota, biblio): New
  procedures.
* Koha/Library.pm (stockrotationstages): New procedure.
* t/db_dependent/Items.t: Add tests.
* t/db_dependent/Koha/Libraries.t: Add tests.

Created attachment 58211 [details] [review]
Bug 11897: Add stockrotation cron script.

* misc/cronjobs/stockrotation.pl: New file.

Created attachment 58212 [details] [review]
Bug 11897: Add Utils library for Stockrotation module

* Koha/Util/Stockrotation.pm: new file

Created attachment 58213 [details] [review]
Bug 11897: REST API - add path defs, controller & tests

* Koha/REST/V1/Stage.pm: New file
* api/v1/swagger/paths.json: Add route for stage manipulation
* api/v1/swagger/paths/rotas.json: New file
* t/db_dependent/api/v1/stockrotationstage.t: New file

Created attachment 58214 [details] [review]
Bug 11897: Add JS to provide rota stage manipulation

* koha-tmpl/intranet-tmpl/prog/js/pages/stockrotation.js: new file

Created attachment 58215 [details] [review]
Bug 11897: Add Stockrotation CSS rules

* koha-tmpl/intranet-tmpl/prog/css/staff-global.css: New rules

Created attachment 58216 [details] [review]
Bug 11897: Add Stock Rotation permissions to list

* koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc: Add new
permissions

Created attachment 58217 [details] [review]
Bug 11897: Interface for rotas from Tools menu

* tools/stockrotation.pl: New file
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt: New file
* koha-tmpl/intranet-tmpl/prog/en/includes/stockrotation-toolbar.inc:
New file

Created attachment 58218 [details] [review]
Bug 11897: Interface for rota items from biblio

* catalogue/stockrotation.pl: New file
* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt:
New file

Created attachment 58219 [details] [review]
Bug 11897: Add menu options for Stock Rotation

* koha-tmpl/intranet-tmpl/prog/en/includes/biblio-view-menu.inc: Add
menu option
* koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc: Add menu
option
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt: Add menu
option

The patch set should apply cleanly on sandboxes and on devboxes without additional setup needing to be configured.

- On sandboxes the stockrotation cron script cannot be tested
- On devboxes the cron script can be tested and should be run first with the -h flag for documentation

Please let us know if you have any questions about this module or it's intended use cases.

Comment on attachment 58218 [details] [review]
Bug 11897: Interface for rota items from biblio

This is being obsoleted, replacement patch to be attached.

Comment on attachment 58219 [details] [review]
Bug 11897: Add menu options for Stock Rotation

This is being obsoleted, replacement patch to be attached.

Created attachment 58280 [details] [review]
Bug 11897: Interface for rota items from biblio

* catalogue/stockrotation.pl: New file
* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt:
New file

Created attachment 58281 [details] [review]
Bug 11897: Add menu options for Stock Rotation

* koha-tmpl/intranet-tmpl/prog/en/includes/biblio-view-menu.inc: Add
menu option
* koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc: Add menu
option
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt: Add menu
option

I've done a quick run-through of this and nothing looks broken. The tests all pass. I think the biggest gap is documentation. There should be a help file for the whole process, especially the things which aren't self-explanatory. For instance:

- What does the 'cyclical' setting do?
- What is the purpose of the "Start" item in the stages
  and duration column of the manage items table?
- What does the "In demand" setting do?

I take it the process is: Create rota -> Add items -> Wait for cron job to put items in transit? I'm curious whether available items should be put in transit automatically after putting them on a rota.

Minor problems:

- Our capitalization rules say the heading should say
  "Stock rotation" instead of "Stock Rotation"
  https://wiki.koha-community.org/wiki/Coding_Guidelines#HTML4:_Upper_and_lower_cases_in_strings

- I don't the the alert-style div is correct for showing
  a confirmation of items added. I would put the confirmation
  message in a <div class="dialog message"></div> and show a
  table of results below that.

- In the manage items table, there is no <tr> wrapping <th>.

- The page title and breadcrumbs should be made more specific
  for each page. For instance, when on the manage items page
  the breadcrumbs should be:

  Home › Tools › Stock rotation > Manage items

  This provides an easily accessible link back to the main
  stock rotation page.

Suggestions:

- Add a toolbar to the manage items page with a 'New rota'
  and 'Edit rota' buttons.

- Move the add items forms to the top of the 

- In the manage items table, I think title and barcode
  should link to the bibliographic record and items page,
  respectively.

- Make the manage items table a DataTable with sorting and
  searching. Unless you think the items table will typically
  hold so much data this would become cumbersome?

- Should item status information be displayed in the manage
  items table? Is it relevant to the process to know whether
  something is checked out, lost, etc?

- Since the drag-to-sort interaction is new to Koha it
  probably needs a hint at the top to suggest it. (Looks 
  good and work well, though).

I'm marking this as failed for now for the minor fixes and to get more information. Thanks.

Created attachment 58687 [details] [review]
Bug 11897: Add Stockrotation CSS rules

* koha-tmpl/intranet-tmpl/prog/css/staff-global.css: New rules

Created attachment 58692 [details] [review]
Bug 11897: Add Stockrotation CSS rules

* koha-tmpl/intranet-tmpl/prog/css/staff-global.css: New rules

Created attachment 58693 [details] [review]
Bug 11897: Add Stock Rotation permissions to list

* koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc: Add new
permissions

Created attachment 58694 [details] [review]
Bug 11897: Interface for rotas from Tools menu

* tools/stockrotation.pl: New file
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt: New file
* koha-tmpl/intranet-tmpl/prog/en/includes/stockrotation-toolbar.inc:

Created attachment 58695 [details] [review]
Bug 11897: Interface for rota items from biblio

* catalogue/stockrotation.pl: New file
* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt:
New file

Created attachment 58696 [details] [review]
Bug 11897: Add menu options for Stock Rotation

* koha-tmpl/intranet-tmpl/prog/en/includes/biblio-view-menu.inc: Add
menu option
* koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc: Add menu
option
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt: Add menu
option

(In reply to Owen Leonard from comment #22)

Hi Owen

Thanks *very* much for your feedback, hugely useful. I've just attached a bunch of modified patches that address most of the points, pertinent comments below each of your points.

> I've done a quick run-through of this and nothing looks broken. The tests
> all pass. I think the biggest gap is documentation. There should be a help
> file for the whole process, especially the things which aren't
> self-explanatory. For instance:
> 
> - What does the 'cyclical' setting do?
> - What is the purpose of the "Start" item in the stages
>   and duration column of the manage items table?
> - What does the "In demand" setting do?

Excellent point, we'll address that.

> Minor problems:
> 
> - Our capitalization rules say the heading should say
>   "Stock rotation" instead of "Stock Rotation"

Fixed  

> 
> - I don't the the alert-style div is correct for showing
>   a confirmation of items added. I would put the confirmation
>   message in a <div class="dialog message"></div> and show a
>   table of results below that.

Yes, agreed, fixed.

> - In the manage items table, there is no <tr> wrapping <th>.

I couldn't replicate this. I looked in the markup for the manage items table and there was definitely a <tr> wrapping <th>. I'm not sure if I'm misunderstanding where you mean.

> - The page title and breadcrumbs should be made more specific
>   for each page. For instance, when on the manage items page
>   the breadcrumbs should be:
> 
>   Home › Tools › Stock rotation > Manage items
> 
>   This provides an easily accessible link back to the main
>   stock rotation page.

Yes, excellent point. This has now been addressed throughout.

> Suggestions:
> 
> - Add a toolbar to the manage items page with a 'New rota'
>   and 'Edit rota' buttons.

Done.

> - Move the add items forms to the top of the [items table]

Done.

> - In the manage items table, I think title and barcode
>   should link to the bibliographic record and items page,
>   respectively.

Done.

> - Make the manage items table a DataTable with sorting and
>   searching. Unless you think the items table will typically
>   hold so much data this would become cumbersome?

I don't imagine it will, good suggestion. Done.

> - Should item status information be displayed in the manage
>   items table? Is it relevant to the process to know whether
>   something is checked out, lost, etc?

Not sure how immediately useful this will be, particularly as we're linking directly to the item from the barcode in the row. I got a consensus of opinions here and the majority felt it wasn't necessary. Thanks for the suggestion though.

> - Since the drag-to-sort interaction is new to Koha it
>   probably needs a hint at the top to suggest it. (Looks 
>   good and work well, though).

Done.

Again, thanks for your feedback. If you find time to go through the latest changes, I'd be very grateful. I'll update this bug when we have some accompanying docs.

Cheers
Andrew

Created attachment 60175 [details]
Draft Documentation (authored by Fiona Borthwick)

I've added a new attachment: a PDF document with proposed draft documentation for the module.

This should provide sufficient information for any user wanting to play with the module.

As it is intended for users it does not cover use of the cron script.

The manual was authored by Fiona Borthwick here at PTFS Europe.

Cheers,
Alex

Created attachment 60715 [details]
Documentation for Koha manual

This documentation PDF is a file generated from the kohadocs project. The patch for that can be submitted at the same time that stock rotation is merged into master.

Comments on the documentation, here attached as PDF, for the whole Tools chapter (search for 'Stock Rotation' to get to the relevant section), very welcome.

Alex

Hi,

Quick comments, as I have a client interested in this patch:

- the changes to the .sql files are missing comments at the end of the lines, so they cause the installer to throw a fit, please fix. 
- The stages cannot be reordered, the ajax response says  Saving changes... Error: Method Not Allowed 
- the button styles in the toolbar need to be consistent with the rest of Koha please.
- it is a nice to have to eliminate "submit" and swap for the button label "Save"

I'll add more as I find them and I am sure my client will bring up whatever they find as well.

Cheers,
Liz

Hi Liz,

Thank you for the comments!

(In reply to Liz Rea from comment #32)
> Hi,
> 
> Quick comments, as I have a client interested in this patch:
> 
> - the changes to the .sql files are missing comments at the end of the
> lines, so they cause the installer to throw a fit, please fix. 

Indeed, I've fixed this bug in my local dev version, will push it shortly to this patch.

> - The stages cannot be reordered, the ajax response says  Saving changes...
> Error: Method Not Allowed 

This is an error in the koha-httpd config for standard and single installs I've submitted a bug report for the Makefile that will fix this. (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18175)

Just to be sure — what kind of install did you use to test this?

> - the button styles in the toolbar need to be consistent with the rest of
> Koha please.

Will fix :-)

> - it is a nice to have to eliminate "submit" and swap for the button label
> "Save"

Agreed, will fix this too.

 
> I'll add more as I find them and I am sure my client will bring up whatever
> they find as well.

Thanks, keep 'em coming :-)

Ah, it's probably because my apache configs are older, I'll add those paths and try it again. (and make a note that they need to be added in upgrades)

Created attachment 61825 [details] [review]
Bug 11897: Add Stock Rotation atomic update schema.

* installer/data/mysql/atomicupdate/stockrot_tables.sql: New file.
* installer/data/mysql/kohastructure.sql (stockrotationrotas)
  (stockrotationstages, stockrotationitems): New tables.
* installer/data/mysql/sysprefs.sql: Add Stockrotation sysprefs.
* installer/data/mysql/userflags.sql: Add Stockrotaiton userflag.
* installer/data/mysql/userpermissions.sql: Add Stockrotation
  userpermissions.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:
  Add Stockrotation menu.

Created attachment 61826 [details] [review]
Bug 11897: DO NOT PUSH: Add stock rotation schema files.

* Koha/Schema/Result/Branch.pm: New file.
* Koha/Schema/Result/Item.pm: New file.
* Koha/Schema/Result/Stockrotationitem.pm: New file.
* Koha/Schema/Result/Stockrotationrota.pm: New file.
* Koha/Schema/Result/Stockrotationstage.pm: New file.

Created attachment 61827 [details] [review]
Bug 11897: DBIX::Class::Ordered in Stockrotationstage.

* Koha/Schema/Result/Stockrotationstage.pm: Load `Ordered`, add
  grouping_column.

Created attachment 61828 [details] [review]
Bug 11897: Add Koha::Objects for Stockrotation module.

* Koha/Stockrotationitem.pm: New file.
* Koha/Stockrotationitems.pm: New file.
* Koha/Stockrotationrota.pm: New file.
* Koha/Stockrotationrotas.pm: New file.
* Koha/Stockrotationstage.pm: New file.
* Koha/Stockrotationstages.pm: New file.
* t/db_dependent/Stockrotationitems.t: New file.
* t/db_dependent/Stockrotationrotas.t: New file.
* t/db_dependent/Stockrotationstages.t: New file.
* Koha/Item.pm (stockrotationitem, add_to_rota, biblio): New
  procedures.
* Koha/Library.pm (stockrotationstages): New procedure.
* t/db_dependent/Items.t: Add tests.
* t/db_dependent/Koha/Libraries.t: Add tests.

Created attachment 61829 [details] [review]
Bug 11897: Add stockrotation cron script.

* misc/cronjobs/stockrotation.pl: New file.

Created attachment 61830 [details] [review]
Bug 11897: Add Utils library for Stockrotation module

* Koha/Util/Stockrotation.pm: new file

Created attachment 61831 [details] [review]
Bug 11897: REST API - add path defs, controller & tests

* Koha/REST/V1/Stage.pm: New file
* api/v1/swagger/paths.json: Add route for stage manipulation
* api/v1/swagger/paths/rotas.json: New file
* t/db_dependent/api/v1/stockrotationstage.t: New file

Created attachment 61832 [details] [review]
Bug 11897: Add JS to provide rota stage manipulation

* koha-tmpl/intranet-tmpl/prog/js/pages/stockrotation.js: new file

Created attachment 61833 [details] [review]
Bug 11897: Add Stockrotation CSS rules

* koha-tmpl/intranet-tmpl/prog/css/staff-global.css: New rules

Created attachment 61834 [details] [review]
Bug 11897: Add Stock Rotation permissions to list

* koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc: Add new
permissions

Created attachment 61835 [details] [review]
Bug 11897: Interface for rotas from Tools menu

* tools/stockrotation.pl: New file
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt: New file
* koha-tmpl/intranet-tmpl/prog/en/includes/stockrotation-toolbar.inc:

Created attachment 61836 [details] [review]
Bug 11897: Interface for rota items from biblio

* catalogue/stockrotation.pl: New file
* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt:
New file

Created attachment 61837 [details] [review]
Bug 11897: Add menu options for Stock Rotation

* koha-tmpl/intranet-tmpl/prog/en/includes/biblio-view-menu.inc: Add
menu option
* koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc: Add menu
option
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt: Add menu
option

Created attachment 61838 [details] [review]
Bug 11897: Bugfixes, unit tests, feedback, documentation.

A (squashed) patch implementing significant changes to many parts of
the module.

Also adds thorough unit tests.

Created attachment 61839 [details] [review]
Bug 11897: Fix style of button links, & form wording.

* koha-tmpl/intranet-tmpl/prog/en/includes/stockrotation-toolbar.inc
* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt

I'm reposting the test plan, after some further extensive modifications (some internal redesign, add loads of unit tests, act upon feedback).

We believe this is now of sufficient quality to be included in Koha.

Test Plan
=========

Setup
-----

- Ensure the patches apply cleanly and without problems
- Run the unit tests, and ensure no regressions.  In particular:
  * t/db_dependent/Stockrotationitems.t
  * t/db_dependent/Stockrotationrotas.t
  * t/db_dependent/Stockrotationstages.t
  * t/db_dependent/Items.t
  * t/db_dependent/Koha/Libraries.t

User Interface
--------------

- Enable the stockrotation system preference
- A new menu, "stockrotation" should have appeared under the "tools" page. Test:
  * creating rota
  * editing rota
  * adding stages
  * adding items
  * removing items
  * "move item to next stage"
- Any biblio should now have a "rotas" tab in the biblio view
- From there any items attached to that biblio can be added/removed to/from rotas

Cron Script
-----------

The task of the cron script is to traverse all the items in all rotas
and to see whether any of them need to be moved on to the next stage.

It does this by, for each item, checking whether it has spent more
days than "duration" at the current branch since it's last
(stockrotation) branch transfer.

To test the script, run it with the "-h" flag.  You should get
documentation.

If you want to see the script actually do something, you should:
- setup an item as described above
- run the script to create initial stock rotation branch transfers
  (don't forget the "-x" flag!)
- fiddle the database, so that the initial stock rotation branch
  transfer is in the past by at least the "duration" of the stage
  that the item is currently at
- run the script again (without the "-x" flag): you should see that
  the cron script would now move the item to the next stage
- running it with the "-x" flag will create the new branch transfer
  and will emit a report for the library currently holding the item
  to inform them it needs to be fetched from shelf to be put in
  transit.

If you prefer you can have the script send emails.  Follow the
documentation for this.


Please let me know if you have any questions!  I'll be happy to help
:-)

Alex

Created attachment 61885 [details] [review]
Bug 11897: Add documentation for the cron script.

* misc/cronjobs/stockrotation.pl: Add documentation.

Everything was working up until just after adding a list of barcodes to a rota. When I try to go back to 'manage items' I get an error:

Proxy Error

The proxy server received an invalid response from an upstream server.
The proxy server could not handle the request GET /cgi-bin/koha/tools/stockrotation.pl.

The log says:

[Mon Apr 10 17:50:22.708218 2017] [proxy_http:error] [pid 4055] (20014)Internal error: [client 192.168.50.1:54951] AH01102: error reading status line from remote server httpd-UDS:0, referer: http://kohadev-staff.mylocalkoha.org/cgi-bin/koha/tools/stockrotation.pl
[Mon Apr 10 17:50:22.710180 2017] [proxy:error] [pid 4055] [client 192.168.50.1:54951] AH00898: Error reading from remote server returned by /cgi-bin/koha/tools/stockrotation.pl, referer: http://kohadev-staff.mylocalkoha.org/cgi-bin/koha/tools/stockrotation.pl

Hi Owen, 

I've been trying to reproduce this, but cannot.

(In reply to Owen Leonard from comment #52)
> Everything was working up until just after adding a list of barcodes to a
> rota. When I try to go back to 'manage items' I get an error:
> 
> Proxy Error
> 
> The proxy server received an invalid response from an upstream server.
> The proxy server could not handle the request GET
> /cgi-bin/koha/tools/stockrotation.pl.

Is this a persistent error?

When you say you try to go back to 'manage items' does this mean you succesfully get to the rotas overview screen, but when you click on 'manage' for the rota you just added the items to, and then 'items', that's when the error appears?

There should be no proxying going on here…

What testing environment are you using? Is it a devbox?

If so, is it with Plack switched on? Would you be able to retest this particular operation with Plack switched off?

Thanks for testing so far!

Alex

Testing again I can't reproduce the error. I will continue to look.

Can you please camel case your module names for readability ( e.g. Koha::StockRotationItem instead of Koha::Stockrotationitem )?

Thanks!

Hi Kyle,

I've been playing around with this, and am happy to do so (I will upload a patch to this effect a little later), but from my perspective I'm not a fan of the change:

- the database tables are called stockrotationitem etc., so the dbix class files use the non-camel cased versions.
- changing the Koha namespace entries to camel case means we now have a separation between dbix classes and the koha object code
- when working with the unit test builder interface, you now have to remember to use non camel case, but when using the koha objects you have to use camel case…

Also, given that the object is called a 'stockrotationitem' for instance, to me it seems natural that the camel case boundary would be, for instance, StockrotationitemDeliverer.

But then maybe the last point is the result of my German background, where ridiculously long compound nouns are not seen as problematic ;-)

WDYT?

Alex

(In reply to Kyle M Hall from comment #55)
> Can you please camel case your module names for readability ( e.g.
> Koha::StockRotationItem instead of Koha::Stockrotationitem )?
> 
> Thanks!

Also, as an aside, and to potentially save time here, I don't think there is a code guideline about this (pointing us in either direction), though there does seem to be a large precedent to do as you suggest, and only 2 other modules not using camel case in their names.

Alex

Finally, there is the added complication of relations and other methods that relate to stockrotation objects:

e.g. Koha::Item, line 189 defines the relation between an item and its stock rotation item.  That method is currently, quite naturally, called 'stockrotationitem' ("give me the stockrotationitem object associated with this item").

Should this now be:

stockRotationItem - this seems terrible and has no precedent, so probably ignore this.

stock_rotation_item - so now we introduce yet another name for the same object; just in a different context.  And what if we had a method like "stock_rotation_item_index"?  Would this be an index of Koha::Items associated with stock rotation, or would this be an index of stockrotationitems?

stockrotationitem - leave it as is.  But this seems inconsistent with the renaming of the modules themselves.

I hope all of this does not come across as pedantic — I guess simply don't know what the right approach is, and I'm not clear whether Koha community has a clear policy on this.  If there is, I'm very happy to adopt that.

If there isn't, like I say, I have a preference for the approach I've taken so far.

Cheers,
Alex

Created attachment 62730 [details] [review]
Use camel case for stockrotation modules.

As requested by Kyle, the last patch implement camel casing for the stockrotation modules.

Alex

Created attachment 62750 [details] [review]
Bug 11897: Add Stock Rotation atomic update schema.

* installer/data/mysql/atomicupdate/stockrot_tables.sql: New file.
* installer/data/mysql/kohastructure.sql (stockrotationrotas)
  (stockrotationstages, stockrotationitems): New tables.
* installer/data/mysql/sysprefs.sql: Add Stockrotation sysprefs.
* installer/data/mysql/userflags.sql: Add Stockrotaiton userflag.
* installer/data/mysql/userpermissions.sql: Add Stockrotation
  userpermissions.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:
  Add Stockrotation menu.

Created attachment 62751 [details] [review]
Bug 11897: DO NOT PUSH: Add stock rotation schema files.

* Koha/Schema/Result/Branch.pm: New file.
* Koha/Schema/Result/Item.pm: New file.
* Koha/Schema/Result/Stockrotationitem.pm: New file.
* Koha/Schema/Result/Stockrotationrota.pm: New file.
* Koha/Schema/Result/Stockrotationstage.pm: New file.

Created attachment 62752 [details] [review]
Bug 11897: DBIX::Class::Ordered in Stockrotationstage.

* Koha/Schema/Result/Stockrotationstage.pm: Load `Ordered`, add
  grouping_column.

Created attachment 62753 [details] [review]
Bug 11897: Add Koha::Objects for Stockrotation module.

* Koha/Stockrotationitem.pm: New file.
* Koha/Stockrotationitems.pm: New file.
* Koha/Stockrotationrota.pm: New file.
* Koha/Stockrotationrotas.pm: New file.
* Koha/Stockrotationstage.pm: New file.
* Koha/Stockrotationstages.pm: New file.
* t/db_dependent/Stockrotationitems.t: New file.
* t/db_dependent/Stockrotationrotas.t: New file.
* t/db_dependent/Stockrotationstages.t: New file.
* Koha/Item.pm (stockrotationitem, add_to_rota, biblio): New
  procedures.
* Koha/Library.pm (stockrotationstages): New procedure.
* t/db_dependent/Items.t: Add tests.
* t/db_dependent/Koha/Libraries.t: Add tests.

Created attachment 62754 [details] [review]
Bug 11897: Add stockrotation cron script.

* misc/cronjobs/stockrotation.pl: New file.

Created attachment 62755 [details] [review]
Bug 11897: Add Utils library for Stockrotation module

* Koha/Util/Stockrotation.pm: new file

Created attachment 62756 [details] [review]
Bug 11897: REST API - add path defs, controller & tests

* Koha/REST/V1/Stage.pm: New file
* api/v1/swagger/paths.json: Add route for stage manipulation
* api/v1/swagger/paths/rotas.json: New file
* t/db_dependent/api/v1/stockrotationstage.t: New file

Created attachment 62757 [details] [review]
Bug 11897: Add JS to provide rota stage manipulation

* koha-tmpl/intranet-tmpl/prog/js/pages/stockrotation.js: new file

Created attachment 62758 [details] [review]
Bug 11897: Add Stockrotation CSS rules

* koha-tmpl/intranet-tmpl/prog/css/staff-global.css: New rules

Created attachment 62759 [details] [review]
Bug 11897: Add Stock Rotation permissions to list

* koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc: Add new
permissions

Created attachment 62760 [details] [review]
Bug 11897: Interface for rotas from Tools menu

* tools/stockrotation.pl: New file
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt: New file
* koha-tmpl/intranet-tmpl/prog/en/includes/stockrotation-toolbar.inc:

Created attachment 62761 [details] [review]
Bug 11897: Interface for rota items from biblio

* catalogue/stockrotation.pl: New file
* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt:
New file

Created attachment 62762 [details] [review]
Bug 11897: Add menu options for Stock Rotation

* koha-tmpl/intranet-tmpl/prog/en/includes/biblio-view-menu.inc: Add
menu option
* koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc: Add menu
option
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt: Add menu
option

Created attachment 62763 [details] [review]
Bug 11897: Bugfixes, unit tests, feedback, documentation.

A (squashed) patch implementing significant changes to many parts of
the module.

Also adds thorough unit tests.

Created attachment 62764 [details] [review]
Bug 11897: Fix style of button links, & form wording.

* koha-tmpl/intranet-tmpl/prog/en/includes/stockrotation-toolbar.inc
* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt

Created attachment 62765 [details] [review]
Bug 11897: Add documentation for the cron script.

* misc/cronjobs/stockrotation.pl: Add documentation.

Created attachment 62766 [details] [review]
Bug 11897: Use camel case in Koha namespace modules.

* Koha/Item.pm (stockrotationitem, add_to_rota): Use camel case.
* Koha/Library.pm (stockrotationstages): Use camel case.
* Koha/REST/V1/Stage.pm (move): Use camel case.
* Koha/StockRotationItem.pm: New name for 'Koha/Stockrotationitem.pm'.
  (stage, advance): Use camel case.
* Koha/StockRotationItems.pm: New name for 'Koha/Stockrotationitems.pm'.
  (object_class): Use camel case.
* Koha/StockRotationRota.pm: New name for 'Koha/Stockrotationrota.pm'.
  (stockrotationstages, stockrotationitems, add_item): Use camel case.
* Koha/StockRotationRotas.pm: New name for 'Koha/Stockrotationrotas.pm'.
  (object_class): Use camel case.
* Koha/StockRotationStage.pm: New name for 'Koha/Stockrotationstage.pm'.
  (stockrotationitems, rota, siblings, next_siblings, previous_siblings)
  (next_sibling, previous_sibling, first_sibling, last_sibling): Use
  camel case.
* Koha/StockRotationStages.pm: New name for 'Koha/Stockrotationstages.pm'.
  (object_class): Use camel case.
* Koha/Util/StockRotation.pm: New name for 'Koha/Util/Stockrotation.pm'.
  (toggle_indemand, move_to_next_stage, remove_from_stage): Use camel case.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:
  Improve wording.
* misc/cronjobs/stockrotation.pl: Use camel case.
* catalogue/stockrotation.pl: Use camel case.
* tools/stockrotation.pl: Use camel case.
* t/db_dependent/Items.t: Use camel case.
* t/db_dependent/Koha/Libraries.t: Use camel case.
* t/db_dependent/StockRotationItems.t: New name for
  't/db_dependent/Stockrotationitems.t' & use camel case.
* t/db_dependent/StockRotationRotas.t: New name for
  't/db_dependent/Stockrotationrotas.t' & use camel case.
* t/db_dependent/StockRotationStages.t: New name for
  't/db_dependent/Stockrotationstages.t' & use camel case.
* t/db_dependent/api/v1/stockrotationstage.t: Use camel case.

Created attachment 62767 [details] [review]
Bug 11897: Bugfix: Handle empty rotas.

* Koha/StockRotationRota.pm (investigate): If current rota has no
  stages, return empty items & log entries in report.

Owen,

Sorry for no apply!  I've rebased and added a tiny fix to an edge case we missed before.  Should apply now.

Alex

Sorry, don't want to break something - tried to resolve the conflicts, but better leaving it up to the author.

Created attachment 63063 [details] [review]
Bug 11897: Add Stock Rotation atomic update schema.

* installer/data/mysql/atomicupdate/stockrot_tables.sql: New file.
* installer/data/mysql/kohastructure.sql (stockrotationrotas)
  (stockrotationstages, stockrotationitems): New tables.
* installer/data/mysql/sysprefs.sql: Add Stockrotation sysprefs.
* installer/data/mysql/userflags.sql: Add Stockrotaiton userflag.
* installer/data/mysql/userpermissions.sql: Add Stockrotation
  userpermissions.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:
  Add Stockrotation menu.

Created attachment 63064 [details] [review]
Bug 11897: DO NOT PUSH: Add stock rotation schema files.

* Koha/Schema/Result/Branch.pm: New file.
* Koha/Schema/Result/Item.pm: New file.
* Koha/Schema/Result/Stockrotationitem.pm: New file.
* Koha/Schema/Result/Stockrotationrota.pm: New file.
* Koha/Schema/Result/Stockrotationstage.pm: New file.

Created attachment 63065 [details] [review]
Bug 11897: DBIX::Class::Ordered in Stockrotationstage.

* Koha/Schema/Result/Stockrotationstage.pm: Load `Ordered`, add
  grouping_column.

Created attachment 63066 [details] [review]
Bug 11897: Add Koha::Objects for Stockrotation module.

* Koha/Stockrotationitem.pm: New file.
* Koha/Stockrotationitems.pm: New file.
* Koha/Stockrotationrota.pm: New file.
* Koha/Stockrotationrotas.pm: New file.
* Koha/Stockrotationstage.pm: New file.
* Koha/Stockrotationstages.pm: New file.
* t/db_dependent/Stockrotationitems.t: New file.
* t/db_dependent/Stockrotationrotas.t: New file.
* t/db_dependent/Stockrotationstages.t: New file.
* Koha/Item.pm (stockrotationitem, add_to_rota, biblio): New
  procedures.
* Koha/Library.pm (stockrotationstages): New procedure.
* t/db_dependent/Items.t: Add tests.
* t/db_dependent/Koha/Libraries.t: Add tests.

Created attachment 63067 [details] [review]
Bug 11897: Add stockrotation cron script.

* misc/cronjobs/stockrotation.pl: New file.

Created attachment 63068 [details] [review]
Bug 11897: Add Utils library for Stockrotation module

* Koha/Util/Stockrotation.pm: new file

Created attachment 63069 [details] [review]
Bug 11897: REST API - add path defs, controller & tests

* Koha/REST/V1/Stage.pm: New file
* api/v1/swagger/paths.json: Add route for stage manipulation
* api/v1/swagger/paths/rotas.json: New file
* t/db_dependent/api/v1/stockrotationstage.t: New file

Created attachment 63070 [details] [review]
Bug 11897: Add JS to provide rota stage manipulation

* koha-tmpl/intranet-tmpl/prog/js/pages/stockrotation.js: new file

Created attachment 63071 [details] [review]
Bug 11897: Add Stockrotation CSS rules

* koha-tmpl/intranet-tmpl/prog/css/staff-global.css: New rules

Created attachment 63072 [details] [review]
Bug 11897: Add Stock Rotation permissions to list

* koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc: Add new
permissions

Created attachment 63073 [details] [review]
Bug 11897: Interface for rotas from Tools menu

* tools/stockrotation.pl: New file
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt: New file
* koha-tmpl/intranet-tmpl/prog/en/includes/stockrotation-toolbar.inc:

Created attachment 63074 [details] [review]
Bug 11897: Interface for rota items from biblio

* catalogue/stockrotation.pl: New file
* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt:
New file

Created attachment 63075 [details] [review]
Bug 11897: Add menu options for Stock Rotation

* koha-tmpl/intranet-tmpl/prog/en/includes/biblio-view-menu.inc: Add
menu option
* koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc: Add menu
option
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt: Add menu
option

Created attachment 63076 [details] [review]
Bug 11897: Bugfixes, unit tests, feedback, documentation.

A (squashed) patch implementing significant changes to many parts of
the module.

Also adds thorough unit tests.

Created attachment 63077 [details] [review]
Bug 11897: Fix style of button links, & form wording.

* koha-tmpl/intranet-tmpl/prog/en/includes/stockrotation-toolbar.inc
* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt

Created attachment 63078 [details] [review]
Bug 11897: Add documentation for the cron script.

* misc/cronjobs/stockrotation.pl: Add documentation.

Created attachment 63079 [details] [review]
Bug 11897: Use camel case in Koha namespace modules.

* Koha/Item.pm (stockrotationitem, add_to_rota): Use camel case.
* Koha/Library.pm (stockrotationstages): Use camel case.
* Koha/REST/V1/Stage.pm (move): Use camel case.
* Koha/StockRotationItem.pm: New name for 'Koha/Stockrotationitem.pm'.
  (stage, advance): Use camel case.
* Koha/StockRotationItems.pm: New name for 'Koha/Stockrotationitems.pm'.
  (object_class): Use camel case.
* Koha/StockRotationRota.pm: New name for 'Koha/Stockrotationrota.pm'.
  (stockrotationstages, stockrotationitems, add_item): Use camel case.
* Koha/StockRotationRotas.pm: New name for 'Koha/Stockrotationrotas.pm'.
  (object_class): Use camel case.
* Koha/StockRotationStage.pm: New name for 'Koha/Stockrotationstage.pm'.
  (stockrotationitems, rota, siblings, next_siblings, previous_siblings)
  (next_sibling, previous_sibling, first_sibling, last_sibling): Use
  camel case.
* Koha/StockRotationStages.pm: New name for 'Koha/Stockrotationstages.pm'.
  (object_class): Use camel case.
* Koha/Util/StockRotation.pm: New name for 'Koha/Util/Stockrotation.pm'.
  (toggle_indemand, move_to_next_stage, remove_from_stage): Use camel case.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:
  Improve wording.
* misc/cronjobs/stockrotation.pl: Use camel case.
* catalogue/stockrotation.pl: Use camel case.
* tools/stockrotation.pl: Use camel case.
* t/db_dependent/Items.t: Use camel case.
* t/db_dependent/Koha/Libraries.t: Use camel case.
* t/db_dependent/StockRotationItems.t: New name for
  't/db_dependent/Stockrotationitems.t' & use camel case.
* t/db_dependent/StockRotationRotas.t: New name for
  't/db_dependent/Stockrotationrotas.t' & use camel case.
* t/db_dependent/StockRotationStages.t: New name for
  't/db_dependent/Stockrotationstages.t' & use camel case.
* t/db_dependent/api/v1/stockrotationstage.t: Use camel case.

Created attachment 63080 [details] [review]
Bug 11897: Bugfix: Handle empty rotas.

* Koha/StockRotationRota.pm (investigate): If current rota has no
  stages, return empty items & log entries in report.

(In reply to Katrin Fischer from comment #79)
> Sorry, don't want to break something - tried to resolve the conflicts, but
> better leaving it up to the author.

Ah, fast moving pre-release Koha :-)

Should apply now.

Thanks for trying!

Alex

Created attachment 63114 [details] [review]
Bug 11897: Follow up: Update userflags/permission id.

* installer/data/mysql/atomicupdate/stockrot_tables.sql: Set the ID
  for permissions and userflags to 22.

Just tested now and have some minor comments: 

1) I was able to go to Stock rotation tool even when the StockRotation preference was n't enabled...

2) All the actions buttons should be smaller - we use btn-xs class everywhere  - should be consistant I think

Setting failed because of 1), the 2) could be discussed - Owen, do you have same feeling?

Created attachment 64850 [details] [review]
Bug 11897: Follow up: Use "btn-xs" instead of "btn-sm".

* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt:
  Replace "btn-sm" with "btn-xs".
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt:
  Replace "btn-sm" with "btn-xs".

Created attachment 64851 [details] [review]
Bug 11897: Follow up: Check syspref before stockrotation access.

* koha-tmpl/intranet-tmpl/prog/en/includes/biblio-view-menu.inc: Add
  check for syspref.
* koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc: Add check
  for syspref.
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt: Add
  check for syspref.
* tools/stockrotation.pl: Add check for syspref.
* catalogue/stockrotation.pl: Add check for syspref.

Hi Josef,

Thank you very much for testing — and apologies for not getting right on this.  I know how hard it is to context switch back and forth to a complex bug like this.

(In reply to Josef Moravec from comment #100)
> Just tested now and have some minor comments: 
> 
> 1) I was able to go to Stock rotation tool even when the StockRotation
> preference was n't enabled...

This is now fixed by adding checks to the various .tt files (to make the links only appear when appropriate), and by adding redirects to .pl files if the syspref is switched off.

> 2) All the actions buttons should be smaller - we use btn-xs class
> everywhere  - should be consistant I think

I have changed the buttons to btn-xs now.

> Setting failed because of 1), the 2) could be discussed - Owen, do you have
> same feeling?

Please could you test again / sign off if you believe this was the final blocker from your side?

Best wishes,

Alex

Created attachment 65715 [details] [review]
Bug 11897: Add Stock Rotation atomic update schema.

* installer/data/mysql/atomicupdate/stockrot_tables.sql: New file.
* installer/data/mysql/kohastructure.sql (stockrotationrotas)
  (stockrotationstages, stockrotationitems): New tables.
* installer/data/mysql/sysprefs.sql: Add Stockrotation sysprefs.
* installer/data/mysql/userflags.sql: Add Stockrotaiton userflag.
* installer/data/mysql/userpermissions.sql: Add Stockrotation
  userpermissions.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:
  Add Stockrotation menu.

Created attachment 65716 [details] [review]
Bug 11897: DO NOT PUSH: Add stock rotation schema files.

* Koha/Schema/Result/Branch.pm: New file.
* Koha/Schema/Result/Item.pm: New file.
* Koha/Schema/Result/Stockrotationitem.pm: New file.
* Koha/Schema/Result/Stockrotationrota.pm: New file.
* Koha/Schema/Result/Stockrotationstage.pm: New file.

Created attachment 65717 [details] [review]
Bug 11897: DBIX::Class::Ordered in Stockrotationstage.

* Koha/Schema/Result/Stockrotationstage.pm: Load `Ordered`, add
  grouping_column.

Created attachment 65718 [details] [review]
Bug 11897: Add Koha::Objects for Stockrotation module.

* Koha/Stockrotationitem.pm: New file.
* Koha/Stockrotationitems.pm: New file.
* Koha/Stockrotationrota.pm: New file.
* Koha/Stockrotationrotas.pm: New file.
* Koha/Stockrotationstage.pm: New file.
* Koha/Stockrotationstages.pm: New file.
* t/db_dependent/Stockrotationitems.t: New file.
* t/db_dependent/Stockrotationrotas.t: New file.
* t/db_dependent/Stockrotationstages.t: New file.
* Koha/Item.pm (stockrotationitem, add_to_rota, biblio): New
  procedures.
* Koha/Library.pm (stockrotationstages): New procedure.
* t/db_dependent/Items.t: Add tests.
* t/db_dependent/Koha/Libraries.t: Add tests.

Created attachment 65719 [details] [review]
Bug 11897: Add stockrotation cron script.

* misc/cronjobs/stockrotation.pl: New file.

Created attachment 65720 [details] [review]
Bug 11897: Add Utils library for Stockrotation module

* Koha/Util/Stockrotation.pm: new file

Created attachment 65721 [details] [review]
Bug 11897: REST API - add path defs, controller & tests

* Koha/REST/V1/Stage.pm: New file
* api/v1/swagger/paths.json: Add route for stage manipulation
* api/v1/swagger/paths/rotas.json: New file
* t/db_dependent/api/v1/stockrotationstage.t: New file

Created attachment 65722 [details] [review]
Bug 11897: Add JS to provide rota stage manipulation

* koha-tmpl/intranet-tmpl/prog/js/pages/stockrotation.js: new file

Created attachment 65723 [details] [review]
Bug 11897: Add Stockrotation CSS rules

* koha-tmpl/intranet-tmpl/prog/css/staff-global.css: New rules

Created attachment 65724 [details] [review]
Bug 11897: Add Stock Rotation permissions to list

* koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc: Add new
permissions

Created attachment 65725 [details] [review]
Bug 11897: Interface for rotas from Tools menu

* tools/stockrotation.pl: New file
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt: New file
* koha-tmpl/intranet-tmpl/prog/en/includes/stockrotation-toolbar.inc:

Created attachment 65726 [details] [review]
Bug 11897: Interface for rota items from biblio

* catalogue/stockrotation.pl: New file
* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt:
New file

Created attachment 65727 [details] [review]
Bug 11897: Add menu options for Stock Rotation

* koha-tmpl/intranet-tmpl/prog/en/includes/biblio-view-menu.inc: Add
menu option
* koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc: Add menu
option
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt: Add menu
option

Created attachment 65728 [details] [review]
Bug 11897: Bugfixes, unit tests, feedback, documentation.

A (squashed) patch implementing significant changes to many parts of
the module.

Also adds thorough unit tests.

Created attachment 65729 [details] [review]
Bug 11897: Fix style of button links, & form wording.

* koha-tmpl/intranet-tmpl/prog/en/includes/stockrotation-toolbar.inc
* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt

Created attachment 65730 [details] [review]
Bug 11897: Add documentation for the cron script.

* misc/cronjobs/stockrotation.pl: Add documentation.

Created attachment 65731 [details] [review]
Bug 11897: Use camel case in Koha namespace modules.

* Koha/Item.pm (stockrotationitem, add_to_rota): Use camel case.
* Koha/Library.pm (stockrotationstages): Use camel case.
* Koha/REST/V1/Stage.pm (move): Use camel case.
* Koha/StockRotationItem.pm: New name for 'Koha/Stockrotationitem.pm'.
  (stage, advance): Use camel case.
* Koha/StockRotationItems.pm: New name for 'Koha/Stockrotationitems.pm'.
  (object_class): Use camel case.
* Koha/StockRotationRota.pm: New name for 'Koha/Stockrotationrota.pm'.
  (stockrotationstages, stockrotationitems, add_item): Use camel case.
* Koha/StockRotationRotas.pm: New name for 'Koha/Stockrotationrotas.pm'.
  (object_class): Use camel case.
* Koha/StockRotationStage.pm: New name for 'Koha/Stockrotationstage.pm'.
  (stockrotationitems, rota, siblings, next_siblings, previous_siblings)
  (next_sibling, previous_sibling, first_sibling, last_sibling): Use
  camel case.
* Koha/StockRotationStages.pm: New name for 'Koha/Stockrotationstages.pm'.
  (object_class): Use camel case.
* Koha/Util/StockRotation.pm: New name for 'Koha/Util/Stockrotation.pm'.
  (toggle_indemand, move_to_next_stage, remove_from_stage): Use camel case.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:
  Improve wording.
* misc/cronjobs/stockrotation.pl: Use camel case.
* catalogue/stockrotation.pl: Use camel case.
* tools/stockrotation.pl: Use camel case.
* t/db_dependent/Items.t: Use camel case.
* t/db_dependent/Koha/Libraries.t: Use camel case.
* t/db_dependent/StockRotationItems.t: New name for
  't/db_dependent/Stockrotationitems.t' & use camel case.
* t/db_dependent/StockRotationRotas.t: New name for
  't/db_dependent/Stockrotationrotas.t' & use camel case.
* t/db_dependent/StockRotationStages.t: New name for
  't/db_dependent/Stockrotationstages.t' & use camel case.
* t/db_dependent/api/v1/stockrotationstage.t: Use camel case.

Created attachment 65732 [details] [review]
Bug 11897: Bugfix: Handle empty rotas.

* Koha/StockRotationRota.pm (investigate): If current rota has no
  stages, return empty items & log entries in report.

Created attachment 65733 [details] [review]
Bug 11897: Follow up: Update userflags/permission id.

* installer/data/mysql/atomicupdate/stockrot_tables.sql: Set the ID
  for permissions and userflags to 22.

Created attachment 65734 [details] [review]
Bug 11897: Follow up: Use "btn-xs" instead of "btn-sm".

* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt:
  Replace "btn-sm" with "btn-xs".
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt:
  Replace "btn-sm" with "btn-xs".

Created attachment 65735 [details] [review]
Bug 11897: Follow up: Check syspref before stockrotation access.

* koha-tmpl/intranet-tmpl/prog/en/includes/biblio-view-menu.inc: Add
  check for syspref.
* koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc: Add check
  for syspref.
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt: Add
  check for syspref.
* tools/stockrotation.pl: Add check for syspref.
* catalogue/stockrotation.pl: Add check for syspref.

Created attachment 65736 [details] [review]
Bug 11897: Move from ::Swagger to ::OpenAPI.

* Koha/REST/V1/Stage.pm
* api/v1/swagger/paths/rotas.json

This has today been rebased on master and is now compatible with the new OpenAPI API schema from patch 18137.

Best wishes,

Alex

Created attachment 65737 [details] [review]
Bug 11897: Add Stock Rotation atomic update schema.

* installer/data/mysql/atomicupdate/stockrot_tables.sql: New file.
* installer/data/mysql/kohastructure.sql (stockrotationrotas)
  (stockrotationstages, stockrotationitems): New tables.
* installer/data/mysql/sysprefs.sql: Add Stockrotation sysprefs.
* installer/data/mysql/userflags.sql: Add Stockrotaiton userflag.
* installer/data/mysql/userpermissions.sql: Add Stockrotation
  userpermissions.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:
  Add Stockrotation menu.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65738 [details] [review]
Bug 11897: DO NOT PUSH: Add stock rotation schema files.

* Koha/Schema/Result/Branch.pm: New file.
* Koha/Schema/Result/Item.pm: New file.
* Koha/Schema/Result/Stockrotationitem.pm: New file.
* Koha/Schema/Result/Stockrotationrota.pm: New file.
* Koha/Schema/Result/Stockrotationstage.pm: New file.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65739 [details] [review]
Bug 11897: DBIX::Class::Ordered in Stockrotationstage.

* Koha/Schema/Result/Stockrotationstage.pm: Load `Ordered`, add
  grouping_column.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65740 [details] [review]
Bug 11897: Add Koha::Objects for Stockrotation module.

* Koha/Stockrotationitem.pm: New file.
* Koha/Stockrotationitems.pm: New file.
* Koha/Stockrotationrota.pm: New file.
* Koha/Stockrotationrotas.pm: New file.
* Koha/Stockrotationstage.pm: New file.
* Koha/Stockrotationstages.pm: New file.
* t/db_dependent/Stockrotationitems.t: New file.
* t/db_dependent/Stockrotationrotas.t: New file.
* t/db_dependent/Stockrotationstages.t: New file.
* Koha/Item.pm (stockrotationitem, add_to_rota, biblio): New
  procedures.
* Koha/Library.pm (stockrotationstages): New procedure.
* t/db_dependent/Items.t: Add tests.
* t/db_dependent/Koha/Libraries.t: Add tests.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65741 [details] [review]
Bug 11897: Add stockrotation cron script.

* misc/cronjobs/stockrotation.pl: New file.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65742 [details] [review]
Bug 11897: Add Utils library for Stockrotation module

* Koha/Util/Stockrotation.pm: new file

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65743 [details] [review]
Bug 11897: REST API - add path defs, controller & tests

* Koha/REST/V1/Stage.pm: New file
* api/v1/swagger/paths.json: Add route for stage manipulation
* api/v1/swagger/paths/rotas.json: New file
* t/db_dependent/api/v1/stockrotationstage.t: New file

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65744 [details] [review]
Bug 11897: Add JS to provide rota stage manipulation

* koha-tmpl/intranet-tmpl/prog/js/pages/stockrotation.js: new file

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65745 [details] [review]
Bug 11897: Add Stockrotation CSS rules

* koha-tmpl/intranet-tmpl/prog/css/staff-global.css: New rules

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65746 [details] [review]
Bug 11897: Add Stock Rotation permissions to list

* koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc: Add new
permissions

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65747 [details] [review]
Bug 11897: Interface for rotas from Tools menu

* tools/stockrotation.pl: New file
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt: New file
* koha-tmpl/intranet-tmpl/prog/en/includes/stockrotation-toolbar.inc:

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65748 [details] [review]
Bug 11897: Interface for rota items from biblio

* catalogue/stockrotation.pl: New file
* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt:
New file

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65749 [details] [review]
Bug 11897: Add menu options for Stock Rotation

* koha-tmpl/intranet-tmpl/prog/en/includes/biblio-view-menu.inc: Add
menu option
* koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc: Add menu
option
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt: Add menu
option

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65750 [details] [review]
Bug 11897: Bugfixes, unit tests, feedback, documentation.

A (squashed) patch implementing significant changes to many parts of
the module.

Also adds thorough unit tests.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65751 [details] [review]
Bug 11897: Fix style of button links, & form wording.

* koha-tmpl/intranet-tmpl/prog/en/includes/stockrotation-toolbar.inc
* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65752 [details] [review]
Bug 11897: Add documentation for the cron script.

* misc/cronjobs/stockrotation.pl: Add documentation.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65753 [details] [review]
Bug 11897: Use camel case in Koha namespace modules.

* Koha/Item.pm (stockrotationitem, add_to_rota): Use camel case.
* Koha/Library.pm (stockrotationstages): Use camel case.
* Koha/REST/V1/Stage.pm (move): Use camel case.
* Koha/StockRotationItem.pm: New name for 'Koha/Stockrotationitem.pm'.
  (stage, advance): Use camel case.
* Koha/StockRotationItems.pm: New name for 'Koha/Stockrotationitems.pm'.
  (object_class): Use camel case.
* Koha/StockRotationRota.pm: New name for 'Koha/Stockrotationrota.pm'.
  (stockrotationstages, stockrotationitems, add_item): Use camel case.
* Koha/StockRotationRotas.pm: New name for 'Koha/Stockrotationrotas.pm'.
  (object_class): Use camel case.
* Koha/StockRotationStage.pm: New name for 'Koha/Stockrotationstage.pm'.
  (stockrotationitems, rota, siblings, next_siblings, previous_siblings)
  (next_sibling, previous_sibling, first_sibling, last_sibling): Use
  camel case.
* Koha/StockRotationStages.pm: New name for 'Koha/Stockrotationstages.pm'.
  (object_class): Use camel case.
* Koha/Util/StockRotation.pm: New name for 'Koha/Util/Stockrotation.pm'.
  (toggle_indemand, move_to_next_stage, remove_from_stage): Use camel case.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:
  Improve wording.
* misc/cronjobs/stockrotation.pl: Use camel case.
* catalogue/stockrotation.pl: Use camel case.
* tools/stockrotation.pl: Use camel case.
* t/db_dependent/Items.t: Use camel case.
* t/db_dependent/Koha/Libraries.t: Use camel case.
* t/db_dependent/StockRotationItems.t: New name for
  't/db_dependent/Stockrotationitems.t' & use camel case.
* t/db_dependent/StockRotationRotas.t: New name for
  't/db_dependent/Stockrotationrotas.t' & use camel case.
* t/db_dependent/StockRotationStages.t: New name for
  't/db_dependent/Stockrotationstages.t' & use camel case.
* t/db_dependent/api/v1/stockrotationstage.t: Use camel case.

https://bugs.koha-community.org/show_bug.cgi?id=11897

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65754 [details] [review]
Bug 11897: Bugfix: Handle empty rotas.

* Koha/StockRotationRota.pm (investigate): If current rota has no
  stages, return empty items & log entries in report.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65755 [details] [review]
Bug 11897: Follow up: Update userflags/permission id.

* installer/data/mysql/atomicupdate/stockrot_tables.sql: Set the ID
  for permissions and userflags to 22.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65756 [details] [review]
Bug 11897: Follow up: Use "btn-xs" instead of "btn-sm".

* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt:
  Replace "btn-sm" with "btn-xs".
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt:
  Replace "btn-sm" with "btn-xs".

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65757 [details] [review]
Bug 11897: Follow up: Check syspref before stockrotation access.

* koha-tmpl/intranet-tmpl/prog/en/includes/biblio-view-menu.inc: Add
  check for syspref.
* koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc: Add check
  for syspref.
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt: Add
  check for syspref.
* tools/stockrotation.pl: Add check for syspref.
* catalogue/stockrotation.pl: Add check for syspref.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 65758 [details] [review]
Bug 11897: Move from ::Swagger to ::OpenAPI.

* Koha/REST/V1/Stage.pm
* api/v1/swagger/paths/rotas.json

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Signed off by customer using system in production environment.

Ready for QA.

Alex

Hi guys, I've been looking at this patchset without trying it yet. Everything looks as expected :-D
The only thing I would like to mention is that the auth chain added a check for maintenance mode (usually on upgrades) that requires some more error codes to be added to the spec. Can you please take a look at (say) patrons.json from bug 18137? It's pretty straightforward.

Created attachment 66381 [details] [review]
Bug 11897: Follow up: Add more error conditions.

* api/v1/swagger/paths/rotas.json: Add 401, 500 & 503 error
  conditions.

Hi Tomás,

(In reply to Tomás Cohen Arazi from comment #150)
> Hi guys, I've been looking at this patchset without trying it yet.
> Everything looks as expected :-D
> The only thing I would like to mention is that the auth chain added a check
> for maintenance mode (usually on upgrades) that requires some more error
> codes to be added to the spec. Can you please take a look at (say)
> patrons.json from bug 18137? It's pretty straightforward.

I believe I have added a patch now that implements what you requested.  Feel free to let me know if you think I missed something!

Alex

Created attachment 70496 [details] [review]
Bug 11897: Add Stock Rotation atomic update schema.

* installer/data/mysql/atomicupdate/stockrot_tables.sql: New file.
* installer/data/mysql/kohastructure.sql (stockrotationrotas)
  (stockrotationstages, stockrotationitems): New tables.
* installer/data/mysql/sysprefs.sql: Add Stockrotation sysprefs.
* installer/data/mysql/userflags.sql: Add Stockrotaiton userflag.
* installer/data/mysql/userpermissions.sql: Add Stockrotation
  userpermissions.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:
  Add Stockrotation menu.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70497 [details] [review]
Bug 11897: Add Stock Rotation atomic update schema.

* installer/data/mysql/atomicupdate/stockrot_tables.sql: New file.
* installer/data/mysql/kohastructure.sql (stockrotationrotas)
  (stockrotationstages, stockrotationitems): New tables.
* installer/data/mysql/sysprefs.sql: Add Stockrotation sysprefs.
* installer/data/mysql/userflags.sql: Add Stockrotaiton userflag.
* installer/data/mysql/userpermissions.sql: Add Stockrotation
  userpermissions.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:
  Add Stockrotation menu.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70498 [details] [review]
Bug 11897: DO NOT PUSH: Add stock rotation schema files.

* Koha/Schema/Result/Branch.pm: New file.
* Koha/Schema/Result/Item.pm: New file.
* Koha/Schema/Result/Stockrotationitem.pm: New file.
* Koha/Schema/Result/Stockrotationrota.pm: New file.
* Koha/Schema/Result/Stockrotationstage.pm: New file.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70499 [details] [review]
Bug 11897: DBIX::Class::Ordered in Stockrotationstage.

* Koha/Schema/Result/Stockrotationstage.pm: Load `Ordered`, add
  grouping_column.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70500 [details] [review]
Bug 11897: Add Koha::Objects for Stockrotation module.

* Koha/Stockrotationitem.pm: New file.
* Koha/Stockrotationitems.pm: New file.
* Koha/Stockrotationrota.pm: New file.
* Koha/Stockrotationrotas.pm: New file.
* Koha/Stockrotationstage.pm: New file.
* Koha/Stockrotationstages.pm: New file.
* t/db_dependent/Stockrotationitems.t: New file.
* t/db_dependent/Stockrotationrotas.t: New file.
* t/db_dependent/Stockrotationstages.t: New file.
* Koha/Item.pm (stockrotationitem, add_to_rota, biblio): New
  procedures.
* Koha/Library.pm (stockrotationstages): New procedure.
* t/db_dependent/Items.t: Add tests.
* t/db_dependent/Koha/Libraries.t: Add tests.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70501 [details] [review]
Bug 11897: Add stockrotation cron script.

* misc/cronjobs/stockrotation.pl: New file.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70502 [details] [review]
Bug 11897: Add Utils library for Stockrotation module

* Koha/Util/Stockrotation.pm: new file

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70503 [details] [review]
Bug 11897: REST API - add path defs, controller & tests

* Koha/REST/V1/Stage.pm: New file
* api/v1/swagger/paths.json: Add route for stage manipulation
* api/v1/swagger/paths/rotas.json: New file
* t/db_dependent/api/v1/stockrotationstage.t: New file

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70504 [details] [review]
Bug 11897: Add JS to provide rota stage manipulation

* koha-tmpl/intranet-tmpl/prog/js/pages/stockrotation.js: new file

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70505 [details] [review]
Bug 11897: Add Stockrotation CSS rules

* koha-tmpl/intranet-tmpl/prog/css/staff-global.css: New rules

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70506 [details] [review]
Bug 11897: Add Stock Rotation permissions to list

* koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc: Add new
permissions

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70507 [details] [review]
Bug 11897: Interface for rotas from Tools menu

* tools/stockrotation.pl: New file
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt: New file
* koha-tmpl/intranet-tmpl/prog/en/includes/stockrotation-toolbar.inc:

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70508 [details] [review]
Bug 11897: Interface for rota items from biblio

* catalogue/stockrotation.pl: New file
* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt:
New file

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70509 [details] [review]
Bug 11897: Add menu options for Stock Rotation

* koha-tmpl/intranet-tmpl/prog/en/includes/biblio-view-menu.inc: Add
menu option
* koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc: Add menu
option
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt: Add menu
option

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70510 [details] [review]
Bug 11897: Bugfixes, unit tests, feedback, documentation.

A (squashed) patch implementing significant changes to many parts of
the module.

Also adds thorough unit tests.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70511 [details] [review]
Bug 11897: Fix style of button links, & form wording.

* koha-tmpl/intranet-tmpl/prog/en/includes/stockrotation-toolbar.inc
* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70512 [details] [review]
Bug 11897: Add documentation for the cron script.

* misc/cronjobs/stockrotation.pl: Add documentation.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70513 [details] [review]
Bug 11897: Use camel case in Koha namespace modules.

* Koha/Item.pm (stockrotationitem, add_to_rota): Use camel case.
* Koha/Library.pm (stockrotationstages): Use camel case.
* Koha/REST/V1/Stage.pm (move): Use camel case.
* Koha/StockRotationItem.pm: New name for 'Koha/Stockrotationitem.pm'.
  (stage, advance): Use camel case.
* Koha/StockRotationItems.pm: New name for 'Koha/Stockrotationitems.pm'.
  (object_class): Use camel case.
* Koha/StockRotationRota.pm: New name for 'Koha/Stockrotationrota.pm'.
  (stockrotationstages, stockrotationitems, add_item): Use camel case.
* Koha/StockRotationRotas.pm: New name for 'Koha/Stockrotationrotas.pm'.
  (object_class): Use camel case.
* Koha/StockRotationStage.pm: New name for 'Koha/Stockrotationstage.pm'.
  (stockrotationitems, rota, siblings, next_siblings, previous_siblings)
  (next_sibling, previous_sibling, first_sibling, last_sibling): Use
  camel case.
* Koha/StockRotationStages.pm: New name for 'Koha/Stockrotationstages.pm'.
  (object_class): Use camel case.
* Koha/Util/StockRotation.pm: New name for 'Koha/Util/Stockrotation.pm'.
  (toggle_indemand, move_to_next_stage, remove_from_stage): Use camel case.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:
  Improve wording.
* misc/cronjobs/stockrotation.pl: Use camel case.
* catalogue/stockrotation.pl: Use camel case.
* tools/stockrotation.pl: Use camel case.
* t/db_dependent/Items.t: Use camel case.
* t/db_dependent/Koha/Libraries.t: Use camel case.
* t/db_dependent/StockRotationItems.t: New name for
  't/db_dependent/Stockrotationitems.t' & use camel case.
* t/db_dependent/StockRotationRotas.t: New name for
  't/db_dependent/Stockrotationrotas.t' & use camel case.
* t/db_dependent/StockRotationStages.t: New name for
  't/db_dependent/Stockrotationstages.t' & use camel case.
* t/db_dependent/api/v1/stockrotationstage.t: Use camel case.

https://bugs.koha-community.org/show_bug.cgi?id=11897

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70514 [details] [review]
Bug 11897: Bugfix: Handle empty rotas.

* Koha/StockRotationRota.pm (investigate): If current rota has no
  stages, return empty items & log entries in report.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70515 [details] [review]
Bug 11897: Follow up: Update userflags/permission id.

* installer/data/mysql/atomicupdate/stockrot_tables.sql: Set the ID
  for permissions and userflags to 22.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70516 [details] [review]
Bug 11897: Follow up: Use "btn-xs" instead of "btn-sm".

* koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/stockrotation.tt:
  Replace "btn-sm" with "btn-xs".
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/stockrotation.tt:
  Replace "btn-sm" with "btn-xs".

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70517 [details] [review]
Bug 11897: Follow up: Check syspref before stockrotation access.

* koha-tmpl/intranet-tmpl/prog/en/includes/biblio-view-menu.inc: Add
  check for syspref.
* koha-tmpl/intranet-tmpl/prog/en/includes/tools-menu.inc: Add check
  for syspref.
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt: Add
  check for syspref.
* tools/stockrotation.pl: Add check for syspref.
* catalogue/stockrotation.pl: Add check for syspref.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70518 [details] [review]
Bug 11897: Move from ::Swagger to ::OpenAPI.

* Koha/REST/V1/Stage.pm
* api/v1/swagger/paths/rotas.json

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70519 [details] [review]
Bug 11897: Follow up: Add more error conditions.

* api/v1/swagger/paths/rotas.json: Add 401, 500 & 503 error
  conditions.

Created attachment 70520 [details] [review]
Bug 11897: Rebase bugfixes.

* api/v1/swagger/paths.json: Fix missing commas.
* api/v1/swagger/paths/rotas.json: Fix missing commas.
* installer/data/mysql/atomicupdate/stockrot_tables.sql: Up perms
  counter to 23.
* installer/data/mysql/userpermissions.sql: Up perms counter to 23.
* t/db_dependent/StockRotationItems.t: Fix builder semantics.

Created attachment 70521 [details] [review]
Bug 11897: Add Stock Rotation atomic update schema.

* installer/data/mysql/atomicupdate/stockrot_tables.sql: New file.
* installer/data/mysql/kohastructure.sql (stockrotationrotas)
  (stockrotationstages, stockrotationitems): New tables.
* installer/data/mysql/sysprefs.sql: Add Stockrotation sysprefs.
* installer/data/mysql/userflags.sql: Add Stockrotaiton userflag.
* installer/data/mysql/userpermissions.sql: Add Stockrotation
  userpermissions.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:
  Add Stockrotation menu.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70522 [details] [review]
Bug 11897: DO NOT PUSH: Add stock rotation schema files.

* Koha/Schema/Result/Branch.pm: New file.
* Koha/Schema/Result/Item.pm: New file.
* Koha/Schema/Result/Stockrotationitem.pm: New file.
* Koha/Schema/Result/Stockrotationrota.pm: New file.
* Koha/Schema/Result/Stockrotationstage.pm: New file.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70523 [details] [review]
Bug 11897: DBIX::Class::Ordered in Stockrotationstage.

* Koha/Schema/Result/Stockrotationstage.pm: Load `Ordered`, add
  grouping_column.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 70524 [details] [review]
Bug 11897: Add bulk of Stockrotation module.

This commit replaces a set of about 20 commits into one large
commit.  This makes working with the module way easier from bugzilla.

As it stands the patches on this bug provide a fully functioning stock
rotation feature set for Koha.

All previous patches that added functionality were signed off, so I'm
retaining that flag.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

I've tried to get these patches to apply, but I'd like a proper rebase. 

1) Database update doesn't work
DEV atomic update: stockrot_tables.sql
C4::Installer::load_sql returned the following errors while attempting to load /home/vagrant/kohaclone/installer/data/mysql/atomicupdate/stockrot_tables.sql:

Will continue comments using splinter.

Comment on attachment 70521 [details] [review]
Bug 11897: Add Stock Rotation atomic update schema.

Review of attachment 70521 [details] [review]:
-----------------------------------------------------------------

::: installer/data/mysql/atomicupdate/stockrot_tables.sql
@@ +6,5 @@
> +    description text NOT NULL default '',   -- Description for this rota
> +    cyclical tinyint(1) NOT NULL default 0, -- Should items on this rota keep cycling?
> +    active tinyint(1) NOT NULL default 0,   -- Is this rota currently active?
> +    PRIMARY KEY (`rota_id`)
> +) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;

2) Should now be: ... ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; (see bug 18336)

::: installer/data/mysql/kohastructure.sql
@@ +4156,5 @@
> +    description text NOT NULL default '',   -- Description for this rota
> +    cyclical tinyint(1) NOT NULL default 0, -- Should items on this rota keep cycling?
> +    active tinyint(1) NOT NULL default 0,   -- Is this rota currently active?
> +    PRIMARY KEY (`rota_id`)
> +) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;

3) Same here as above:
ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;

::: installer/data/mysql/sysprefs.sql
@@ +601,5 @@
>  ('XSLTResultsDisplay','default','','Enable XSL stylesheet control over results page display on intranet','Free'),
>  ('z3950AuthorAuthFields','701,702,700',NULL,'Define the MARC biblio fields for Personal Name Authorities to fill biblio.author','free'),
> +('z3950NormalizeAuthor','0','','If ON, Personal Name Authorities will replace authors in biblio.author','YesNo'),
> +('StockRotation','0',NULL,'If ON, enables the stock rotation module','YesNo'),
> +('RotationPreventTransfers','0',NULL,'If ON, prevent any transfers for items on stock rotation rotas, except for stock rotation transfers','YesNo')

4) Preferences in this file are sorted alphabetically, please move into the right spot.

::: koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc
@@ +21,4 @@
>      [%- CASE 'lists' -%]<span>Lists</span>
>      [%- CASE 'clubs' -%]<span>Patron clubs</span>
>      [%- CASE 'ill' -%]<span>Create and modify Interlibrary loan requests</span>
> +    [%- CASE 'stockrotation' -%]<span>Manage stockrotation operations</span>

5) Descriptions for the sub-permissions appear to be  missing?

::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
@@ +879,5 @@
> +            - pref: RotationPreventTransfers
> +              choices:
> +                  yes: Enable
> +                  no: Disable
> +            - "Prevent branchtransfers on items in stockrotation rotas"

6) These preferences don't form sentences and are not nicely formatted. Please avoid use of 'branch' in favor of 'library'. Also, should it be stock rotation or stockrotation? Both are used here. It should probably be something like:

- yes: Enable
- no: Disable
- "library transfers on items in stockrotation rotas."

Comment on attachment 70524 [details] [review]
Bug 11897: Add bulk of Stockrotation module.

Review of attachment 70524 [details] [review]:
-----------------------------------------------------------------

::: 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&amp;modules=CATALOGUING&amp;action=MODIFY&amp;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 &rsaquo; Catalog &rsaquo; 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 &rsaquo; 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?

Created attachment 72124 [details] [review]
Bug 11897: Add Stock Rotation atomic update schema.

* installer/data/mysql/atomicupdate/stockrot_tables.sql: New file.
* installer/data/mysql/kohastructure.sql (stockrotationrotas)
  (stockrotationstages, stockrotationitems): New tables.
* installer/data/mysql/sysprefs.sql: Add Stockrotation sysprefs.
* installer/data/mysql/userflags.sql: Add Stockrotaiton userflag.
* installer/data/mysql/userpermissions.sql: Add Stockrotation
  userpermissions.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:
  Add Stockrotation menu.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 72125 [details] [review]
Bug 11897: DO NOT PUSH: Add stock rotation schema files.

* Koha/Schema/Result/Branch.pm: New file.
* Koha/Schema/Result/Item.pm: New file.
* Koha/Schema/Result/Stockrotationitem.pm: New file.
* Koha/Schema/Result/Stockrotationrota.pm: New file.
* Koha/Schema/Result/Stockrotationstage.pm: New file.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 72126 [details] [review]
Bug 11897: DBIX::Class::Ordered in Stockrotationstage.

* Koha/Schema/Result/Stockrotationstage.pm: Load `Ordered`, add
  grouping_column.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 72127 [details] [review]
Bug 11897: Add bulk of Stockrotation module.

This commit replaces a set of about 20 commits into one large
commit.  This makes working with the module way easier from bugzilla.

As it stands the patches on this bug provide a fully functioning stock
rotation feature set for Koha.

All previous patches that added functionality were signed off, so I'm
retaining that flag.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 72128 [details] [review]
Bug 11897: (QA Followup) Implement review feedback.

- Update Table declarations
- Add permissions checks
- Remove unused code
- Fix language & naming
- Make templates translation friendly

Hi Cait,

Thank you for the review — much appreciated!

I have now implemented all your recommendations and further improved the POD for the Stockrotation API.

There are two caveats:
- I have not modified the API file (outside of POD) as it works as is?
- I have not implemented notice template based report for the stockrotation cronscript.

For the latter, to be honest, I'm in over my head.  If you feel that is a blocker, then perhaps someone who has worked with the templates before can point me in the right direction?

Alternatively, patches / follow-up bugs greatly appreciated!

I have rebased — this should apply cleanly on master.

Let me know if you run into problems!

Alex

Hi Alex,

patches now apply and updatedatabase works.

1) QA script returns some errors (have removed the OK parts):

 FAIL	t/db_dependent/Koha/Libraries.t

   FAIL	  valid
		Global symbol "$retrieved_category_2" requires explicit package name 
		Global symbol "$nb_of_categories" requires explicit package name 
		t/db_dependent/Koha/Libraries.t had compilation errors.

 FAIL	Koha/StockRotationStages.pm
   FAIL	  pod coverage
		POD is missing for 'object_class'

 FAIL	Koha/StockRotationRotas.pm
   FAIL	  pod coverage
		POD is missing for 'object_class'

 FAIL	Koha/StockRotationItems.pm
   FAIL	  pod coverage
		POD is missing for 'object_class'


 FAIL	Koha/Schema/Result/Stockrotationstage.pm
   FAIL	  pod coverage
		POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666

 FAIL	Koha/Schema/Result/Stockrotationitem.pm
   FAIL	  pod coverage
		POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666

 FAIL	Koha/Schema/Result/Stockrotationrota.pm
   FAIL	  pod coverage
		POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666

Not sure about the last 3, but please take a look.

2) Missing notice template

> I have now implemented all your recommendations and further improved the POD
> for the Stockrotation API.
> 
> There are two caveats:
> - I have not modified the API file (outside of POD) as it works as is?
> - I have not implemented notice template based report for the stockrotation
> cronscript.
> 
> For the latter, to be honest, I'm in over my head.  If you feel that is a
> blocker, then perhaps someone who has worked with the templates before can
> point me in the right direction?

I have thought about it, but I think we should really not introduce a harcoded notice like this as it will be a step back from what we have achieved now with all messages being translatable. I'd suggest starting at the GUI, taking a look at messages that have recently been added, like the article request ones and see how it was done there maybe? 

3) Documentation

Can you share how the attached PDF was created? Would it be possible to submit a patch to our manual too? (note: we are using Sphinx now) I read it and thought it was very helpful, but I couldn't figure out what 'Add "in demand"' does, can you explain?

Created attachment 74003 [details] [review]
Bug 11897: (QA Followup) Implement review feedback.

- Update Table declarations
- Add permissions checks
- Remove unused code
- Fix language & naming
- Make templates translation friendly

Created attachment 74004 [details] [review]
Bug 11897: Add Stock Rotation atomic update schema.

* installer/data/mysql/atomicupdate/stockrot_tables.sql: New file.
* installer/data/mysql/kohastructure.sql (stockrotationrotas)
  (stockrotationstages, stockrotationitems): New tables.
* installer/data/mysql/sysprefs.sql: Add Stockrotation sysprefs.
* installer/data/mysql/userflags.sql: Add Stockrotaiton userflag.
* installer/data/mysql/userpermissions.sql: Add Stockrotation
  userpermissions.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:
  Add Stockrotation menu.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 74005 [details] [review]
Bug 11897: DO NOT PUSH: Add stock rotation schema files.

* Koha/Schema/Result/Branch.pm: New file.
* Koha/Schema/Result/Item.pm: New file.
* Koha/Schema/Result/Stockrotationitem.pm: New file.
* Koha/Schema/Result/Stockrotationrota.pm: New file.
* Koha/Schema/Result/Stockrotationstage.pm: New file.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 74006 [details] [review]
Bug 11897: DBIX::Class::Ordered in Stockrotationstage.

* Koha/Schema/Result/Stockrotationstage.pm: Load `Ordered`, add
  grouping_column.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 74007 [details] [review]
Bug 11897: Add bulk of Stockrotation module.

This commit replaces a set of about 20 commits into one large
commit.  This makes working with the module way easier from bugzilla.

As it stands the patches on this bug provide a fully functioning stock
rotation feature set for Koha.

All previous patches that added functionality were signed off, so I'm
retaining that flag.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 74008 [details] [review]
Bug 11897: (QA Followup) Implement review feedback.

- Update Table declarations
- Add permissions checks
- Remove unused code
- Fix language & naming
- Make templates translation friendly

Just a rebase.. now onto addressing the QA issues.

Created attachment 74010 [details] [review]
Bug 11897: Add Stock Rotation atomic update schema.

* installer/data/mysql/atomicupdate/stockrot_tables.sql: New file.
* installer/data/mysql/kohastructure.sql (stockrotationrotas)
  (stockrotationstages, stockrotationitems): New tables.
* installer/data/mysql/sysprefs.sql: Add Stockrotation sysprefs.
* installer/data/mysql/userflags.sql: Add Stockrotaiton userflag.
* installer/data/mysql/userpermissions.sql: Add Stockrotation
  userpermissions.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref:
  Add Stockrotation menu.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>

Created attachment 74011 [details] [review]
Bug 11897: DO NOT PUSH: Add stock rotation schema files.

* Koha/Schema/Result/Branch.pm: New file.
* Koha/Schema/Result/Item.pm: New file.
* Koha/Schema/Result/Stockrotationitem.pm: New file.
* Koha/Schema/Result/Stockrotationrota.pm: New file.
* Koha/Schema/Result/Stockrotationstage.pm: New file.

Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>