Bug 26814 - Add onsite_checkout to circulation rules
Summary: Add onsite_checkout to circulation rules
Status: Patch doesn't apply
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Lari Taskula
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 24101
  Show dependency treegraph
 
Reported: 2020-10-24 02:36 UTC by Lari Taskula
Modified: 2020-12-15 23:32 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 26814: Add a new column onsite_checkout to circulation_rules table (3.36 KB, patch)
2020-10-24 04:20 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: Schema changes (2.03 KB, patch)
2020-10-24 04:20 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: Add onsite_checkout to Koha::CirculationRules (18.53 KB, patch)
2020-10-24 04:20 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: Remove on-site specific circulation rules (24.41 KB, patch)
2020-10-24 04:20 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: Add onsite_checkout to set_rule (9.74 KB, patch)
2020-10-24 04:20 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: Add onsite_checkout to set_rules (34.28 KB, patch)
2020-10-24 04:20 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: Add onsite_checkout to get_effective_rule (23.28 KB, patch)
2020-10-24 04:20 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: Add onsite_checkout to get_effective_rules (29.05 KB, patch)
2020-10-24 04:20 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: (SMART-RULES GUI) Add onsite_checkout into user interface (32.25 KB, patch)
2020-10-24 04:21 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: (SMART-RULES GUI) Indent and add onsite_checkout to all_rules hash (30.82 KB, patch)
2020-10-24 04:21 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: (SMART-RULES GUI) Disable hold rules when on-site option selected (3.81 KB, patch)
2020-10-28 02:51 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: (SMART-RULES GUI) Add onsite_checkout into user interface (32.31 KB, patch)
2020-10-28 03:02 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: (SMART-RULES GUI) Indent and add onsite_checkout to all_rules hash (30.82 KB, patch)
2020-10-28 03:02 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: (SMART-RULES GUI) Disable hold rules when on-site option selected (3.88 KB, patch)
2020-10-28 03:02 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: Add onsite_checkout to set_rules (34.61 KB, patch)
2020-10-28 03:22 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: Add onsite_checkout to get_effective_rule (23.28 KB, patch)
2020-10-28 03:22 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: Add onsite_checkout to get_effective_rules (29.05 KB, patch)
2020-10-28 03:22 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: (SMART-RULES GUI) Add onsite_checkout into user interface (32.31 KB, patch)
2020-10-28 03:22 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: (SMART-RULES GUI) Indent and add onsite_checkout to all_rules hash (30.82 KB, patch)
2020-10-28 03:22 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: (SMART-RULES GUI) Disable hold rules when on-site option selected (3.88 KB, patch)
2020-10-28 03:23 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: (SMART-RULES GUI) Add onsite_checkout into user interface (34.66 KB, patch)
2020-10-28 04:52 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: (SMART-RULES GUI) Indent and add onsite_checkout to all_rules hash (30.95 KB, patch)
2020-10-28 04:52 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: (SMART-RULES GUI) Disable hold rules when on-site option selected (3.88 KB, patch)
2020-10-28 04:52 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: (SMART-RULES GUI) Add onsite_checkout into user interface (34.55 KB, patch)
2020-10-28 23:30 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: (SMART-RULES GUI) Indent and add onsite_checkout to all_rules hash (31.11 KB, patch)
2020-10-28 23:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: (SMART-RULES GUI) Disable hold rules when on-site option selected (3.88 KB, patch)
2020-10-28 23:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: Add a new column onsite_checkout to circulation_rules table (3.36 KB, patch)
2020-12-15 23:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: Schema changes (2.03 KB, patch)
2020-12-15 23:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: Add onsite_checkout to Koha::CirculationRules (19.18 KB, patch)
2020-12-15 23:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: Remove on-site specific circulation rules (24.84 KB, patch)
2020-12-15 23:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: Add onsite_checkout to set_rule (13.48 KB, patch)
2020-12-15 23:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: Add onsite_checkout to set_rules (36.20 KB, patch)
2020-12-15 23:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: Add onsite_checkout to get_effective_rule (23.75 KB, patch)
2020-12-15 23:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: Add onsite_checkout to get_effective_rules (32.11 KB, patch)
2020-12-15 23:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: (SMART-RULES GUI) Add onsite_checkout into user interface (34.69 KB, patch)
2020-12-15 23:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: (SMART-RULES GUI) Indent and add onsite_checkout to all_rules hash (32.12 KB, patch)
2020-12-15 23:32 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 26814: (SMART-RULES GUI) Disable hold rules when on-site option selected (3.88 KB, patch)
2020-12-15 23:32 UTC, Lari Taskula
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lari Taskula 2020-10-24 02:36:20 UTC
We need the ability to define different circulation rules for normal and on-site checkouts.

