Bug 25037 - Add checkout_type to checkouts
Summary: Add checkout_type to checkouts
Status: Patch doesn't apply
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low new feature
Assignee: Lari Taskula
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 25089 25103
  Show dependency treegraph
 
Reported: 2020-04-02 05:26 UTC by Lari Taskula
Modified: 2020-10-28 02:52 UTC (History)
6 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 25037: Add authorised value for checkout types (2.24 KB, patch)
2020-04-02 05:28 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Add authorised values to sample data (7.71 KB, patch)
2020-04-02 05:28 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Convert issues.onsite_checkout to issues.checkout_type (6.48 KB, patch)
2020-04-02 05:28 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Schema changes (5.32 KB, patch)
2020-04-02 05:28 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Replace onsite_checkout with checkout_type (17.29 KB, patch)
2020-04-02 05:29 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Replace onsite_checkout with checkout_type (17.29 KB, patch)
2020-04-02 05:33 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Add authorised value for checkout types (2.24 KB, patch)
2020-04-02 06:05 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Add authorised values to sample data (7.71 KB, patch)
2020-04-02 06:05 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Convert issues.onsite_checkout to issues.checkout_type (6.48 KB, patch)
2020-04-02 06:05 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Schema changes (5.32 KB, patch)
2020-04-02 06:05 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Replace occurences to issues.onsite_checkout (17.44 KB, patch)
2020-04-02 06:05 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Validate checkout_type in Koha::Checkout->store (4.81 KB, patch)
2020-04-02 07:02 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Add authorised value for checkout types (2.29 KB, patch)
2020-04-05 19:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Add authorised values to sample data (7.75 KB, patch)
2020-04-05 19:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Convert issues.onsite_checkout to issues.checkout_type (6.53 KB, patch)
2020-04-05 19:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Schema changes (5.37 KB, patch)
2020-04-05 19:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Replace occurences to issues.onsite_checkout (17.49 KB, patch)
2020-04-05 19:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Validate checkout_type in Koha::Checkout->store (4.85 KB, patch)
2020-04-05 19:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Add authorised value for checkout types (2.52 KB, patch)
2020-04-05 20:19 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Add authorised values to sample data (9.05 KB, patch)
2020-04-05 20:19 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Convert issues.onsite_checkout to issues.checkout_type (6.87 KB, patch)
2020-04-05 20:19 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Schema changes (5.43 KB, patch)
2020-04-05 20:19 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Replace occurences to issues.onsite_checkout (17.42 KB, patch)
2020-04-05 20:19 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Validate checkout_type in Koha::Checkout->store (4.85 KB, patch)
2020-04-05 20:19 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Add authorised value for checkout types (2.52 KB, patch)
2020-04-08 18:34 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Add authorised values to sample data (9.05 KB, patch)
2020-04-08 18:34 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Convert issues.onsite_checkout to issues.checkout_type (6.90 KB, patch)
2020-04-08 18:34 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Schema changes (5.36 KB, patch)
2020-04-08 18:34 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Add Koha::Checkouts checkout_type constants and TT plugin (6.04 KB, patch)
2020-04-08 18:34 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Validate checkout_type in Koha::Checkout->store (4.89 KB, patch)
2020-04-08 18:34 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Replace occurences to issues.onsite_checkout (18.58 KB, patch)
2020-04-08 18:34 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Add authorised value for checkout types (2.52 KB, patch)
2020-04-08 18:45 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Add authorised values to sample data (9.05 KB, patch)
2020-04-08 18:46 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Convert issues.onsite_checkout to issues.checkout_type (6.90 KB, patch)
2020-04-08 18:46 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Schema changes (5.36 KB, patch)
2020-04-08 18:46 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Add Koha::Checkouts checkout_type constants and TT plugin (6.02 KB, patch)
2020-04-08 18:46 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Validate checkout_type in Koha::Checkout->store (4.88 KB, patch)
2020-04-08 18:46 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Replace occurences to issues.onsite_checkout (18.58 KB, patch)
2020-04-08 18:46 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Convert issues.onsite_checkout to issues.checkout_type (5.92 KB, patch)
2020-04-09 19:47 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Schema changes (3.08 KB, patch)
2020-04-09 19:48 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Add Koha::Checkouts checkout_type constants and TT plugin (6.02 KB, patch)
2020-04-09 19:48 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Replace occurences to issues.onsite_checkout (18.58 KB, patch)
2020-04-09 19:48 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Convert issues.onsite_checkout to issues.checkout_type (5.91 KB, patch)
2020-04-09 20:00 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Schema changes (3.34 KB, patch)
2020-04-09 20:00 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Add Koha::Checkouts checkout_type constants and TT plugin (6.02 KB, patch)
2020-04-09 20:00 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Replace occurences to issues.onsite_checkout (18.58 KB, patch)
2020-04-09 20:00 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Convert issues.onsite_checkout to issues.checkout_type (4.91 KB, patch)
2020-04-09 20:08 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Schema changes (3.34 KB, patch)
2020-04-09 20:09 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Add Koha::Checkouts checkout_type constants and TT plugin (6.02 KB, patch)
2020-04-09 20:09 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Replace occurences to issues.onsite_checkout (18.58 KB, patch)
2020-04-09 20:09 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Add Koha::Checkouts checkout_type constants and TT plugin (6.05 KB, patch)
2020-04-09 21:37 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Replace occurences to issues.onsite_checkout (18.58 KB, patch)
2020-04-09 21:37 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Add Koha::Checkouts checkout_type constants and TT plugin (6.09 KB, patch)
2020-05-04 12:40 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Replace occurences to issues.onsite_checkout (18.61 KB, patch)
2020-05-04 12:41 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: (follow-up) Fix checkout_type property description (1.82 KB, patch)
2020-06-08 11:21 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25037: Convert issues.onsite_checkout to issues.checkout_type (5.07 KB, patch)
2020-06-12 09:47 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 25037: Schema changes (3.49 KB, patch)
2020-06-12 09:47 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 25037: Add Koha::Checkouts checkout_type constants and TT plugin (6.25 KB, patch)
2020-06-12 09:47 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 25037: Replace occurences to issues.onsite_checkout (18.82 KB, patch)
2020-06-12 09:47 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 25037: (follow-up) Fix checkout_type property description (1.97 KB, patch)
2020-06-12 09:47 UTC, Joonas Kylmälä
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-04-02 05:26:30 UTC
This is a blocker to Bug 24101.

