Bug 20912 - Rental fees based on time period
Summary: Rental fees based on time period
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Kyle M Hall
QA Contact: Martin Renvoize
URL:
Keywords: Manual
Depends on: 21721
Blocks: 21443 23382
  Show dependency treegraph
 
Reported: 2018-06-12 04:00 UTC by Kyle M Hall
Modified: 2019-08-06 13:32 UTC (History)
8 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Sponsored by: Huntsville-Madison County Public Library (http://hmcpl.org/) This new feature adds the ability to define a per day (or per hour) rental fee for rental items. The new fee may be used as an alternative to the fixed price rental fee or in conjunction with it.
Version(s) released in:
19.05.00


Attachments
Bug 20912: Update database (1.74 KB, patch)
2018-06-14 18:36 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Rental Fees based on Time Period (21.55 KB, patch)
2018-06-14 18:36 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Update schema file (1.53 KB, patch)
2018-06-14 18:36 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Rental Fees based on Time Period (23.53 KB, patch)
2018-06-14 18:51 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Update schema file (1.53 KB, patch)
2018-06-14 18:52 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Update database (1.77 KB, patch)
2018-07-27 11:48 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Rental Fees based on Time Period (23.56 KB, patch)
2018-07-27 11:48 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Update schema file (1.56 KB, patch)
2018-07-27 11:48 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Update database (1.79 KB, patch)
2018-10-25 12:42 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Rental Fees based on Time Period (22.74 KB, patch)
2018-10-25 12:42 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Update schema file (1.53 KB, patch)
2018-10-25 12:42 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix number of tests and correct typos (5.47 KB, patch)
2018-10-29 06:39 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20912: Update database (1.79 KB, patch)
2018-10-31 19:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Rental Fees based on Time Period (22.75 KB, patch)
2018-10-31 19:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Update schema file (1.53 KB, patch)
2018-10-31 19:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix number of tests and correct typos (5.48 KB, patch)
2018-10-31 19:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test (955 bytes, patch)
2018-10-31 19:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings (3.29 KB, patch)
2018-10-31 19:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Move calculation to Koha::Fees (20.42 KB, patch)
2018-11-09 18:34 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Move calculation to Koha::Fees (20.61 KB, patch)
2018-11-09 18:53 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Update database (1.79 KB, patch)
2019-01-25 17:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Rental Fees based on Time Period (22.19 KB, patch)
2019-01-25 17:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Update schema file (1.53 KB, patch)
2019-01-25 17:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix number of tests and correct typos (5.50 KB, patch)
2019-01-25 17:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test (955 bytes, patch)
2019-01-25 17:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings (3.29 KB, patch)
2019-01-25 17:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Move calculation to Koha::Fees (20.06 KB, patch)
2019-01-25 17:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912 (QA Follow-up): Fix up Koha::Fees (1.18 KB, patch)
2019-01-25 17:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912 (QA Follow-up): Fix up unit tests (1.05 KB, patch)
2019-01-25 17:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Update database (1.83 KB, patch)
2019-01-31 15:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Rental Fees based on Time Period (22.24 KB, patch)
2019-01-31 15:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Update schema file (1.57 KB, patch)
2019-01-31 15:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix number of tests and correct typos (5.55 KB, patch)
2019-01-31 15:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test (1004 bytes, patch)
2019-01-31 15:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings (3.34 KB, patch)
2019-01-31 15:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Move calculation to Koha::Fees (20.11 KB, patch)
2019-01-31 15:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up): Fix up Koha::Fees (1.23 KB, patch)
2019-01-31 15:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up): Fix up unit tests (1.09 KB, patch)
2019-01-31 15:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Move Fees to Charges:: (3.27 KB, patch)
2019-01-31 15:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Test for distinct accountlines (2.57 KB, patch)
2019-01-31 15:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make DB Update idempotent (1.84 KB, patch)
2019-01-31 15:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (follow-up) Improve test coverage (17.95 KB, patch)
2019-01-31 15:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (follow-up) Add hourly support (12.32 KB, patch)
2019-01-31 15:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix TestBuilder (784 bytes, patch)
2019-01-31 15:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix some test failures (2.37 KB, patch)
2019-01-31 15:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Update database (1.85 KB, patch)
2019-02-01 13:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Rental Fees based on Time Period (22.25 KB, patch)
2019-02-01 13:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Update schema file (1.64 KB, patch)
2019-02-01 13:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix number of tests and correct typos (5.62 KB, patch)
2019-02-01 13:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test (1.05 KB, patch)
2019-02-01 13:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings (3.41 KB, patch)
2019-02-01 13:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Move calculation to Koha::Fees (20.18 KB, patch)
2019-02-01 13:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up): Fix up Koha::Fees (1.30 KB, patch)
2019-02-01 13:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up): Fix up unit tests (1.17 KB, patch)
2019-02-01 13:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Move Fees to Charges:: (3.34 KB, patch)
2019-02-01 13:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Test for distinct accountlines (2.64 KB, patch)
2019-02-01 13:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make DB Update idempotent (1.91 KB, patch)
2019-02-01 13:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (follow-up) Improve test coverage (18.02 KB, patch)
2019-02-01 13:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (follow-up) Add hourly support (12.39 KB, patch)
2019-02-01 13:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix TestBuilder (856 bytes, patch)
2019-02-01 13:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix some test failures (2.44 KB, patch)
2019-02-01 13:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Update database (1.89 KB, patch)
2019-02-04 14:37 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20912: Rental Fees based on Time Period (22.29 KB, patch)
2019-02-04 14:37 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20912: Update schema file (1.68 KB, patch)
2019-02-04 14:37 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix number of tests and correct typos (5.66 KB, patch)
2019-02-04 14:37 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test (1.09 KB, patch)
2019-02-04 14:37 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings (3.45 KB, patch)
2019-02-04 14:37 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20912: Move calculation to Koha::Fees (20.21 KB, patch)
2019-02-04 14:38 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20912: (QA follow-up): Fix up Koha::Fees (1.34 KB, patch)
2019-02-04 14:38 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20912: (QA follow-up): Fix up unit tests (1.20 KB, patch)
2019-02-04 14:38 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Move Fees to Charges:: (3.37 KB, patch)
2019-02-04 14:38 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Test for distinct accountlines (2.67 KB, patch)
2019-02-04 14:38 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make DB Update idempotent (1.95 KB, patch)
2019-02-04 14:38 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20912: (follow-up) Improve test coverage (18.06 KB, patch)
2019-02-04 14:38 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20912: (follow-up) Add hourly support (11.95 KB, patch)
2019-02-04 14:38 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix TestBuilder (840 bytes, patch)
2019-02-04 14:38 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix some test failures (2.42 KB, patch)
2019-02-04 14:38 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) POD fix (649 bytes, patch)
2019-02-04 14:38 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) The default rule already has renewalsallowed=0 (933 bytes, patch)
2019-02-04 14:38 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 20912: Update database (1.90 KB, patch)
2019-02-19 18:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Rental Fees based on Time Period (22.07 KB, patch)
2019-02-19 18:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Update schema file (1.70 KB, patch)
2019-02-19 18:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix number of tests and correct typos (5.67 KB, patch)
2019-02-19 18:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test (1.10 KB, patch)
2019-02-19 18:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings (3.46 KB, patch)
2019-02-19 18:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Move calculation to Koha::Fees (20.23 KB, patch)
2019-02-19 18:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up): Fix up Koha::Fees (1.35 KB, patch)
2019-02-19 18:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up): Fix up unit tests (1.22 KB, patch)
2019-02-19 18:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Move Fees to Charges:: (3.39 KB, patch)
2019-02-19 18:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Test for distinct accountlines (2.69 KB, patch)
2019-02-19 18:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make DB Update idempotent (1.96 KB, patch)
2019-02-19 18:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (follow-up) Improve test coverage (18.08 KB, patch)
2019-02-19 18:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (follow-up) Add hourly support (11.97 KB, patch)
2019-02-19 18:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix TestBuilder (856 bytes, patch)
2019-02-19 18:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix some test failures (2.44 KB, patch)
2019-02-19 18:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) POD fix (665 bytes, patch)
2019-02-19 18:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) The default rule already has renewalsallowed=0 (949 bytes, patch)
2019-02-19 18:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (follow-up) Prevent double charging on renewal (3.03 KB, patch)
2019-02-20 08:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Update database (1.89 KB, patch)
2019-02-22 14:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Rental Fees based on Time Period (21.60 KB, patch)
2019-02-22 14:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Update schema file (1.68 KB, patch)
2019-02-22 14:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix number of tests and correct typos (5.66 KB, patch)
2019-02-22 14:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test (1.09 KB, patch)
2019-02-22 14:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings (3.45 KB, patch)
2019-02-22 14:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Move calculation to Koha::Fees (20.21 KB, patch)
2019-02-22 14:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up): Fix up Koha::Fees (1.34 KB, patch)
2019-02-22 14:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up): Fix up unit tests (1.20 KB, patch)
2019-02-22 14:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Move Fees to Charges:: (3.37 KB, patch)
2019-02-22 14:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Test for distinct accountlines (2.67 KB, patch)
2019-02-22 14:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make DB Update idempotent (1.95 KB, patch)
2019-02-22 14:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (follow-up) Improve test coverage (18.09 KB, patch)
2019-02-22 14:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (follow-up) Add hourly support (11.95 KB, patch)
2019-02-22 14:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix TestBuilder (840 bytes, patch)
2019-02-22 14:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix some test failures (2.42 KB, patch)
2019-02-22 14:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) POD fix (649 bytes, patch)
2019-02-22 14:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) The default rule already has renewalsallowed=0 (933 bytes, patch)
2019-02-22 14:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (follow-up) Prevent double charging on renewal (3.03 KB, patch)
2019-02-22 14:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Warn patron of daily and hourly renewal fees (3.41 KB, patch)
2019-02-25 16:04 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Warn patron of daily and hourly renewal fees (3.41 KB, patch)
2019-02-25 18:08 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: Update database (1.89 KB, patch)
2019-03-06 12:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Rental Fees based on Time Period (21.12 KB, patch)
2019-03-06 12:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Update schema file (1.68 KB, patch)
2019-03-06 12:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix number of tests and correct typos (5.66 KB, patch)
2019-03-06 12:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test (1.09 KB, patch)
2019-03-06 12:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings (3.45 KB, patch)
2019-03-06 12:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Move calculation to Koha::Fees (19.76 KB, patch)
2019-03-06 12:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up): Fix up Koha::Fees (1.34 KB, patch)
2019-03-06 12:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up): Fix up unit tests (1.20 KB, patch)
2019-03-06 12:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Move Fees to Charges:: (3.37 KB, patch)
2019-03-06 12:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Test for distinct accountlines (2.67 KB, patch)
2019-03-06 12:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make DB Update idempotent (1.95 KB, patch)
2019-03-06 12:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (follow-up) Improve test coverage (18.13 KB, patch)
2019-03-06 12:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (follow-up) Add hourly support (11.95 KB, patch)
2019-03-06 12:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix TestBuilder (840 bytes, patch)
2019-03-06 12:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix some test failures (2.44 KB, patch)
2019-03-06 12:56 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) POD fix (649 bytes, patch)
2019-03-06 12:56 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) The default rule already has renewalsallowed=0 (933 bytes, patch)
2019-03-06 12:56 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (follow-up) Prevent double charging on renewal (3.03 KB, patch)
2019-03-06 12:56 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Warn patron of daily and hourly renewal fees (3.46 KB, patch)
2019-03-06 12:56 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Update database (1.89 KB, patch)
2019-03-07 14:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Rental Fees based on Time Period (21.12 KB, patch)
2019-03-07 14:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Update schema file (1.68 KB, patch)
2019-03-07 14:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix number of tests and correct typos (5.66 KB, patch)
2019-03-07 14:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test (1.09 KB, patch)
2019-03-07 14:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings (3.45 KB, patch)
2019-03-07 14:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Move calculation to Koha::Fees (19.76 KB, patch)
2019-03-07 14:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up): Fix up Koha::Fees (1.34 KB, patch)
2019-03-07 14:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up): Fix up unit tests (1.20 KB, patch)
2019-03-07 14:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Move Fees to Charges:: (3.37 KB, patch)
2019-03-07 14:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Test for distinct accountlines (2.67 KB, patch)
2019-03-07 14:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make DB Update idempotent (1.95 KB, patch)
2019-03-07 14:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (follow-up) Improve test coverage (18.13 KB, patch)
2019-03-07 14:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (follow-up) Add hourly support (11.95 KB, patch)
2019-03-07 14:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix TestBuilder (840 bytes, patch)
2019-03-07 14:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix some test failures (2.44 KB, patch)
2019-03-07 14:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) POD fix (649 bytes, patch)
2019-03-07 14:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) The default rule already has renewalsallowed=0 (933 bytes, patch)
2019-03-07 14:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (follow-up) Prevent double charging on renewal (3.03 KB, patch)
2019-03-07 14:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Warn patron of daily and hourly renewal fees (3.46 KB, patch)
2019-03-07 14:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Rebase error corrections (23.83 KB, patch)
2019-03-07 14:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Update database (1.89 KB, patch)
2019-03-07 16:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Rental Fees based on Time Period (21.12 KB, patch)
2019-03-07 16:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Update schema file (1.68 KB, patch)
2019-03-07 16:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix number of tests and correct typos (5.66 KB, patch)
2019-03-07 16:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test (1.09 KB, patch)
2019-03-07 16:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings (3.45 KB, patch)
2019-03-07 16:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Move calculation to Koha::Fees (19.76 KB, patch)
2019-03-07 16:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up): Fix up Koha::Fees (1.34 KB, patch)
2019-03-07 16:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up): Fix up unit tests (1.20 KB, patch)
2019-03-07 16:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Move Fees to Charges:: (3.37 KB, patch)
2019-03-07 16:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Test for distinct accountlines (2.67 KB, patch)
2019-03-07 16:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make DB Update idempotent (1.95 KB, patch)
2019-03-07 16:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (follow-up) Improve test coverage (18.13 KB, patch)
2019-03-07 16:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (follow-up) Add hourly support (11.95 KB, patch)
2019-03-07 16:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix TestBuilder (840 bytes, patch)
2019-03-07 16:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix some test failures (2.44 KB, patch)
2019-03-07 16:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) POD fix (649 bytes, patch)
2019-03-07 16:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) The default rule already has renewalsallowed=0 (933 bytes, patch)
2019-03-07 16:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (follow-up) Prevent double charging on renewal (3.03 KB, patch)
2019-03-07 16:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Warn patron of daily and hourly renewal fees (3.46 KB, patch)
2019-03-07 16:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Rebase error corrections (23.93 KB, patch)
2019-03-07 16:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Update database (1.89 KB, patch)
2019-03-07 17:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Rental Fees based on Time Period (21.12 KB, patch)
2019-03-07 17:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Update schema file (1.68 KB, patch)
2019-03-07 17:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix number of tests and correct typos (5.66 KB, patch)
2019-03-07 17:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test (1.09 KB, patch)
2019-03-07 17:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings (3.45 KB, patch)
2019-03-07 17:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: Move calculation to Koha::Fees (19.76 KB, patch)
2019-03-07 17:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up): Fix up Koha::Fees (1.34 KB, patch)
2019-03-07 17:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up): Fix up unit tests (1.20 KB, patch)
2019-03-07 17:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Move Fees to Charges:: (3.37 KB, patch)
2019-03-07 17:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Test for distinct accountlines (2.67 KB, patch)
2019-03-07 17:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Make DB Update idempotent (1.95 KB, patch)
2019-03-07 17:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (follow-up) Improve test coverage (18.13 KB, patch)
2019-03-07 17:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (follow-up) Add hourly support (11.95 KB, patch)
2019-03-07 17:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix TestBuilder (840 bytes, patch)
2019-03-07 17:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix some test failures (2.44 KB, patch)
2019-03-07 17:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) POD fix (649 bytes, patch)
2019-03-07 17:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) The default rule already has renewalsallowed=0 (933 bytes, patch)
2019-03-07 17:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (follow-up) Prevent double charging on renewal (3.03 KB, patch)
2019-03-07 17:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Warn patron of daily and hourly renewal fees (3.46 KB, patch)
2019-03-07 17:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Rebase error corrections (24.23 KB, patch)
2019-03-07 17:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Add on_editing to $Price filter (2.55 KB, patch)
2019-03-07 17:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix for missed signature failure (2.56 KB, patch)
2019-04-03 15:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix for missed signature failure (2.63 KB, patch)
2019-04-04 13:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20912: (QA follow-up) Fix for missed signature failure (2.67 KB, patch)
2019-04-10 08:37 UTC, Dobrica Pavlinusic
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 2018-06-12 04:00:09 UTC
Work to be done:
1) Add new column for daily rental fee to itemtypes
2) Add new syspref to control if daily rental fee includes holidays
3) Add code to C4::Circulation::AddIssue that will check the item's type to determine if it has a daily rental fee
a) If it does, the rental fee will be calculated in full as "checkout length" times "daily rental fee", less holidays if syspref is set to ignore them
4) Add unit tests
Comment 1 Kyle M Hall 2018-06-14 18:36:40 UTC
Created attachment 76065 [details] [review]
Bug 20912: Update database
Comment 2 Kyle M Hall 2018-06-14 18:36:48 UTC
Created attachment 76066 [details] [review]
Bug 20912: Rental Fees based on Time Period

