Bug 18936 - Move issuingrules into circulation_rules
Summary: Move issuingrules into circulation_rules
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Joonas Kylmälä
URL: https://gitlab.com/joubu/Koha/commits...
Keywords:
: 8858 (view as bug list)
Depends on: 18930
Blocks: 15522 23323 24083 24159 24602 24669 24670 25556 26911 29705 36017
  Show dependency treegraph
 
Reported: 2017-07-13 11:17 UTC by Kyle M Hall
Modified: 2024-02-07 14:40 UTC (History)
13 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
As part of ongoing efforts to simplify and enhance the circulation rules system, the `issuingrules` table has been removed and replaced by a new `circulation_rules` table. Any reports that may have used the issueingrules table will need to be updated to utilise the updated database structure.
Version(s) released in:
20.05.00


Attachments
Bug 18936 - Convert issuingrules fields to circulation_rules (154.00 KB, patch)
2017-07-25 15:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18936: (followup) Fix upgrade step (2.78 KB, patch)
2017-07-25 15:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18936: Fix api/v1/holds.t (1.23 KB, patch)
2017-07-25 15:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18936: (follow-up) Add foreign key and scope enhancement to circ rules (117.02 KB, patch)
2017-10-05 06:15 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936: (follow-up) Update REST definition for /.../kinds (1.50 KB, patch)
2017-10-06 19:22 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936: (follow-up) fix rental odiscount lookup (1.11 KB, patch)
2017-10-17 23:26 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936 - Convert issuingrules fields to circulation_rules (155.73 KB, patch)
2018-01-29 23:08 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936: (followup) Fix upgrade step (2.79 KB, patch)
2018-01-29 23:08 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936: Fix api/v1/holds.t (1.24 KB, patch)
2018-01-29 23:08 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936: (follow-up) Add foreign key and scope enhancement to circ rules (119.40 KB, patch)
2018-01-29 23:08 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936: (follow-up) Update REST definition for /.../kinds (1.50 KB, patch)
2018-01-29 23:08 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936: (follow-up) fix rental odiscount lookup (1.11 KB, patch)
2018-01-29 23:08 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936: (follow-up) Remove issuingrules from kohastructure.sql (3.81 KB, patch)
2018-01-29 23:08 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936: (follow-up) fix tests, null vs. empty behavior for limiting rules (16.96 KB, patch)
2018-01-29 23:08 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936 - Convert issuingrules fields to circulation_rules (155.26 KB, patch)
2018-02-26 19:18 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936: (followup) Fix upgrade step (2.79 KB, patch)
2018-02-26 19:18 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936: Fix api/v1/holds.t (1.22 KB, patch)
2018-02-26 19:18 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936: (follow-up) Add foreign key and scope enhancement to circ rules (120.15 KB, patch)
2018-02-26 19:18 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936: (follow-up) Update REST definition for /.../kinds (1.50 KB, patch)
2018-02-26 19:18 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936: (follow-up) fix rental odiscount lookup (1.11 KB, patch)
2018-02-26 19:18 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936: (follow-up) Remove issuingrules from kohastructure.sql (3.82 KB, patch)
2018-02-26 19:18 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936: (follow-up) fix tests, null vs. empty behavior for limiting rules (16.78 KB, patch)
2018-02-26 19:18 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936: (follow-up) Fix tests, replace old get_onshelfholds_policy method (11.25 KB, patch)
2018-02-26 19:18 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 18936: Convert issuingrules fields to circulation_rules (156.22 KB, patch)
2019-08-04 20:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (followup) Fix upgrade step (2.79 KB, patch)
2019-08-04 20:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix api/v1/holds.t (995 bytes, patch)
2019-08-04 20:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Add foreign key and scope enhancement to circ rules (105.56 KB, patch)
2019-08-04 20:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Update REST definition for /.../kinds (1.50 KB, patch)
2019-08-04 20:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) fix rental odiscount lookup (1.11 KB, patch)
2019-08-04 20:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Remove issuingrules from kohastructure.sql (4.25 KB, patch)
2019-08-04 20:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) fix tests, null vs. empty behavior for limiting rules (9.42 KB, patch)
2019-08-04 20:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Fix tests, replace old get_onshelfholds_policy method (10.46 KB, patch)
2019-08-04 20:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: move get_opacitemholds_policy method (1.57 KB, patch)
2019-08-04 20:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Move guess_article_requestable_itemtypes method (10.74 KB, patch)
2019-08-04 20:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Remove last bit of Koha::IssuingRule (9.18 KB, patch)
2019-08-04 20:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix syntax issue (1.29 KB, patch)
2019-08-04 20:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix several issues (2.14 KB, patch)
2019-08-04 20:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: chargename removed by bug 21753 (658 bytes, patch)
2019-08-04 20:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: new suspension_chargeperiod (4.47 KB, patch)
2019-08-04 20:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: new holds_per_day (801 bytes, patch)
2019-08-04 20:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Replace an other insert into issuingrules (2.30 KB, patch)
2019-08-04 20:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix Circulation.t (817 bytes, patch)
2019-08-04 20:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: More fixes (33.79 KB, patch)
2019-08-04 20:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Add missing filters (9.12 KB, patch)
2019-08-04 20:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix Update DB entry (1.93 KB, patch)
2019-08-04 20:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix Update DB entry (1.80 KB, patch)
2019-08-04 20:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Convert issuingrules fields to circulation_rules (156.73 KB, patch)
2019-10-15 12:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (followup) Fix upgrade step (2.79 KB, patch)
2019-10-15 12:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix api/v1/holds.t (1.01 KB, patch)
2019-10-15 12:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Add foreign key and scope enhancement to circ rules (105.56 KB, patch)
2019-10-15 12:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Update REST definition for /.../kinds (1.50 KB, patch)
2019-10-15 12:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) fix rental odiscount lookup (1.11 KB, patch)
2019-10-15 12:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Remove issuingrules from kohastructure.sql (4.25 KB, patch)
2019-10-15 12:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) fix tests, null vs. empty behavior for limiting rules (9.42 KB, patch)
2019-10-15 12:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Fix tests, replace old get_onshelfholds_policy method (10.46 KB, patch)
2019-10-15 12:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: move get_opacitemholds_policy method (1.57 KB, patch)
2019-10-15 12:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Move guess_article_requestable_itemtypes method (10.77 KB, patch)
2019-10-15 12:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Remove last bit of Koha::IssuingRule (9.20 KB, patch)
2019-10-15 12:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix syntax issue (1.29 KB, patch)
2019-10-15 12:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix several issues (2.14 KB, patch)
2019-10-15 12:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: chargename removed by bug 21753 (658 bytes, patch)
2019-10-15 12:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: new suspension_chargeperiod (4.48 KB, patch)
2019-10-15 12:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: new holds_per_day (801 bytes, patch)
2019-10-15 12:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Replace an other insert into issuingrules (2.30 KB, patch)
2019-10-15 12:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix Circulation.t (817 bytes, patch)
2019-10-15 12:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: More fixes (33.82 KB, patch)
2019-10-15 12:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Add missing filters (8.98 KB, patch)
2019-10-15 12:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix Update DB entry (1.80 KB, patch)
2019-10-15 12:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Convert issuingrules fields to circulation_rules (157.60 KB, patch)
2020-02-03 16:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (followup) Fix upgrade step (2.99 KB, patch)
2020-02-03 16:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix api/v1/holds.t (1.21 KB, patch)
2020-02-03 16:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Add foreign key and scope enhancement to circ rules (105.74 KB, patch)
2020-02-03 16:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Update REST definition for /.../kinds (1.70 KB, patch)
2020-02-03 16:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) fix rental odiscount lookup (1.31 KB, patch)
2020-02-03 16:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Remove issuingrules from kohastructure.sql (4.16 KB, patch)
2020-02-03 16:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) fix tests, null vs. empty behavior for limiting rules (9.59 KB, patch)
2020-02-03 16:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Fix tests, replace old get_onshelfholds_policy method (10.62 KB, patch)
2020-02-03 16:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: move get_opacitemholds_policy method (1.77 KB, patch)
2020-02-03 16:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Move guess_article_requestable_itemtypes method (10.97 KB, patch)
2020-02-03 16:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Remove last bit of Koha::IssuingRule (9.40 KB, patch)
2020-02-03 16:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix syntax issue (1.49 KB, patch)
2020-02-03 16:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix several issues (1.52 KB, patch)
2020-02-03 16:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: chargename removed by bug 21753 (863 bytes, patch)
2020-02-03 16:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: new suspension_chargeperiod (4.14 KB, patch)
2020-02-03 16:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: new holds_per_day (1006 bytes, patch)
2020-02-03 16:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Replace an other insert into issuingrules (2.50 KB, patch)
2020-02-03 16:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix Circulation.t (1022 bytes, patch)
2020-02-03 16:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: More fixes (39.09 KB, patch)
2020-02-03 16:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Add missing filters (9.18 KB, patch)
2020-02-03 16:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix Update DB entry (2.00 KB, patch)
2020-02-03 16:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: fixes from comment #88 (2.80 KB, patch)
2020-02-03 16:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix warning about page about wrong lengthunit (2.45 KB, patch)
2020-02-03 16:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Don't retrieve unused issuing rule (920 bytes, patch)
2020-02-03 16:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix some more tests (19.56 KB, patch)
2020-02-03 16:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Add missing circulation rule to fix tests (1.61 KB, patch)
2020-02-03 16:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Make sure we default to '*' if no rule exist (2.14 KB, patch)
2020-02-03 16:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Insert default rules during the onboarding tool (3.86 KB, patch)
2020-02-03 16:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Replace '*' with undef (1.14 KB, patch)
2020-02-03 16:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix the delete rule buttons (2.86 KB, patch)
2020-02-03 16:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up 2) Add foreign key and scope enhancement to circ rules (1.28 KB, patch)
2020-02-03 16:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Fix typo in documentation (847 bytes, patch)
2020-02-03 16:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Add missing import of Koha::CirculationRules (897 bytes, patch)
2020-02-03 16:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix inconsistencies in IssuingRules.t (16.76 KB, patch)
2020-02-03 16:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up2) Fix tests, replace old get_onshelfholds_policy method (1.26 KB, patch)
2020-02-03 16:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Add cloning of circulation rules back to Koha (9.31 KB, patch)
2020-02-03 16:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Make possible to have unlimited value for holds_per_day (1.88 KB, patch)
2020-02-03 16:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up2) Make possible to have unlimited value for holds_per_day (1.88 KB, patch)
2020-02-03 16:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix IssuingRules/guess_article_requestable_itemtypes.t (3.86 KB, patch)
2020-02-03 16:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Adjust circ rule related tests so that they pass (8.90 KB, patch)
2020-02-03 16:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up2) Adjust circ rule related tests so that they pass (1.25 KB, patch)
2020-02-03 16:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) Fix checkouts endpoint and its tests (2.87 KB, patch)
2020-02-03 16:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (QA follow-up) Add missing POD (944 bytes, patch)
2020-02-03 16:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (follow-up) More fixes (1.08 KB, patch)
2020-02-03 16:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (QA follow-up) Convert remaining tests to use Koha::CirculationRules (3.23 KB, patch)
2020-02-03 16:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Remove 2 new occurrences of Koha::IssuingRules (1.90 KB, patch)
2020-02-03 16:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: (QA follow-up) Fix failing test (1.01 KB, patch)
2020-02-03 16:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix failing test in api/v1/holds.t (1.14 KB, patch)
2020-02-03 16:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Display 'Unlimited' for Holds allowed (total) (1.70 KB, patch)
2020-02-04 15:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18936: Fix selenium/administration_tasks.t (1.17 KB, patch)
2020-02-04 15:00 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2017-07-13 11:17:44 UTC

    
Comment 1 Kyle M Hall 2017-07-25 15:44:31 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2017-07-25 15:44:37 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2017-07-25 15:44:39 UTC Comment hidden (obsolete)
Comment 4 Jesse Weaver 2017-10-05 06:15:33 UTC Comment hidden (obsolete)
Comment 5 Jesse Weaver 2017-10-06 19:22:49 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2017-10-08 11:20:37 UTC
Please don't forget to assign your bugs to yourself! (Is it Kyle or Jesse here?)
Comment 7 Jesse Weaver 2017-10-17 23:26:41 UTC Comment hidden (obsolete)
Comment 8 David Bourgault 2017-11-03 20:30:21 UTC
No test plan?
Comment 9 Jesse Weaver 2018-01-29 23:08:20 UTC Comment hidden (obsolete)
Comment 10 Jesse Weaver 2018-01-29 23:08:23 UTC Comment hidden (obsolete)
Comment 11 Jesse Weaver 2018-01-29 23:08:26 UTC Comment hidden (obsolete)
Comment 12 Jesse Weaver 2018-01-29 23:08:29 UTC Comment hidden (obsolete)
Comment 13 Jesse Weaver 2018-01-29 23:08:31 UTC Comment hidden (obsolete)
Comment 14 Jesse Weaver 2018-01-29 23:08:34 UTC Comment hidden (obsolete)
Comment 15 Jesse Weaver 2018-01-29 23:08:36 UTC Comment hidden (obsolete)
Comment 16 Jesse Weaver 2018-01-29 23:08:38 UTC Comment hidden (obsolete)
Comment 17 Jesse Weaver 2018-02-26 19:18:26 UTC Comment hidden (obsolete)
Comment 18 Jesse Weaver 2018-02-26 19:18:29 UTC Comment hidden (obsolete)
Comment 19 Jesse Weaver 2018-02-26 19:18:32 UTC Comment hidden (obsolete)
Comment 20 Jesse Weaver 2018-02-26 19:18:35 UTC Comment hidden (obsolete)
Comment 21 Jesse Weaver 2018-02-26 19:18:38 UTC Comment hidden (obsolete)
Comment 22 Jesse Weaver 2018-02-26 19:18:42 UTC Comment hidden (obsolete)
Comment 23 Jesse Weaver 2018-02-26 19:18:45 UTC Comment hidden (obsolete)
Comment 24 Jesse Weaver 2018-02-26 19:18:48 UTC Comment hidden (obsolete)
Comment 25 Jesse Weaver 2018-02-26 19:18:51 UTC Comment hidden (obsolete)
Comment 26 Séverine Queune 2018-04-19 10:45:46 UTC
Hi Jesse, how could this one be tested ?
Comment 27 Katrin Fischer 2018-07-11 05:53:40 UTC
Kyle, assigning this to you, ok?
Comment 28 Jonathan Druart 2018-10-24 23:45:35 UTC
I have started a rebase but it's getting very hard here. I will wait a bit for other patches to get attention.
Comment 29 Jonathan Druart 2018-10-25 16:27:24 UTC
Just for reference: https://gitlab.com/joubu/Koha/commits/bug_18936