As of now we have two checkout types, a normal checkout and an on-site checkout type.

In order to target circulation rules to a specific checkout type, the issues table must have information on which type of checkout it is. On-site checkout is currently determined by a boolean value in issues.onsite_checkout database column. For supporting multiple checkout types, we should convert this column into a generic checkout_type field.

This Bug also sets an easy-to-extend base for further checkout types. In theory a library could define any types of checkouts dynamically, but support for that is outside the scope of this Bug.
Comment 1 Lari Taskula 2020-04-02 05:28:49 UTC Comment hidden (obsolete)
Comment 2 Lari Taskula 2020-04-02 05:28:52 UTC Comment hidden (obsolete)
Comment 3 Lari Taskula 2020-04-02 05:28:55 UTC Comment hidden (obsolete)
Comment 4 Lari Taskula 2020-04-02 05:28:58 UTC Comment hidden (obsolete)
Comment 5 Lari Taskula 2020-04-02 05:29:01 UTC Comment hidden (obsolete)
Comment 6 Lari Taskula 2020-04-02 05:33:54 UTC Comment hidden (obsolete)
Comment 7 Lari Taskula 2020-04-02 06:05:33 UTC Comment hidden (obsolete)
Comment 8 Lari Taskula 2020-04-02 06:05:36 UTC Comment hidden (obsolete)
Comment 9 Lari Taskula 2020-04-02 06:05:39 UTC Comment hidden (obsolete)
Comment 10 Lari Taskula 2020-04-02 06:05:42 UTC Comment hidden (obsolete)
Comment 11 Lari Taskula 2020-04-02 06:05:45 UTC Comment hidden (obsolete)
Comment 12 Lari Taskula 2020-04-02 07:02:06 UTC Comment hidden (obsolete)
Comment 13 Lari Taskula 2020-04-05 19:31:17 UTC Comment hidden (obsolete)
Comment 14 Lari Taskula 2020-04-05 19:31:20 UTC Comment hidden (obsolete)
Comment 15 Lari Taskula 2020-04-05 19:31:23 UTC Comment hidden (obsolete)
Comment 16 Lari Taskula 2020-04-05 19:31:27 UTC Comment hidden (obsolete)
Comment 17 Lari Taskula 2020-04-05 19:31:30 UTC Comment hidden (obsolete)
Comment 18 Lari Taskula 2020-04-05 19:31:33 UTC Comment hidden (obsolete)
Comment 19 Lari Taskula 2020-04-05 20:19:32 UTC Comment hidden (obsolete)
Comment 20 Lari Taskula 2020-04-05 20:19:35 UTC Comment hidden (obsolete)
Comment 21 Lari Taskula 2020-04-05 20:19:38 UTC Comment hidden (obsolete)
Comment 22 Lari Taskula 2020-04-05 20:19:42 UTC Comment hidden (obsolete)
Comment 23 Lari Taskula 2020-04-05 20:19:47 UTC Comment hidden (obsolete)
Comment 24 Lari Taskula 2020-04-05 20:19:50 UTC Comment hidden (obsolete)
Comment 25 Lari Taskula 2020-04-08 18:34:22 UTC Comment hidden (obsolete)
Comment 26 Lari Taskula 2020-04-08 18:34:26 UTC Comment hidden (obsolete)
Comment 27 Lari Taskula 2020-04-08 18:34:29 UTC Comment hidden (obsolete)
Comment 28 Lari Taskula 2020-04-08 18:34:33 UTC Comment hidden (obsolete)
Comment 29 Lari Taskula 2020-04-08 18:34:37 UTC Comment hidden (obsolete)
Comment 30 Lari Taskula 2020-04-08 18:34:41 UTC Comment hidden (obsolete)
Comment 31 Lari Taskula 2020-04-08 18:34:45 UTC Comment hidden (obsolete)
Comment 32 Lari Taskula 2020-04-08 18:45:56 UTC Comment hidden (obsolete)
Comment 33 Lari Taskula 2020-04-08 18:46:01 UTC Comment hidden (obsolete)
Comment 34 Lari Taskula 2020-04-08 18:46:06 UTC Comment hidden (obsolete)
Comment 35 Lari Taskula 2020-04-08 18:46:10 UTC Comment hidden (obsolete)
Comment 36 Lari Taskula 2020-04-08 18:46:13 UTC Comment hidden (obsolete)
Comment 37 Lari Taskula 2020-04-08 18:46:17 UTC Comment hidden (obsolete)
Comment 38 Lari Taskula 2020-04-08 18:46:20 UTC Comment hidden (obsolete)
Comment 39 Katrin Fischer 2020-04-08 21:44:18 UTC
Hi Lari,