Some libraries would like to be able to charge a rental fee based on the
number of days and item will be checked out, as opposed to the flat fee
currently offered by Koha.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Edit an itemtype, add a daily rental fee of 1.00
4) Check an item of that itemtype out for 7 days
5) Verify the patron now has rental fee of 7.00
Comment 3 Kyle M Hall 2018-06-14 18:36:50 UTC
Created attachment 76067 [details] [review]
Bug 20912: Update schema file
Comment 4 Kyle M Hall 2018-06-14 18:51:57 UTC
Created attachment 76068 [details] [review]
Bug 20912: Rental Fees based on Time Period

Some libraries would like to be able to charge a rental fee based on the
number of days and item will be checked out, as opposed to the flat fee
currently offered by Koha.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Edit an itemtype, add a daily rental fee of 1.00
4) Check an item of that itemtype out for 7 days
5) Verify the patron now has rental fee of 7.00
Comment 5 Kyle M Hall 2018-06-14 18:52:04 UTC
Created attachment 76069 [details] [review]
Bug 20912: Update schema file
Comment 6 Martha Fuerst 2018-07-26 20:50:25 UTC
Tested - works fine for me!

Signed off - Martha Fuerst
Comment 7 Kyle M Hall 2018-07-27 11:48:29 UTC
Created attachment 77302 [details] [review]
Bug 20912: Update database

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Comment 8 Kyle M Hall 2018-07-27 11:48:40 UTC
Created attachment 77303 [details] [review]
Bug 20912: Rental Fees based on Time Period