This Bug aims to add support for such functionality.
Comment 1 Lari Taskula 2020-10-24 04:20:25 UTC
Created attachment 112376 [details] [review]
Bug 26814: Add a new column onsite_checkout to circulation_rules table

To add:
1. perl installer/data/mysql/updatedatabase.pl

Sponsored-by: The National Library of Finland
Comment 2 Lari Taskula 2020-10-24 04:20:30 UTC
Created attachment 112377 [details] [review]
Bug 26814: Schema changes

Sponsored-by: The National Library of Finland
Comment 3 Lari Taskula 2020-10-24 04:20:35 UTC
Created attachment 112378 [details] [review]
Bug 26814: Add onsite_checkout to Koha::CirculationRules

To test:
1. prove t/db_dependent/Koha/CirculationRules.t

Sponsored-by: The National Library of Finland
Comment 4 Lari Taskula 2020-10-24 04:20:39 UTC
Created attachment 112379 [details] [review]
Bug 26814: Remove on-site specific circulation rules

To apply:
1. perl installer/data/mysql/updatedatabase.pl

To test:
1. Search for patron_maxonsiteissueqty rule

   $ grep -rn 'patron_maxonsiteissueqty'

2. Observe no results in Koha source files (excluding .git, atomicupdates nor
   updatedatabase.pl, no release notes)
3. Search for maxonsiteissueqty rule

   $ grep -rn 'maxonsiteissueqty'

4. Observe no results (excluding .git, atomicupdates and updatedatabase.pl
   and release notes)
5. Check modification made to C4::Circulation::GetBranchBorrowerCircRule().
   Make sure the new parameter is being used everywhere. You can search
   the usage of this subroutine with similar grep commands as above.

6. Run the following tests:

prove t/db_dependent/Circulation/Branch.t
prove t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
prove t/db_dependent/Circulation/TooMany.t

Sponsored-by: The National Library of Finland
Comment 5 Lari Taskula 2020-10-24 04:20:44 UTC
Created attachment 112380 [details] [review]
Bug 26814: Add onsite_checkout to set_rule

To test:
1. Find all occurrences of set_rule that are missing onsite_checkout
   where required

grep --exclude-dir '.git' --exclude-dir 'misc/translator' \
--exclude-dir 'koha-tmpl' -Przo '(?s)(::|->)set_rule(?!s).*?\)' | \
grep -avz 'onsite_checkout' | grep -Pavz 'hold|reserves|article_requests' \
&& echo ""

   The only occasions this should return anything are cases where
   a HASH or HASHref is given to set_rule(), or that we are
   explicitly testing missing parameters in an unit test.

   1.2 Verify the HASH/HASHref cases. The hash should contain
             a onsite_checkout (unless the rule is related to holds)

2. Find all subroutines using set_rule()