I really like the idea of circulation types - I think I actually suggested something like that when on-site was introduced at the time, because I know other systems use this concept.

I have 2 comments for discussion/consideration.

1) "This commit adds an authorised value category "CHECKOUT_TYPE" and
an authorised value "OS" (on-site) and "C" (normal checkout)."

I feel this is not necessary and would vote for hard-coding the circulation types and their descriptions. It will make it translatable and keep people from 'messing' with it. Unless you plan to really allow people to add a lot more of those?  I'd also use more speakign abbreviations, maybe 'ONSITE' and CHECKOUT - no need to make things more cryptical than necessary. It would help doing reports etc. too.

2) On the mailing list it was suggested to have 'normal' 'on-site' and 'all'. I am not sure about the 'all'. I feel it might be complicating things more than necessary. If all was to preserve current behaviour, I don't think that would work as on-site currently is 1 day by default, independent of the normal loan period.
Comment 40 Lari Taskula 2020-04-09 19:30:10 UTC
(In reply to Katrin Fischer from comment #39)
> Hi Lari,
> 
> I really like the idea of circulation types - I think I actually suggested
> something like that when on-site was introduced at the time, because I know
> other systems use this concept.
Just curious, which systems and how does it work in them?

> I have 2 comments for discussion/consideration.
> 
> 1) "This commit adds an authorised value category "CHECKOUT_TYPE" and
> an authorised value "OS" (on-site) and "C" (normal checkout)."
> 
> I feel this is not necessary and would vote for hard-coding the circulation
> types and their descriptions. It will make it translatable and keep people
> from 'messing' with it. Unless you plan to really allow people to add a lot
> more of those?
The default values are both hardcoded and exist as authorised value. While we are at it, my idea was to add database support for dynamicity. Although, fully supporting such feature would require more changes in the GUI and circulation logic which I do not plan to implement.