Some libraries would like to be able to charge a rental fee based on the
number of days and item will be checked out, as opposed to the flat fee
currently offered by Koha.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Edit an itemtype, add a daily rental fee of 1.00
4) Check an item of that itemtype out for 7 days
5) Verify the patron now has rental fee of 7.00

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Comment 9 Kyle M Hall 2018-07-27 11:48:43 UTC
Created attachment 77304 [details] [review]
Bug 20912: Update schema file

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Comment 10 Katrin Fischer 2018-08-24 06:18:22 UTC
Patch doesn't apply - please rebase! (might need some more XSS filters)
Comment 11 Kyle M Hall 2018-10-25 12:42:38 UTC
Created attachment 81141 [details] [review]
Bug 20912: Update database

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Comment 12 Kyle M Hall 2018-10-25 12:42:48 UTC
Created attachment 81142 [details] [review]
Bug 20912: Rental Fees based on Time Period

Some libraries would like to be able to charge a rental fee based on the
number of days and item will be checked out, as opposed to the flat fee
currently offered by Koha.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Edit an itemtype, add a daily rental fee of 1.00
4) Check an item of that itemtype out for 7 days
5) Verify the patron now has rental fee of 7.00

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Comment 13 Kyle M Hall 2018-10-25 12:42:51 UTC
Created attachment 81143 [details] [review]
Bug 20912: Update schema file
Comment 14 Katrin Fischer 2018-10-28 22:56:05 UTC
Starting here with QA script and code review:

1) QA Script
 FAIL	t/db_dependent/Circulation.t
   OK	  critic
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   FAIL	  spelling
		 calulated  ==> calculated
   OK	  valid

 FAIL	t/db_dependent/Koha/ItemTypes.t
   OK	  critic
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   FAIL	  spelling
		 calulated  ==> calculated
   OK	  valid

2) Code review

2.1)
<span class="hint">This fee is charged a checkout/renewal time for each day between the checkout/renewal date and due date.</span>
==> charged at...

2.2) Why the check on notforloan? What if the notforloan is overriden?
+            <td>
+            [% UNLESS ( itemtype.notforloan ) %]
+              [% itemtype.rental_charge_daily | $Price %]
+            [% END %]
+            </td>

3) Tests

Circulation.t needs to be updated to run 120 tests (not 119)

There is a new warn in the other tests:
t/db_dependent/Koha/ItemTypes.t .. 23/25 Use of uninitialized value in string eq at /home/vagrant/kohaclone/Koha/ItemType.pm line 43.
Comment 15 Katrin Fischer 2018-10-29 06:39:26 UTC
Created attachment 81450 [details] [review]
Bug 20912: (QA follow-up) Fix number of tests and correct typos
Comment 16 Katrin Fischer 2018-10-29 06:40:08 UTC
Fixed 1) and the first part of 3). Still needs some love :)
Comment 17 Kyle M Hall 2018-10-31 19:15:15 UTC
Created attachment 81771 [details] [review]
Bug 20912: Update database

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Comment 18 Kyle M Hall 2018-10-31 19:16:03 UTC
Created attachment 81772 [details] [review]
Bug 20912: Rental Fees based on Time Period

Some libraries would like to be able to charge a rental fee based on the
number of days and item will be checked out, as opposed to the flat fee
currently offered by Koha.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Edit an itemtype, add a daily rental fee of 1.00
4) Check an item of that itemtype out for 7 days
5) Verify the patron now has rental fee of 7.00

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Comment 19 Kyle M Hall 2018-10-31 19:16:07 UTC
Created attachment 81773 [details] [review]
Bug 20912: Update schema file
Comment 20 Kyle M Hall 2018-10-31 19:16:10 UTC
Created attachment 81774 [details] [review]
Bug 20912: (QA follow-up) Fix number of tests and correct typos
Comment 21 Kyle M Hall 2018-10-31 19:16:13 UTC
Created attachment 81775 [details] [review]
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test
Comment 22 Kyle M Hall 2018-10-31 19:30:16 UTC
Created attachment 81778 [details] [review]
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings
Comment 23 Katrin Fischer 2018-11-02 09:15:38 UTC
I won't be able to finish here today - would be really happy if someone else from the QA team could have a look.
Comment 24 Martin Renvoize 2018-11-02 16:03:49 UTC
I'd like to see some tests for various combinations of process_fee, rentalcharge and rental_charge_daily before passing this.

Looking at the code there seems to be a little disparity between charging the amount returned by GetIssuingCharges followed by a second charge calculated using rental_charge_daily.. sometimes it looks like a combined charge, sometimes it looks like two charges and sometimes two charges with the second change also being a combined charge.
 
Hence the asking for a few more tests ;)

Also.. i think I'd prefer that the calculation was folded into GetIssuingCharges myself.. it seems somewhat fraught with peril to be duplicating this code.
Comment 25 Tomás Cohen Arazi 2018-11-08 09:22:06 UTC
I have a few remarks:

         my ($rentalCharge) = GetIssuingCharges( $item->{'itemnumber'}, $patron->borrowernumber );
+        my $itemtype = Koha::ItemTypes->find( $item->{itype} ); # GetItem sets effective itemtype
+        $rentalCharge += $itemtype->calc_rental_charge_daily( { from => dt_from_string(), to => $duedate } );

This doesn't look right, shouldn't it be either a fixed charge or the daily calculated one but not both?? If it was two different charges, we might better have separate offsets for them, right?

I would remove any fee calculation from Koha::ItemType (same for using userenv). It should only return the 'daily' charge and let some other class handle the calculation. I've been thinking of

my $fee = Koha::Fee->new({ library => $library, patron => $patron, item => $item, datetime_due => $dt });