It is not supposed to be tested, I am not expecting it to work correctly.
Comment 30 Jonathan Druart 2018-12-06 20:11:04 UTC
Remove branch rebased and updated.

TODO: "notes for rule" is certainly broken (bug 12365)

Kyle, I have now 1 line per category and item type on the circ rule admin page, is that expected?
Comment 31 Kyle M Hall 2018-12-07 17:28:53 UTC
(In reply to Jonathan Druart from comment #30)
> Remove branch rebased and updated.
> 
> TODO: "notes for rule" is certainly broken (bug 12365)
> 
> Kyle, I have now 1 line per category and item type on the circ rule admin
> page, is that expected?

If I understand what you are saying, the answer is yes. From the user perspective, the circ rules should look pretty much the same as they always have.
Comment 32 Jonathan Druart 2018-12-07 19:14:37 UTC
No it did not, that's why I asked :)
I pushed another commit that fixes the problem I got.
Thanks for the answer!
Comment 33 Owen Leonard 2019-02-20 16:06:38 UTC
Is this ready for testing? When I run updatedatabase I get some errors:

DEV atomic update: bug_18925.perl
DBD::mysql::db selectrow_array failed: Table 'koha_kohadev.branch_borrower_circ_rules' doesn't exist [for Statement "
        SHOW COLUMNS FROM branch_borrower_circ_rules
        WHERE Field = ?
        "] at /home/vagrant/kohaclone/C4/Installer.pm line 531.

...etc.
Comment 34 Jonathan Druart 2019-02-21 22:18:01 UTC
(In reply to Owen Leonard from comment #33)
> Is this ready for testing? When I run updatedatabase I get some errors:
> 
> DEV atomic update: bug_18925.perl
> DBD::mysql::db selectrow_array failed: Table
> 'koha_kohadev.branch_borrower_circ_rules' doesn't exist [for Statement "
>         SHOW COLUMNS FROM branch_borrower_circ_rules
>         WHERE Field = ?
>         "] at /home/vagrant/kohaclone/C4/Installer.pm line 531.
> 
> ...etc.

Hi Owen,
I have rebased the whole dependency tree and updated the remote branch.
The upgrade process should pass if the DB structure comes from master. However I have seen those errors if you run it twice. It is a bug in column_exists that does not test if the table still exist.

I am not confident with the patches from this bug report (lot of rebases), maybe it would be better to validate that the ones from the deps are ok.
Comment 35 Jonathan Druart 2019-03-06 17:46:10 UTC
Remote branch rebased.
Comment 36 Jonathan Druart 2019-04-28 21:09:54 UTC
*** Bug 8858 has been marked as a duplicate of this bug. ***
Comment 37 Victor Grousset/tuxayo 2019-05-24 15:25:17 UTC
Should this bug be in "assigned" instead of "Needs Signoff"?


updatedabase fails:

logs:
https://framabin.org/p/?ee2bc83c735fb710#wvTUZlLVsrUIoShkp6kyGxpjY9Ov3osvFLltCqpwFNU=

dump: latest MARC21
http://git.koha-community.org/gitweb/?p=contrib/global.git;a=history;f=sandbox/sql/sandbox1.sql.gz;h=19268bccb43b2a33d5644b7d86cbb1abb323016b;hb=HEAD

code:7ca6504be2d9ce96ef929c90b3b83a84a935244e
https://gitlab.com/joubu/Koha/commits/bug_18936
Comment 38 Liz Rea 2019-05-24 16:06:04 UTC
I think these errors will put this on a failed QA state (Sorry Jonathan...)

Liz
Comment 39 Jonathan Druart 2019-05-24 21:46:47 UTC
(In reply to Jonathan Druart from comment #29)
> Just for reference: https://gitlab.com/joubu/Koha/commits/bug_18936
> 
> It is not supposed to be tested, I am not expecting it to work correctly.

Remote branch rebased. Previous comment still applies, please focus on dependencies first.
Comment 40 Jonathan Druart 2019-08-04 20:09:19 UTC
Created attachment 91953 [details] [review]
Bug 18936: Convert issuingrules fields to circulation_rules
Comment 41 Jonathan Druart 2019-08-04 20:09:24 UTC
Created attachment 91954 [details] [review]
Bug 18936: (followup) Fix upgrade step
Comment 42 Jonathan Druart 2019-08-04 20:09:29 UTC
Created attachment 91955 [details] [review]
Bug 18936: Fix api/v1/holds.t
Comment 43 Jonathan Druart 2019-08-04 20:09:35 UTC
Created attachment 91956 [details] [review]
Bug 18936: (follow-up) Add foreign key and scope enhancement to circ rules

This necessitates moving the circ rules from using '*' to using
undef/NULL.
Comment 44 Jonathan Druart 2019-08-04 20:09:39 UTC
Created attachment 91957 [details] [review]
Bug 18936: (follow-up) Update REST definition for /.../kinds
Comment 45 Jonathan Druart 2019-08-04 20:09:43 UTC
Created attachment 91958 [details] [review]
Bug 18936: (follow-up) fix rental odiscount lookup
Comment 46 Jonathan Druart 2019-08-04 20:09:48 UTC
Created attachment 91959 [details] [review]
Bug 18936: (follow-up) Remove issuingrules from kohastructure.sql
Comment 47 Jonathan Druart 2019-08-04 20:09:53 UTC
Created attachment 91960 [details] [review]
Bug 18936: (follow-up) fix tests, null vs. empty behavior for limiting rules
Comment 48 Jonathan Druart 2019-08-04 20:09:57 UTC
Created attachment 91961 [details] [review]
Bug 18936: (follow-up) Fix tests, replace old get_onshelfholds_policy method
Comment 49 Jonathan Druart 2019-08-04 20:10:02 UTC
Created attachment 91962 [details] [review]
Bug 18936: move get_opacitemholds_policy method
Comment 50 Jonathan Druart 2019-08-04 20:10:07 UTC
Created attachment 91963 [details] [review]
Bug 18936: Move guess_article_requestable_itemtypes method
Comment 51 Jonathan Druart 2019-08-04 20:10:11 UTC
Created attachment 91964 [details] [review]
Bug 18936: Remove last bit of Koha::IssuingRule
Comment 52 Jonathan Druart 2019-08-04 20:10:16 UTC
Created attachment 91965 [details] [review]
Bug 18936: Fix syntax issue

TODO: Replace croak with exceptions
Comment 53 Jonathan Druart 2019-08-04 20:10:21 UTC
Created attachment 91966 [details] [review]
Bug 18936: Fix several issues
Comment 54 Jonathan Druart 2019-08-04 20:10:25 UTC
Created attachment 91967 [details] [review]
Bug 18936: chargename removed by bug 21753
Comment 55 Jonathan Druart 2019-08-04 20:10:30 UTC
Created attachment 91968 [details] [review]
Bug 18936: new suspension_chargeperiod
Comment 56 Jonathan Druart 2019-08-04 20:10:34 UTC
Created attachment 91969 [details] [review]
Bug 18936: new holds_per_day
Comment 57 Jonathan Druart 2019-08-04 20:10:39 UTC
Created attachment 91970 [details] [review]
Bug 18936: Replace an other insert into issuingrules
Comment 58 Jonathan Druart 2019-08-04 20:10:43 UTC
Created attachment 91971 [details] [review]
Bug 18936: Fix Circulation.t
Comment 59 Jonathan Druart 2019-08-04 20:10:48 UTC
Created attachment 91972 [details] [review]
Bug 18936: More fixes
Comment 60 Jonathan Druart 2019-08-04 20:10:52 UTC
Created attachment 91973 [details] [review]
Bug 18936: Add missing filters
Comment 61 Jonathan Druart 2019-08-04 20:10:59 UTC
Created attachment 91974 [details] [review]
Bug 18936: Fix Update DB entry
Comment 62 Jonathan Druart 2019-08-04 20:11:34 UTC
Created attachment 91975 [details] [review]
Bug 18936: Fix Update DB entry
Comment 63 Jonathan Druart 2019-08-04 20:15:42 UTC
This is now ready for testing!

The remote branch has been rebased and adjusted. Patches attached to this bug report as well.

Test plan:
- checkout master and create several circulation rules
- apply these patches and run the updatedatabase.pl script
- confirm that the screen displays the correct value
- add/edit/delete rules and confirm that there is no regression
- focus on empty strings vs 0 vs undef values
- pick some rules you know well and confirm the restrictions applies accordingly to the values you have defined (renewalsallowed for instance)
Comment 64 Jonathan Druart 2019-10-15 12:32:52 UTC
Created attachment 94179 [details] [review]
Bug 18936: Convert issuingrules fields to circulation_rules
Comment 65 Jonathan Druart 2019-10-15 12:32:57 UTC
Created attachment 94180 [details] [review]
Bug 18936: (followup) Fix upgrade step
Comment 66 Jonathan Druart 2019-10-15 12:33:01 UTC
Created attachment 94181 [details] [review]
Bug 18936: Fix api/v1/holds.t
Comment 67 Jonathan Druart 2019-10-15 12:33:06 UTC
Created attachment 94182 [details] [review]
Bug 18936: (follow-up) Add foreign key and scope enhancement to circ rules

This necessitates moving the circ rules from using '*' to using
undef/NULL.
Comment 68 Jonathan Druart 2019-10-15 12:33:10 UTC
Created attachment 94183 [details] [review]
Bug 18936: (follow-up) Update REST definition for /.../kinds
Comment 69 Jonathan Druart 2019-10-15 12:33:15 UTC
Created attachment 94184 [details] [review]
Bug 18936: (follow-up) fix rental odiscount lookup
Comment 70 Jonathan Druart 2019-10-15 12:33:19 UTC
Created attachment 94185 [details] [review]
Bug 18936: (follow-up) Remove issuingrules from kohastructure.sql
Comment 71 Jonathan Druart 2019-10-15 12:33:23 UTC
Created attachment 94186 [details] [review]
Bug 18936: (follow-up) fix tests, null vs. empty behavior for limiting rules
Comment 72 Jonathan Druart 2019-10-15 12:33:28 UTC
Created attachment 94187 [details] [review]
Bug 18936: (follow-up) Fix tests, replace old get_onshelfholds_policy method
Comment 73 Jonathan Druart 2019-10-15 12:33:32 UTC
Created attachment 94188 [details] [review]
Bug 18936: move get_opacitemholds_policy method
Comment 74 Jonathan Druart 2019-10-15 12:33:36 UTC
Created attachment 94189 [details] [review]
Bug 18936: Move guess_article_requestable_itemtypes method
Comment 75 Jonathan Druart 2019-10-15 12:33:40 UTC
Created attachment 94190 [details] [review]
Bug 18936: Remove last bit of Koha::IssuingRule
Comment 76 Jonathan Druart 2019-10-15 12:33:44 UTC
Created attachment 94191 [details] [review]
Bug 18936: Fix syntax issue

TODO: Replace croak with exceptions
Comment 77 Jonathan Druart 2019-10-15 12:33:48 UTC
Created attachment 94192 [details] [review]
Bug 18936: Fix several issues
Comment 78 Jonathan Druart 2019-10-15 12:33:53 UTC
Created attachment 94193 [details] [review]
Bug 18936: chargename removed by bug 21753
Comment 79 Jonathan Druart 2019-10-15 12:33:58 UTC
Created attachment 94194 [details] [review]
Bug 18936: new suspension_chargeperiod
Comment 80 Jonathan Druart 2019-10-15 12:34:04 UTC
Created attachment 94195 [details] [review]
Bug 18936: new holds_per_day
Comment 81 Jonathan Druart 2019-10-15 12:34:09 UTC
Created attachment 94196 [details] [review]
Bug 18936: Replace an other insert into issuingrules
Comment 82 Jonathan Druart 2019-10-15 12:34:14 UTC
Created attachment 94197 [details] [review]
Bug 18936: Fix Circulation.t
Comment 83 Jonathan Druart 2019-10-15 12:34:19 UTC
Created attachment 94198 [details] [review]
Bug 18936: More fixes
Comment 84 Jonathan Druart 2019-10-15 12:34:23 UTC
Created attachment 94199 [details] [review]
Bug 18936: Add missing filters
Comment 85 Jonathan Druart 2019-10-15 12:34:28 UTC
Created attachment 94200 [details] [review]
Bug 18936: Fix Update DB entry
Comment 86 Josef Moravec 2019-10-30 18:53:23 UTC
sorry, tried to apply, but got:

Applying: Bug 18936: More fixes
error: sha1 information is lacking or useless (C4/Reserves.pm).
Comment 87 Martin Renvoize 2019-10-31 12:48:30 UTC
I think this bug probably warrants the 'Just a git branch' approach and should have the patches removed from here.. those capable of testing this should be capable of adding a remote and checking out the branch that Jonathan is maintaining :)
Comment 88 Josef Moravec 2019-11-03 16:27:51 UTC
I tested using git branch and found two issues:

- holds per day are not transferred in database update
- in note column, I can see link "view notes" even if there is no note
- if the rule is specific for library and patron category, or for library and itemtype, the rules for current checkouts allowed and current on-site checkouts allowed are not transferred
- No automatic renewal after (hard limit) does not respect dateformat and is shown in iso format
- Overdue fines cap (amount) is not possible to edit in GUI
Comment 89 Jonathan Druart 2019-11-26 11:09:07 UTC
(In reply to Josef Moravec from comment #88)

Hi Josef, thanks for testing this!

> I tested using git branch and found two issues:
> 
> - holds per day are not transferred in database update

Fixed!

> - in note column, I can see link "view notes" even if there is no note

Fixed!

> - if the rule is specific for library and patron category, or for library
> and itemtype, the rules for current checkouts allowed and current on-site
> checkouts allowed are not transferred

I do not recreate that.
checkout master
create specific rule for CPL and a given patron's category
checkout bug_18936
update
=> I do see the value for current checkouts allowed

> - No automatic renewal after (hard limit) does not respect dateformat and is
> shown in iso format

Fixed!

> - Overdue fines cap (amount) is not possible to edit in GUI

I do not recreate:
Edit the line
Modify the value for 'Overdue fines cap (amount)'
Save
=> The value is correctly displayed in the table
Comment 90 Martin Renvoize 2019-12-12 13:03:56 UTC
This all scans well, if you get a chance it would be great to have a final SO/QA by January as it's one I'm targetting for an early push.
Comment 91 Joonas Kylmälä 2020-01-15 12:30:39 UTC
Hi Jonathan,

failing the patches "Bug 18936: More fixes", "Bug 18936: Fix Circulation.t", "Bug 18936: Fix several issues" and "Bug 18936: Fix Update DB entry" since they miss description of the changes so there is no way to know what is the intended change here therefore testing will be super difficult. Also with "Bug 18936: Fix several issues" it would be more logical to have then several commits for those.

So I will take a look again once the descriptions of changes are added, ok?
Comment 92 Jonathan Druart 2020-01-21 09:14:23 UTC
(In reply to Joonas Kylmälä from comment #91)
> Hi Jonathan,
> 
> failing the patches "Bug 18936: More fixes", "Bug 18936: Fix Circulation.t",
> "Bug 18936: Fix several issues" and "Bug 18936: Fix Update DB entry" since
> they miss description of the changes so there is no way to know what is the
> intended change here therefore testing will be super difficult. Also with
> "Bug 18936: Fix several issues" it would be more logical to have then
> several commits for those.
> 
> So I will take a look again once the descriptions of changes are added, ok?

Hi Joonas,
Those patches are follow-up sI submitted when I retested the feature after a feature. I can understand they do not make sense in the git log, but they do for me (as I am rebasing this for 2 years now). I would suggest to squash the patches when reviewing them, and you will see that the changes make sense.

Fix Circulation.t: Make the tests Circulation.t pass (maxissueqty was missing when the rule was created)

Fix several issues: branchcode, categorycode and itemtype are no longer mandatory to create a new rule

Fix Update DB entry: Some rules were missing (or added when they should not). Also branchcode, categorycode and itemtype must not be '*' but NULL, otherwise the FK constraint will not be satisfied.

We are at Signoff step, that should not block testers to test this feature.
Comment 93 Jonathan Druart 2020-01-22 14:14:04 UTC
Patches rebased.

There is one test that is failing I do not manage to fix (t/db_dependent/Koha/Charges/Fees.t).
Comment 94 Jonathan Druart 2020-01-23 15:20:13 UTC
Testers should consider using the branch bug_18936 from my koha-misc4dev repo to fix reset_all: https://gitlab.com/joubu/koha-misc4dev/commits/bug_18936
Will be cleaned and merged into master when these patches will be pushed.
Comment 95 Minna Kivinen 2020-01-28 09:48:19 UTC
Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>
Tested following:
- Added several rules for different combinations of patron category and item type
- Modified rules
- Removed rules
- Added new rules
- Tested functionalities against rules: Hard due date, Loan period, Loans allowed (total), Renewals allowed (count), No renewal before, Holds allowed (total)
Comment 96 Joonas Kylmälä 2020-01-28 10:35:54 UTC
Problems I noticed after doing database upgrade and comparing the web page before db upgrade and after db upgrade: 

- The "Clone these rules to: " feature is missing after the patch set

- "Holds allowed (daily)" was previously empty now with the patch set it says "Unlimited"

- "Fine amount" is now with 6 decimals instead of previous 2
Comment 97 Joonas Kylmälä 2020-01-28 10:50:19 UTC
(In reply to Joonas Kylmälä from comment #96)
> - "Holds allowed (daily)" was previously empty now with the patch set it
> says "Unlimited"

This is fine, the empty value previously also meant "Unlimited", here it is just made explicit.
Comment 98 Joonas Kylmälä 2020-01-28 14:36:30 UTC
Even though it should be allowed, I get this error message when making a hold for Student patron for Music item: "Daily hold limit reached for patron".

I have rules:

category = Student, itemtype = Music, Holds allowed (daily) = Unlimited

and

category = All, itemtype = ALL, Holds allowed (daily) = 0

If for the latter "All rule" I change it to 1 then things work.
Comment 99 Minna Kivinen 2020-01-31 08:17:34 UTC
Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>
Tested following:
- Added several rules for different combinations of patron category and item type
- Modified rules
- Removed rules
- Added new rules
- Tested functionalities against rules: Hard due date, Loan period, Loans allowed (total), Renewals allowed (count), No renewal before, Holds allowed (total)
- Tested especially empty values on rules
Comment 100 Jonathan Druart 2020-01-31 09:47:36 UTC
(In reply to Minna Kivinen from comment #99)
> Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>
> Tested following:
> - Added several rules for different combinations of patron category and item
> type
> - Modified rules
> - Removed rules
> - Added new rules
> - Tested functionalities against rules: Hard due date, Loan period, Loans
> allowed (total), Renewals allowed (count), No renewal before, Holds allowed
> (total)
> - Tested especially empty values on rules

Thank you! I have added your signed-off-by lines to the commit messages.
Comment 101 Joonas Kylmälä 2020-01-31 11:54:17 UTC
The qa-koha.pl tool detected an issue with constant being redefined:

> FAIL   Koha/CirculationRules.pm              
>   FAIL   valid                  
>                Constant subroutine >Koha::CirculationRules::GUESSED_ITEMTYPES_KEY redefined

This appears to be from circular dependency:

Koha/CirculationRules.pm -> Koha/CirculationRule.pm -> Koha/ItemTypes.pm -> Koha/ItemType.pm -> Koha/CirculationRules.pm

if someone has the time to remove the circularity here would be great!
Comment 102 Jonathan Druart 2020-01-31 15:25:41 UTC
Minor:
Koha::CirculationRule ->item_type, ->library and ->patron_category must use the DBIC RS
Comment 103 Jonathan Druart 2020-01-31 15:29:43 UTC
(In reply to Joonas Kylmälä from comment #101)
> The qa-koha.pl tool detected an issue with constant being redefined:
> 
> > FAIL   Koha/CirculationRules.pm              
> >   FAIL   valid                  
> >                Constant subroutine >Koha::CirculationRules::GUESSED_ITEMTYPES_KEY redefined
> 
> This appears to be from circular dependency:
> 
> Koha/CirculationRules.pm -> Koha/CirculationRule.pm -> Koha/ItemTypes.pm ->
> Koha/ItemType.pm -> Koha/CirculationRules.pm
> 
> if someone has the time to remove the circularity here would be great!

I would not care much about that.
Note that the QA script is ignoring the "subroutine ... redefined" warning.
Here it displays "Constant subroutine ... redefined" and that is why it's not ignored.

The circular dependency already existed:
% git checkout master
% perl -wc Koha/CirculationRules.pm 
Subroutine get_effective_rule redefined at Koha/CirculationRules.pm line 42.
Subroutine set_rule redefined at Koha/CirculationRules.pm line 82.
Subroutine set_rules redefined at Koha/CirculationRules.pm line 140.
Subroutine delete redefined at Koha/CirculationRules.pm line 171.
Subroutine _type redefined at Koha/CirculationRules.pm line 184.
Subroutine object_class redefined at Koha/CirculationRules.pm line 192.
Koha/CirculationRules.pm syntax OK
Comment 104 Martin Renvoize 2020-01-31 15:50:40 UTC
I've given this a look over myself now and am pretty happy.. as soon as we have your stamp of approval Joonas we'll go for it! :)

Well done guys!
Comment 105 Joonas Kylmälä 2020-02-03 15:04:10 UTC
I would still like to wait Joubu to update https://gitlab.com/joubu/koha-misc4dev/commits/bug_18936 so that using that branch all the tests pass again. The defaults currently differ from the original values and therefore the tests don't pass.
Comment 106 Joonas Kylmälä 2020-02-03 15:08:28 UTC
(In reply to Joonas Kylmälä from comment #105)
> I would still like to wait Joubu to update
> https://gitlab.com/joubu/koha-misc4dev/commits/bug_18936 so that using that
> branch all the tests pass again. The defaults currently differ from the
> original values and therefore the tests don't pass.

I can actually just do upgradedatabase from master and run the tests so this is not necessary. Sorry for the noise.
Comment 107 Joonas Kylmälä 2020-02-03 16:01:22 UTC
I followed the test plan, and as requested there to test some specific rules I know, I choose to test hold related rules and renewal rules.

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Big thanks for all involved on this 2+ years old bug!
Comment 108 Jonathan Druart 2020-02-03 16:05:48 UTC
Created attachment 98299 [details] [review]
Bug 18936: Convert issuingrules fields to circulation_rules

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 109 Jonathan Druart 2020-02-03 16:05:54 UTC
Created attachment 98300 [details] [review]
Bug 18936: (followup) Fix upgrade step

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 110 Jonathan Druart 2020-02-03 16:05:58 UTC
Created attachment 98301 [details] [review]
Bug 18936: Fix api/v1/holds.t

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 111 Jonathan Druart 2020-02-03 16:06:05 UTC
Created attachment 98302 [details] [review]
Bug 18936: (follow-up) Add foreign key and scope enhancement to circ rules

This necessitates moving the circ rules from using '*' to using
undef/NULL.

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 112 Jonathan Druart 2020-02-03 16:06:10 UTC
Created attachment 98303 [details] [review]
Bug 18936: (follow-up) Update REST definition for /.../kinds

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 113 Jonathan Druart 2020-02-03 16:06:15 UTC
Created attachment 98304 [details] [review]
Bug 18936: (follow-up) fix rental odiscount lookup

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 114 Jonathan Druart 2020-02-03 16:06:20 UTC
Created attachment 98305 [details] [review]
Bug 18936: (follow-up) Remove issuingrules from kohastructure.sql

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 115 Jonathan Druart 2020-02-03 16:06:25 UTC
Created attachment 98306 [details] [review]
Bug 18936: (follow-up) fix tests, null vs. empty behavior for limiting rules

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 116 Jonathan Druart 2020-02-03 16:06:30 UTC
Created attachment 98307 [details] [review]
Bug 18936: (follow-up) Fix tests, replace old get_onshelfholds_policy method

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 117 Jonathan Druart 2020-02-03 16:06:35 UTC
Created attachment 98308 [details] [review]
Bug 18936: move get_opacitemholds_policy method

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 118 Jonathan Druart 2020-02-03 16:06:40 UTC
Created attachment 98309 [details] [review]
Bug 18936: Move guess_article_requestable_itemtypes method

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 119 Jonathan Druart 2020-02-03 16:06:45 UTC
Created attachment 98310 [details] [review]
Bug 18936: Remove last bit of Koha::IssuingRule

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 120 Jonathan Druart 2020-02-03 16:06:50 UTC
Created attachment 98311 [details] [review]
Bug 18936: Fix syntax issue

TODO: Replace croak with exceptions

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 121 Jonathan Druart 2020-02-03 16:06:55 UTC
Created attachment 98312 [details] [review]
Bug 18936: Fix several issues

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 122 Jonathan Druart 2020-02-03 16:07:01 UTC
Created attachment 98313 [details] [review]
Bug 18936: chargename removed by bug 21753

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 123 Jonathan Druart 2020-02-03 16:07:06 UTC
Created attachment 98314 [details] [review]
Bug 18936: new suspension_chargeperiod

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 124 Jonathan Druart 2020-02-03 16:07:11 UTC
Created attachment 98315 [details] [review]
Bug 18936: new holds_per_day

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 125 Jonathan Druart 2020-02-03 16:07:16 UTC
Created attachment 98316 [details] [review]
Bug 18936: Replace an other insert into issuingrules

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 126 Jonathan Druart 2020-02-03 16:07:21 UTC
Created attachment 98317 [details] [review]
Bug 18936: Fix Circulation.t

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 127 Jonathan Druart 2020-02-03 16:07:27 UTC
Created attachment 98318 [details] [review]
Bug 18936: More fixes

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 128 Jonathan Druart 2020-02-03 16:07:32 UTC
Created attachment 98319 [details] [review]
Bug 18936: Add missing filters

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 129 Jonathan Druart 2020-02-03 16:07:37 UTC
Created attachment 98320 [details] [review]
Bug 18936: Fix Update DB entry

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 130 Jonathan Druart 2020-02-03 16:07:43 UTC
Created attachment 98321 [details] [review]
Bug 18936: fixes from comment #88

- holds per day are not transferred in database update (actually not
displayed)
- in note column, I can see link "view notes" even if there is no note
- No automatic renewal after (hard limit) does not respect dateformat
and is shown in iso format

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 131 Jonathan Druart 2020-02-03 16:07:48 UTC
Created attachment 98322 [details] [review]
Bug 18936: Fix warning about page about wrong lengthunit

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 132 Jonathan Druart 2020-02-03 16:07:53 UTC
Created attachment 98323 [details] [review]
Bug 18936: (follow-up) Don't retrieve unused issuing rule

The chargename rule is not used anywhere in the code

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 133 Jonathan Druart 2020-02-03 16:07:59 UTC
Created attachment 98324 [details] [review]
Bug 18936: Fix some more tests

* CanItemBeReserved
Prior to "Bug 18936: Convert issuingrules fields to circulation_rules",
GetHoldRule returned holds_per_record even if no reservesallowed was
defined. This change restores this behavior.
FIXME Note: In GetHoldRule we return itemtype only if reservesallowed is set,
not sure it is correct.

* t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
When setting returnbranch, holdallowed and hold_fulfillment_policy, we
should not provide categorycode.

* t/db_dependent/Holds.t
Prefer to keep the existing rules instead of removing them. It got quite
hard to understand what was going on here because of the mixup with
the rule reservesallowed that was in issuingrules, and the other rules
we used for the tests. Also, categorycode should not be passed to set
those 3 rules (holdallowed, hold_fulfillment_policy and returnbranch)

* t/db_dependent/Circulation.t
Setting lengthunit to 'hours', no need to make sure the rule has been
correctly be saved

* t/db_dependent/Circulation/CalcDateDue.t
It uses hardcoded data that is not in the sample data (categorycode=C).
Let use K that exists and postpone a refactore of the whole script (to
make it create the data it needs).

* t/db_dependent/Circulation/ReturnClaims.t
* t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t
Simple replace Koha::IssuingRule with Koha::CirculationRules

* t/db_dependent/Koha/Charges/Fees.t
=> FIXME Still failing, stuck here, need help

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 134 Jonathan Druart 2020-02-03 16:08:04 UTC
Created attachment 98325 [details] [review]
Bug 18936: (follow-up) Add missing circulation rule to fix tests

The tests passed before by a chance because the default kohadevbox
installation script added a issuing rule that had lengthunit =
days. If you use a Koha version before the commits introduced by Bug
18936 you can reproduce the Fees.t tests failing by deleting all the
issuing rules from database with

> delete issuingrules;

and then running

$ prove t/db_dependent/Koha/Charges/Fees.t

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 135 Jonathan Druart 2020-02-03 16:08:09 UTC
Created attachment 98326 [details] [review]
Bug 18936: Make sure we default to '*' if no rule exist

Making sure the code is doing what we expect.
Also this patch fix CirculationRules.t

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 136 Jonathan Druart 2020-02-03 16:08:14 UTC
Created attachment 98327 [details] [review]
Bug 18936: Insert default rules during the onboarding tool

Those default values have been picked from bug 15522
(Koha::CirculationRules)

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 137 Jonathan Druart 2020-02-03 16:08:20 UTC
Created attachment 98328 [details] [review]
Bug 18936: Replace '*' with undef

No test was provided initially so considering out of the scope for this
one as well, but that's lacking terribly!

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 138 Jonathan Druart 2020-02-03 16:08:25 UTC
Created attachment 98329 [details] [review]
Bug 18936: Fix the delete rule buttons

The itemtype and categorycode were always set to '*'

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 139 Jonathan Druart 2020-02-03 16:08:31 UTC
Created attachment 98330 [details] [review]
Bug 18936: (follow-up 2) Add foreign key and scope enhancement to circ rules

The previous patch modifies:
-                categorycode => undef,
-                itemtype     => undef,
-                branchcode   => $branch,
+                branchcode   => undef,

But in that condition $branchcode is not '*' and so we should not set it
to undef

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 140 Jonathan Druart 2020-02-03 16:08:36 UTC
Created attachment 98331 [details] [review]
Bug 18936: (follow-up) Fix typo in documentation

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 141 Jonathan Druart 2020-02-03 16:08:41 UTC
Created attachment 98332 [details] [review]
Bug 18936: (follow-up) Add missing import of Koha::CirculationRules

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 142 Jonathan Druart 2020-02-03 16:08:47 UTC
Created attachment 98333 [details] [review]
Bug 18936: Fix inconsistencies in IssuingRules.t

* get_effective_rule should not pass rule_value
* indentation fix
* add comment to list the rules we have when the test _is_row_match
is executed
* compare ->rule_name and ->rule_value returns

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 143 Jonathan Druart 2020-02-03 16:08:52 UTC
Created attachment 98334 [details] [review]
Bug 18936: (follow-up2) Fix tests, replace old get_onshelfholds_policy method

Previous change to this file was different from the one that existed
prior to this patchset.
Restoring value for onshelfholds, note that it did not change anything
in the test result.

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 144 Jonathan Druart 2020-02-03 16:08:58 UTC
Created attachment 98335 [details] [review]
Bug 18936: (follow-up) Add cloning of circulation rules back to Koha

The "Clone these rules" feature in admin/smart-rules.pl was
dropped. This re-implements the cloning using Koha objects.

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 145 Jonathan Druart 2020-02-03 16:09:03 UTC
Created attachment 98336 [details] [review]
Bug 18936: (follow-up) Make possible to have unlimited value for holds_per_day

Without this it is not possible to make for a specific patron category
or itemtype a more specific unlimited holds_per_day rule if you have a
catch all rule (categorycode = *, itemtype = *) with limited
holds_per_day value.

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 146 Jonathan Druart 2020-02-03 16:09:08 UTC
Created attachment 98337 [details] [review]
Bug 18936: (follow-up2) Make possible to have unlimited value for holds_per_day

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 147 Jonathan Druart 2020-02-03 16:09:14 UTC
Created attachment 98338 [details] [review]
Bug 18936: Fix IssuingRules/guess_article_requestable_itemtypes.t

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 148 Jonathan Druart 2020-02-03 16:09:19 UTC
Created attachment 98339 [details] [review]
Bug 18936: (follow-up) Adjust circ rule related tests so that they pass

Bunch of tests broke after moving from issuingrules to
circulation_rules. This accomodates the tests for this refactoring and
DB change.

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 149 Jonathan Druart 2020-02-03 16:09:25 UTC
Created attachment 98340 [details] [review]
Bug 18936: (follow-up2) Adjust circ rule related tests so that they pass

The itemtype did not exist in the DB. We create it at the beginning of
the tests. Ideally we would like to create it filling the form.

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 150 Jonathan Druart 2020-02-03 16:09:30 UTC
Created attachment 98341 [details] [review]
Bug 18936: (follow-up) Fix checkouts endpoint and its tests

- Fixes a bug caused by the refactoring in
Koha/REST/V1/Checkouts.pm (run the tests without this patch and you
will see the tests fails because 'rule_name' parameter was
forgotten).

- Changes the tests to use the new Koha::CirculationRule objects
  instead of directly using issuingrules DB table

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 151 Jonathan Druart 2020-02-03 16:09:35 UTC
Created attachment 98342 [details] [review]
Bug 18936: (QA follow-up) Add missing POD

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 152 Jonathan Druart 2020-02-03 16:09:41 UTC
Created attachment 98343 [details] [review]
Bug 18936: (follow-up) More fixes

Previous "More fixes" patch added a debugging statement, not needed.

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 153 Jonathan Druart 2020-02-03 16:09:46 UTC
Created attachment 98344 [details] [review]
Bug 18936: (QA follow-up) Convert remaining tests to use Koha::CirculationRules

These two uses of Koha::IssuingRule were forgotten so this converts
them to use the new Koha::CirculationRules object.

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 154 Jonathan Druart 2020-02-03 16:09:52 UTC
Created attachment 98345 [details] [review]
Bug 18936: Remove 2 new occurrences of Koha::IssuingRules

Fix conflict with bug 13121

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 155 Jonathan Druart 2020-02-03 16:09:57 UTC
Created attachment 98346 [details] [review]
Bug 18936: (QA follow-up) Fix failing test

Using categorycode as a scope is not possible for any of these rules
so lets remove it.

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 156 Jonathan Druart 2020-02-03 16:10:03 UTC
Created attachment 98347 [details] [review]
Bug 18936: Fix failing test in api/v1/holds.t

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 157 Martin Renvoize 2020-02-04 10:04:24 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 158 Martin Renvoize 2020-02-04 10:10:45 UTC
Guys, this has been a real stirling effort!

Thanks to all involved in pushing it over the finish line, amazing work!
Comment 159 Jonathan Druart 2020-02-04 15:00:39 UTC
Created attachment 98407 [details] [review]
Bug 18936: Display 'Unlimited' for Holds allowed (total)

holds_per_day and reservesallowed are set to unlimited in the rules
inserted by misc4dev
Comment 160 Jonathan Druart 2020-02-04 15:00:45 UTC
Created attachment 98408 [details] [review]
Bug 18936: Fix selenium/administration_tasks.t
Comment 161 Martin Renvoize 2020-02-04 15:05:47 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 162 Joy Nelson 2020-02-10 23:40:11 UTC
Not backporting to 19.11.x