What comes to messing with the values - we can set a RESTRICT flag into the foreign key which means one cannot change nor delete the authorised values, unless the whole issues and old_issues table is empty. This way a librarian cannot accidentally mess up a production system (unless they really did not had any checkouts (active or returned) yet).

But we can proceed with only hardcoding the values. We can move the authorised value patches to another Bug for someone else to continue from there. In order to protect issues.checkout_type from non-existing types, we could use an ENUM data type that only allows a value from a set of possible hardcoded values.

I will do that now so we can get this Bug moving easier. We can then later choose whether to include auth value functionality or not.

> I'd also use more speakign abbreviations, maybe 'ONSITE' and
> CHECKOUT - no need to make things more cryptical than necessary. It would
> help doing reports etc. too.
I don't have any preference on the code choices. We can make it more readable.

> 2) On the mailing list it was suggested to have 'normal' 'on-site' and
> 'all'. I am not sure about the 'all'. I feel it might be complicating things
> more than necessary. If all was to preserve current behaviour, I don't think
> that would work as on-site currently is 1 day by default, independent of the
> normal loan period.
The circulation rules part of this Bug was moved to Bug 25089. "All" is useful if you only need to separate one or two rules between the types of checkouts. As an example, you could then have one full set of rules with "All" scope, and loan period = 1, renewals allowed = 0 with "on-site" scope.

Thx for your input!
Comment 41 Lari Taskula 2020-04-09 19:47:58 UTC Comment hidden (obsolete)
Comment 42 Lari Taskula 2020-04-09 19:48:01 UTC Comment hidden (obsolete)
Comment 43 Lari Taskula 2020-04-09 19:48:05 UTC Comment hidden (obsolete)
Comment 44 Lari Taskula 2020-04-09 19:48:08 UTC Comment hidden (obsolete)
Comment 45 Lari Taskula 2020-04-09 20:00:46 UTC Comment hidden (obsolete)
Comment 46 Lari Taskula 2020-04-09 20:00:51 UTC Comment hidden (obsolete)
Comment 47 Lari Taskula 2020-04-09 20:00:54 UTC Comment hidden (obsolete)
Comment 48 Lari Taskula 2020-04-09 20:00:58 UTC Comment hidden (obsolete)
Comment 49 Lari Taskula 2020-04-09 20:08:59 UTC
Created attachment 102663 [details] [review]
Bug 25037: Convert issues.onsite_checkout to issues.checkout_type

If onsite_checkout=1, checkout_type becomes "ONSITE"
Else checkout_type becomes "CHECKOUT"

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

Sponsored-by: The National Library of Finland
Comment 50 Lari Taskula 2020-04-09 20:09:03 UTC
Created attachment 102664 [details] [review]
Bug 25037: Schema changes

Sponsored-by: The National Library of Finland
Comment 51 Lari Taskula 2020-04-09 20:09:07 UTC Comment hidden (obsolete)
Comment 52 Lari Taskula 2020-04-09 20:09:11 UTC Comment hidden (obsolete)
Comment 53 Lari Taskula 2020-04-09 21:37:43 UTC
Created attachment 102675 [details] [review]
Bug 25037: Add Koha::Checkouts checkout_type constants and TT plugin

Also make it available for templates.

To test:
1. prove t/db_dependent/Koha/Checkouts.t
2. prove t/db_dependent/Template/Plugin/Checkouts.t