This is of course out of the scope of this bug, but with that in mind, I concur on Martin's proposal of having GetIssuingCharges do it on this bug. The only concern I have is offsets.
Comment 26 Kyle M Hall 2018-11-08 11:31:59 UTC
(In reply to Tomás Cohen Arazi from comment #25)
> I have a few remarks:
> 
>          my ($rentalCharge) = GetIssuingCharges( $item->{'itemnumber'},
> $patron->borrowernumber );
> +        my $itemtype = Koha::ItemTypes->find( $item->{itype} ); # GetItem
> sets effective itemtype
> +        $rentalCharge += $itemtype->calc_rental_charge_daily( { from =>
> dt_from_string(), to => $duedate } );
> 
> This doesn't look right, shouldn't it be either a fixed charge or the daily
> calculated one but not both?? If it was two different charges, we might
> better have separate offsets for them, right?

Yes, a library should be allowed to have a flat fee plus a daily fee if they wish. This patch set already ensures they will have separate offsets.


> I would remove any fee calculation from Koha::ItemType (same for using
> userenv). It should only return the 'daily' charge and let some other class
> handle the calculation. 

That's fine by me!

> I've been thinking of 
> my $fee = Koha::Fee->new({ library => $library, patron => $patron, item =>
> $item, datetime_due => $dt });
> 
> This is of course out of the scope of this bug, but with that in mind, I
> concur on Martin's proposal of having GetIssuingCharges do it on this bug.
> The only concern I have is offsets.

Integrating this into GetIssuingCharges seems like a step backwards. What if I created Koha::Fees and moved the method to it? What I imagine is basically

my @fees = Koha::Fee->new({ library => $library, patron => $patron, item =>
> $item, datetime_due => $dt })->get_fees, where the return is a list of Koha::Fee objects. To limit the scope creep, I will only move calc_rental_charge_daily into Fees.pm, and leave the breaking up and moving of the rest of GetIssuingCharges as a later exercise.

What do you think?
Comment 27 Tomás Cohen Arazi 2018-11-08 15:29:30 UTC
(In reply to Kyle M Hall from comment #26)
> (In reply to Tomás Cohen Arazi from comment #25)
> > I have a few remarks:
> > 
> >          my ($rentalCharge) = GetIssuingCharges( $item->{'itemnumber'},
> > $patron->borrowernumber );
> > +        my $itemtype = Koha::ItemTypes->find( $item->{itype} ); # GetItem
> > sets effective itemtype
> > +        $rentalCharge += $itemtype->calc_rental_charge_daily( { from =>
> > dt_from_string(), to => $duedate } );
> > 
> > This doesn't look right, shouldn't it be either a fixed charge or the daily
> > calculated one but not both?? If it was two different charges, we might
> > better have separate offsets for them, right?
> 
> Yes, a library should be allowed to have a flat fee plus a daily fee if they
> wish. This patch set already ensures they will have separate offsets.
> 
> 
> > I would remove any fee calculation from Koha::ItemType (same for using
> > userenv). It should only return the 'daily' charge and let some other class
> > handle the calculation. 
> 
> That's fine by me!
> 
> > I've been thinking of 
> > my $fee = Koha::Fee->new({ library => $library, patron => $patron, item =>
> > $item, datetime_due => $dt });
> > 
> > This is of course out of the scope of this bug, but with that in mind, I
> > concur on Martin's proposal of having GetIssuingCharges do it on this bug.
> > The only concern I have is offsets.
> 
> Integrating this into GetIssuingCharges seems like a step backwards. What if
> I created Koha::Fees and moved the method to it? What I imagine is basically
> 
> my @fees = Koha::Fee->new({ library => $library, patron => $patron, item =>
> > $item, datetime_due => $dt })->get_fees, where the return is a list of Koha::Fee objects. To limit the scope creep, I will only move calc_rental_charge_daily into Fees.pm, and leave the breaking up and moving of the rest of GetIssuingCharges as a later exercise.
> 
> What do you think?

I agree.
Comment 28 Kyle M Hall 2018-11-09 18:34:40 UTC
Created attachment 82195 [details] [review]
Bug 20912: Move calculation to Koha::Fees
Comment 29 Kyle M Hall 2018-11-09 18:53:44 UTC
Created attachment 82196 [details] [review]
Bug 20912: Move calculation to Koha::Fees
Comment 30 Kyle M Hall 2019-01-25 17:50:00 UTC
Created attachment 84412 [details] [review]
Bug 20912: Update database

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Comment 31 Kyle M Hall 2019-01-25 17:50:31 UTC
Created attachment 84413 [details] [review]
Bug 20912: Rental Fees based on Time Period

Some libraries would like to be able to charge a rental fee based on the
number of days and item will be checked out, as opposed to the flat fee
currently offered by Koha.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Edit an itemtype, add a daily rental fee of 1.00
4) Check an item of that itemtype out for 7 days
5) Verify the patron now has rental fee of 7.00

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Comment 32 Kyle M Hall 2019-01-25 17:50:34 UTC
Created attachment 84414 [details] [review]
Bug 20912: Update schema file
Comment 33 Kyle M Hall 2019-01-25 17:50:38 UTC
Created attachment 84415 [details] [review]
Bug 20912: (QA follow-up) Fix number of tests and correct typos
Comment 34 Kyle M Hall 2019-01-25 17:50:41 UTC
Created attachment 84416 [details] [review]
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test
Comment 35 Kyle M Hall 2019-01-25 17:50:45 UTC
Created attachment 84417 [details] [review]
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings
Comment 36 Kyle M Hall 2019-01-25 17:50:48 UTC
Created attachment 84418 [details] [review]
Bug 20912: Move calculation to Koha::Fees
Comment 37 Kyle M Hall 2019-01-25 17:50:52 UTC
Created attachment 84419 [details] [review]
Bug 20912 (QA Follow-up): Fix up Koha::Fees
Comment 38 Kyle M Hall 2019-01-25 17:50:55 UTC
Created attachment 84420 [details] [review]
Bug 20912 (QA Follow-up): Fix up unit tests
Comment 39 Martin Renvoize 2019-01-31 15:29:24 UTC
Created attachment 84570 [details] [review]
Bug 20912: Update database

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 40 Martin Renvoize 2019-01-31 15:29:27 UTC
Created attachment 84571 [details] [review]
Bug 20912: Rental Fees based on Time Period

Some libraries would like to be able to charge a rental fee based on the
number of days an item will be checked out, as opposed to the flat fee
currently offered by Koha.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Edit an itemtype, add a daily rental fee of 1.00
4) Check an item of that itemtype out for 7 days
5) Verify the patron now has rental fee of 7.00

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 41 Martin Renvoize 2019-01-31 15:29:31 UTC
Created attachment 84572 [details] [review]
Bug 20912: Update schema file

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 42 Martin Renvoize 2019-01-31 15:29:35 UTC
Created attachment 84573 [details] [review]
Bug 20912: (QA follow-up) Fix number of tests and correct typos

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 43 Martin Renvoize 2019-01-31 15:29:39 UTC
Created attachment 84574 [details] [review]
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 44 Martin Renvoize 2019-01-31 15:29:42 UTC
Created attachment 84575 [details] [review]
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 45 Martin Renvoize 2019-01-31 15:29:46 UTC
Created attachment 84576 [details] [review]
Bug 20912: Move calculation to Koha::Fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 46 Martin Renvoize 2019-01-31 15:29:50 UTC
Created attachment 84577 [details] [review]
Bug 20912: (QA follow-up): Fix up Koha::Fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 47 Martin Renvoize 2019-01-31 15:29:53 UTC
Created attachment 84578 [details] [review]
Bug 20912: (QA follow-up): Fix up unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 48 Martin Renvoize 2019-01-31 15:29:57 UTC
Created attachment 84579 [details] [review]
Bug 20912: (QA follow-up) Move Fees to Charges::

In preparation for the introduction of Koha::Charges::Fines I have moved
this ::Fees class into the Koha::Charges:: namespace

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 49 Martin Renvoize 2019-01-31 15:30:00 UTC
Created attachment 84580 [details] [review]
Bug 20912: (QA follow-up) Test for distinct accountlines

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 50 Martin Renvoize 2019-01-31 15:30:05 UTC
Created attachment 84581 [details] [review]
Bug 20912: (QA follow-up) Make DB Update idempotent

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 51 Martin Renvoize 2019-01-31 15:30:08 UTC
Created attachment 84582 [details] [review]
Bug 20912: (follow-up) Improve test coverage

Increase test coverage for CanBookBeIssued and fix a introduced during
the refactoring to Koha::Fees.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 52 Martin Renvoize 2019-01-31 15:30:12 UTC
Created attachment 84583 [details] [review]
Bug 20912: (follow-up) Add hourly support

It seemed strange to only add support for Daily loans in the feature
when Koha also support loan periods in hours. This patch adds parallel
functionaliy for hourly loan periods.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 53 Martin Renvoize 2019-01-31 15:30:16 UTC
Created attachment 84584 [details] [review]
Bug 20912: (QA follow-up) Fix TestBuilder

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 54 Martin Renvoize 2019-01-31 15:30:20 UTC
Created attachment 84585 [details] [review]
Bug 20912: (QA follow-up) Fix some test failures

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 55 Martin Renvoize 2019-01-31 15:32:29 UTC
I have given this a thorough testing and added a few qa followups to fix some failing test cases and improve test coverage.

I also felt it important to add support for hourly loan periods and as such, I've added a pair of follow-ups to add this. As such, although I'd PQA on all work preceding my own, I've asked Tomas and Kyle to cast their eyes over the followup as a final QA signoff.
Comment 56 Kyle M Hall 2019-02-01 13:49:35 UTC
Created attachment 84610 [details] [review]
Bug 20912: Update database

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 57 Kyle M Hall 2019-02-01 13:49:45 UTC
Created attachment 84611 [details] [review]
Bug 20912: Rental Fees based on Time Period

Some libraries would like to be able to charge a rental fee based on the
number of days an item will be checked out, as opposed to the flat fee
currently offered by Koha.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Edit an itemtype, add a daily rental fee of 1.00
4) Check an item of that itemtype out for 7 days
5) Verify the patron now has rental fee of 7.00

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 58 Kyle M Hall 2019-02-01 13:49:48 UTC
Created attachment 84612 [details] [review]
Bug 20912: Update schema file

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 59 Kyle M Hall 2019-02-01 13:49:52 UTC
Created attachment 84613 [details] [review]
Bug 20912: (QA follow-up) Fix number of tests and correct typos

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 60 Kyle M Hall 2019-02-01 13:49:55 UTC
Created attachment 84614 [details] [review]
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 61 Kyle M Hall 2019-02-01 13:49:58 UTC
Created attachment 84615 [details] [review]
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 62 Kyle M Hall 2019-02-01 13:50:02 UTC
Created attachment 84616 [details] [review]
Bug 20912: Move calculation to Koha::Fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 63 Kyle M Hall 2019-02-01 13:50:06 UTC
Created attachment 84617 [details] [review]
Bug 20912: (QA follow-up): Fix up Koha::Fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 64 Kyle M Hall 2019-02-01 13:50:10 UTC
Created attachment 84618 [details] [review]
Bug 20912: (QA follow-up): Fix up unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 65 Kyle M Hall 2019-02-01 13:50:13 UTC
Created attachment 84619 [details] [review]
Bug 20912: (QA follow-up) Move Fees to Charges::

In preparation for the introduction of Koha::Charges::Fines I have moved
this ::Fees class into the Koha::Charges:: namespace

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 66 Kyle M Hall 2019-02-01 13:50:17 UTC
Created attachment 84620 [details] [review]
Bug 20912: (QA follow-up) Test for distinct accountlines

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 67 Kyle M Hall 2019-02-01 13:50:21 UTC
Created attachment 84621 [details] [review]
Bug 20912: (QA follow-up) Make DB Update idempotent

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 68 Kyle M Hall 2019-02-01 13:50:25 UTC
Created attachment 84622 [details] [review]
Bug 20912: (follow-up) Improve test coverage

Increase test coverage for CanBookBeIssued and fix a introduced during
the refactoring to Koha::Fees.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 69 Kyle M Hall 2019-02-01 13:50:29 UTC
Created attachment 84623 [details] [review]
Bug 20912: (follow-up) Add hourly support