git grep --no-index -n -p -P 'set_rule\s*\(' | grep -v 'sub {' \
| grep -P 'sub .*'

   2.1 The only return should be
       Koha/CirculationRules.pm=331=sub set_rules {

3. The boring step. Find all occurrences of set_rule() with
   the following command:

grep --exclude-dir='.git' -Prn 'set_rule' | grep -v set_rules

   Go through this list and make sure all neccessary locations are
   updated. A change is required when the rule scope includes
   onsite_checkout. See Koha/CirculationRules.pm for scopes.
   Hold/reserve related rules do not require it.

Sponsored-by: The National Library of Finland
Comment 6 Lari Taskula 2020-10-24 04:20:49 UTC Comment hidden (obsolete)
Comment 7 Lari Taskula 2020-10-24 04:20:54 UTC Comment hidden (obsolete)
Comment 8 Lari Taskula 2020-10-24 04:20:59 UTC Comment hidden (obsolete)
Comment 9 Lari Taskula 2020-10-24 04:21:04 UTC Comment hidden (obsolete)
Comment 10 Lari Taskula 2020-10-24 04:21:09 UTC Comment hidden (obsolete)
Comment 11 Lari Taskula 2020-10-28 02:51:13 UTC Comment hidden (obsolete)
Comment 12 Lari Taskula 2020-10-28 03:02:14 UTC Comment hidden (obsolete)
Comment 13 Lari Taskula 2020-10-28 03:02:18 UTC Comment hidden (obsolete)
Comment 14 Lari Taskula 2020-10-28 03:02:26 UTC Comment hidden (obsolete)
Comment 15 Lari Taskula 2020-10-28 03:22:32 UTC
Created attachment 112623 [details] [review]
Bug 26814: Add onsite_checkout to set_rules

To test:
1. Find all occurrences of set_rules that are missing onsite_checkout
   where required

grep --exclude-dir '.git' --exclude-dir 'misc/translator' \
--exclude-dir 'koha-tmpl' -Przo '(?s)(::|->)set_rules.*?\)' \
--exclude="smart-rules.pl" --exclude="smart-rules.tt" | \
grep -avz 'onsite_checkout' && echo ""

   The only occasions this should return anything are cases where
   a HASH or HASHref is given to set_rules(), or that we are
   explicitly testing missing parameters in an unit test.

   1.2 Verify the HASH/HASHref cases. The hash should contain
       a onsite_checkout (unless the rule is related to holds)

2. Find all subroutines using set_rules()

git grep --no-index -n -p -P 'set_rules\s*\(' | grep -v 'sub {' \
| grep -P 'sub .*'

   2.1 The only return should be
       t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t=401=
         sub set_holdallowed_rule {

3. The boring step. Find all occurrences of set_rules() with
   the following command:

grep --exclude-dir='.git' -Prn 'set_rules'

    Go through this list and make sure all neccessary locations are
    updated. A change is required when the rule scope includes
    onsite_checkout. See Koha/CirculationRules.pm for scopes.
    Hold/reserve related rules do not require it.

Sponsored-by: The National Library of Finland
Comment 16 Lari Taskula 2020-10-28 03:22:37 UTC
Created attachment 112624 [details] [review]
Bug 26814: Add onsite_checkout to get_effective_rule

To test:
1. Find all occurrences of get_effective_rule that are missing
   onsite_checkout where required

grep --exclude-dir '.git' --exclude-dir 'misc/translator' \
--exclude-dir 'koha-tmpl' \
-Przo '(?s)(::|->)get_effective_rule(?!s).*?\)' | \
grep -avz 'onsite_checkout' | \
grep -Pavz 'hold|reserves|article_requests' && echo ""

   The only occasions this should return anything are cases where
   a HASH or HASHref is given to get_effective_rule(), or that we
   are explicitly testing missing parameters in an unit test.

   1.2 Verify the HASH/HASHref cases. The hash should contain
       a onsite_checkout (unless the rule is related to holds)

   1.3 Verify the other cases. If you find another result than
       what is mentioned above, then this test fails.

2. Find all subroutines using get_effective_rule()

git grep --no-index -n -p -P 'get_effective_rule\s*\(' \
| grep -v 'sub {' | grep -P 'sub .*'

    This list should be returned:

    C4/Circulation.pm=394=sub TooMany {
    C4/Circulation.pm=1375=sub AddIssue {
    C4/Circulation.pm=1697=sub GetBranchBorrowerCircRule {
    C4/Circulation.pm=1753=sub GetBranchItemRule {
    C4/Circulation.pm=3040=sub GetRenewCount {
    C4/Reserves.pm=359=sub CanItemBeReserved {
    C4/Reserves.pm=2245=sub GetHoldRule {
    Koha/Biblio.pm=295=sub article_request_type_for_bib {
    Koha/Charges/Fees.pm=93=sub accumulate_rentalcharge {
    Koha/CirculationRules.pm=221=sub get_effective_rules {
    Koha/CirculationRules.pm=395=sub get_opacitemholds_policy {
    Koha/CirculationRules.pm=421=sub get_onshelfholds_policy {
    Koha/CirculationRules.pm=443=sub get_lostreturn_policy {
    Koha/CirculationRules.pm=538=sub get_effective_daysmode {
    Koha/Item.pm=617=sub article_request_type {
    Koha/REST/V1/Checkouts.pm=192=sub allows_renewal {
    Koha/Template/Plugin/CirculationRules.pm=26=sub Get {

    Check these subroutines and make sure they are using
    get_effective_rule() with the new scope, unless not needed.
    When fetching a hold-related rules, onsite_checkout is not needed.

    These subroutines and classes need changes to sub params:

    2.1. Check modifications to Koha::Charges::Fees
    2.2. Check modification made to C4::Circulation::_debar_user_on_return().
    2.3. Check modification made to C4::Circulation::_calculate_new_debar_dt().

3. Find all occurrences of get_effective_rule with the following command:
      grep --exclude-dir='.git' -Prn 'get_effective_rule(?!s)'

    Make sure all matches (where rule scope allows onsite_checkout) are
    updated.

Sponsored-by: The National Library of Finland
Comment 17 Lari Taskula 2020-10-28 03:22:43 UTC
Created attachment 112625 [details] [review]
Bug 26814: Add onsite_checkout to get_effective_rules

To test:
1. Find all occurrences of get_effective_rules that are missing
   onsite_checkout where required

grep --exclude-dir '.git' --exclude-dir 'misc/translator' \
--exclude-dir 'koha-tmpl' \
-Przo '(?s)(::|->)get_effective_rules.*?\)' | \
grep -avz 'onsite_checkout' && echo ""

   1.1. Verify command output. Only reserve related calls to
        get_effective_rules() should be returned.

2. Find all subroutines using get_effective_rules()

git grep --no-index -n -p -P 'get_effective_rules\s*\(' \
| grep -v 'sub {' | grep -P 'sub .*'

    This list should be returned:

    C4/Circulation.pm=1611=sub GetLoanLength {
    C4/Circulation.pm=1650=sub GetHardDueDate {
    C4/Circulation.pm=2301=sub _calculate_new_debar_dt {
    C4/Circulation.pm=2654=sub CanBookBeRenewed {
    C4/Circulation.pm=3107=sub GetSoonestRenewDate {
    C4/Circulation.pm=3171=sub GetLatestAutoRenewDate {
    C4/Overdues.pm=224=sub CalcFine {
    C4/Reserves.pm=2245=sub GetHoldRule {

    Check these subroutines and make sure they are using
    get_effective_rules() with the new scope, unless not needed.
    When fetching a hold-related rules, checkout_type is not needed.

3. Check modification made to C4::Circulation::CalcDateDue()
4. Check modification made to C4::Circulation::CalcFine()
5. Check modification made to C4::Circulation::GetLoanLength()
6. Check modification made to C4::Circulation::GetHardDueDate()

    Make sure all matches (where rule scope allows onsite_checkout) are
    updated.

7. Run these unit tests:

grep -Prl --color=never 'get_effective_rule\(|set_rule\(|set_rules\(' \
t/db_dependent/ | grep '^.*\.t$' | xargs prove

8. Test onboarding tool
8.1. Drop&create your database (take backups first if needed)
8.2. Navigate to web installer
8.3. Continue until you reach onboarding tool step where circulation
     rules are added.
8.4. Observe new drop down selection "Checkout type".
8.5. Select any checkout type and continue the onboarding process.
8.6. Investigate your database table "circulation_rules". You should
     now have rules with onsite_checkout of your choice.

Sponsored-by: The National Library of Finland
Comment 18 Lari Taskula 2020-10-28 03:22:48 UTC Comment hidden (obsolete)
Comment 19 Lari Taskula 2020-10-28 03:22:55 UTC Comment hidden (obsolete)
Comment 20 Lari Taskula 2020-10-28 03:23:02 UTC Comment hidden (obsolete)
Comment 21 Lari Taskula 2020-10-28 04:52:10 UTC Comment hidden (obsolete)
Comment 22 Lari Taskula 2020-10-28 04:52:14 UTC Comment hidden (obsolete)
Comment 23 Lari Taskula 2020-10-28 04:52:18 UTC Comment hidden (obsolete)
Comment 24 Lari Taskula 2020-10-28 23:30:56 UTC
Created attachment 112651 [details] [review]
Bug 26814: (SMART-RULES GUI) Add onsite_checkout into user interface

SMART-RULES GUI tag means that this patch will apply before Bug 15522.

To test:
1. Go to Circulation rules page in Koha
2. Observe checkout type column in the rule matrix
3. Test circulation rules matrix by setting, modifying and deleting
   rules.
4. Test default checkout, hold and return policy settings.
5. Test default checkout, hold policy by patron category settings.

Sponsored-by: The National Library of Finland
Comment 25 Lari Taskula 2020-10-28 23:31:01 UTC
Created attachment 112652 [details] [review]
Bug 26814: (SMART-RULES GUI) Indent and add onsite_checkout to all_rules hash

SMART-RULES GUI tag means that this patch will apply before Bug 15522.

No other changes in this patch.

To test:
1. git diff --word-diff=color --ignore-all-space HEAD~1 -- koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
2. Observe only $osc has been added

Sponsored-by: The National Library of Finland
Comment 26 Lari Taskula 2020-10-28 23:31:05 UTC
Created attachment 112653 [details] [review]
Bug 26814: (SMART-RULES GUI) Disable hold rules when on-site option selected

SMART-RULES GUI tag means that this patch will apply before Bug 15522.

To test:
1. Apply patch
2. Go to Circulation and fine rules in staff client
3. Observe the main circulation rules matrix, observe the following columns:

   Holds allowed (total)
   Holds allowed (daily)
   Holds per record (count)
   On shelf holds allowed
   OPAC item level holds
   Article requests

4. Set "On-site checkout" to "No" or "On-site"
5. Observe the columns in step 3, they should now be disabled
6. Set "On-site checkout" to "All"
7. Observe the columns in step 3, they should now be enabled
8. Add a new rule:

   On-site checkout: "On-site"
   Current checkouts allowed: 1

   and click Save.

9. Find the row you just added and press Edit.
10. Observe the columns in step 3, they should now be disabled
11. Press Edit for any other row where On-site checkout is "All"
12. Observe the columns in step 3, they should now be enabled

Sponsored-by: The National Library of Finland
Comment 27 Lari Taskula 2020-10-29 03:28:21 UTC
What options should circulation rules have for onsite-checkout?
Option 1:
"No"    circulation_rules.onsite_checkout => 0
"Yes"   circulation_rules.onsite_checkout => 1

or

Option 2:
"All"   circulation_rules.onsite_checkout => undef
"No"    circulation_rules.onsite_checkout => 0
"Yes"   circulation_rules.onsite_checkout => 1

The problem has to do with how we process circulation_rules scopes and the "*" aka undef value.

1. With option 1 it becomes impossible to share rules for normal and on-site checkouts and likely requires a lot of rule duplication. Also, how would we handle cases where the value for onsite-checkout is not known as there is no '*' rule? Just use the "No" option?

2. My reasoning for option 2 was that it would enable sharing of rules between those two checkout types. For example, sharing a fine rule:

branch | onsite | ccode | itype | name            | value
*      | *      | *     | *     | fine            | 5
*      | 1      | *     | *     | renewalsallowed | 1
*      | 0      | *     | *     | renewalsallowed | 5

But it seems such flexibility is not supported by the current graphical user interface because it just sets all the rules in that row for that scope, even if you don't touch them (to test: simply create a new rule using a new scope and press Save, and you will see all rule columns were added into circulation_rules). So we cannot share the rules unless we do it directly in the database. Not sure how it works in the upcoming GUI.
Comment 28 Joonas Kylmälä 2020-12-11 08:08:47 UTC
Doesn't apply anymore.
Comment 29 Lari Taskula 2020-12-15 23:31:23 UTC
Created attachment 114417 [details] [review]
Bug 26814: Add a new column onsite_checkout to circulation_rules table

To add:
1. perl installer/data/mysql/updatedatabase.pl

Sponsored-by: The National Library of Finland
Comment 30 Lari Taskula 2020-12-15 23:31:26 UTC
Created attachment 114418 [details] [review]
Bug 26814: Schema changes

Sponsored-by: The National Library of Finland
Comment 31 Lari Taskula 2020-12-15 23:31:31 UTC
Created attachment 114419 [details] [review]
Bug 26814: Add onsite_checkout to Koha::CirculationRules

To test:
1. prove t/db_dependent/Koha/CirculationRules.t

Sponsored-by: The National Library of Finland
Comment 32 Lari Taskula 2020-12-15 23:31:36 UTC
Created attachment 114420 [details] [review]
Bug 26814: Remove on-site specific circulation rules

To apply:
1. perl installer/data/mysql/updatedatabase.pl

To test:
1. Search for patron_maxonsiteissueqty rule

   $ grep -rn 'patron_maxonsiteissueqty'

2. Observe no results in Koha source files (excluding .git, atomicupdates,
   updatedatabase.pl, release notes, smart-rules.pl, smart-rules.tt)
3. Search for maxonsiteissueqty rule

   $ grep -rn 'maxonsiteissueqty'

4. Observe no results (excluding .git, atomicupdates and updatedatabase.pl,
   release notes, smart-rules.pl, smart-rules.tt)
5. Check modification made to C4::Circulation::GetBranchBorrowerCircRule().
   Make sure the new parameter is being used everywhere. You can search
   the usage of this subroutine with similar grep commands as above.

6. Run the following tests:

prove t/db_dependent/Circulation/Branch.t
prove t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
prove t/db_dependent/Circulation/TooMany.t

Sponsored-by: The National Library of Finland
Comment 33 Lari Taskula 2020-12-15 23:31:40 UTC
Created attachment 114421 [details] [review]
Bug 26814: Add onsite_checkout to set_rule

To test:
1. Find all occurrences of set_rule that are missing onsite_checkout
   where required

grep --exclude-dir '.git' --exclude-dir 'misc/translator' \
--exclude-dir 'koha-tmpl' -Przo '(?s)(::|->)set_rule(?!s).*?\)' | \
grep -avz 'onsite_checkout' | grep -Pavz 'hold|reserves|article_requests|lostreturn' \
&& echo ""

   The only occasions this should return anything are cases where
   a HASH or HASHref is given to set_rule(), or that we are
   explicitly testing missing parameters in an unit test.

   1.2 Verify the HASH/HASHref cases. The hash should contain
             a onsite_checkout (unless the rule is related to holds)

2. Find all subroutines using set_rule()

git grep --no-index -n -p -P 'set_rule\s*\(' | grep -v 'sub {' \
| grep -P 'sub .*'

   2.1 The only return should be
       Koha/CirculationRules.pm=331=sub set_rules {

3. The boring step. Find all occurrences of set_rule() with
   the following command:

grep --exclude-dir='.git' -Prn 'set_rule' | grep -v set_rules

   Go through this list and make sure all neccessary locations are
   updated. A change is required when the rule scope includes
   onsite_checkout. See Koha/CirculationRules.pm for scopes.
   Hold/reserve related rules do not require it.

4. Check changes to CirculationRule objects built with TestBuilder

cd t && grep -Prn "CirculationRules?'"

   Make sure all these cases are updated with onsite_checkout => undef,
   otherwise tests will fail later on.

Sponsored-by: The National Library of Finland
Comment 34 Lari Taskula 2020-12-15 23:31:45 UTC
Created attachment 114422 [details] [review]
Bug 26814: Add onsite_checkout to set_rules

To test:
1. Find all occurrences of set_rules that are missing onsite_checkout
   where required

grep --exclude-dir '.git' --exclude-dir 'misc/translator' \
--exclude-dir 'koha-tmpl' -Przo '(?s)(::|->)set_rules.*?\)' \
--exclude="smart-rules.pl" --exclude="smart-rules.tt" | \
grep -avz 'onsite_checkout' | less

   The only occasions this should return anything are cases where
   a HASH or HASHref is given to set_rules(), or that we are
   explicitly testing missing parameters in an unit test, or that
   rules are only related to holds

   1.2 Verify the HASH/HASHref cases. The hash should contain
       a onsite_checkout (unless the rule is related to holds)

2. Find all subroutines using set_rules()

git grep --no-index -n -p -P 'set_rules\s*\(' | grep -v 'sub {' \
| grep -P 'sub .*'

   2.1 The only return should be
       t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t=401=
         sub set_holdallowed_rule {

3. The boring step. Find all occurrences of set_rules() with
   the following command:

grep --exclude-dir='.git' -Prn 'set_rules'

    Go through this list and make sure all neccessary locations are
    updated. A change is required when the rule scope includes
    onsite_checkout. See Koha/CirculationRules.pm for scopes.
    Hold/reserve related rules do not require it.

Sponsored-by: The National Library of Finland
Comment 35 Lari Taskula 2020-12-15 23:31:49 UTC
Created attachment 114423 [details] [review]
Bug 26814: Add onsite_checkout to get_effective_rule

To test:
1. Find all occurrences of get_effective_rule that are missing
   onsite_checkout where required

grep --exclude-dir '.git' --exclude-dir 'misc/translator' \
--exclude-dir 'koha-tmpl' \
-Przo '(?s)(::|->)get_effective_rule(?!s).*?\)' | \
grep -avz 'onsite_checkout' | \
grep -Pavz 'hold|reserves|article_requests|lostreturn' | less

   The only occasions this should return anything are cases where
   a HASH or HASHref is given to get_effective_rule(), or that we
   are explicitly testing missing parameters in an unit test.

   1.2 Verify the HASH/HASHref cases. The hash should contain
       a onsite_checkout (unless the rule is related to holds)

   1.3 Verify the other cases. If you find another result than
       what is mentioned above, then this test fails.

2. Find all subroutines using get_effective_rule()

git grep --no-index -n -p -P 'get_effective_rule\s*\(' \
| grep -v 'sub {' | grep -P 'sub .*'

    This list should be returned (line numbers not meaningful):

    C4/Circulation.pm=394=sub TooMany {
    C4/Circulation.pm=1375=sub AddIssue {
    C4/Circulation.pm=1697=sub GetBranchBorrowerCircRule {
    C4/Circulation.pm=1753=sub GetBranchItemRule {
    C4/Circulation.pm=3040=sub GetRenewCount {
    C4/Reserves.pm=359=sub CanItemBeReserved {
    C4/Reserves.pm=2245=sub GetHoldRule {
    Koha/Biblio.pm=295=sub article_request_type_for_bib {
    Koha/Charges/Fees.pm=93=sub accumulate_rentalcharge {
    Koha/CirculationRules.pm=221=sub get_effective_rules {
    Koha/CirculationRules.pm=395=sub get_opacitemholds_policy {
    Koha/CirculationRules.pm=421=sub get_onshelfholds_policy {
    Koha/CirculationRules.pm=443=sub get_lostreturn_policy {
    Koha/CirculationRules.pm=538=sub get_effective_daysmode {
    Koha/Item.pm=617=sub article_request_type {
    Koha/REST/V1/Checkouts.pm=192=sub allows_renewal {
    Koha/Template/Plugin/CirculationRules.pm=26=sub Get {

    Check these subroutines and make sure they are using
    get_effective_rule() with the new scope, unless not needed.
    When fetching a hold-related rules, onsite_checkout is not needed.

    These subroutines and classes need changes to sub params:

    2.1. Check modifications to Koha::Charges::Fees
    2.2. Check modification made to C4::Circulation::_debar_user_on_return().
    2.3. Check modification made to C4::Circulation::_calculate_new_debar_dt().

3. Find all occurrences of get_effective_rule with the following command:
      grep --exclude-dir='.git' -Prn 'get_effective_rule(?!s)'

    Make sure all matches (where rule scope allows onsite_checkout) are
    updated.

Sponsored-by: The National Library of Finland
Comment 36 Lari Taskula 2020-12-15 23:31:53 UTC
Created attachment 114424 [details] [review]
Bug 26814: Add onsite_checkout to get_effective_rules

To test:
1. Find all occurrences of get_effective_rules that are missing
   onsite_checkout where required

grep --exclude-dir '.git' --exclude-dir 'misc/translator' \
--exclude-dir 'koha-tmpl' \
-Przo '(?s)(::|->)get_effective_rules.*?\)' | \
grep -avz 'onsite_checkout' | less

   1.1. Verify command output. Only reserve related calls to
        get_effective_rules() should be returned.

2. Find all subroutines using get_effective_rules()

git grep --no-index -n -p -P 'get_effective_rules\s*\(' \
| grep -v 'sub {' | grep -P 'sub .*'

    This list should be returned:

C4/Circulation.pm=1663=sub GetLoanLength {
C4/Circulation.pm=1702=sub GetHardDueDate {
C4/Circulation.pm=2386=sub _calculate_new_debar_dt {
C4/Circulation.pm=2736=sub CanBookBeRenewed {
C4/Circulation.pm=3160=sub GetRenewCount {
C4/Circulation.pm=3239=sub GetSoonestRenewDate {
C4/Circulation.pm=3303=sub GetLatestAutoRenewDate {
C4/Overdues.pm=224=sub CalcFine {
C4/Reserves.pm=2263=sub GetHoldRule {

    Check these subroutines and make sure they are using
    get_effective_rules() with the new scope, unless not needed.
    When fetching a hold-related rules, checkout_type is not needed.

3. Check modification made to C4::Circulation::CalcDateDue()
4. Check modification made to C4::Circulation::CalcFine()
5. Check modification made to C4::Circulation::GetLoanLength()
6. Check modification made to C4::Circulation::GetHardDueDate()

    Make sure all matches (where rule scope allows onsite_checkout) are
    updated.

7. Run these unit tests:

grep -Prl --color=never 'get_effective_rule\(|set_rule\(|set_rules\(' \
t/db_dependent/ | grep '^.*\.t$' | xargs prove

8. Test onboarding tool
8.1. Drop&create your database (take backups first if needed)
8.2. Navigate to web installer
8.3. Continue until you reach onboarding tool step where circulation
     rules are added.
8.4. Observe new drop down selection "Checkout type".
8.5. Select any checkout type and continue the onboarding process.
8.6. Investigate your database table "circulation_rules". You should
     now have rules with onsite_checkout of your choice.

Sponsored-by: The National Library of Finland
Comment 37 Lari Taskula 2020-12-15 23:31:57 UTC
Created attachment 114425 [details] [review]
Bug 26814: (SMART-RULES GUI) Add onsite_checkout into user interface

SMART-RULES GUI tag means that this patch will apply before Bug 15522.

To test:
1. Go to Circulation rules page in Koha
2. Observe checkout type column in the rule matrix
3. Test circulation rules matrix by setting, modifying and deleting
   rules.
4. Test default checkout, hold and return policy settings.
5. Test default checkout, hold policy by patron category settings.

Sponsored-by: The National Library of Finland
Comment 38 Lari Taskula 2020-12-15 23:32:01 UTC
Created attachment 114426 [details] [review]
Bug 26814: (SMART-RULES GUI) Indent and add onsite_checkout to all_rules hash

SMART-RULES GUI tag means that this patch will apply before Bug 15522.

No other changes in this patch.

To test:
1. git diff --word-diff=color --ignore-all-space HEAD~1 -- koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
2. Observe only $osc has been added

Sponsored-by: The National Library of Finland
Comment 39 Lari Taskula 2020-12-15 23:32:06 UTC
Created attachment 114427 [details] [review]
Bug 26814: (SMART-RULES GUI) Disable hold rules when on-site option selected

SMART-RULES GUI tag means that this patch will apply before Bug 15522.

To test:
1. Apply patch
2. Go to Circulation and fine rules in staff client
3. Observe the main circulation rules matrix, observe the following columns:

   Holds allowed (total)
   Holds allowed (daily)
   Holds per record (count)
   On shelf holds allowed
   OPAC item level holds
   Article requests

4. Set "On-site checkout" to "No" or "On-site"
5. Observe the columns in step 3, they should now be disabled
6. Set "On-site checkout" to "All"
7. Observe the columns in step 3, they should now be enabled
8. Add a new rule:

   On-site checkout: "On-site"
   Current checkouts allowed: 1

   and click Save.

9. Find the row you just added and press Edit.
10. Observe the columns in step 3, they should now be disabled
11. Press Edit for any other row where On-site checkout is "All"
12. Observe the columns in step 3, they should now be enabled

Sponsored-by: The National Library of Finland