Sponsored-by: The National Library of Finland
Comment 54 Lari Taskula 2020-04-09 21:37:47 UTC
Created attachment 102676 [details] [review]
Bug 25037: Replace occurences to issues.onsite_checkout

This patch replaces all occurences to issues.onsite_checkout so that
on-site checkouts work normally.

It does not replace any on-site checkout logic.

To test:

1. Investigate Koha source files via command line. We must make sure
   there are no longer references to issues.onsite_checkout column.
   You can do this by searching all occurrences of onsite_checkout.

   grep -rn 'onsite_checkout' | grep -v '\.git'

2. Run all these commands in your command line interface:

grep -rl --color=never 'onsite_checkout' | grep '^.*\.t$' | xargs prove
prove t/db_dependent/Koha/Account/Line.t
prove t/db_dependent/api/v1/checkouts.t

3. Verify koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
   - Create a record and add two items to it
   - Checkout both of them, one as on-site checkout and one as normal
   - Go to INTRANET /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX
     where XXX is your record's biblionumber
   - Observe Holdings table column Status
      => your on-site checked out item should say "Currently in local
         use by ..."
      => your normal checked out item should say "Checked out to ..."

4. Verify koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc
   - Repeat step 3 but this time go to OPAC
     /cgi-bin/koha/opac-detail.pl?biblionumber=XXX where XXX is your
     record's biblionumber
   - Observe Holdings table column Status
      => your on-site checked out item should say "Currently in local
         use"
      => your normal checked out item should say "Checked out"

5. Verify koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt
   - After step 4, go to INTRANET
     /cgi-bin/koha/members/readingrec.pl?borrowernumber=51
   - You will see circulation history table.
   - Observe your two checkouts from step 3
   - Click "Checkouts" tab
   - Observe your normal checkout in the list
   - Click "On-site checkouts"" tab
   - Observe your on-site checkout in the list

6. Verify koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt
   - Repeat step 5 but this time go to OPAC
     /cgi-bin/koha/opac-readingrecord.pl

Sponsored-by: The National Library of Finland
Comment 55 Bernardo Gonzalez Kriegel 2020-04-30 18:38:16 UTC
Hi, two fail sfrom qa-tools

FAIL   Koha/Checkouts.pm
   OK     critic
   OK     forbidden patterns
   OK     git manipulation
   FAIL   pod
                *** ERROR: =cut found outside a pod block.  Skipping to next block.  in file Koha/Checkouts.pm
   OK     pod coverage
   OK     spelling
   OK     valid

 FAIL   Koha/Template/Plugin/Checkouts.pm
   OK     critic
   FAIL   forbidden patterns
                forbidden pattern: Warning: The 'sub type' may be wrong is declared in a Koha::* package (see bug 15446) (line 57)
   OK     git manipulation
   OK     pod
   OK     pod coverage
   OK     spelling
   OK     valid

First is not a problem, the second one needs revision
Comment 56 Lari Taskula 2020-05-04 12:15:37 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #55)
>  FAIL   Koha/Template/Plugin/Checkouts.pm
>    OK     critic
>    FAIL   forbidden patterns
>                 forbidden pattern: Warning: The 'sub type' may be wrong is
> declared in a Koha::* package (see bug 15446) (line 57)
>    OK     git manipulation
>    OK     pod
>    OK     pod coverage
>    OK     spelling
>    OK     valid
> 
> ..., the second one needs revision

It is not really a problem because it is not a Koha::Object, but let's change just for the sake of explicity.
Comment 57 Lari Taskula 2020-05-04 12:40:53 UTC
Created attachment 104267 [details] [review]
Bug 25037: Add Koha::Checkouts checkout_type constants and TT plugin

Also make it available for templates.

To test:
1. prove t/db_dependent/Koha/Checkouts.t
2. prove t/db_dependent/Template/Plugin/Checkouts.t

Sponsored-by: The National Library of Finland
Comment 58 Lari Taskula 2020-05-04 12:41:10 UTC
Created attachment 104268 [details] [review]
Bug 25037: Replace occurences to issues.onsite_checkout