It seemed strange to only add support for Daily loans in the feature
when Koha also support loan periods in hours. This patch adds parallel
functionaliy for hourly loan periods.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 70 Kyle M Hall 2019-02-01 13:50:32 UTC
Created attachment 84624 [details] [review]
Bug 20912: (QA follow-up) Fix TestBuilder

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 71 Kyle M Hall 2019-02-01 13:50:36 UTC
Created attachment 84625 [details] [review]
Bug 20912: (QA follow-up) Fix some test failures

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 72 Katrin Fischer 2019-02-03 09:19:55 UTC
(In reply to Martin Renvoize from comment #55)
> I have given this a thorough testing and added a few qa followups to fix
> some failing test cases and improve test coverage.
> 
> I also felt it important to add support for hourly loan periods and as such,
> I've added a pair of follow-ups to add this. As such, although I'd PQA on
> all work preceding my own, I've asked Tomas and Kyle to cast their eyes over
> the followup as a final QA signoff.

Are we waiting for Tomas or should this be moved on?
Comment 73 Tomás Cohen Arazi 2019-02-04 14:37:35 UTC
Created attachment 84707 [details] [review]
Bug 20912: Update database

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 74 Tomás Cohen Arazi 2019-02-04 14:37:40 UTC
Created attachment 84708 [details] [review]
Bug 20912: Rental Fees based on Time Period

Some libraries would like to be able to charge a rental fee based on the
number of days an item will be checked out, as opposed to the flat fee
currently offered by Koha.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Edit an itemtype, add a daily rental fee of 1.00
4) Check an item of that itemtype out for 7 days
5) Verify the patron now has rental fee of 7.00

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 75 Tomás Cohen Arazi 2019-02-04 14:37:44 UTC
Created attachment 84709 [details] [review]
Bug 20912: Update schema file

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 76 Tomás Cohen Arazi 2019-02-04 14:37:48 UTC
Created attachment 84710 [details] [review]
Bug 20912: (QA follow-up) Fix number of tests and correct typos

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 77 Tomás Cohen Arazi 2019-02-04 14:37:53 UTC
Created attachment 84711 [details] [review]
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 78 Tomás Cohen Arazi 2019-02-04 14:37:57 UTC
Created attachment 84712 [details] [review]
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 79 Tomás Cohen Arazi 2019-02-04 14:38:01 UTC
Created attachment 84713 [details] [review]
Bug 20912: Move calculation to Koha::Fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 80 Tomás Cohen Arazi 2019-02-04 14:38:05 UTC
Created attachment 84714 [details] [review]
Bug 20912: (QA follow-up): Fix up Koha::Fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 81 Tomás Cohen Arazi 2019-02-04 14:38:09 UTC
Created attachment 84715 [details] [review]
Bug 20912: (QA follow-up): Fix up unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 82 Tomás Cohen Arazi 2019-02-04 14:38:14 UTC
Created attachment 84716 [details] [review]
Bug 20912: (QA follow-up) Move Fees to Charges::

In preparation for the introduction of Koha::Charges::Fines I have moved
this ::Fees class into the Koha::Charges:: namespace

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 83 Tomás Cohen Arazi 2019-02-04 14:38:18 UTC
Created attachment 84717 [details] [review]
Bug 20912: (QA follow-up) Test for distinct accountlines

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 84 Tomás Cohen Arazi 2019-02-04 14:38:23 UTC
Created attachment 84718 [details] [review]
Bug 20912: (QA follow-up) Make DB Update idempotent

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 85 Tomás Cohen Arazi 2019-02-04 14:38:27 UTC
Created attachment 84719 [details] [review]
Bug 20912: (follow-up) Improve test coverage

Increase test coverage for CanBookBeIssued and fix a introduced during
the refactoring to Koha::Fees.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 86 Tomás Cohen Arazi 2019-02-04 14:38:32 UTC
Created attachment 84720 [details] [review]
Bug 20912: (follow-up) Add hourly support

It seemed strange to only add support for Daily loans in the feature
when Koha also support loan periods in hours. This patch adds parallel
functionaliy for hourly loan periods.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 87 Tomás Cohen Arazi 2019-02-04 14:38:36 UTC
Created attachment 84721 [details] [review]
Bug 20912: (QA follow-up) Fix TestBuilder

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 88 Tomás Cohen Arazi 2019-02-04 14:38:41 UTC
Created attachment 84722 [details] [review]
Bug 20912: (QA follow-up) Fix some test failures

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 89 Tomás Cohen Arazi 2019-02-04 14:38:45 UTC
Created attachment 84723 [details] [review]
Bug 20912: (QA follow-up) POD fix

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 90 Tomás Cohen Arazi 2019-02-04 14:38:50 UTC
Created attachment 84724 [details] [review]
Bug 20912: (QA follow-up) The default rule already has renewalsallowed=0

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 91 Tomás Cohen Arazi 2019-02-04 14:42:14 UTC
Kyle implemented the things we suggested on QA, and Martin and I discussed the changes he made. We are all on the same page about this.
Comment 92 Katrin Fischer 2019-02-04 16:06:01 UTC
(In reply to Tomás Cohen Arazi from comment #91)
> Kyle implemented the things we suggested on QA, and Martin and I discussed
> the changes he made. We are all on the same page about this.

Great, thx!
Comment 93 Kyle M Hall 2019-02-19 18:00:07 UTC
Created attachment 85291 [details] [review]
Bug 20912: Update database

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 94 Kyle M Hall 2019-02-19 18:00:20 UTC
Created attachment 85292 [details] [review]
Bug 20912: Rental Fees based on Time Period

Some libraries would like to be able to charge a rental fee based on the
number of days an item will be checked out, as opposed to the flat fee
currently offered by Koha.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Edit an itemtype, add a daily rental fee of 1.00
4) Check an item of that itemtype out for 7 days
5) Verify the patron now has rental fee of 7.00

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 95 Kyle M Hall 2019-02-19 18:00:24 UTC
Created attachment 85293 [details] [review]
Bug 20912: Update schema file

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 96 Kyle M Hall 2019-02-19 18:00:29 UTC
Created attachment 85294 [details] [review]
Bug 20912: (QA follow-up) Fix number of tests and correct typos

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 97 Kyle M Hall 2019-02-19 18:00:32 UTC
Created attachment 85295 [details] [review]
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 98 Kyle M Hall 2019-02-19 18:00:37 UTC
Created attachment 85296 [details] [review]
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 99 Kyle M Hall 2019-02-19 18:00:41 UTC
Created attachment 85297 [details] [review]
Bug 20912: Move calculation to Koha::Fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 100 Kyle M Hall 2019-02-19 18:00:45 UTC
Created attachment 85298 [details] [review]
Bug 20912: (QA follow-up): Fix up Koha::Fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 101 Kyle M Hall 2019-02-19 18:00:49 UTC
Created attachment 85299 [details] [review]
Bug 20912: (QA follow-up): Fix up unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 102 Kyle M Hall 2019-02-19 18:00:53 UTC
Created attachment 85300 [details] [review]
Bug 20912: (QA follow-up) Move Fees to Charges::

In preparation for the introduction of Koha::Charges::Fines I have moved
this ::Fees class into the Koha::Charges:: namespace

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 103 Kyle M Hall 2019-02-19 18:00:57 UTC
Created attachment 85301 [details] [review]
Bug 20912: (QA follow-up) Test for distinct accountlines

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 104 Kyle M Hall 2019-02-19 18:01:01 UTC
Created attachment 85302 [details] [review]
Bug 20912: (QA follow-up) Make DB Update idempotent

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 105 Kyle M Hall 2019-02-19 18:01:06 UTC
Created attachment 85303 [details] [review]
Bug 20912: (follow-up) Improve test coverage

Increase test coverage for CanBookBeIssued and fix a introduced during
the refactoring to Koha::Fees.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 106 Kyle M Hall 2019-02-19 18:01:10 UTC
Created attachment 85304 [details] [review]
Bug 20912: (follow-up) Add hourly support

It seemed strange to only add support for Daily loans in the feature
when Koha also support loan periods in hours. This patch adds parallel
functionaliy for hourly loan periods.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 107 Kyle M Hall 2019-02-19 18:01:14 UTC
Created attachment 85305 [details] [review]
Bug 20912: (QA follow-up) Fix TestBuilder

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 108 Kyle M Hall 2019-02-19 18:01:18 UTC
Created attachment 85306 [details] [review]
Bug 20912: (QA follow-up) Fix some test failures

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 109 Kyle M Hall 2019-02-19 18:01:23 UTC
Created attachment 85307 [details] [review]
Bug 20912: (QA follow-up) POD fix

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 110 Kyle M Hall 2019-02-19 18:01:27 UTC
Created attachment 85308 [details] [review]
Bug 20912: (QA follow-up) The default rule already has renewalsallowed=0

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 111 Martin Renvoize 2019-02-19 18:27:55 UTC
Not strictly a blocker for 14825, but I believe it will cause a requirement to rebase or at least revisit the affected routine for the add_devit/add_credit conversion tree so I'm adding it there to aid my own tracking.
Comment 112 Nick Clemens 2019-02-19 20:39:05 UTC
Couple things:
1 - Early renewals:
    The patron is charged for the full checkout duration on each renewal - I think they should only be charged for the additional days? or at least only for the renewal period?
    - if renewal period base is 'current day' and the checkout is not extended by the renewal I think they should not be charged, or charged only the renewal period
    - if the renewal period base is 'old due date' then they are charged for only added days