This patch replaces all occurences to issues.onsite_checkout so that
on-site checkouts work normally.

It does not replace any on-site checkout logic.

To test:

1. Investigate Koha source files via command line. We must make sure
   there are no longer references to issues.onsite_checkout column.
   You can do this by searching all occurrences of onsite_checkout.

   grep -rn 'onsite_checkout' | grep -v '\.git'

2. Run all these commands in your command line interface:

grep -rl --color=never 'onsite_checkout' | grep '^.*\.t$' | xargs prove
prove t/db_dependent/Koha/Account/Line.t
prove t/db_dependent/api/v1/checkouts.t

3. Verify koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
   - Create a record and add two items to it
   - Checkout both of them, one as on-site checkout and one as normal
   - Go to INTRANET /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX
     where XXX is your record's biblionumber
   - Observe Holdings table column Status
      => your on-site checked out item should say "Currently in local
         use by ..."
      => your normal checked out item should say "Checked out to ..."

4. Verify koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc
   - Repeat step 3 but this time go to OPAC
     /cgi-bin/koha/opac-detail.pl?biblionumber=XXX where XXX is your
     record's biblionumber
   - Observe Holdings table column Status
      => your on-site checked out item should say "Currently in local
         use"
      => your normal checked out item should say "Checked out"

5. Verify koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt
   - After step 4, go to INTRANET
     /cgi-bin/koha/members/readingrec.pl?borrowernumber=51
   - You will see circulation history table.
   - Observe your two checkouts from step 3
   - Click "Checkouts" tab
   - Observe your normal checkout in the list
   - Click "On-site checkouts"" tab
   - Observe your on-site checkout in the list

6. Verify koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt
   - Repeat step 5 but this time go to OPAC
     /cgi-bin/koha/opac-readingrecord.pl

Sponsored-by: The National Library of Finland
Comment 59 Joonas Kylmälä 2020-06-05 09:25:39 UTC
Comment on attachment 104268 [details] [review]
Bug 25037: Replace occurences to issues.onsite_checkout

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

I checked all the other patches and they were looking good!

::: api/v1/swagger/definitions/checkout.json
@@ +53,5 @@
>        "description": "Date the item was issued"
>      },
> +    "checkout_type": {
> +      "type": ["string", "null"],
> +      "description": "Checkout type, an authorised value under CHECKOUT_TYPE category"

The description is still from the v1 version where authorized values were used. Could you here a) add enum for values "CHECKOUT" and "ONSITE" and b) update the description.
Comment 60 Lari Taskula 2020-06-08 11:21:30 UTC
Created attachment 105638 [details] [review]
Bug 25037: (follow-up) Fix checkout_type property description

Also adds a test of the property to checkouts.t REST test.

To test:
1. prove t/db_dependent/api/v1/checkouts.t
Comment 61 Lari Taskula 2020-06-08 11:22:35 UTC
(In reply to Joonas Kylmälä from comment #59)
> Comment on attachment 104268 [details] [review] [review]
> Bug 25037: Replace occurences to issues.onsite_checkout
> 
> Review of attachment 104268 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> I checked all the other patches and they were looking good!
> 
> ::: api/v1/swagger/definitions/checkout.json
> @@ +53,5 @@
> >        "description": "Date the item was issued"
> >      },
> > +    "checkout_type": {
> > +      "type": ["string", "null"],
> > +      "description": "Checkout type, an authorised value under CHECKOUT_TYPE category"
> 
> The description is still from the v1 version where authorized values were
> used. Could you here a) add enum for values "CHECKOUT" and "ONSITE" and b)
> update the description.

Done! Thanks for noticing.
Comment 62 Joonas Kylmälä 2020-06-12 09:47:44 UTC
Created attachment 105780 [details] [review]
Bug 25037: Convert issues.onsite_checkout to issues.checkout_type

If onsite_checkout=1, checkout_type becomes "ONSITE"
Else checkout_type becomes "CHECKOUT"

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

Sponsored-by: The National Library of Finland
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 63 Joonas Kylmälä 2020-06-12 09:47:48 UTC
Created attachment 105781 [details] [review]
Bug 25037: Schema changes

Sponsored-by: The National Library of Finland
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 64 Joonas Kylmälä 2020-06-12 09:47:51 UTC
Created attachment 105782 [details] [review]
Bug 25037: Add Koha::Checkouts checkout_type constants and TT plugin

Also make it available for templates.

To test:
1. prove t/db_dependent/Koha/Checkouts.t
2. prove t/db_dependent/Template/Plugin/Checkouts.t

Sponsored-by: The National Library of Finland
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 65 Joonas Kylmälä 2020-06-12 09:47:55 UTC
Created attachment 105783 [details] [review]
Bug 25037: Replace occurences to issues.onsite_checkout

This patch replaces all occurences to issues.onsite_checkout so that
on-site checkouts work normally.

It does not replace any on-site checkout logic.

To test:

1. Investigate Koha source files via command line. We must make sure
   there are no longer references to issues.onsite_checkout column.
   You can do this by searching all occurrences of onsite_checkout.

   grep -rn 'onsite_checkout' | grep -v '\.git'

2. Run all these commands in your command line interface:

grep -rl --color=never 'onsite_checkout' | grep '^.*\.t$' | xargs prove
prove t/db_dependent/Koha/Account/Line.t
prove t/db_dependent/api/v1/checkouts.t

3. Verify koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
   - Create a record and add two items to it
   - Checkout both of them, one as on-site checkout and one as normal
   - Go to INTRANET /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX
     where XXX is your record's biblionumber
   - Observe Holdings table column Status
      => your on-site checked out item should say "Currently in local
         use by ..."
      => your normal checked out item should say "Checked out to ..."

4. Verify koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc
   - Repeat step 3 but this time go to OPAC
     /cgi-bin/koha/opac-detail.pl?biblionumber=XXX where XXX is your
     record's biblionumber
   - Observe Holdings table column Status
      => your on-site checked out item should say "Currently in local
         use"
      => your normal checked out item should say "Checked out"

5. Verify koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt
   - After step 4, go to INTRANET
     /cgi-bin/koha/members/readingrec.pl?borrowernumber=51
   - You will see circulation history table.
   - Observe your two checkouts from step 3
   - Click "Checkouts" tab
   - Observe your normal checkout in the list
   - Click "On-site checkouts"" tab
   - Observe your on-site checkout in the list

6. Verify koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt
   - Repeat step 5 but this time go to OPAC
     /cgi-bin/koha/opac-readingrecord.pl

Sponsored-by: The National Library of Finland
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 66 Joonas Kylmälä 2020-06-12 09:47:59 UTC
Created attachment 105784 [details] [review]
Bug 25037: (follow-up) Fix checkout_type property description

Also adds a test of the property to checkouts.t REST test.

To test:
1. prove t/db_dependent/api/v1/checkouts.t

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 67 Katrin Fischer 2020-08-16 10:08:41 UTC
This patch set on longer applies. Please rebase!

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 25037: Convert issues.onsite_checkout to issues.checkout_type
Applying: Bug 25037: Schema changes
Applying: Bug 25037: Add Koha::Checkouts checkout_type constants and TT plugin
Applying: Bug 25037: Replace occurences to issues.onsite_checkout
Using index info to reconstruct a base tree...
M	C4/Circulation.pm
M	C4/Items.pm
M	koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt
M	koha-tmpl/intranet-tmpl/prog/js/checkouts.js
M	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt
M	t/db_dependent/Circulation.t
M	t/db_dependent/Items.t
M	t/db_dependent/Koha/Account/Line.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Koha/Account/Line.t
Auto-merging t/db_dependent/Items.t
Auto-merging t/db_dependent/Circulation.t
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/js/checkouts.js
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt
Auto-merging C4/Items.pm
Auto-merging C4/Circulation.pm
CONFLICT (content): Merge conflict in C4/Circulation.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 25037: Replace occurences to issues.onsite_checkout
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-25037-Replace-occurences-to-issuesonsitechecko-4HO8b_.patch