2 - The OPAC patron summary lists any general rental fees, but does not list the periodic rental fees - ideally I think the patron would have to confirm acceptance of charges before completing the transaction, but warning ahead of time is needed at a minimum - I can file a new bug for confirmation of renewing with rental charges
Comment 113 Martin Renvoize 2019-02-20 08:52:19 UTC
Created attachment 85316 [details] [review]
Bug 20912: (follow-up) Prevent double charging on renewal

This patch updates the tests the ensure we do not double charge for
renewals that take place before the original due date and fixes the
corresponding loging in C4::Circulation::AddRenewal.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 114 Martin Renvoize 2019-02-20 08:54:08 UTC
Followup added to fix the first issue Nick found.. can't believe that got through so many QA eyes.. it was even incorrectly tested for in the tests, highlighting the issue.

I'm not sure I fully understand your second comment though Nick... is that something that can go in a second bug with a clear description of what you think should happen?
Comment 115 Martin Renvoize 2019-02-22 14:54:20 UTC
Created attachment 85543 [details] [review]
Bug 20912: Update database

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 116 Martin Renvoize 2019-02-22 14:54:25 UTC
Created attachment 85544 [details] [review]
Bug 20912: Rental Fees based on Time Period

Some libraries would like to be able to charge a rental fee based on the
number of days an item will be checked out, as opposed to the flat fee
currently offered by Koha.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Edit an itemtype, add a daily rental fee of 1.00
4) Check an item of that itemtype out for 7 days
5) Verify the patron now has rental fee of 7.00

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 117 Martin Renvoize 2019-02-22 14:54:29 UTC
Created attachment 85545 [details] [review]
Bug 20912: Update schema file

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 118 Martin Renvoize 2019-02-22 14:54:33 UTC
Created attachment 85546 [details] [review]
Bug 20912: (QA follow-up) Fix number of tests and correct typos

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 119 Martin Renvoize 2019-02-22 14:54:37 UTC
Created attachment 85547 [details] [review]
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 120 Martin Renvoize 2019-02-22 14:54:41 UTC
Created attachment 85548 [details] [review]
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 121 Martin Renvoize 2019-02-22 14:54:46 UTC
Created attachment 85549 [details] [review]
Bug 20912: Move calculation to Koha::Fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 122 Martin Renvoize 2019-02-22 14:54:50 UTC
Created attachment 85550 [details] [review]
Bug 20912: (QA follow-up): Fix up Koha::Fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 123 Martin Renvoize 2019-02-22 14:54:54 UTC
Created attachment 85551 [details] [review]
Bug 20912: (QA follow-up): Fix up unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 124 Martin Renvoize 2019-02-22 14:54:58 UTC
Created attachment 85552 [details] [review]
Bug 20912: (QA follow-up) Move Fees to Charges::

In preparation for the introduction of Koha::Charges::Fines I have moved
this ::Fees class into the Koha::Charges:: namespace

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 125 Martin Renvoize 2019-02-22 14:55:03 UTC
Created attachment 85553 [details] [review]
Bug 20912: (QA follow-up) Test for distinct accountlines

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 126 Martin Renvoize 2019-02-22 14:55:08 UTC
Created attachment 85554 [details] [review]
Bug 20912: (QA follow-up) Make DB Update idempotent

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 127 Martin Renvoize 2019-02-22 14:55:12 UTC
Created attachment 85555 [details] [review]
Bug 20912: (follow-up) Improve test coverage

Increase test coverage for CanBookBeIssued and fix a introduced during
the refactoring to Koha::Fees.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 128 Martin Renvoize 2019-02-22 14:55:16 UTC
Created attachment 85556 [details] [review]
Bug 20912: (follow-up) Add hourly support

It seemed strange to only add support for Daily loans in the feature
when Koha also support loan periods in hours. This patch adds parallel
functionaliy for hourly loan periods.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 129 Martin Renvoize 2019-02-22 14:55:21 UTC
Created attachment 85557 [details] [review]
Bug 20912: (QA follow-up) Fix TestBuilder

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 130 Martin Renvoize 2019-02-22 14:55:25 UTC
Created attachment 85558 [details] [review]
Bug 20912: (QA follow-up) Fix some test failures

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 131 Martin Renvoize 2019-02-22 14:55:29 UTC
Created attachment 85559 [details] [review]
Bug 20912: (QA follow-up) POD fix

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 132 Martin Renvoize 2019-02-22 14:55:35 UTC
Created attachment 85560 [details] [review]
Bug 20912: (QA follow-up) The default rule already has renewalsallowed=0

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 133 Martin Renvoize 2019-02-22 14:55:40 UTC
Created attachment 85561 [details] [review]
Bug 20912: (follow-up) Prevent double charging on renewal

This patch updates the tests the ensure we do not double charge for
renewals that take place before the original due date and fixes the
corresponding loging in C4::Circulation::AddRenewal.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 134 Martin Renvoize 2019-02-22 14:57:12 UTC
Minor rebase to fit it on top of the add_debits accounts bug tree.
Comment 135 Martin Renvoize 2019-02-22 16:09:20 UTC
(In reply to Nick Clemens from comment #112)
> 2 - The OPAC patron summary lists any general rental fees, but does not list
> the periodic rental fees - ideally I think the patron would have to confirm
> acceptance of charges before completing the transaction, but warning ahead
> of time is needed at a minimum - I can file a new bug for confirmation of
> renewing with rental charges

OK, I understand now.. I'll mark it back to failed qa.. hopefully one of us will get to it some time soon.

In short, I think we need to add Koha::Charges::Fees into opac/opac-user.pl to add in predicted renewal fees for a period based fee so that they display in the checkouts table where the user can opt to renew items.
Comment 136 Kyle M Hall 2019-02-22 16:43:12 UTC
(In reply to Martin Renvoize from comment #135)
> (In reply to Nick Clemens from comment #112)
> > 2 - The OPAC patron summary lists any general rental fees, but does not list
> > the periodic rental fees - ideally I think the patron would have to confirm
> > acceptance of charges before completing the transaction, but warning ahead
> > of time is needed at a minimum - I can file a new bug for confirmation of
> > renewing with rental charges
> 
> OK, I understand now.. I'll mark it back to failed qa.. hopefully one of us
> will get to it some time soon.
> 
> In short, I think we need to add Koha::Charges::Fees into opac/opac-user.pl
> to add in predicted renewal fees for a period based fee so that they display
> in the checkouts table where the user can opt to renew items.

Does the existing rental charge do this?
Comment 137 Martin Renvoize 2019-02-22 19:57:37 UTC
Yup, nick even showed me a screenshot 😉. It's not obvious though to be fair, it's just a note within renew field of the table. It's says "Fee for item type 'Book': 1.00" - https://snag.gy/MWntYk.jpg
Comment 138 Katrin Fischer 2019-02-24 11:51:45 UTC
(In reply to Martin Renvoize from comment #137)
> Yup, nick even showed me a screenshot 😉. It's not obvious though to be
> fair, it's just a note within renew field of the table. It's says "Fee for
> item type 'Book': 1.00" - https://snag.gy/MWntYk.jpg

I think this was added not too long ago, which might explain it not being well known: 

Bug 13796 - Alert in OPAC when renewing an item with a rental charge (signed by Kyle :) )

I agree that information/confirmation is needed before charging from a user's point of view, but I think also probably legally in some cases. I remember discussions here about labelling buttons for ordering clearly. Better to be safe.
Comment 139 Kyle M Hall 2019-02-25 16:04:32 UTC
Created attachment 85628 [details] [review]
Bug 20912: (QA follow-up) Warn patron of daily and hourly renewal fees
Comment 140 Kyle M Hall 2019-02-25 18:08:49 UTC
Created attachment 85631 [details] [review]
Bug 20912: (QA follow-up) Warn patron of daily and hourly renewal fees
Comment 141 Martin Renvoize 2019-03-06 12:54:55 UTC
Created attachment 86140 [details] [review]
Bug 20912: Update database

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 142 Martin Renvoize 2019-03-06 12:54:59 UTC
Created attachment 86141 [details] [review]
Bug 20912: Rental Fees based on Time Period

Some libraries would like to be able to charge a rental fee based on the
number of days an item will be checked out, as opposed to the flat fee
currently offered by Koha.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Edit an itemtype, add a daily rental fee of 1.00
4) Check an item of that itemtype out for 7 days
5) Verify the patron now has rental fee of 7.00

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 143 Martin Renvoize 2019-03-06 12:55:04 UTC
Created attachment 86142 [details] [review]
Bug 20912: Update schema file

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 144 Martin Renvoize 2019-03-06 12:55:08 UTC
Created attachment 86143 [details] [review]
Bug 20912: (QA follow-up) Fix number of tests and correct typos

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 145 Martin Renvoize 2019-03-06 12:55:12 UTC
Created attachment 86144 [details] [review]
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 146 Martin Renvoize 2019-03-06 12:55:16 UTC
Created attachment 86145 [details] [review]
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 147 Martin Renvoize 2019-03-06 12:55:21 UTC
Created attachment 86146 [details] [review]
Bug 20912: Move calculation to Koha::Fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 148 Martin Renvoize 2019-03-06 12:55:25 UTC
Created attachment 86147 [details] [review]
Bug 20912: (QA follow-up): Fix up Koha::Fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 149 Martin Renvoize 2019-03-06 12:55:30 UTC
Created attachment 86148 [details] [review]
Bug 20912: (QA follow-up): Fix up unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 150 Martin Renvoize 2019-03-06 12:55:34 UTC
Created attachment 86149 [details] [review]
Bug 20912: (QA follow-up) Move Fees to Charges::

In preparation for the introduction of Koha::Charges::Fines I have moved
this ::Fees class into the Koha::Charges:: namespace

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 151 Martin Renvoize 2019-03-06 12:55:38 UTC
Created attachment 86151 [details] [review]
Bug 20912: (QA follow-up) Test for distinct accountlines

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 152 Martin Renvoize 2019-03-06 12:55:42 UTC
Created attachment 86152 [details] [review]
Bug 20912: (QA follow-up) Make DB Update idempotent

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 153 Martin Renvoize 2019-03-06 12:55:47 UTC
Created attachment 86153 [details] [review]
Bug 20912: (follow-up) Improve test coverage

Increase test coverage for CanBookBeIssued and fix a introduced during
the refactoring to Koha::Fees.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 154 Martin Renvoize 2019-03-06 12:55:51 UTC
Created attachment 86154 [details] [review]
Bug 20912: (follow-up) Add hourly support

It seemed strange to only add support for Daily loans in the feature
when Koha also support loan periods in hours. This patch adds parallel
functionaliy for hourly loan periods.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 155 Martin Renvoize 2019-03-06 12:55:55 UTC
Created attachment 86155 [details] [review]
Bug 20912: (QA follow-up) Fix TestBuilder

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 156 Martin Renvoize 2019-03-06 12:56:00 UTC
Created attachment 86156 [details] [review]
Bug 20912: (QA follow-up) Fix some test failures

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 157 Martin Renvoize 2019-03-06 12:56:04 UTC
Created attachment 86157 [details] [review]
Bug 20912: (QA follow-up) POD fix

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 158 Martin Renvoize 2019-03-06 12:56:08 UTC
Created attachment 86158 [details] [review]
Bug 20912: (QA follow-up) The default rule already has renewalsallowed=0

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 159 Martin Renvoize 2019-03-06 12:56:13 UTC
Created attachment 86159 [details] [review]
Bug 20912: (follow-up) Prevent double charging on renewal

This patch updates the tests the ensure we do not double charge for
renewals that take place before the original due date and fixes the
corresponding loging in C4::Circulation::AddRenewal.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 160 Martin Renvoize 2019-03-06 12:56:17 UTC
Created attachment 86160 [details] [review]
Bug 20912: (QA follow-up) Warn patron of daily and hourly renewal fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 161 Martin Renvoize 2019-03-07 14:00:04 UTC
Created attachment 86244 [details] [review]
Bug 20912: Update database

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 162 Martin Renvoize 2019-03-07 14:00:09 UTC
Created attachment 86245 [details] [review]
Bug 20912: Rental Fees based on Time Period

Some libraries would like to be able to charge a rental fee based on the
number of days an item will be checked out, as opposed to the flat fee
currently offered by Koha.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Edit an itemtype, add a daily rental fee of 1.00
4) Check an item of that itemtype out for 7 days
5) Verify the patron now has rental fee of 7.00

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 163 Martin Renvoize 2019-03-07 14:00:14 UTC
Created attachment 86246 [details] [review]
Bug 20912: Update schema file

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 164 Martin Renvoize 2019-03-07 14:00:18 UTC
Created attachment 86247 [details] [review]
Bug 20912: (QA follow-up) Fix number of tests and correct typos

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 165 Martin Renvoize 2019-03-07 14:00:23 UTC
Created attachment 86248 [details] [review]
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 166 Martin Renvoize 2019-03-07 14:00:27 UTC
Created attachment 86249 [details] [review]
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 167 Martin Renvoize 2019-03-07 14:00:32 UTC
Created attachment 86250 [details] [review]
Bug 20912: Move calculation to Koha::Fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 168 Martin Renvoize 2019-03-07 14:00:36 UTC
Created attachment 86251 [details] [review]
Bug 20912: (QA follow-up): Fix up Koha::Fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 169 Martin Renvoize 2019-03-07 14:00:41 UTC
Created attachment 86252 [details] [review]
Bug 20912: (QA follow-up): Fix up unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 170 Martin Renvoize 2019-03-07 14:00:45 UTC
Created attachment 86253 [details] [review]
Bug 20912: (QA follow-up) Move Fees to Charges::

In preparation for the introduction of Koha::Charges::Fines I have moved
this ::Fees class into the Koha::Charges:: namespace

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 171 Martin Renvoize 2019-03-07 14:00:50 UTC
Created attachment 86254 [details] [review]
Bug 20912: (QA follow-up) Test for distinct accountlines

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 172 Martin Renvoize 2019-03-07 14:00:54 UTC
Created attachment 86255 [details] [review]
Bug 20912: (QA follow-up) Make DB Update idempotent

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 173 Martin Renvoize 2019-03-07 14:00:59 UTC
Created attachment 86256 [details] [review]
Bug 20912: (follow-up) Improve test coverage

Increase test coverage for CanBookBeIssued and fix a introduced during
the refactoring to Koha::Fees.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 174 Martin Renvoize 2019-03-07 14:01:03 UTC
Created attachment 86257 [details] [review]
Bug 20912: (follow-up) Add hourly support

It seemed strange to only add support for Daily loans in the feature
when Koha also support loan periods in hours. This patch adds parallel
functionaliy for hourly loan periods.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 175 Martin Renvoize 2019-03-07 14:01:08 UTC
Created attachment 86258 [details] [review]
Bug 20912: (QA follow-up) Fix TestBuilder

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 176 Martin Renvoize 2019-03-07 14:01:12 UTC
Created attachment 86259 [details] [review]
Bug 20912: (QA follow-up) Fix some test failures

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 177 Martin Renvoize 2019-03-07 14:01:16 UTC
Created attachment 86260 [details] [review]
Bug 20912: (QA follow-up) POD fix

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 178 Martin Renvoize 2019-03-07 14:01:21 UTC
Created attachment 86261 [details] [review]
Bug 20912: (QA follow-up) The default rule already has renewalsallowed=0

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 179 Martin Renvoize 2019-03-07 14:01:25 UTC
Created attachment 86262 [details] [review]
Bug 20912: (follow-up) Prevent double charging on renewal

This patch updates the tests the ensure we do not double charge for
renewals that take place before the original due date and fixes the
corresponding loging in C4::Circulation::AddRenewal.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 180 Martin Renvoize 2019-03-07 14:01:30 UTC
Created attachment 86263 [details] [review]
Bug 20912: (QA follow-up) Warn patron of daily and hourly renewal fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 181 Martin Renvoize 2019-03-07 14:01:34 UTC
Created attachment 86264 [details] [review]
Bug 20912: (QA follow-up) Rebase error corrections

The automatic rebase after bug 21206 required a helping hand.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 182 Martin Renvoize 2019-03-07 14:02:28 UTC
Fresh rebase, should all be working again now.
Comment 183 Martin Renvoize 2019-03-07 16:36:42 UTC
Created attachment 86282 [details] [review]
Bug 20912: Update database

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 184 Martin Renvoize 2019-03-07 16:36:46 UTC
Created attachment 86283 [details] [review]
Bug 20912: Rental Fees based on Time Period

Some libraries would like to be able to charge a rental fee based on the
number of days an item will be checked out, as opposed to the flat fee
currently offered by Koha.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Edit an itemtype, add a daily rental fee of 1.00
4) Check an item of that itemtype out for 7 days
5) Verify the patron now has rental fee of 7.00

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 185 Martin Renvoize 2019-03-07 16:36:51 UTC
Created attachment 86284 [details] [review]
Bug 20912: Update schema file

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 186 Martin Renvoize 2019-03-07 16:36:55 UTC
Created attachment 86285 [details] [review]
Bug 20912: (QA follow-up) Fix number of tests and correct typos

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 187 Martin Renvoize 2019-03-07 16:37:00 UTC
Created attachment 86286 [details] [review]
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 188 Martin Renvoize 2019-03-07 16:37:04 UTC
Created attachment 86287 [details] [review]
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 189 Martin Renvoize 2019-03-07 16:37:09 UTC
Created attachment 86288 [details] [review]
Bug 20912: Move calculation to Koha::Fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 190 Martin Renvoize 2019-03-07 16:37:14 UTC
Created attachment 86289 [details] [review]
Bug 20912: (QA follow-up): Fix up Koha::Fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 191 Martin Renvoize 2019-03-07 16:37:18 UTC
Created attachment 86290 [details] [review]
Bug 20912: (QA follow-up): Fix up unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 192 Martin Renvoize 2019-03-07 16:37:23 UTC
Created attachment 86291 [details] [review]
Bug 20912: (QA follow-up) Move Fees to Charges::

In preparation for the introduction of Koha::Charges::Fines I have moved
this ::Fees class into the Koha::Charges:: namespace

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 193 Martin Renvoize 2019-03-07 16:37:27 UTC
Created attachment 86292 [details] [review]
Bug 20912: (QA follow-up) Test for distinct accountlines

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 194 Martin Renvoize 2019-03-07 16:37:32 UTC
Created attachment 86293 [details] [review]
Bug 20912: (QA follow-up) Make DB Update idempotent

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 195 Martin Renvoize 2019-03-07 16:37:37 UTC
Created attachment 86294 [details] [review]
Bug 20912: (follow-up) Improve test coverage

Increase test coverage for CanBookBeIssued and fix a introduced during
the refactoring to Koha::Fees.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 196 Martin Renvoize 2019-03-07 16:37:41 UTC
Created attachment 86295 [details] [review]
Bug 20912: (follow-up) Add hourly support

It seemed strange to only add support for Daily loans in the feature
when Koha also support loan periods in hours. This patch adds parallel
functionaliy for hourly loan periods.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 197 Martin Renvoize 2019-03-07 16:37:46 UTC
Created attachment 86296 [details] [review]
Bug 20912: (QA follow-up) Fix TestBuilder

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 198 Martin Renvoize 2019-03-07 16:37:50 UTC
Created attachment 86297 [details] [review]
Bug 20912: (QA follow-up) Fix some test failures

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 199 Martin Renvoize 2019-03-07 16:37:55 UTC
Created attachment 86298 [details] [review]
Bug 20912: (QA follow-up) POD fix

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 200 Martin Renvoize 2019-03-07 16:37:59 UTC
Created attachment 86299 [details] [review]
Bug 20912: (QA follow-up) The default rule already has renewalsallowed=0

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 201 Martin Renvoize 2019-03-07 16:38:04 UTC
Created attachment 86300 [details] [review]
Bug 20912: (follow-up) Prevent double charging on renewal

This patch updates the tests the ensure we do not double charge for
renewals that take place before the original due date and fixes the
corresponding loging in C4::Circulation::AddRenewal.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 202 Martin Renvoize 2019-03-07 16:38:09 UTC
Created attachment 86301 [details] [review]
Bug 20912: (QA follow-up) Warn patron of daily and hourly renewal fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 203 Martin Renvoize 2019-03-07 16:38:14 UTC
Created attachment 86302 [details] [review]
Bug 20912: (QA follow-up) Rebase error corrections

The automatic rebase after bug 21206 required a helping hand.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 204 Martin Renvoize 2019-03-07 17:20:51 UTC
Created attachment 86306 [details] [review]
Bug 20912: Update database

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 205 Martin Renvoize 2019-03-07 17:20:56 UTC
Created attachment 86307 [details] [review]
Bug 20912: Rental Fees based on Time Period

Some libraries would like to be able to charge a rental fee based on the
number of days an item will be checked out, as opposed to the flat fee
currently offered by Koha.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Edit an itemtype, add a daily rental fee of 1.00
4) Check an item of that itemtype out for 7 days
5) Verify the patron now has rental fee of 7.00

Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 206 Martin Renvoize 2019-03-07 17:21:00 UTC
Created attachment 86308 [details] [review]
Bug 20912: Update schema file

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 207 Martin Renvoize 2019-03-07 17:21:05 UTC
Created attachment 86309 [details] [review]
Bug 20912: (QA follow-up) Fix number of tests and correct typos

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 208 Martin Renvoize 2019-03-07 17:21:10 UTC
Created attachment 86310 [details] [review]
Bug 20912: (QA follow-up) Set daily charge to 0 for item type not being used for test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 209 Martin Renvoize 2019-03-07 17:21:14 UTC
Created attachment 86311 [details] [review]
Bug 20912: (QA follow-up) Make unit tests reliable and get rid of perl warnings

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 210 Martin Renvoize 2019-03-07 17:21:19 UTC
Created attachment 86312 [details] [review]
Bug 20912: Move calculation to Koha::Fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 211 Martin Renvoize 2019-03-07 17:21:24 UTC
Created attachment 86313 [details] [review]
Bug 20912: (QA follow-up): Fix up Koha::Fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 212 Martin Renvoize 2019-03-07 17:21:28 UTC
Created attachment 86314 [details] [review]
Bug 20912: (QA follow-up): Fix up unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 213 Martin Renvoize 2019-03-07 17:21:33 UTC
Created attachment 86315 [details] [review]
Bug 20912: (QA follow-up) Move Fees to Charges::

In preparation for the introduction of Koha::Charges::Fines I have moved
this ::Fees class into the Koha::Charges:: namespace

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 214 Martin Renvoize 2019-03-07 17:21:37 UTC
Created attachment 86316 [details] [review]
Bug 20912: (QA follow-up) Test for distinct accountlines

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 215 Martin Renvoize 2019-03-07 17:21:43 UTC
Created attachment 86317 [details] [review]
Bug 20912: (QA follow-up) Make DB Update idempotent

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 216 Martin Renvoize 2019-03-07 17:21:47 UTC
Created attachment 86318 [details] [review]
Bug 20912: (follow-up) Improve test coverage

Increase test coverage for CanBookBeIssued and fix a introduced during
the refactoring to Koha::Fees.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 217 Martin Renvoize 2019-03-07 17:21:52 UTC
Created attachment 86319 [details] [review]
Bug 20912: (follow-up) Add hourly support

It seemed strange to only add support for Daily loans in the feature
when Koha also support loan periods in hours. This patch adds parallel
functionaliy for hourly loan periods.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 218 Martin Renvoize 2019-03-07 17:21:57 UTC
Created attachment 86320 [details] [review]
Bug 20912: (QA follow-up) Fix TestBuilder

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 219 Martin Renvoize 2019-03-07 17:22:01 UTC
Created attachment 86321 [details] [review]
Bug 20912: (QA follow-up) Fix some test failures

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 220 Martin Renvoize 2019-03-07 17:22:07 UTC
Created attachment 86322 [details] [review]
Bug 20912: (QA follow-up) POD fix

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 221 Martin Renvoize 2019-03-07 17:22:11 UTC
Created attachment 86323 [details] [review]
Bug 20912: (QA follow-up) The default rule already has renewalsallowed=0

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 222 Martin Renvoize 2019-03-07 17:22:16 UTC
Created attachment 86324 [details] [review]
Bug 20912: (follow-up) Prevent double charging on renewal

This patch updates the tests the ensure we do not double charge for
renewals that take place before the original due date and fixes the
corresponding loging in C4::Circulation::AddRenewal.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 223 Martin Renvoize 2019-03-07 17:22:21 UTC
Created attachment 86325 [details] [review]
Bug 20912: (QA follow-up) Warn patron of daily and hourly renewal fees

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 224 Martin Renvoize 2019-03-07 17:22:25 UTC
Created attachment 86326 [details] [review]
Bug 20912: (QA follow-up) Rebase error corrections

The automatic rebase after bug 21206 required a helping hand.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 225 Martin Renvoize 2019-03-07 17:22:30 UTC
Created attachment 86327 [details] [review]
Bug 20912: (QA follow-up) Add on_editing to $Price filter

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 226 Nick Clemens 2019-03-07 17:40:55 UTC
Awesome work all!

Pushed to master for 19.05
Comment 227 Martin Renvoize 2019-03-12 08:34:29 UTC
Enhancement will not be backported to 18.11.x series.
Comment 228 Dobrica Pavlinusic 2019-04-03 09:11:59 UTC
I'm afraid that this patch broke AddRenewal which now calculates wrong duedate because of:

-        $datedue =  CalcDateDue($datedue, $itemtype, _GetCircControlBranch($item_unblessed, $patron_unblessed), $patron_unblessed, 'is a renewal');
+        $datedue =  CalcDateDue($datedue, $itemtype, $circ_library, $patron_unblessed, 'is a renewal');


This will pass $circ_library object instead of branch name so $duedate will be calculated wrongly.
Comment 229 Martin Renvoize 2019-04-03 10:12:37 UTC
Dobrica, long time no see.. nice to have you back.

Very annoying this wasn't caught by a test.. I'll open it in a new bug as I think it'll only add to the confusion adding followups here.
Comment 230 Martin Renvoize 2019-04-03 15:01:35 UTC
Created attachment 87363 [details] [review]
Bug 20912: (QA follow-up) Fix for missed signature failure

We missed a change in calling parameters passed to CalcDateDue in AddRenewal
during the initial QAing. This patch corrects the call and adds a test to catch
regressions.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 231 Kyle M Hall 2019-04-04 13:32:24 UTC
Created attachment 87413 [details] [review]
Bug 20912: (QA follow-up) Fix for missed signature failure

We missed a change in calling parameters passed to CalcDateDue in AddRenewal
during the initial QAing. This patch corrects the call and adds a test to catch
regressions.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 232 Dobrica Pavlinusic 2019-04-10 08:37:11 UTC
Created attachment 87703 [details] [review]
Bug 20912: (QA follow-up) Fix for missed signature failure
Comment 233 Dobrica Pavlinusic 2019-04-10 08:40:05 UTC
Thanks for patch, I signed-off on it.
Comment 234 Nick Clemens 2019-04-11 13:55:13 UTC
(In reply to Dobrica Pavlinusic from comment #232)
> Created attachment 87703 [details] [review] [review]
> Bug 20912: (QA follow-up) Fix for missed signature failure

Awesome work all!

follow-up pushed to master for 19.05