Bug 17015 - New Koha Calendar
Summary: New Koha Calendar
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Holidays (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement with 1 vote (vote)
Assignee: Charles Farmer
QA Contact: Testopia
URL:
Keywords:
Depends on: 20660
Blocks: 17983 18485 18486
  Show dependency treegraph
 
Reported: 2016-08-01 18:07 UTC by Mehdi Hamidi
Modified: 2020-03-03 21:23 UTC (History)
28 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Large patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug - 17015 New calendar (22.61 KB, patch)
2016-08-01 18:26 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - New koha calendar (73.99 KB, patch)
2016-08-11 18:52 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - New koha calendar (73.98 KB, patch)
2016-08-13 00:45 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (78.25 KB, patch)
2017-01-18 16:11 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (78.25 KB, patch)
2017-01-18 16:20 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Tests for DiscreteCalendar (12.05 KB, patch)
2017-01-18 16:20 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Koha now uses the new DiscreteCalendar module (26.06 KB, patch)
2017-01-18 16:20 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Updated Koha tests to use the new DiscreteCalendar module (5.63 KB, patch)
2017-01-18 16:21 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Peformance tests for Koha::DiscreteCalendar (255.84 KB, patch)
2017-01-18 16:21 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (78.54 KB, patch)
2017-01-19 15:33 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Tests for DiscreteCalendar (12.12 KB, patch)
2017-01-19 15:33 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Koha now uses the new DiscreteCalendar module (12.15 KB, patch)
2017-01-19 15:33 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Updated Koha tests to use the new DiscreteCalendar module (5.91 KB, patch)
2017-01-19 15:33 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Peformance tests for Koha::DiscreteCalendar (255.84 KB, patch)
2017-01-19 15:33 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Updated Koha tests to use the new DiscreteCalendar module (7.26 KB, patch)
2017-01-23 19:36 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (78.55 KB, patch)
2017-01-24 17:19 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Tests for DiscreteCalendar (11.74 KB, patch)
2017-01-24 17:19 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Koha now uses the new DiscreteCalendar module (12.15 KB, patch)
2017-01-24 17:19 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Updated Koha tests to use the new DiscreteCalendar module (7.26 KB, patch)
2017-01-24 17:19 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Peformance tests for Koha::DiscreteCalendar (255.84 KB, patch)
2017-01-24 17:19 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Fix for sandbox users, updatedatebase.pl now calls generate_discrete_calendar.pl (819 bytes, patch)
2017-01-25 20:36 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (78.68 KB, patch)
2017-01-25 20:50 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - sandbox fixe (1.78 KB, patch)
2017-01-26 15:50 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (78.18 KB, patch)
2017-01-26 17:17 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (79.26 KB, patch)
2017-02-03 15:27 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Tests for DiscreteCalendar (11.81 KB, patch)
2017-02-03 15:28 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (79.71 KB, patch)
2017-03-07 20:10 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Tests for DiscreteCalendar (11.83 KB, patch)
2017-03-07 20:10 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Koha now uses the new DiscreteCalendar module (12.15 KB, patch)
2017-03-07 20:13 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Updated Koha tests to use the new DiscreteCalendar module (7.28 KB, patch)
2017-03-07 20:13 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Peformance tests for Koha::DiscreteCalendar (255.85 KB, patch)
2017-03-07 20:13 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (80.01 KB, patch)
2017-03-10 12:21 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (80.05 KB, patch)
2017-03-30 13:07 UTC, Blou
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (82.47 KB, patch)
2017-04-19 14:29 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Tests for DiscreteCalendar (11.83 KB, patch)
2017-04-19 14:32 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Updated Koha tests to use the new DiscreteCalendar module (7.28 KB, patch)
2017-04-19 14:40 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Koha now uses the new DiscreteCalendar module (12.73 KB, patch)
2017-05-08 19:45 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (82.40 KB, patch)
2017-05-09 12:14 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (82.70 KB, patch)
2017-05-10 17:33 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (82.70 KB, patch)
2017-05-10 17:34 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Tests for DiscreteCalendar (13.05 KB, patch)
2017-05-10 17:34 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Koha now uses the new DiscreteCalendar module (13.53 KB, patch)
2017-05-10 17:35 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Updated Koha tests to use the new DiscreteCalendar module (5.80 KB, patch)
2017-05-10 17:35 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Peformance tests for Koha::DiscreteCalendar (255.85 KB, patch)
2017-05-10 17:35 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (88.07 KB, patch)
2017-06-12 20:05 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (88.31 KB, patch)
2017-07-17 14:56 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Tests for DiscreteCalendar (13.05 KB, patch)
2017-07-17 14:57 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Koha now uses the new DiscreteCalendar module (13.64 KB, patch)
2017-07-17 14:57 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Updated Koha tests to use the new DiscreteCalendar module (5.80 KB, patch)
2017-07-17 14:57 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - Peformance tests for Koha::DiscreteCalendar (255.85 KB, patch)
2017-07-17 14:57 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (88.31 KB, patch)
2017-10-06 15:26 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015 - Tests for DiscreteCalendar (13.05 KB, patch)
2017-10-06 15:26 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015 - Koha now uses the new DiscreteCalendar module (13.55 KB, patch)
2017-10-06 15:26 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015 - Updated Koha tests to use the new DiscreteCalendar module (5.80 KB, patch)
2017-10-06 15:26 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015 - Peformance tests for Koha::DiscreteCalendar (255.85 KB, patch)
2017-10-06 15:26 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015 - Fix QA tools criticism (16.39 KB, patch)
2017-10-10 15:09 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (88.44 KB, patch)
2017-10-11 09:24 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 17015 - Tests for DiscreteCalendar (13.16 KB, patch)
2017-10-11 09:26 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 17015 - Koha now uses the new DiscreteCalendar module (13.75 KB, patch)
2017-10-11 09:35 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 17015 - Updated Koha tests to use the new DiscreteCalendar module (5.99 KB, patch)
2017-10-11 09:38 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 17015 - Peformance tests for Koha::DiscreteCalendar (255.93 KB, patch)
2017-10-11 09:44 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 17015 - Fix QA tools criticism (16.53 KB, patch)
2017-10-11 09:53 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (88.44 KB, patch)
2017-10-23 13:44 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015 - Tests for DiscreteCalendar (13.16 KB, patch)
2017-10-23 13:44 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015 - Koha now uses the new DiscreteCalendar module (13.78 KB, patch)
2017-10-23 13:44 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015 - Updated Koha tests to use the new DiscreteCalendar module (5.99 KB, patch)
2017-10-23 13:45 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015 - Peformance tests for Koha::DiscreteCalendar (255.93 KB, patch)
2017-10-23 13:45 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015 - Fix QA tools criticism (16.53 KB, patch)
2017-10-23 13:45 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts (95.27 KB, patch)
2018-02-14 19:17 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015 - Tests for DiscreteCalendar (15.50 KB, patch)
2018-02-14 19:17 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015 - Koha now uses the new DiscreteCalendar module (21.55 KB, patch)
2018-02-14 19:17 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015 - Peformance tests for Koha::DiscreteCalendar (255.86 KB, patch)
2018-02-14 19:18 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015 - Remove old Koha::Calendar (55.52 KB, patch)
2018-02-14 19:18 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015 - Correction on QA feedback (110.66 KB, patch)
2018-03-14 16:11 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015 - Add gen_time to TestBuilder (1.10 KB, patch)
2018-03-21 18:06 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015: DiscreteCalendar UI, Back-End and necessary scripts (95.27 KB, patch)
2018-04-25 20:25 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015: Tests for DiscreteCalendar (15.50 KB, patch)
2018-04-25 20:25 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015: Koha now uses the new DiscreteCalendar module (21.55 KB, patch)
2018-04-25 20:25 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015: Peformance tests for Koha::DiscreteCalendar (255.86 KB, patch)
2018-04-25 20:26 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015: Remove old Koha::Calendar (55.97 KB, patch)
2018-04-25 20:26 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015: Code quality fixes on QA feedback (110.66 KB, patch)
2018-04-25 20:26 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015: Add gen_time to TestBuilder (1.10 KB, patch)
2018-04-25 20:26 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015: Updated patch after bug 19204 (9.19 KB, patch)
2018-04-25 20:26 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015: Updated patch after bug 19204 (9.16 KB, patch)
2018-05-18 21:04 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015: Improve stability for very old loans (1.23 KB, patch)
2018-05-22 18:42 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015: Improved UI readability (5.31 KB, patch)
2018-06-29 20:41 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015: Fixes to migration script (3.59 KB, patch)
2018-07-24 20:58 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015: Fixes to cronjob (2.82 KB, patch)
2018-07-24 21:20 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 17015: Code quality fixes on QA feedback (110.64 KB, patch)
2018-08-09 02:35 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Add gen_time to TestBuilder (1.10 KB, patch)
2018-08-09 02:35 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Updated patch after bug 19204 (9.16 KB, patch)
2018-08-09 02:35 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Improve stability for very old loans (1.23 KB, patch)
2018-08-09 02:35 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Improved UI readability (5.31 KB, patch)
2018-08-09 02:35 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Fixes to migration script (3.59 KB, patch)
2018-08-09 02:35 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Fixes to cronjob (2.82 KB, patch)
2018-08-09 02:37 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Updated patch after bug 19204 (9.46 KB, patch)
2018-08-13 21:27 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Improve stability for very old loans (1.23 KB, patch)
2018-08-13 21:27 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Improved UI readability (5.31 KB, patch)
2018-08-13 21:27 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Fixes to migration script (3.59 KB, patch)
2018-08-13 21:27 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Fixes to cronjob (2.82 KB, patch)
2018-08-13 21:27 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: (follow-up) keep kohastructure.sql clean to avoid bugs when installing a fresh koha directly over this patchset (3.50 KB, patch)
2018-09-17 18:21 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: DiscreteCalendar UI, Back-End and necessary scripts (95.27 KB, patch)
2018-10-12 15:49 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Tests for DiscreteCalendar (15.50 KB, patch)
2018-10-12 15:49 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Koha now uses the new DiscreteCalendar module (22.02 KB, patch)
2018-10-12 15:50 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Peformance tests for Koha::DiscreteCalendar (255.86 KB, patch)
2018-10-12 15:50 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Remove old Koha::Calendar (55.97 KB, patch)
2018-10-12 15:50 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Code quality fixes on QA feedback (110.64 KB, patch)
2018-10-12 15:50 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Add gen_time to TestBuilder (1.10 KB, patch)
2018-10-12 15:50 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Updated patch after bug 19204 (9.47 KB, patch)
2018-10-12 15:50 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Improve stability for very old loans (1.23 KB, patch)
2018-10-12 15:51 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Improved UI readability (5.31 KB, patch)
2018-10-12 15:51 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Fixes to migration script (3.59 KB, patch)
2018-10-12 15:51 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Fixes to cronjob (2.82 KB, patch)
2018-10-12 15:51 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: (follow-up) keep kohastructure.sql clean to avoid bugs when installing a fresh koha directly over this patchset (3.50 KB, patch)
2018-10-12 15:51 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: (follow-up) Fixes a holiday bug when going from 'none' to 'empty' (862 bytes, patch)
2018-12-28 21:53 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: (follow-up) move javascript .inc files inside jsinclude block (1.43 KB, patch)
2019-02-20 17:12 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: DiscreteCalendar UI, Back-End and necessary scripts (95.27 KB, patch)
2019-02-26 05:22 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Tests for DiscreteCalendar (15.50 KB, patch)
2019-02-26 05:22 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Koha now uses the new DiscreteCalendar module (21.54 KB, patch)
2019-02-26 05:22 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Peformance tests for Koha::DiscreteCalendar (255.86 KB, patch)
2019-02-26 05:23 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Remove old Koha::Calendar (55.96 KB, patch)
2019-02-26 05:23 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Code quality fixes on QA feedback (109.77 KB, patch)
2019-02-26 05:23 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Add gen_time to TestBuilder (1.10 KB, patch)
2019-02-26 05:23 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Updated patch after bug 19204 (9.31 KB, patch)
2019-02-26 05:23 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Improve stability for very old loans (1.23 KB, patch)
2019-02-26 05:23 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Improved UI readability (5.31 KB, patch)
2019-02-26 05:23 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Fixes to migration script (3.59 KB, patch)
2019-02-26 05:24 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Fixes to cronjob (2.82 KB, patch)
2019-02-26 05:24 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: (follow-up) keep kohastructure.sql clean to avoid bugs when installing a fresh koha directly over this patchset (3.50 KB, patch)
2019-02-26 05:24 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: (follow-up) Fixes a holiday bug when going from 'none' to 'empty' (862 bytes, patch)
2019-02-26 05:24 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: (follow-up) move javascript .inc files inside jsinclude block (1.43 KB, patch)
2019-02-26 05:24 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Performance comparison (images) (255.84 KB, patch)
2019-09-04 19:01 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: DiscreteCalendar UI, Back-End and necessary scripts (162.58 KB, patch)
2019-09-04 19:01 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Install scripts for DiscreteCalendar (8.77 KB, patch)
2019-09-04 19:01 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: Tests for DiscreteCalendar (56.94 KB, patch)
2019-09-04 19:01 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 17015: (follow-up) accept Koha::Library objects in place of a branchcode string, always use the syspref 'dateformat' (2.36 KB, patch)
2020-03-03 21:23 UTC, Charles Farmer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mehdi Hamidi 2016-08-01 18:07:25 UTC
We are trying to create a new Koha calendar system. The one available right now is way too slow. This is the first patch of the calendar, it doesn't affect the functioning of the current calendar, but it works in parallel with it. Instead of having multiple tables for the holidays, we created one table, called discrete_calendar. This table contains the dates,branchcode, opening/closing hours, a note about the holiday and whether the library is open or not. 

The script misc/generate_discrete_calendar.pl fills the table discrete_calendar based on tables : repeatable_holidays, special_holidays, openinghours and branches. The script, by default, will add 2 years from the past and 1 year in the future in the table discrete_calendar. It also has an option to added more than 1 year in the future.

The script misc/cronjobs/add_days_discrete_calendar.pl will add days in the future in the table discrete calendar. A day's opening and closing hours will be based on the same weekday from the week before. It will also check if the same day from last year was a holiday, if so, this will be counted as a holiday.
The script makes sur there's at least 1 year in the future. So if it hasn't ran for 8 days, 8 days will be added in he future the next time the script is executed.

There's also an atomic update to create the table.

The current options are not final and they can be changed for convenience.
Comment 1 Mehdi Hamidi 2016-08-01 18:26:18 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2016-08-01 20:35:02 UTC
>It will also check if the
>same day from last year was a holiday, if so, this will be counted as a
>holiday.

This sounds like a wrong assumption. Most of the holidays here are not on the same date every year - so this would create a horrible mess. It's the reason why you can choose to have unique holidays and holidays that repeat every year with the current system.
Comment 3 Chris Cormack 2016-08-01 20:45:13 UTC
(In reply to Katrin Fischer from comment #2)
> >It will also check if the
> >same day from last year was a holiday, if so, this will be counted as a
> >holiday.
> 
> This sounds like a wrong assumption. Most of the holidays here are not on
> the same date every year - so this would create a horrible mess. It's the
> reason why you can choose to have unique holidays and holidays that repeat
> every year with the current system.

I agree this sounds like a big step backwards in functionality. The first step of any rewrite/refactor should be to make sure all existing functionality is not lost.
The calendar is vitally important, so many things depend on it, it is vital we don't lose functionality.
Comment 4 Mehdi Hamidi 2016-08-11 18:52:29 UTC Comment hidden (obsolete)
Comment 5 Mehdi Hamidi 2016-08-13 00:45:50 UTC Comment hidden (obsolete)
Comment 6 Mehdi Hamidi 2017-01-18 16:11:49 UTC Comment hidden (obsolete)
Comment 7 Mehdi Hamidi 2017-01-18 16:20:21 UTC Comment hidden (obsolete)
Comment 8 Mehdi Hamidi 2017-01-18 16:20:36 UTC Comment hidden (obsolete)
Comment 9 Mehdi Hamidi 2017-01-18 16:20:55 UTC Comment hidden (obsolete)
Comment 10 Mehdi Hamidi 2017-01-18 16:21:06 UTC Comment hidden (obsolete)
Comment 11 Mehdi Hamidi 2017-01-18 16:21:21 UTC Comment hidden (obsolete)
Comment 12 Mehdi Hamidi 2017-01-18 16:23:05 UTC
Hello everyone, this is our new Calendar. This calendar is the combination of Koha::Calendar and C4::Calendar.
The current calendar is very slow, especially for returns past due date. DateTime Objects are very huge and resource consuming.
Our new calendar creates a Database Table to keep track of dates, holidays, open and close hours per branch.
This way we can keep information about past, current and future dates/holidays.
We also depend on the database to make the calculations instead of DateTime Objects which makes our new calendar incredibly faster than the current one for fines and check-ins.
The last patch if for performance purposes and it show improvement in comparison to the current calendar

-What we added/changed:
1- New UI
2- Change open and close hours for single and on range dates.
- Holiday types:
    - Float : like Thanksgiving, holiday without a fixed date (same as exception, might be removed)
    - Needs validation : When a day is added in the future and it was an Exception or Floating holiday 365 days in the past.
        This day is considered opened, so if the holiday still applies, it needs to be changed to correct holiday.
- Change holiday type, override existing holiday.
- Copy range of holidays :
    - 1 day to multiple dates.
    - 7 days to another 7 days , apply  day of the week to the same day of the week.
    - Any number of dates to the same number of dates.
  PS: This will copy everything, isopened, open/close hours and notes.
- Delete repeated and weekly holidays for a single date or all of them.
- Delete holidays on the range without a type.
- Calculate hours opened between dates based on open and close hours of the branch.
- Copy calendar to another branch.

UI tips and tricks:
- Open/close hour, date type inputs will be emptied when selecting a range that doesn't share the same values. Only different values will be emptied.
- Leaving one these (open/close hour, date type) inputs empty means they won't be updated.
- Leaving the title input empty means it will be empty/deleted.
- Only dates in discrete_calendar table are selectable on the calendar.
- You can't update the information about a past date, meaning you can only update today and future dates. This also applies to weekly holidays
- Checking delete type, will delete all :
    - Weekly holidays of that day, example : all Sundays.
    - Repeatable holidays, example : all 1 of January.
- Weekly holidays will apply the same open/close hours for all the dates affected unless these inputs are left empty,
    this can be used to update all open/close hours of a given day of the week, then removing the holiday.

Scripts :
  - misc/generate_discrete_calendar.pl
      This script fills the table discrete_calendar based on tables : repeatable_holidays, special_holidays and branches.
      The script, by default, will add 2 years from the past and 1 year in the future in the table discrete_calendar.
      It also has an option to add more than 1 year in the future.
      Opening and closing hours are by default 9:00 and 17:00 respectively, they can be changed using the UI
  - add_days_discrete_calendar.pl
      This script will be used to add days in the future for the calendar, default is 1 day, this can be changed using -n option for the script.
      The add day will have the following values:
          - Open/close hours based on the same day of the week, a week beofre it. Example : new day : Friday 2018-01-19, a week before Friday 2018-01-12
          - HolidayType based on the same day a year ago, however if the holiday type was :
                Float/Exception : the holidaytype of the the new day will be marked as Needs validation.
                Weekly : the new day will be marked as normal day(To discuss further).
                Fixed  : the new day will be closed as fixed holiday.

Test plan :
1- Apply patch
2- Run installer/data/mysql/updatedatabase.pl
3- Have atleast these branches CPL, FFL, LPL for test purposes (specific test : t/db_dependent/HoldsQueue.t)
4- Run the script misc/generate_discrete_calendar.pl (one time only, this may take a while)
5- Run the script add_days_discrete_calendar.pl to add days into the future
    - Check if the max date is now today + 1 day
6- Generate schemas by running misc/devel/update_dbix_class_files.pl
7- Run test t/db_dependent/DiscreteCalendar.t
8- Run all koha test t/

Use Discrete Calendar in Tools.
Comment 13 Josef Moravec 2017-01-18 17:41:22 UTC
I am always for making Koha faster ;)

I just did first round of testing:

scripts
generate_discrete_calendar.pl
and
add_days_discrete_calendar.pl
needs to have set execution permission in filesystem


the need to run generate_discrete_calendar.pl should be somehow documented in UI, adding holidays does not work without it and the can't know why

qa tools complain:
 FAIL	C4/Circulation.pm
   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 3418)
		forbidden pattern: tab char (line 394)
		forbidden pattern: tab char (line 676)
		forbidden pattern: tab char (line 3575)
		forbidden pattern: tab char (line 3553)

 FAIL	Koha/DiscreteCalendar.pm
   FAIL	  critic
		Variable declared in conditional statement at line 862, column 5. Declare variables outside of the condition.
		Variable declared in conditional statement at line 861, column 5. Declare variables outside of the condition.
   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 413)
		forbidden pattern: tab char (line 412)
		forbidden pattern: tab char (line 411)
   FAIL	  pod
		Spurious text after =cut
		Spurious =cut command
		
		*** ERROR: 
		 
		 in file Koha/DiscreteCalendar.pm
   FAIL	  spelling
		 differnt  ==> different


 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/tools/discrete_calendar.tt
   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 440)
		forbidden pattern: intranet-tmpl should certainly replaced with [% interface %] (line 7)
		forbidden pattern: tab char (line 439)
		forbidden pattern: tab char (line 441)


 FAIL	misc/cronjobs/staticfines.pl
   FAIL	  valid
		Can't locate C4/DiscreteCalendar.pm in @INC (you may need to install the C4::DiscreteCalendar module) (@INC contains: /home/vagrant/kohaclone /home/vagrant/qa-test-tools /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.20.2 /usr/local/share/perl/5.20.2 /usr/lib/x86_64-linux-gnu/perl5/5.20 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.20 /usr/share/perl/5.20 /usr/local/lib/site_perl .) 

Ad point 3 in your test plan: It would be better not to rely on existing branchcodes, but use TestBuilder to make one you need
Comment 14 Mehdi Hamidi 2017-01-18 20:44:41 UTC
(In reply to Josef Moravec from comment #13)

Hello Josef, 
thank you for testing our patches. I will push a fix for errors you mentioned very soon.  
As for your concerns : 
>- the need to run generate_discrete_calendar.pl should be somehow documented in >UI, adding holidays does not work without it and the can't know why

I added a warning to let the user know about that. As for why you can't add holidays, it's because we need to have the date in the database beforehand, which means if the table is empty, there's date to edit. Holidays are just date where the column isopen=0.

>Ad point 3 in your test plan: It would be better not to rely on existing >branchcodes, but use TestBuilder to make one you need

Our calendar needs to have valid branchcodes where their date are already in table discrete_calendar. Using TestBuilder gives random branchcodes so the dates cannot be retrieved from database since they are generated by generate_discrete_calendar.pl and add_days_discrete_calendar.pl.

If you have a suggestion to bypass the hard coded branchcodes, let us know!
Comment 15 Mehdi Hamidi 2017-01-18 23:25:04 UTC
(In reply to Josef Moravec from comment #13)

> >Ad point 3 in your test plan: It would be better not to rely on existing >branchcodes, but use TestBuilder to make one you need
> 
> Our calendar needs to have valid branchcodes where their date are already in
> table discrete_calendar. Using TestBuilder gives random branchcodes so the
> dates cannot be retrieved from database since they are generated by
> generate_discrete_calendar.pl and add_days_discrete_calendar.pl.
> 
> If you have a suggestion to bypass the hard coded branchcodes, let us know!

Please discard this part.
Comment 16 Mehdi Hamidi 2017-01-19 15:33:11 UTC Comment hidden (obsolete)
Comment 17 Mehdi Hamidi 2017-01-19 15:33:27 UTC Comment hidden (obsolete)
Comment 18 Mehdi Hamidi 2017-01-19 15:33:35 UTC Comment hidden (obsolete)
Comment 19 Mehdi Hamidi 2017-01-19 15:33:41 UTC Comment hidden (obsolete)
Comment 20 Mehdi Hamidi 2017-01-19 15:33:54 UTC Comment hidden (obsolete)
Comment 21 Mehdi Hamidi 2017-01-19 15:44:27 UTC
Hello everyone,
I fixed the problems mentioned by Josef Moravec above, thank you once again!

Also, you no longer need to have valid branchcodes to run tests.
Comment 22 Mehdi Hamidi 2017-01-23 19:36:20 UTC Comment hidden (obsolete)
Comment 23 Mehdi Hamidi 2017-01-24 17:19:09 UTC Comment hidden (obsolete)
Comment 24 Mehdi Hamidi 2017-01-24 17:19:17 UTC Comment hidden (obsolete)
Comment 25 Mehdi Hamidi 2017-01-24 17:19:27 UTC Comment hidden (obsolete)
Comment 26 Mehdi Hamidi 2017-01-24 17:19:42 UTC Comment hidden (obsolete)
Comment 27 Mehdi Hamidi 2017-01-24 17:19:51 UTC Comment hidden (obsolete)
Comment 28 Mehdi Hamidi 2017-01-24 20:00:48 UTC
Hello everyone, we added a more detailed test plan for the UI:
- Go tools and choose Discrete Calendar
- The field title is optional.

[Cant change past]
- Choose any date from the calendar that is yesterday or older.
- Change it's details and save. 
Result: **Nothing should change.**

[change from past to future]
- Choose any date from the calendar that is yesterday or older.
- Choose another date in the To date field that is today or in the future.
- Update the details and save.
Result: **Only dates starting from today are affected.**

Adding holidays
[Single holiday]
- Choose any date from the calendar that is today or in the future.
- Choose another date in the To date field that is in the future (optional, for range only).
- Change Date type to : exception.
- Save.
Result: **All dates in the range should be marked red for unique holiday.**

[Weekly]
- Choose any Sunday from the calendar that is in the past or future.
- Change Date type to : Weekly.
- Save.
Result: **All upcomming (starting from today) Sundays will be holidays.**
PS: Open and close hour will be affected **to all Sundays** unless inputs are empty.

[Fixed]
- Choose any date from the calendar that is today or in the future.
- Change Date type to : Fixed.
- Save
Result: **The selected date and the same day in the future (if available in Calendar) will be holiday.**

[Fixed from the past]
- Choose any date from the calendar that is in the past.
- Change Date type to : Fixed.
- Save
Result: **The selected date  is unchanged but the same day in the future (if available in Calendar) will be holiday.**

Delete holidays
[Single date]
- Choose any date from the calendar that is today or in the future and is also a holiday.
- Change Date type to : Normal day.
- Save.
Result: **The selected date should be a normal day**

[Range of dates]
- Choose any date from the calendar that is today or in the future.
- Choose another date in the To date field that is in the future.
- Change date type to : Normal day
- Save.
Result: **The selected dates should be a normal days**

[Delete type]
- Choose any date from the calendar that is today or in the future and is a weekly or a fixed holiday.
- Change date type to : Normal day.
- Check : Delete this type CheckBox
- Save.
Result: -Weekly, all upcoming weekly holidays if that day will be normal days.
            Example : all upcoming Sundays will be normal days
        -Fixed, all upcoming fixed holidays of that day will be normal days. 
            Example : all upcoming 25 December will be normal days

[Copy dates, 1 day to multiple]
- Choose starting date from the calendar.
- Choose en date in the To date field.
- Select Copy to different dates
    - Choose starting date.
    - Choose end date date.
- Save.
Result: **The selected day details will be copied to all the dates in the range.**

[Copy dates, 1 week to 1 week]
- Choose starting date from the calendar.
- Choose en date in the To date field to make a whole week(7 days).
- Select Copy to different dates
    - Choose starting date.
    - Choose end date date to make a whole week(7 days).
- Save.
Result: **Every day of week from the first range will be applied to the second range. Independently of the order. Exemple : Sunday1 to Sunday2, Monday1 to Monday2, etc.**

[Copy dates, multiple dates to multiple dates]
- Choose start date from the calendar.
- Choose end date in the To date field.
- Select Copy to different dates
    - Choose start date.
    - Choose end date date to have the same number of days as the first range
- Save.
Result: **Each day from the first range will be applied the second range in the same order. Exemple : days1 to day1, day2 to day2, etc.**

[Copy to branch]
 - Select any date to make the side panel appear.
 - Select : Copy calendar to another branch.
 - Choose a calendar to copy to.
 - Save.
 - Choose copied to calendar in the dropdown : Define the holidays for.
Result: **All of the information in the currect calendar will be copied to the chosen calendar**
Comment 29 J Schmidt 2017-01-25 18:17:20 UTC
This is what I did on 1/25/17:
1. Used the ByWater sandbox.
2. Went to Tools > Discrete calendar.
3. Page opens /tools/discrete_calendar.pl but yellow error states "You have to run generate_discrete_calendar.pl in order to use Discrete Calendar.
4. I pushed on and went back a month and clicked on Dec. 1, 2016. Nothing. 
FYI. jenny~
Comment 30 Mehdi Hamidi 2017-01-25 18:40:44 UTC
Hello jenny, 
First, thank you for testing our patch!

Did you run the script generate_discrete_calendar.pl? If so, was there any errors?

Discrete Calendar needs to have valid dates in database in order to work. Without them, an error message is shown and the calendar doesn't work.

Mehdi.
Comment 31 Blou 2017-01-25 19:13:16 UTC
Mehdi,
I think that in order to be tested using those soap box, you'll need something in atomicupdates/ that does the equivalent of your init script, or just call it.  The box users have no control over running manual scripts, iiuc.

So maybe add a patch clearly identifying that it's just doing that (and being useless otherwise).

Otherwise, the patch is no "Patch doesn't apply".  Soap boxes limits is not a code error per se.
Comment 32 Mehdi Hamidi 2017-01-25 20:36:17 UTC Comment hidden (obsolete)
Comment 33 Mehdi Hamidi 2017-01-25 20:50:11 UTC Comment hidden (obsolete)
Comment 34 Mehdi Hamidi 2017-01-25 21:10:02 UTC
(In reply to Blou from comment #31)
> Mehdi,
> I think that in order to be tested using those soap box, you'll need
> something in atomicupdates/ that does the equivalent of your init script, or
> just call it.  The box users have no control over running manual scripts,
> iiuc.
> 
> So maybe add a patch clearly identifying that it's just doing that (and
> being useless otherwise).
> 
> Otherwise, the patch is no "Patch doesn't apply".  Soap boxes limits is not
> a code error per se.

Thank you for the insight Phillipe!

I Changed  the test plan so sandbox users can test the patch.
Test plan :
1- Apply patch
2- Run installer/data/mysql/updatedatabase.pl
3- Run the script add_days_discrete_calendar.pl to add days into the future
    - Check if the max date is now today + 1 day
4- Generate schemas by running misc/devel/update_dbix_class_files.pl
5- Run test t/db_dependent/DiscreteCalendar.t
6- Run all of Koha's tests in t/
Comment 35 J Schmidt 2017-01-25 21:39:47 UTC
Again on 1/25/17 3:32pm CST (US), I went back into the sandbox for ByWater (first one). The .pl script and patch seems to have been applied.
Steps:
1.) Tools > Discrete Calendar for Alphaville.
2.) Changed the calendar back to December 2016 and selected Dec 1st.
3.) Received the Add new holiday dialog box.
4.) Filled in "To date"; "title" Date type: "Weekly" Open hours;
5.) Filled in the hour/minutes (12:30) and (16:30).
6.) Left default Edit selected dates. then clicked Save.
7.) Received a software error, The 'month' parameter ("30") to DateTime::new did not pass the 'an integer between 1 and 12' callback at /usr/local/lib/perl/5.14.2/DateTime.pm line 197. 
DateTime::new(undef, "hour", 0, "minute", 0, "second", 0, "month", 30, ...) called at /var/lib/koha/sandbox1/kohaclone/Koha/DateUtils.pm line 172
	Koha::DateUtils::dt_from_string("04/30/2017") called at /var/lib/koha/sandbox1/kohaclone/tools/discrete_calendar.pl line 94
	main::edit_holiday() called at /var/lib/koha/sandbox1/kohaclone/tools/discrete_calendar.pl line 60
Comment 36 Mehdi Hamidi 2017-01-26 15:29:38 UTC
(In reply to J Schmidt from comment #35)
Hello Jenny,
I just went on By water solutions sandbox to test the bug, there seems to be a problem with the datepicker. The reason you'are getting this error :

>The 'month' parameter ("30") to DateTime::new did not pass the 'an integer >between 1 and 12' callback at /usr/local/lib/perl/5.14.2/DateTime.pm line 197. 
>DateTime::new(undef, "hour", 0, "minute", 0, "second", 0, "month", 30, ...) >called at /var/lib/koha/sandbox1/kohaclone/Koha/DateUtils.pm line 172
>	Koha::DateUtils::dt_from_string("04/30/2017") called at >/var/lib/koha/sandbox1/kohaclone/tools/discrete_calendar.pl line 94
>	main::edit_holiday() called at >/var/lib/koha/sandbox1/kohaclone/tools/discrete_calendar.pl line 60

It's because the date format is not correct, it is set to dd/mm/yyyy in the settings, so you get an error saying the month can't be 30 when the date entered is : 04/30/2017.

I'm working on to find why the datepickers are not showing on the sandbox, because they are working correctly on local installations. 

Also, for weekly holidays, they are only for single day, so the to date will be ignored.

Thank you again for testing!

Mehdi Hamidi
Comment 37 Mehdi Hamidi 2017-01-26 15:50:26 UTC Comment hidden (obsolete)
Comment 38 Mehdi Hamidi 2017-01-26 17:17:09 UTC Comment hidden (obsolete)
Comment 39 Mehdi Hamidi 2017-01-26 19:24:34 UTC
Hello Jenny,

I found the reason why you were getting the error. There seems to be a problem  with the sandbox that prevents the datepicker from showing the correct date
format which is  selected in the system preferences and this makes the calendar unusable. 
If you wish to proceed on testing, you will have to change the system preference dateformat to mm/dd/yyyy, it seems like it's the only option that works for now.

Mehdi Hamidi
Comment 40 J Schmidt 2017-01-26 20:48:46 UTC
Went into ByWater sandbox on 1/26/17. Steps taken:
1.) Went to Koha Admin > modified the dateFormat preference (I18N/L10N) from dd/mm/yyyy to mm/dd/yyyy. Saved.
2.) Went to Tools > Discrete Calendar
3.) Select January 6, 2017. Dialog box for "add new holiday" appears.
4.) Initial date is Friday 1/6/2017. Filled in To date to be 02/17/2017; Title was laptop loaning office hours. Date type: weekly; open hours stayed at 09:00:00; Close hours set to 19:30:00. Kept Edit selected dates and Save. 
5.) Weekly repeatable holiday is added to the Calendar page display as it normally would. From that line item on the side there is no hours/times listed (just an fyi).
6.) Did another entry into the future but that did not take. It did not, however, generate an error so that's good.
7.) I did another entry of a past date (Jan 16th, Monday as a weekly repeatable holiday). I entered in the times using minutes as well and then selected Copy calendar to another branch. Betatown. I saved. I did not receive an error. It doesn't appear under the Betatown calendar listings however. I did that again. It still doesn't appear under Betatown's calendar listings... but no error. 

The defining a calendar listing in the past, with hours seems to work.  Transferring to another branch does not... Not sure what to put as the status but it's partially working so kudos for that. jenny~
Comment 41 Mehdi Hamidi 2017-01-27 12:59:39 UTC
Hello Jenny,

Went into ByWater sandbox on 1/27/17.
- Adding an entry in the future worked for me, I added an exception and a fixed holiday and they were both added and their hours changed.

- Copy calendar to an other branch will take the current state of the calendar and copy it to another. Holidays that there were not saved will not be copied. So you will have to save your changes, and then, copy calendar.
I tested it and it copied the Alphaville to Betatown.

Mehdi Hamidi
Comment 42 J Schmidt 2017-01-31 16:25:05 UTC
Testing again. 1/31/2016.
Here are the steps taken:
1.) Went to Koha Admin > modified the dateFormat preference (I18N/L10N) from dd/mm/yyyy to mm/dd/yyyy. Saved.
2.) Went to Tools > Discrete Calendar
3.) Selected December 2nd, 2016. The dialog box to "add new holiday" appears.
4.) I left the "To date:" blank. Date type: fixed. Open hours: 08:00:00 to Close hours: 19:45:00. Kept the "Edit selected dates" and clicked. Save.
5.) The new calendar "holiday" is placed on the right as an entry. However, there are no times displaying. Only the date displays. Is the time desired here as well?
6.) Clicked on 1/1/2017 to insert another "holiday".
7.) Parameters filled in this time were: To date: 01/15/2017. Title: Sundays Limited Hours. Date type: Weekly; Open hours: 08:00:00 - 12:30:00.  I chose Copy calendar to another branch: Gammatron College. I clicked Save. That did not seem to work. No new holiday was added under the Alphaville branch calendar area OR under the Gammatron branch calendar page. 
8.) To confirm this I went to Set library > Choose library as Gammatron College. When I went to look at Gammatron College's calendar being a Gammatron College staffer, the weekly Sundays limited hours was not listed there.
9.) Changing to Alphaville. I create another calendar entry under Alphaville that is for "Sunday limited hours." It's a weekly event with set hours for 1/1/2017-1/15/2017. I leave the "Edit selected dates and click Save and it is there. appearing under this branch's calendar events. 
10.) I click on January 16, 2017 because it is earlier than today. I'm in the Alphaville branch's calendar area. The "Add new holiday" box comes up. I do not add any date/times into this box but simply click the "Copy calendar to another branch" option, choose Gammatron College, and click Save. Now I move to the Gammatron College branch but I do not see two entries from Alphaville's calendar under Gammatron's calendar. I am hoping to see the Sundays limited hours entry I created.. 
11.) It seems you have to first create an entry in one calendar and save it before you are able to copy it to another branch's calendar but it also seems you end up recreating the new holiday entry for that branch anyway. 
12.) I went into Alphaville and clicked on Jan 5, 2016. I created a new calendar event to test the "Normal" date types.  After entering in information in the dialog box and leaving Edit selected dates as is, then Save. the calendar would not save the new holiday entry to the default branch's calendar area. So Normal date types might not work. After clicking save when trying to fill in a "Normal date type" it moved me over to Gammatron College's calendar area.
13.) I used the drop down to return to Alphaville's calendar area. I clicked on 1/5/2016 and was prompted with the new holiday entry dialog box. This time I say from 1/5/2016 to 1/5/2016 (same day) and I made the Date type Exception. (I filled in the other boxes for title and hours....Upon clicking save, it jumped me over to Gammatron College's calendar (that's which library my log in is set to by the way)... The new calendar entry is not under Gammatron College's area OR under Alphaville.
14.) I change my log home branch now. I set the library BACK to be Alphaville. Submit.
15.) I look under Alphaville's calendar area but my previously attempted calendar entries are still not there. They are also not under Gammatron College's calendar area.  
12.) From my testing, it saves new holiday entries for any branch but not all date types work. I find that the Normal and weekly work but not the others. I am also not able to copy the calendar to another branch.  I was using Chrome.
Comment 43 Mehdi Hamidi 2017-02-03 15:27:55 UTC Comment hidden (obsolete)
Comment 44 Mehdi Hamidi 2017-02-03 15:28:19 UTC Comment hidden (obsolete)
Comment 45 Mehdi Hamidi 2017-02-03 16:30:23 UTC
Hello everyone, 
We updated the UI a little a bit and changed the behavior of the add_days script.
UI updates:
    - Moved copy to another branch to the top, just like in circulation rules, to follow the current Koha pattern.
    - Inputs for past days are now disabled for clarity since they can't be edited.
    - Renamed Normal day to Working day for clarity.
    - Renamed Fixed holiday to Repeatable holiday for clarity.
    - Renamed Exception holiday to Unique holiday for clarity.
    - Added coloring to all datepickers.
    
Script add_days_discrete_calendar.pl:
    - Unique holidays will no longer be "Need validation" when copied from a past date.
    - Added debug option to display the details about the added date.
    - Branchcodes will now be retrieved from table discrete_calendar instead of branches to prevent empty data and errors.

Patch test plan:

1- Apply patch
2- Run installer/data/mysql/updatedatabase.pl
3- Run the script add_days_discrete_calendar.pl with debug option
4- Generate schemas by running misc/devel/update_dbix_class_files.pl
5- Run test t/db_dependent/DiscreteCalendar.t
6- Run all of Koha's tests in t/

UI test plan :
- Go tools and choose Discrete Calendar
- The field title is optional.

[Change past]
- Choose any date from the calendar that is yesterday or older.
- Everything should be disabled except the To date. You can only use past dates to copy into different dates. 

Adding holidays :

[Unique holiday]
- Choose any date from the calendar that is today or in the future.
- Choose another date in the To date field that is in the future (optional, for range only).
- Change Date type to : Unique holiday.
- Save.
Result: **All dates in the range should be marked red for unique holiday.**

[Weekly]
- Choose a Sunday from the calendar that is editable (today or in the future).
- Change Date type to : Weekly.
- Save.
Result: **All upcoming (starting from today) Sundays will be holidays.**
PS: Title, open and close hour will be affected to **ALL SUNDAYS** unless the inputs are empty.

[Repeatable]
- Choose any date from the calendar that is today or in the future.
- Change Date type to : Repeatable holiday.
- Save
Result: **The selected date and the same day in the future (if available in Calendar) will be holiday.**

Delete holidays :

[Single date]
- Choose any date from the calendar that is today or in the future and is also a holiday.
- Change Date type to : Working day.
- Save.
Result: **The selected date should be a Working day**

[Range of dates]
- Choose any date from the calendar that is today or in the future.
- Choose another date in the To date field that is in the future.
- Change date type to : Normal day
- Save.
Result: **The selected range of dates should be Working days**

[Delete type]
- Choose any date from the calendar that is today or in the future and is a weekly or a repeatable holiday.
- Change date type to : Working day.
- Check : Delete this type CheckBox
- Save.
Result: -Weekly, all upcoming weekly holidays if that day will be normal days.
            Example : all upcoming Sundays will be working days
        -Fixed, all upcoming fixed holidays of that day will be normal days. 
            Example : all upcoming 25 December will be working days

[Copy dates, 1 day to multiple]
- Choose starting date from the calendar.
- Select Copy to different dates
    - Choose starting date.
    - Choose end date date.
- Save.
Result: **The selected day details will be copied to all the dates in the range.**

[Copy dates, 1 week to 1 week]
- Choose starting date from the calendar.
- Choose end date in the To date field to make a whole week(7 days).
- Select Copy to different dates
    - Choose starting date.
    - Choose end date date to make a whole week(7 days).
- Save.
Result: **Every day of week from the first range will be applied to the second range. Independently of the order. Exemple : Sunday1 to Sunday2, Monday1 to Monday2, etc.**

[Copy dates, multiple dates to multiple dates]
- Choose start date from the calendar.
- Choose end date in the To date field.
- Select Copy to different dates
    - Choose start date.
    - Choose end date date to have the same number of days as the first range
- Save.
Result: **Each day from the first range will be applied the second range in the same order. Exemple : day1 to day1, day2 to day2, etc.**

[Copy to branch]
 - The from branch is the current used calendar (Define the holidays for)
 - Choose a calendar to copy TO, from the second Dropdown.
 - Click on Clone.
 - Choose Copy calendar to in the Dropdown : Define the holidays for.
Result: **All of the information in the current calendar will be copied to the selected calendar**
Comment 46 Francois Charbonnier 2017-02-03 17:20:00 UTC
When I read again the description here, I felt that an overview of the work and its benefits will help you anyone who wants to get involved. 

Short version :

The communinity and some of our clients noticed two issues :

1. Hour loans doesn't know when a library is closed
2. We have performance issues when checking in overdue book or managing overdue fines.

We identified that both issues are related to the way Koha does date calculation. To improve and fix these issues, we developped a new calendar with a couple of new features.

The benefits of this new calendar are :

1. management of the libraries opening hours (new feature).
2. performance improvement when circulating items and managing fines. In particular, when checking in overdue book or managing overdue fines.
3. help to setup holidays/closed days from one year to another (new feature)

To acheive this, we redesigned the calendar database table and rewrote part of the code in charge of the date calculation. We used this work to move part of the code and separate what's in charge of the calendar and what's used to do the calculation.

This way, we solved two bugzilla issues opened a long time ago : 
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11211 (Move calculation code out of C4::Calendar) (opened by Jesse Weaver on the 2013-11-06)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8133 (hourly loans doesn't know when library closed) (opended by Nicole C. Engard on the 2012-05-21)

For now, BZ11211 blocked BZ8133.

All features from the old calendar is available in the new calendar.

The new calendar passed all the unit tests.

Long version :

The calendar's database table contains only rules to set whether the library is closed or not. If Koha needs a date or a range of date (was the library closed that day? how long is this book overdue? etc.), it has to be calculated using these rules. Depending on the type of transactions, how Koha does the calculation is not always efficient. For example, a return of overdue books can take as long as many seconds (for 2 months old overdues for example). We redesigned the calendar table to be able to give Koha the information it needs to do the calculation in a more efficient way.

In the calendar's new table, we don't store rules anymore. Each day from the calendar is now stored (an entry for each day). For each day, we can indicate if the library is open or closed. We took this redesign opportunity to add the library opening hours as well. That way, Koha gets straight the information it needs to determine if the library is open or not, it can take into consideration the opening hours (good for hour loans and the date range calculation is simplified by using the information stored in the database. No need to calculate it based on different rules anymore.

Among the patches we submitted, we have a script to benchmark checkin and checkout transactions. The benchmark of checkin is more indicative of the performance improvement. Here is an example of collected statistics. When an item is returned the day the checkout expires, the calculation time decreased of 22% (for 100 checkins, the calculation time drops from 52.2s to 40.7s). When an item is 2 months overdue and returned, the calculation time decreased of 70% (for 100 checkins, the calculation time drops from 154s to 46.6s).

How does the new calendar work ?

Before the first use, a script will populate the new calendar table with on entry for each day of the year. This script can retreive your actual setting to keep track of your old set up. It also creates all the entries for future years. The basic set up would be : the library wants to have a 1 year overview of the calendar so they can set it up in advance. The script creates every entry in the table to be able to set up the calendar 1 year in advance.

Then, another script will be scheduled to run every day. This script will add a new entry for every day spent. In this manner, Koha keeps a 1 year advance overview at all time. 

The advantage of this last script is to help the koha admin to set up the calendar in advance. Each time the script will add a new entry (a new day), it will check one year in the past how the same day was set up to try to determine if the library should be closed or not :

- If the date is a unique holiday : the script will not keep it for the new entry.
- If the date is a repeatable holiday : the script will keep it. (For example, a library closed every saturday)
- If the day type was "floatting" (it's a new type of holiday) : the new entry status is "needs confirmation". Thus, when setting up the calendar, I have two options : 
1. I know it's a repeatable holiday every year or very week, I choose the date type 'repeatable'. When the script add a new entry, it keeps the holiday.
2. I want to check the holiday every year to adjust the settings (for example, Easter stat holiday), I choose the new date type "floatting". Then, when the script add a new entry, it doesn't assume the library is closed but change the date status to "needs confirmation". Then I will be able to overview these "need confirmation" holidays and set them up when I get the information.

The script manage the opening hours as well. It checks the previous week settings instead of the previous year.

The other new feature to help setting up the calendar is in the way Koha can create holidays between two dates. When creating this range of dates, you can :

- either use a specific day and copy its settings to all the dates in the range.
- or use a range of dates and copy the settings of each day to each day of the new range.

Besides that, the calendar keeps all its actual features.
Comment 47 Olli-Antti Kivilahti 2017-02-23 11:38:38 UTC
Thanks Francois Charbonnier for an excellent summary! It was much needed.

Would it be possible to make a small screencast about this feature?
I have used Istanbul in linux to do screencasts. It is super easy to use.

Screencasts can easily communicate the feature, and the details can be much willingly disseminated after interest has been garnered.


I need to be able to set the opening hours for libraries.
Mainly self-service opening hours to allow/disallow self-service access by access-control systems.
There actually are different "opening hours" for different levels of services in libraries.

-We have normal opening hours when staff is present.
-We have self-service period when staff is not present.
-We might have a period during which checkouts must be returned, or fines start cumulating. This can be bigger or smaller than the real library opening hours, if using for ex. outdoor checkin-automatons.

And those periods can interact in interesting ways.

This would require an extra layer on top of the calendar rewrite where you could set different types of opening hours.


Having something more visual which shows the feature in action, would help us decide if our interests align on this feature.
Comment 48 Christopher Brannon 2017-02-23 15:17:44 UTC
(In reply to Olli-Antti Kivilahti from comment #47)
> I need to be able to set the opening hours for libraries.
> Mainly self-service opening hours to allow/disallow self-service access by
> access-control systems.
> There actually are different "opening hours" for different levels of
> services in libraries.
> 
> -We have normal opening hours when staff is present.
> -We have self-service period when staff is not present.
> -We might have a period during which checkouts must be returned, or fines
> start cumulating. This can be bigger or smaller than the real library
> opening hours, if using for ex. outdoor checkin-automatons.
> 
> And those periods can interact in interesting ways.
> 
> This would require an extra layer on top of the calendar rewrite where you
> could set different types of opening hours.

I know the idea here is to keep "branch" schedules together, but would it work to have your different schedules as different branches for the time being?
Comment 49 Blou 2017-02-24 21:43:51 UTC
Hi Olli-Antti,
I've opened a task here, so we'll be working on that visual cue very soon.  I understand the benefits, for you and every other potential users.

I admit not understanding exactly your need, but I liked the use of "interact in interesting ways".  Always a good clue things can be complicated.  One thing I'd bet on is that this new calendar would make it much easier for you to stack on extra constraints.  Christopher's suggestion would be my starting point, but adding an extra column to the "discrete_calendar" table to represent a combo (branch, checkouttype) would be easy.  Not in this patch, though :)

We'll get back with the visual stuff....
Comment 50 Olli-Antti Kivilahti 2017-03-02 09:08:33 UTC
(In reply to Blou from comment #49)

> I admit not understanding exactly your need, but I liked the use of
> "interact in interesting ways".
I mean there are different types of opening hours in a library.
Different services are available on different hours.

> One thing I'd bet on is that this new calendar would make it
> much easier for you to stack on extra constraints.
What I am concerned is the choice of GUI technologies, if they allow easily to set different types of opening hours in the calendar and can visualize those to the user.
I was dabbling with https://fullcalendar.io/ on our overduenotices rewrite.
It is pretty cool.

> But adding an extra column to the
> "discrete_calendar" table to represent a combo (branch, checkouttype) would
> be easy.  Not in this patch, though :)
Adding a new column was what I was thinking of. Should be rather simple.
We can provide a followup when we start working on that.


Also popped into my head. You should add a REST API endpoint for CRUD:ing the calendar entries.
Then write the GUI on top of the REST API.
This way you have architecturally solid separation of backend-logic from front-end logic. And can make some super snappy UIs using AJAX.
Also we can write our own GUI to interface with the discrete_calendar if we want.
Our pretty much anybody can do it on top a of a well defined API.

...but maybe not in this rewrite. Up to you how you want to upgrade your workflow.



Atomicupdate:
installer/data/mysql/atomicupdate/generate_discrete_calendar.perl

Not sure what is the community recommendation for naming conventions here.
I find it polite to include the ticket number there, eg:

installer/data/mysql/atomicupdate/Bug-17015-generate_discrete_calendar.perl



tests++
Comment 51 Mehdi Hamidi 2017-03-07 20:10:49 UTC Comment hidden (obsolete)
Comment 52 Mehdi Hamidi 2017-03-07 20:10:57 UTC Comment hidden (obsolete)
Comment 53 Mehdi Hamidi 2017-03-07 20:13:10 UTC Comment hidden (obsolete)
Comment 54 Mehdi Hamidi 2017-03-07 20:13:16 UTC Comment hidden (obsolete)
Comment 55 Mehdi Hamidi 2017-03-07 20:13:23 UTC Comment hidden (obsolete)
Comment 56 Mehdi Hamidi 2017-03-07 20:25:48 UTC Comment hidden (obsolete)
Comment 57 Mehdi Hamidi 2017-03-10 12:21:38 UTC Comment hidden (obsolete)
Comment 58 Mehdi Hamidi 2017-03-10 20:46:33 UTC
Hello everyone,

here's a link to our screencast : 

https://www.youtube.com/watch?v=QLxE9ToRJgE 

Feel free to leave us feedback!
Comment 59 Andreas Hedström Mace 2017-03-29 21:17:20 UTC
The patch applies, but I get an error running updatedatabase.pl, with lots and lots of insert into fails.  Is this just a problem that happens for me, or are others experiencing the same?
Comment 60 Blou 2017-03-30 13:07:36 UTC Comment hidden (obsolete)
Comment 61 Blou 2017-03-30 13:31:04 UTC
Thanks Andreas.  

I reproduced your problem, caused because we got two files in atomtic update, and due to alphabets(duh!) they were called in wrong order.  Basically, that hadn't been tested with latest patch, it seems.

I fixed the problem, you should be able to run it without glitch.  Please note I had one error appear during update (dublicate bla) and took a note for the dev to fix it later.  It should have no impact on your testing.
Comment 62 Andreas Hedström Mace 2017-03-30 15:27:31 UTC
Sorry, I tried today with a fresh new branch, but the same problem still persists. The first part of the error message reads:

C4::Installer::load_sql returned the following errors while attempting to load /home/vagrant/kohaclone/installer/data/mysql/atomicupdate/bug_17015_part1_create_discrete_calendar.sql:
DEV atomic update: bug_17015_part2_fill_discrete_calendar.perl
DBD::mysql::st execute failed: Duplicate entry 'DFLT' for key 'PRIMARY' [for Statement "INSERT INTO branches (branchname, branchcode) VALUES(?,?)" with ParamValues: 0='Default', 1='DFLT'] at (eval 1087) line 58.
DBD::mysql::st execute failed: Duplicate entry 'DFLT-2015-03-26 00:00:00' for key 'PRIMARY' [for Statement "INSERT INTO discrete_calendar (date,branchcode,isopened,holidaytype,note,openhour,closehour) VALUES (?,?,?,?,?,?,?)" with ParamValues: 0=DateTime=HASH(0x57ab3f0), 1='DFLT', 2=1, 3='', 4='', 5='09:00:00', 6='17:00:00'] at (eval 1087) line 163.

And then it goes on for quite a while, with failed insert into lines.
Comment 63 Blou 2017-03-30 15:52:59 UTC
I also have the  "Duplicate entry 'DFLT' " error.  But as I said, it had no impact.  

Then it takes a loooong time (you can see the progress through a "select count(*) from discrete_calendar".  This part will be improved, I have a note.

But then "with failed insert into lines.".  Could you elaborate ?
Comment 64 Andreas Hedström Mace 2017-03-30 20:04:52 UTC
It lines and lines of this type:

DBD::mysql::st execute failed: Duplicate entry 'FRESC-2017-03-24 00:00:00' for key 'PRIMARY' [for Statement "INSERT INTO discrete_calendar (date,branchcode,isopened,holidaytype,note,openhour,closehour) VALUES (?,?,?,?,?,?,?)" with ParamValues: 0=DateTime=HASH(0x57ab3f0), 1='FRESC', 2=1, 3='', 4='', 5='09:00:00', 6='17:00:00'] at (eval 1087) line 163.
DBD::mysql::st execute failed: Duplicate entry 'GEO-2017-03-24 00:00:00' for key 'PRIMARY' [for Statement "INSERT INTO discrete_calendar (date,branchcode,isopened,holidaytype,note,openhour,closehour) VALUES (?,?,?,?,?,?,?)" with ParamValues: 0=DateTime=HASH(0x57ab3f0), 1='GEO', 2=1, 3='', 4='', 5='09:00:00', 6='17:00:00'] at (eval 1087) line 163.
DBD::mysql::st execute failed: Duplicate entry 'DFLT-2017-03-25 00:00:00' for key 'PRIMARY' [for Statement "INSERT INTO discrete_calendar (date,branchcode,isopened,holidaytype,note,openhour,closehour) VALUES (?,?,?,?,?,?,?)" with ParamValues: 0=DateTime=HASH(0x57ab3f0), 1='DFLT', 2=1, 3='', 4='', 5='09:00:00', 6='17:00:00'] at (eval 1087) line 163.
DBD::mysql::st execute failed: Duplicate entry 'FRESC-2017-03-25 00:00:00' for key 'PRIMARY' [for Statement "INSERT INTO discrete_calendar (date,branchcode,isopened,holidaytype,note,openhour,closehour) VALUES (?,?,?,?,?,?,?)" with ParamValues: 0=DateTime=HASH(0x57ab3f0), 1='FRESC', 2=1, 3='', 4='', 5='09:00:00', 6='17:00:00'] at (eval 1087) line 163.
DBD::mysql::st execute failed: Duplicate entry 'GEO-2017-03-25 00:00:00' for key 'PRIMARY' [for Statement "INSERT INTO discrete_calendar (date,branchcode,isopened,holidaytype,note,openhour,closehour) VALUES (?,?,?,?,?,?,?)" with ParamValues: 0=DateTime=HASH(0x57ab3f0), 1='GEO', 2=1, 3='', 4='', 5='09:00:00', 6='17:00:00'] at (eval 1087) line 163.

FRESC, GEO and DFLT are of course the branchcodes. It can be as you say that this is not important for testing.

But when I try to go to Tools > Discrete calendar in the staff interface I encounter a "not found" page.
Comment 65 Blou 2017-04-03 17:33:07 UTC
I don't reproduce it, but I'll try different scenarios.  Expect some new patches coming from me or Mehdi in the comming week.
Comment 66 Mehdi Hamidi 2017-04-19 14:29:38 UTC Comment hidden (obsolete)
Comment 67 Mehdi Hamidi 2017-04-19 14:32:47 UTC Comment hidden (obsolete)
Comment 68 Mehdi Hamidi 2017-04-19 14:40:56 UTC Comment hidden (obsolete)
Comment 69 Mehdi Hamidi 2017-04-19 15:00:24 UTC
Hello Andreas,

Here's what's been done : 

- Fixed patch, not it applies properly on the current master.

- Update the insert script so it makes one commit to the database and it's now way faster than the previous script.

- We added Koha/Schema/Result/DiscreteCalendar.pm to the "DiscreteCalendar UI, Back-End and necessary scripts" patch, so now need to run the script misc/devel/update_dbix_class_files.pl to generate the schemas.

However I'm not able to to reproduce your  problem : "not found" page. I also tested the patch on a sandbox and the bug doesn't happen.

If you wish to test on a sandbox, make sur to read my comment #39

Thank you for testing!

Mehdi Hamidi
Comment 70 Mehdi Hamidi 2017-05-08 19:45:59 UTC Comment hidden (obsolete)
Comment 71 Mehdi Hamidi 2017-05-09 12:14:13 UTC Comment hidden (obsolete)
Comment 72 Mehdi Hamidi 2017-05-09 13:07:17 UTC
Rebased the patch so it applies properly on master. 

We tweaked the code so it works on kohadevbox as well.

Mehdi Hamidi
Comment 73 Mehdi Hamidi 2017-05-10 17:33:40 UTC Comment hidden (obsolete)
Comment 74 Mehdi Hamidi 2017-05-10 17:34:33 UTC Comment hidden (obsolete)
Comment 75 Mehdi Hamidi 2017-05-10 17:34:47 UTC Comment hidden (obsolete)
Comment 76 Mehdi Hamidi 2017-05-10 17:35:00 UTC Comment hidden (obsolete)
Comment 77 Mehdi Hamidi 2017-05-10 17:35:09 UTC Comment hidden (obsolete)
Comment 78 Mehdi Hamidi 2017-05-10 17:35:18 UTC Comment hidden (obsolete)
Comment 79 Nick Clemens 2017-06-09 16:11:09 UTC
I just took a quick peek here today, two things:
1 - Can we get a fleshed out test plan? Steps we should ensure work (checkin/out, testing hourly loans, fines, etc)
2 - I think it would be necessary to have a way to set 'Working day' hours for each library (possibly on the library admin page) rather than having to define a repeated holiday and then remove the holiday
Comment 80 Mehdi Hamidi 2017-06-09 18:33:17 UTC
Hello Nick,

(In reply to Nick Clemens from comment #79)
> I just took a quick peek here today, two things:
> 1 - Can we get a fleshed out test plan? Steps we should ensure work
> (checkin/out, testing hourly loans, fines, etc)
We update koha tests to use the new calendar and they all pass. Or are you talking about something else?

> 2 - I think it would be necessary to have a way to set 'Working day' hours
> for each library (possibly on the library admin page) rather than having to
> define a repeated holiday and then remove the holiday

We will be adding an option to update a specific week day without having to define a repeated holiday and then remove it. But we won't be changing anything on  library admin page, at least not in this bug.

Thank you!

Mehdi Hamidi
Comment 81 Blou 2017-06-09 19:09:04 UTC
I think the link with the Library management page is a good idea.  But I see this more as a great feature to add after (it would be valid with the current code as well!).  I'd rather not add code that will make rebasing the patch more complex until signed off.
Comment 82 Nick Clemens 2017-06-09 19:45:47 UTC
(In reply to Mehdi Hamidi from comment #80)

> We update koha tests to use the new calendar and they all pass. Or are you
> talking about something else?

I missed this in comment 45, a list of steps to test things from UI

> We will be adding an option to update a specific week day without having to
> define a repeated holiday and then remove it. But we won't be changing
> anything on  library admin page, at least not in this bug.

That makes sense, admin page is not necessary, the current workflow is just a little awkward
Comment 83 Mehdi Hamidi 2017-06-12 20:05:53 UTC Comment hidden (obsolete)
Comment 84 Mehdi Hamidi 2017-06-12 20:06:40 UTC
Hello everyone, 
We updated the UI a little a bit.
UI updates:
    - You can now change informations of a specific day of the week in a given range.
    
Perforamce enhancement :
    DiscreteCalendar module and scripts use sql transactions to make less calls to the database and reduce time usage.
    

Patch test plan:

1- Apply patch
2- Run installer/data/mysql/updatedatabase.pl
3- Run the script add_days_discrete_calendar.pl to add couple days in the future with debug option
4- Run test t/db_dependent/DiscreteCalendar.t
5- Run all of Koha's tests in t/

UI test plan :
- Go tools and choose Discrete Calendar
- The field title is optional.

[Change past]
- Choose any date from the calendar that is yesterday or older.
- Everything should be disabled except the To date. You can only use past dates to copy into different dates. 

Adding holidays :

[Unique holiday]
- Choose any date from the calendar that is today or in the future.
- Change Date type to : Unique holiday.
- Save.
Result: **All dates in the range should be marked red for unique holiday.**

[Weekly]
- Choose a Sunday from the calendar that is editable (today or in the future).
- Change Date type to : Weekly.
- Save.
Result: **All upcoming (starting from today) Sundays will be holidays.**
PS:
- Title, open and close hour will be affected to **ALL UPCOMING SUNDAYS** unless the inputs are empty.

[Repeatable]
- Choose any date from the calendar that is today or in the future.
- Change Date type to : Repeatable holiday.
- Save
Result: **The selected date and the same day in the future (if available in Calendar) will be holiday.**

Delete holidays :

[Single date]
- Choose any date from the calendar that is today or in the future and is also a holiday.
- Change Date type to : Working day.
- Save.
Result: **The selected date should now be a Working day**

[Range of dates]
    [Everyday]
    - Choose any date from the calendar that is today or in the future.
    - Choose another date in the To date field that is in the future.
    - Change date type to anything except repeatable.
    - Choose week day : Everyday
    - Save.
    Result: **The selected range of dates should be changed to the selected date type**
    
    [Specific day]
    - Choose any date from the calendar that is today or in the future.
    - Choose another date in the To date field that is in the future.
    - Change date type to anything except repeatable.
    - Choose week day : Fridays
    - Save.
    Result: **All the fridays in the selected range of dates should be changed to the selected date type**
PS: 
    - Everyday will be applied when changing to repeatable holiday of a range.
    - Title, open and close hours will be affected to **ALL OF THE MATCHED RESULTS** unless the inputs are empty.

[Change Open/Close hours specific day of week]
    - Choose any date from the calendar that is today or in the future.
    - Choose another date in the To date field that is in the future.
    - Change date type to empty, so the type won't updated.
    - Choose week day : Sundays.
    - Choose Open hours : 10:00:00
    - Choose Close hours : 17:00:00
    - Save.
    Result : **All the close and open hours of Sundays in the given range should be updated to 10:00:00 and 17:00:00**
    
[Delete type]
- Choose any date from the calendar that is today or in the future and is a weekly or a repeatable holiday.
- Change date type to : Working day.
- Check : Delete this type CheckBox (CHecking this option will change the type to working days automatically)
- Save.
Result: -Weekly, all upcoming weekly holidays of that day will be normal days.
            Example : all upcoming Sundays will be working days
        -Repeatable, all upcoming repeatable holidays of that day will be normal days. 
            Example : all upcoming 25 December will be working days

[Copy dates, 1 day to multiple]
- Choose starting date from the calendar.
- Select Copy to different dates
    - Choose starting date.
    - Choose end date date.
- Save.
Result: **The selected day details will be copied to all the dates in the range.**

[Copy dates, 1 week to 1 week]
- Choose starting date from the calendar.
- Choose end date in the To date field to make a whole week(7 days).
- Select Copy to different dates
    - Choose starting date.
    - Choose end date date to make a whole week(7 days).
- Save.
Result: **Every day of week from the first range will be applied to the second range. Independently of the order. Exemple : Sunday1 to Sunday2, Monday1 to Monday2, etc.**

[Copy dates, multiple dates to multiple dates]
- Choose start date from the calendar.
- Choose end date in the To date field.
- Select Copy to different dates
    - Choose start date.
    - Choose end date date to have the same number of days as the first range
- Save.
Result: **Each day from the first range will be applied the second range in the same order. Exemple : day1 to day1, day2 to day2, etc.**

[Copy to branch]
 - The from branch is the current used calendar (Define the holidays for)
 - Choose a calendar to copy TO, from the second Dropdown.
 - Click on Clone.
 - Choose Copy calendar to in the Dropdown : Define the holidays for.
Result: **All of the information in the current calendar will be copied to the selected calendar**

Thank you!

Mehdi Hamidi
Comment 85 Mehdi Hamidi 2017-07-17 14:56:52 UTC Comment hidden (obsolete)
Comment 86 Mehdi Hamidi 2017-07-17 14:57:00 UTC Comment hidden (obsolete)
Comment 87 Mehdi Hamidi 2017-07-17 14:57:07 UTC Comment hidden (obsolete)
Comment 88 Mehdi Hamidi 2017-07-17 14:57:12 UTC Comment hidden (obsolete)
Comment 89 Mehdi Hamidi 2017-07-17 14:57:19 UTC Comment hidden (obsolete)
Comment 90 Alex Buckley 2017-10-05 01:01:36 UTC
Hi Mehdi

Whilst applying this patch on a clean, up to date branch I got the following merge error (below) the issue is C4/Reserves.pm can you please rebase against master and I will be happy to try testing it again:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts
Applying: Bug 17015 - Tests for DiscreteCalendar
Applying: Bug 17015 - Koha now uses the new DiscreteCalendar module
Using index info to reconstruct a base tree...
M	C4/Circulation.pm
M	C4/HoldsQueue.pm
M	C4/Overdues.pm
M	C4/Reserves.pm
M	Koha/Hold.pm
M	circ/returns.pl
M	misc/cronjobs/overdue_notices.pl
Falling back to patching base and 3-way merge...
Auto-merging misc/cronjobs/overdue_notices.pl
Auto-merging circ/returns.pl
Auto-merging Koha/Hold.pm
Auto-merging C4/Reserves.pm
CONFLICT (content): Merge conflict in C4/Reserves.pm
Auto-merging C4/Overdues.pm
Auto-merging C4/HoldsQueue.pm
Auto-merging C4/Circulation.pm
Failed to merge in the changes.
Patch failed at 0001 Bug 17015 - Koha now uses the new DiscreteCalendar module
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-17015---Koha-now-uses-the-new-DiscreteCalendar-G564_m.patch
You have new mail in /var/mail/vagrant
Comment 91 David Bourgault 2017-10-06 15:26:18 UTC Comment hidden (obsolete)
Comment 92 David Bourgault 2017-10-06 15:26:27 UTC Comment hidden (obsolete)
Comment 93 David Bourgault 2017-10-06 15:26:38 UTC Comment hidden (obsolete)
Comment 94 David Bourgault 2017-10-06 15:26:45 UTC Comment hidden (obsolete)
Comment 95 David Bourgault 2017-10-06 15:26:54 UTC Comment hidden (obsolete)
Comment 96 David Bourgault 2017-10-06 15:27:46 UTC
Rebased patch on master.
All tests passed.
Comment 97 David Bourgault 2017-10-10 15:09:43 UTC Comment hidden (obsolete)
Comment 98 Alex Buckley 2017-10-11 09:24:21 UTC Comment hidden (obsolete)
Comment 99 Alex Buckley 2017-10-11 09:26:28 UTC Comment hidden (obsolete)
Comment 100 Alex Buckley 2017-10-11 09:35:12 UTC Comment hidden (obsolete)
Comment 101 Alex Buckley 2017-10-11 09:38:10 UTC Comment hidden (obsolete)
Comment 102 Alex Buckley 2017-10-11 09:44:16 UTC Comment hidden (obsolete)
Comment 103 Alex Buckley 2017-10-11 09:53:48 UTC Comment hidden (obsolete)
Comment 104 Alex Buckley 2017-10-11 10:05:27 UTC
Excellent new feature for Koha!

One point to keep in mind for the test plan, I may be getting a bit pedantic here but in the future I would change the wording slightly in the following steps:

Copy dates 1 day to multiple,
Copy dates 1 week to 1 week
Copy dates multiple dates to multiple dates

In these sections it is not particularly clear that you have to create the holiday period before you copy the dates it looked to me at first glance that you could copy a holiday that was set to 1 day/1 week/multiple dates in the to/from inputs to another set of dates at the same time.

So I would emphasise that the holiday period must be created first then you can copy the dates.

Another potential UX enhancement for the future (in a separate bug report but don't worry at the moment) to build on your excellent work would be to have the calendar in the 'from' input show the same month/year by default as the 'to' date input. e.g. if I set the 'to' date to 1/10/2018 it would be useful to have the 'from' calendar show January 2018 by default when clicked on rather than the current month October 2017.
Comment 105 Alex Buckley 2017-10-21 01:25:22 UTC
Hi David and Medhi

While testing patch 17983 I applied this dependency patch and found that it no longer applies successfully on a master branch any more, the conflict is in C4/Overdues.pm. Can you please rebase, then after checking everything is still working as it was when I last tested and I will sign off again
Comment 106 David Bourgault 2017-10-23 13:44:33 UTC Comment hidden (obsolete)
Comment 107 David Bourgault 2017-10-23 13:44:43 UTC Comment hidden (obsolete)
Comment 108 David Bourgault 2017-10-23 13:44:51 UTC Comment hidden (obsolete)
Comment 109 David Bourgault 2017-10-23 13:45:00 UTC Comment hidden (obsolete)
Comment 110 David Bourgault 2017-10-23 13:45:11 UTC Comment hidden (obsolete)
Comment 111 David Bourgault 2017-10-23 13:45:21 UTC Comment hidden (obsolete)
Comment 112 David Bourgault 2017-10-23 13:50:21 UTC
Rebased on master.
Comment 113 Andreas Hedström Mace 2017-11-02 11:48:34 UTC
Shouldn't this bug have status Signed off?
Comment 114 David Bourgault 2017-11-02 12:36:58 UTC
The signoff was done before a rebase, so I figured I'd ask for another signoff in case I screwed up my rebase.
Comment 115 David Bourgault 2017-11-02 13:26:52 UTC
I've been told that isn't necessary.
Comment 116 Jonathan Druart 2017-12-12 17:37:51 UTC
(In reply to David Bourgault from comment #115)
> I've been told that isn't necessary.

It depends.
If you are not confident with your rebase it may be helpful to ask for another signoff.
Comment 117 David Bourgault 2017-12-13 14:56:31 UTC
I'm very confident in my rebase, considering the corrections were pretty minor.

Patch still applies on master, QA tool still green.
Comment 118 Jonathan Druart 2018-02-07 15:42:25 UTC
Would be good to have it for 18.05
Comment 119 Jonathan Druart 2018-02-07 16:14:35 UTC
Very quick QA feedbacks:

* +use Koha::Account::Line;
added in C4/Overdues.pm

* in getDatesInfo
+            select  => [ 'date', { DATE => 'date' } ],
+            as      => [qw/ date date /],
What does this?

* Mix of snake_case and camelCase in method names (stick to snake_case)

* Same for columns in DB (isopened => is_opened)

* "DFLT" for the default is confusing, use empty string instead like in other areas of Koha

* Why did not you use Koha::Object[s]?

* +    my ($self, $title, $weekday, $holidaytype, $openHour, $closeHour, $startDate, $endDate, $deleteType, $today) = @_;

You should use a hashref instead

* Types of holidays (E, F, N, R, W): please use global variables with readable value ($holidays->{Exception} = "E"; for instance, or $EXCEPTION_HOLIDAY = 'E')

It will make this kind of lines easier to read:
+    }elsif ($holidaytype eq 'E' || $holidaytype eq 'F' || $holidaytype eq 'N') {

* When you use DBIx::Class you should avoid to write SQL, for instance:
  date BETWEEN DATE(?) AND DATE(?)
should be:
  date => { '>=' => $from, '<=' => $to }
or:
  date => { -between => [$from, $to] }

* Isn't DAYOFWEEK a MySQLism? We think we should find another way to do that.

* At first glance DateTime::Format::Strptime->new should be replaced with my $dtf = $schema->storage->datetime_parser

* misc/cronjobs/add_days_discrete_calendar.pl is using SQL statements, use your new module instead

* This same cronjob must use $schema->storage->txn_begin or ->txn_do instead of turning off the AutoCommit flag of DBI

* t/db_dependent/Fines.t: Do not use DateTime directly, dt_from_string instead: dt_from_string->add( day => 42 )

* tools/discrete_calendar.pl
Use C4::Context::only_my_library for $onlymine
Comment 120 Jonathan Druart 2018-02-07 16:29:38 UTC
* There no link to the new page from the navbar

* What about the old pages?

* What about the old modules? I would expect them to be removed

* cronjob must be added to the debian cronjob file (debian/koha-common.cron.*)
Comment 121 Jonathan Druart 2018-02-07 17:08:40 UTC
I am not familiar enough with the current UI or how the calendar is used by libraries. So it is hard to give my point of view on the UI changes.

About the performances:
* did you take a look at Date::Holidays?
* What about loading the calendar in memory (with the existing data) with a home-made structure and improve is_holiday
* What were the bottlenecks? Do you still have the nytprof raw files, or the html files? You only provided the global result so we cannot identify what are the slow areas.

Switched to in discussion for this comment.
Comment 122 Blou 2018-02-07 18:08:29 UTC
I think the old page was not removed because the developper didn't want to "shock" the testers, and allow for easy comparison.  I think David will create a separate patch for it.

Regarding the other points:
- This was coded 18 months ago.  The nytprof has been written over.  But I remember the main culprit was addDay, which created tons of object for simple calculation.
- Fundamentaly, the old(and current) system is flawed, so although we did use in-memory hash for storing the holidays (along with our custom open/close hours) for years on our clients, that was not a good system.  Calculation on past dates was always flawed because any change to an holiday rule would skewed calculation.  This method stores for good the opening/close dates and hours, so any future change through the interface won't affect past dates.

The new UI offers more flexibility.  The calculations are always good.  And extremely fast.  It adds opening and close time.  And it's clean and easy to add on.
Comment 123 David Bourgault 2018-02-14 19:17:16 UTC Comment hidden (obsolete)
Comment 124 David Bourgault 2018-02-14 19:17:28 UTC Comment hidden (obsolete)
Comment 125 David Bourgault 2018-02-14 19:17:54 UTC Comment hidden (obsolete)
Comment 126 David Bourgault 2018-02-14 19:18:03 UTC Comment hidden (obsolete)
Comment 127 David Bourgault 2018-02-14 19:18:48 UTC Comment hidden (obsolete)
Comment 128 David Bourgault 2018-02-14 19:27:09 UTC
I recreated and rebased the patch to keep the git history clean. I 
also added a patch that removes files used by the old calendar code.

Because this new patch changes database column names (as requested by
Jonathan in his quick feedback), if you've tested this patch before
make sure to drop the table and recreate it.

Test plan and functionality is same as before. This change is only a
code cleanup to match QA standards.


== Changes from last patch ==

* Fixed many SQL queries, removing useless code and changing much of
the "where" clauses to a DBIx:: syntax. Not all queries can be written
using DBIx syntax, so some SQL is still present.

* Change most functions to snake_case (from camelCase). Some functions
have the same name as in Koha::Calendar to minify changes other Koha
files; they were left unchanged.

* Added underscores to database columns, and their matching variables.
THIS WILL CAUSE ERRORS if you still have the table in your database. If
that is the case, drop and recreate the discrete_calendar table before
testing.

* edit_holidays now uses a hashref for parameters. Default parameters
have been made explicit. Calls to the function have been simplified
taking default parameters into account.

* Holiday types ({E, F, N, R, W}) are now called through a global hash
in perl files. (e.g.: $HOLIDAYS->{EXCEPTION})

* add_days_discrete_calendar.pl use txn_begin txn_do for transactions.

* discrete_calendar.pl uses C4::Context::only_my_library


== Unresolved/Unchanged ==

> Isn't DAYOFWEEK an MySQLism?
It is. I was however unable to find a cross-implementation way of
obtaining that information. If anyone has an idea I'm open to
suggestions

> add_days_discrete_calendar.pl is using SQL statements, use your new
> module instead.
Koha::DiscreteCalendar does not have any functions to insert new days
in the database, as this is only done by the cronjob.

>* At first glance DateTime::Format::Strptime->new should be replaced
with my $dtf = $schema->storage->datetime_parser
At first try it cannot.
Comment 129 David Bourgault 2018-02-14 19:29:51 UTC
Test plan in comment 84 is still valid, but if you apply last patch "Discrete Calendar" tool is now simply "Calendar".
Comment 130 Josef Moravec 2018-02-15 08:22:57 UTC
(In reply to Mehdi Hamidi from comment #84)
> Patch test plan:
> 
> 1- Apply patch
> 2- Run installer/data/mysql/updatedatabase.pl
> 3- Run the script add_days_discrete_calendar.pl to add couple days in the
> future with debug option

Will this step be needed on production system after upgrade to 18.05 ?

> 4- Run test t/db_dependent/DiscreteCalendar.t
> 5- Run all of Koha's tests in t/
>
Comment 131 Josef Moravec 2018-02-15 08:25:10 UTC
Just quick test but I always get "No library set! You are using the default calendar." in the UI. I'm not able to copy anything from default to specific library.
Comment 132 Josef Moravec 2018-02-15 09:56:03 UTC
Comment on attachment 71634 [details] [review]
Bug 17015 - DiscreteCalendar UI, Back-End and necessary scripts

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

Just quickly read the code and found few issues which should be addressed I think

::: Koha/DiscreteCalendar.pm
@@ +16,5 @@
> +# along with Koha; if not, see <http://www.gnu.org/licenses>.
> +
> +#####Sets holiday periods for each branch. Datedues will be extended if branch is closed -TG
> +use strict;
> +use warnings;

Should be:
use Modern::Perl;

@@ +72,5 @@
> +
> +=cut
> +
> +sub new {
> +    my ( $classname, %options ) = @_;

You are using hash for options, but should be hashref - we use hashrefs in other packages of Koha namespace at least, see https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL16:_Hashrefs_should_be_used_as_arguments

@@ +156,5 @@
> +=head2 add_new_branch
> +
> +    Koha::DiscreteCalendar->add_new_branch($copyBranch, $newBranch)
> +
> +This methode will copy everything from a given branch to a new branch

Typo: "methode" -> "method"

::: installer/data/mysql/atomicupdate/bug_17015_part1_create_discrete_calendar.sql
@@ +9,5 @@
> +    `holiday_type` varchar(1) DEFAULT '',
> +    `note` varchar(30) DEFAULT '',
> +    `open_hour` time NOT NULL,
> +    `close_hour` time NOT NULL,
> +    PRIMARY KEY (`branchcode`,`date`)

Please specify the encoding of the table.

The table is also missing in kohastructure.sql

::: installer/data/mysql/atomicupdate/bug_17015_part2_fill_discrete_calendar.perl
@@ +29,5 @@
> +
> +if ($help) {
> +    print $usage;
> +    exit;
> +}

I think atomic update should not take params.

::: koha-tmpl/intranet-tmpl/prog/en/modules/tools/discrete_calendar.tt
@@ +3,5 @@
> +<title>Koha &rsaquo; Tools &rsaquo; [% Branches.GetName( branch ) %] calendar</title>
> +[% INCLUDE 'doc-head-close.inc' %]
> +[% INCLUDE 'calendar.inc' %]
> +<link rel="stylesheet" type="text/css" href="[% interface %]/[% theme %]/css/datatables.css" />
> +<script type="text/javascript" src="[% interface %]/lib/jquery/plugins/jquery-ui-timepicker-addon.min.js"></script>

Javascript should be at and of page, see: https://wiki.koha-community.org/wiki/Coding_Guidelines#JS12:_Include_javascript_at_the_end_of_template

@@ +376,5 @@
> +        });
> +    //]]>
> +    </script>
> +    <!-- Datepicker colors -->
> +    <style type="text/css">

Please move this to separate css file.

::: misc/cronjobs/add_days_discrete_calendar.pl
@@ +3,5 @@
> +#
> +#   This script adds one day into discrete_calendar table based on the same day from the week before
> +#
> +use strict;
> +use warnings;

use Modern::Perl;

::: tools/discrete_calendar.pl
@@ +17,5 @@
> +
> +#####Sets holiday periods for each branch. Datedues will be extended if branch is closed -TG
> +use strict;
> +use warnings;
> +

use Modern::Perl;
Comment 133 Josef Moravec 2018-02-15 10:00:20 UTC
There are still some references to C4::Calendar, they should be removed to as well as the C4::Calendar module itself.

Also, when we are removing old modules, we should remove the database tables too.
Comment 134 David Bourgault 2018-03-14 16:11:22 UTC Comment hidden (obsolete)
Comment 135 David Bourgault 2018-03-14 16:14:21 UTC
Forgot to mention : t/db_dependent/TestBuilder.t does fail with error message "Unknown type time for open_hour in DiscreteCalendar at /inlibro/git/koha-master-dev-inlibro/t/lib/TestBuilder.pm line 341."

I'm not sure what to do with this. Isn't 'time' a standard datatype for all SQL implementations? 

Advice welcome.
Comment 136 Josef Moravec 2018-03-15 06:56:29 UTC
(In reply to David Bourgault from comment #135)
> Forgot to mention : t/db_dependent/TestBuilder.t does fail with error
> message "Unknown type time for open_hour in DiscreteCalendar at
> /inlibro/git/koha-master-dev-inlibro/t/lib/TestBuilder.pm line 341."
> 
> I'm not sure what to do with this. Isn't 'time' a standard datatype for all
> SQL implementations? 
> 
> Advice welcome.

It is because test builder does not know the data type "time", look into t/lib/TestBuilder.pm line 358 (sub _gen_type)
Comment 137 David Bourgault 2018-03-21 18:06:14 UTC Comment hidden (obsolete)
Comment 138 David Bourgault 2018-04-25 20:25:34 UTC
Created attachment 74859 [details] [review]
Bug 17015: DiscreteCalendar UI, Back-End and necessary scripts
Comment 139 David Bourgault 2018-04-25 20:25:45 UTC
Created attachment 74860 [details] [review]
Bug 17015: Tests for DiscreteCalendar
Comment 140 David Bourgault 2018-04-25 20:25:55 UTC
Created attachment 74861 [details] [review]
Bug 17015: Koha now uses the new DiscreteCalendar module
Comment 141 David Bourgault 2018-04-25 20:26:05 UTC
Created attachment 74862 [details] [review]
Bug 17015: Peformance tests for Koha::DiscreteCalendar
Comment 142 David Bourgault 2018-04-25 20:26:16 UTC
Created attachment 74863 [details] [review]
Bug 17015: Remove old Koha::Calendar
Comment 143 David Bourgault 2018-04-25 20:26:32 UTC
Created attachment 74864 [details] [review]
Bug 17015: Code quality fixes on QA feedback

Changes:

* Koha::DiscreteCalendar->new uses a hashref instead of a hash
This meant changing every call to the Calendar and explains why this patch is so
big. Most changes are simply adding braces to the new() call.

+ Added table encoding to atomic update
+ Added table definition to kohastructure.sql

- Removed `repeatable_holidays`, `special_holidays` from kohastructure.sql
Also added relevant DROP TABLEs to a new atomicupdate (part3)

+ Added 'override' param to Koha::DiscreteCalendar
Without this param (off by default), edit_holiday will not modify past dates.
This caused issues with some tests that required this functionality to work
properly. Adding 'override => 1' to the function call will allow past dates to
happen, but should only be called for testing.

* Holds.t, Hold.t, CalcDateDue.t have been corrected.
Prior to his patch, theses tests used hardcoded dates to test calendar
functionalities, however, DiscreteCalendar is only usable one year into the past
by default. They have been modified to be centered around '$today'

* 'use strict; use warnings;' changed to 'use Modern::Perl;' in modified files.
* Atomic update no longer takes params
* Javascript moved to the bottom of discrete_calendar.tt
* Calendar css moved out of tt file and into new discrete_calendar.css
* All references to C4::Calendar and Koha::Calendar should now be removed
* Some comments corrected

All relevant tests passed for me on a fresh database. QA tool green.
Test plan has not changed.
Comment 144 David Bourgault 2018-04-25 20:26:43 UTC
Created attachment 74865 [details] [review]
Bug 17015: Add gen_time to TestBuilder

+ Adds a gen_time() function to TestBuilder.
TestBuilder now passes green.
Comment 145 David Bourgault 2018-04-25 20:26:50 UTC Comment hidden (obsolete)
Comment 146 David Bourgault 2018-04-25 20:32:22 UTC
I reuploaded all the patches to improve that patch titles and match them to the community guidelines.

QA tool is green. Tests pass.
Patch is ready for testing and signoff.
Comment 147 David Bourgault 2018-04-25 20:42:44 UTC
Marked bug 20660 as a dependency of this, as is required for the last patch to work.
Comment 148 David Bourgault 2018-05-18 21:04:29 UTC
Created attachment 75444 [details] [review]
Bug 17015: Updated patch after bug 19204

Rebased

~

This is a small modification to bring the patch up-to-date with current master after the changes made in 19204. Tests in db_dependent/Circulation.thave been changed slightly since DiscreteCalendar strictly prevents changing holidays in the past. This patch requires 20660.
Comment 149 David Bourgault 2018-05-22 18:42:03 UTC
Created attachment 75479 [details] [review]
Bug 17015: Improve stability for very old loans

This patch fixes a crash when calculating fines for hourly loans created before the earliest date in the 'discrete_calendar' table. The SQL call has been altered to find the closest day (using DATEDIFF).

QA Green.
Comment 150 David Bourgault 2018-06-29 20:41:41 UTC
Created attachment 76594 [details] [review]
Bug 17015: Improved UI readability

* The calendar tool page no longer shows past holidays in the lists on the right side of the page.
* Improved wording in the "Hints"
Comment 151 David Bourgault 2018-07-24 20:58:13 UTC
Created attachment 77249 [details] [review]
Bug 17015: Fixes to migration script

Fixes a few errors in the migration script. When first generation the discrete_calendar table, holidays should now be correctly imported for all branches.
Comment 152 David Bourgault 2018-07-24 21:20:29 UTC
Created attachment 77250 [details] [review]
Bug 17015: Fixes to cronjob

Fixes a typo in the add_days cronjob.
Comment 153 Josef Moravec 2018-08-02 12:21:19 UTC
Could you rebase please?

Applying: Bug 17015: Code quality fixes on QA feedback
fatal: sha1 information is lacking or useless (C4/Circulation.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 17015: Code quality fixes on QA feedback
Comment 154 Jonathan Druart 2018-08-02 15:14:24 UTC
(In reply to Josef Moravec from comment #153)
> Could you rebase please?

I would not ask for a rebase. The fundamental idea of the patches should be discussed and QAers should tell if it's the best way to go.

Josef, I would suggest you to have a look at the code and let us know what you think about it. See also comment 119 and later.

A remote branch with the patches applied (even if not up-to-date) can be helpful.
Comment 155 Charles Farmer 2018-08-09 02:35:11 UTC Comment hidden (obsolete)
Comment 156 Charles Farmer 2018-08-09 02:35:23 UTC Comment hidden (obsolete)
Comment 157 Charles Farmer 2018-08-09 02:35:31 UTC Comment hidden (obsolete)
Comment 158 Charles Farmer 2018-08-09 02:35:39 UTC Comment hidden (obsolete)
Comment 159 Charles Farmer 2018-08-09 02:35:47 UTC Comment hidden (obsolete)
Comment 160 Charles Farmer 2018-08-09 02:35:56 UTC Comment hidden (obsolete)
Comment 161 Charles Farmer 2018-08-09 02:37:34 UTC Comment hidden (obsolete)
Comment 162 Charles Farmer 2018-08-09 02:38:59 UTC
I don't know how those 'sha1 information is lacking or useless' happen, but it made the stack of patches impossible to apply anywhere on the master tree, period.

Since this is not really a rebase, I went ahead and remade that patch whole from one the that was up there. It should now apply cleanly without any manual intervention with the exact same code.

Unfortunately, because that patch stood in the middle of the others, I had to reupload and obsolete the last half just to place the new one back in its right position. Sorry for the wall of text.
Comment 163 Charles Farmer 2018-08-13 21:27:16 UTC Comment hidden (obsolete)
Comment 164 Charles Farmer 2018-08-13 21:27:25 UTC Comment hidden (obsolete)
Comment 165 Charles Farmer 2018-08-13 21:27:34 UTC Comment hidden (obsolete)
Comment 166 Charles Farmer 2018-08-13 21:27:42 UTC Comment hidden (obsolete)
Comment 167 Charles Farmer 2018-08-13 21:27:50 UTC Comment hidden (obsolete)
Comment 168 Lisette Scheer 2018-09-14 23:09:11 UTC
When I run the update database command, I got the following information:

DEV atomic update: bug_17015_part1_create_discrete_calendar.sql
C4::Installer::load_sql returned the following errors while attempting to load /home/vagrant/kohaclone/installer/data/mysql/atomicupdate/bug_17015_part1_create_discrete_calendar.sql:
DEV atomic update: bug_17015_part2_fill_discrete_calendar.perl
[] Default
[CPL] Centerville
[FFL] Fairfield
[FPL] Fairview
[FRL] Franklin
[IPT] Institut Protestant de Th▒ologie
[LPL] Liberty
[MPL] Midway
[PVL] Pleasant Valley
[RPL] Riverside
[SPL] Springfield
[TPL] Troy
[UPL] Union
DBD::mysql::st execute failed: Table 'koha_kohadev.repeatable_holidays' doesn't exist [for Statement "SELECT weekday, title, day, month FROM repeatable_holidays WHERE branchcode = ?" with ParamValues: 0=''] at (eval 1193) line 63.
Atomic update generated errors: DBD::mysql::st execute failed: Table 'koha_kohadev.repeatable_holidays' doesn't exist [for Statement "SELECT weekday, title, day, month FROM repeatable_holidays WHERE branchcode = ?" with ParamValues: 0=''] at (eval 1193) line 63.

DEV atomic update: bug_17015_part3_drop_calendar.sql

Lisette
Comment 169 Charles Farmer 2018-09-17 15:47:38 UTC
Thank you Lisette. I was made aware of that this morning and you beat me to it.

It's probably because we touched kohastructure.sql when we should've waited until the push to do so, which would in turn install an incomplete database if you're installing a fresh Koha with this bug already applied over master.

Patch coming soon
Comment 170 Charles Farmer 2018-09-17 18:21:34 UTC
Created attachment 79035 [details] [review]
Bug 17015: (follow-up) keep kohastructure.sql clean to avoid bugs when installing a fresh koha directly over this patchset
Comment 171 Michal Denar 2018-09-17 21:02:46 UTC
Hi,
on master I get this error:
DBIx::Class::Storage::DBI::_dbh_execute(): Table 'koha_kohadev.discrete_calendar' doesn't exist at /home/vagrant/kohaclone/Koha/DiscreteCalendar.pm line 99
 at /usr/share/perl5/DBIx/Class/Exception.pm line 77

db was reseted on latest master.
Comment 172 M. Tompsett 2018-09-18 16:56:43 UTC
git grep repeatable_holidays
-- there are still references to this dropped table.
Similar issues exist with the other dropped table.
Comment 173 Charles Farmer 2018-10-10 22:27:19 UTC
I'll try another install with the patch already applied. Perhaps I've missed this last time because I was not using the kohadevbox.

--

About repeatable_holidays, could you be more precise about the lingering references you've found? I've seen a lot of variables named that way, a few references to it in language files, which, to me, feel outside the scope of this POC, but nothing that would prevent someone from testing.

I don't wanna misquote Jonathan's last comment, but this is more about gathering info about the feel of this implementation than ironing all the little details. If, however, you did find something that would prevent an enduser from testing this, don't hesitate to describe clearly which part is affected and I'll be happy to submit a follow-up fix.
Comment 174 Charles Farmer 2018-10-12 15:49:47 UTC Comment hidden (obsolete)
Comment 175 Charles Farmer 2018-10-12 15:49:56 UTC Comment hidden (obsolete)
Comment 176 Charles Farmer 2018-10-12 15:50:08 UTC Comment hidden (obsolete)
Comment 177 Charles Farmer 2018-10-12 15:50:17 UTC Comment hidden (obsolete)
Comment 178 Charles Farmer 2018-10-12 15:50:27 UTC Comment hidden (obsolete)
Comment 179 Charles Farmer 2018-10-12 15:50:37 UTC Comment hidden (obsolete)
Comment 180 Charles Farmer 2018-10-12 15:50:47 UTC Comment hidden (obsolete)
Comment 181 Charles Farmer 2018-10-12 15:50:57 UTC Comment hidden (obsolete)
Comment 182 Charles Farmer 2018-10-12 15:51:07 UTC Comment hidden (obsolete)
Comment 183 Charles Farmer 2018-10-12 15:51:16 UTC Comment hidden (obsolete)
Comment 184 Charles Farmer 2018-10-12 15:51:26 UTC Comment hidden (obsolete)
Comment 185 Charles Farmer 2018-10-12 15:51:36 UTC Comment hidden (obsolete)
Comment 186 Charles Farmer 2018-10-12 15:51:46 UTC Comment hidden (obsolete)
Comment 187 Charles Farmer 2018-10-12 16:00:32 UTC
I've submitted a rebased version of the patchset, since there was another SHA1 problem since I last tried to apply.

--

Michal, could you give me more information about your install process? I was not able to get the error message you showed, and since the calendar table should be created after bug_17015_part1_create_discrete_calendar.sql, I'm lost.

I'm personally using the _english_ web-based installer. I drop the database, git-bz apply 17015, and I visit the intranet page to begin the installation process.
Comment 188 Charles Farmer 2018-12-28 21:53:52 UTC
Created attachment 83547 [details] [review]
Bug 17015: (follow-up) Fixes a holiday bug when going from 'none' to 'empty'
Comment 189 Victor Grousset/tuxayo 2019-02-15 09:49:39 UTC
Hi :) I will try to test this.

Is the test plan from comment #84 still up to date and enough for someone not having followed this bugzilla? (I don't know which other messages are still valid and don't not much about the calendar)
Comment 190 Katrin Fischer 2019-02-15 09:52:38 UTC
I think as this is such a big change it would be really great to have an RFC on the wiki first before putting more work into the patch set. I'd be really interested to see the concept and mechanics and possible consequences laid out first.
Comment 191 Charles Farmer 2019-02-20 17:12:04 UTC
Created attachment 85394 [details] [review]
Bug 17015: (follow-up) move javascript .inc files inside jsinclude block
Comment 192 Charles Farmer 2019-02-20 21:09:25 UTC
> it would be really great to have an RFC on the wiki first before            putting more work into the patch set.

I know the comments and answers on this development are spread between the mailing list, this bug and IRC, so I don't necessarily want to shoot down the idea of an RFC, but it would add work, a whole lot of work that we simply don't have the resources for. The crux of the matter being: how is an RFC different from the comments given in this patch, the comments given on IRC and Philippe's own answers on the mailing list?

We already had comments: from our clients who are using this feature in         production. I cannot let this go back to 'In discussion', but I'd be open for a "refresher", a lengthy message describing a bit more tightly what we're aiming for and what are the beneficial features offered here, with some advices on your part.
Comment 193 Katrin Fischer 2019-02-21 08:40:57 UTC
(In reply to Charles Farmer from comment #192)
> > it would be really great to have an RFC on the wiki first before            putting more work into the patch set.
> 
> I know the comments and answers on this development are spread between the
> mailing list, this bug and IRC, so I don't necessarily want to shoot down
> the idea of an RFC, but it would add work, a whole lot of work that we
> simply don't have the resources for. The crux of the matter being: how is an
> RFC different from the comments given in this patch, the comments given on
> IRC and Philippe's own answers on the mailing list?
> 
> We already had comments: from our clients who are using this feature in     
> production. I cannot let this go back to 'In discussion', but I'd be open
> for a "refresher", a lengthy message describing a bit more tightly what
> we're aiming for and what are the beneficial features offered here, with
> some advices on your part.

Hi Charles,

You are proposing to rewrite a core feature of Koha, a very important functionality that will affect circulation and holds. I think there should have been an RFC from the very beginning to be honest, before even starting this work.
I think it's unfair to expect anyone in the community (testers/QA, often volunteering their time) to go trough 190+ bug comments, mailing list mails and IRC logs to get a full picture of how this is supposed to work, what the advantages are etc. An RFC can serve the purpose of one central piece of documentation that is kept current with changes in the development for a big feature to avoid information being scattered everywhere. The wiki is better suited for this than any of the others.

That's my opinion, but of course, you can also approach the RM and others.
Comment 194 Martin Renvoize 2019-02-21 09:25:56 UTC
I can't really summarise that better than Katrin already has..  I simply can't justify the hours of time it would take for me to filter through 190 comments, mailing list threads and irc conversations to understand such a large change and make sure all the angles have been covered.

As such I could not, and would not, even begin to QA this. An RFC should serve as a summary of said comments and conversations simply showing the conversations have taken place and what the agreed outcomes were of those conversations.

I don't believe this bug will progress further without such a summary in existence. It's all a learning process of course, most of the support companies have been in a similar place at some point and we've tried to learn the lessons from it.

Good luck
Comment 195 Victor Grousset/tuxayo 2019-02-21 13:39:03 UTC
Hi,

(In reply to Charles Farmer from comment #192)
> I'd be open
> for a "refresher", a lengthy message describing a bit more tightly what
> we're aiming for and what are the beneficial features offered here, with
> some advices on your part.

Wouldn't that contain most of the needed stuff for an RFC? ;)

Such a summary will in any case be helpful to have solid bases before wanting to sign off, QA or get involved in any other way.
Comment 196 Charles Farmer 2019-02-21 15:28:21 UTC
> Wouldn't that contain most of the needed stuff for an RFC? ;)

Well, I might be arguing semantics here, but I feel like what we could provide is a very well documented _test plan_, without the strings attached of turning this into a _discussion_.

There have been discussions, from all the way back when François was here and argued that the calendar had to be made more effective. We had experimental evidence that this development was needed, it came to be, and it's now used in our production environment after years of additional feedback from our clients.

> You are proposing to rewrite a core feature of Koha, a very important functionality that will affect circulation and holds.

I'm not really proposing to rewrite a core part of Koha, we _did_ rewrite a core part of Koha! That's, I think, the core disagreement here. I cannot let this return to a full fledged battleground of comments where we're asked to rewrite, rearchitecture, and pretty much redo a whole slew of commits when what we offer here works.

> An RFC should serve as a summary of said comments and conversations simply showing the conversations have taken place and what the agreed outcomes were of those conversations.

And if the scope of the RFC stays there, I believe this is a middle ground that's perfectly acceptable. If I has to QA this, I would definitely appreciate a concise overview of what's offered.

Documentation is on the way.
Comment 197 Katrin Fischer 2019-02-21 15:49:51 UTC
Charles, I cannot promise you no discussion, this is not how the community works.

I don't remember this being voted on in a general meeting (sorry if I am mistaken), but that is probably the only formal way we have to say 'yes, we agree to do it that way' See votes on the new REST API for an example.

I am not out to upset you, I really appreciate your and InLibro's contributions to the community a lot. But this code has been started in 2016... I think it can't really work the way you want it to just going in without discussion and possibly changes.
Comment 198 Charles Farmer 2019-02-26 05:22:43 UTC Comment hidden (obsolete)
Comment 199 Charles Farmer 2019-02-26 05:22:50 UTC Comment hidden (obsolete)
Comment 200 Charles Farmer 2019-02-26 05:22:58 UTC Comment hidden (obsolete)
Comment 201 Charles Farmer 2019-02-26 05:23:06 UTC Comment hidden (obsolete)
Comment 202 Charles Farmer 2019-02-26 05:23:13 UTC Comment hidden (obsolete)
Comment 203 Charles Farmer 2019-02-26 05:23:22 UTC Comment hidden (obsolete)
Comment 204 Charles Farmer 2019-02-26 05:23:31 UTC Comment hidden (obsolete)
Comment 205 Charles Farmer 2019-02-26 05:23:38 UTC Comment hidden (obsolete)
Comment 206 Charles Farmer 2019-02-26 05:23:46 UTC Comment hidden (obsolete)
Comment 207 Charles Farmer 2019-02-26 05:23:54 UTC Comment hidden (obsolete)
Comment 208 Charles Farmer 2019-02-26 05:24:02 UTC Comment hidden (obsolete)
Comment 209 Charles Farmer 2019-02-26 05:24:10 UTC Comment hidden (obsolete)
Comment 210 Charles Farmer 2019-02-26 05:24:17 UTC Comment hidden (obsolete)
Comment 211 Charles Farmer 2019-02-26 05:24:25 UTC Comment hidden (obsolete)
Comment 212 Charles Farmer 2019-02-26 05:24:33 UTC Comment hidden (obsolete)
Comment 213 Josef Moravec 2019-02-26 08:33:06 UTC
Hi Charles, I just look to your code quickly and have some comments:

1) Bug 22015 added datatables.css to doc-head-close include file, so it is no more needed to add it to every template and should be removed from koha-tmpl/intranet-tmpl/prog/en/modules/tools/discrete_calendar.tt

2) Why are you using DateTime::Format::Strptime instead of datetime_parser from schema when working with database? I don't think it is right

3) Koha/Schema/* files should always be in they own patch, this patch is not pushed, but is here for ease testing, and schema is regenerated by release manager when patchset is pushed

4) Use DateTime->clone->subtract or add in bug_17015_part2_fill_discrete_calendar.perl when setting $startDate and $endDate, it would be much more readable
my $startDate = DateTime->new(
day       => $currentDate->day(),
month     => $currentDate->month(),
year      => $currentDate->year()-2,
time_zone => C4::Context->tz()
)->truncate( to => 'day' );

5) Is this really needed?
#Added a default (standard) branch.
my $add_default_branch = 'INSERT IGNORE INTO branches (branchname, branchcode) VALUES(?,?)';
my $add_Branch_Sth = $dbh->prepare($add_default_branch);
$add_Branch_Sth->execute('Default', '');

I don't think it is good idea to add a fake data to branches...

6) t/db_dependent/Circulation.t still references C4::Calendar

7) koha-tmpl/intranet-tmpl/prog/en/modules/tools/discrete_calendar.tt has many missing filters

These are just few things which would need attention, but first please, write down a short description/summary and bring it to meeting to discuss and vote for this by community.

The description is needed for people who are not aware of this report to summary the pros and cons to make them able to decide how this would continue.

Thanks for your contribution!
Comment 214 Liz Rea 2019-04-30 00:50:07 UTC
Hi,

Failed QA until you respond to the questions in comment 213.

Thanks!

Liz
Comment 215 Charles Farmer 2019-09-04 19:01:05 UTC Comment hidden (obsolete)
Comment 216 Charles Farmer 2019-09-04 19:01:39 UTC Comment hidden (obsolete)
Comment 217 Charles Farmer 2019-09-04 19:01:47 UTC Comment hidden (obsolete)
Comment 218 Charles Farmer 2019-09-04 19:01:55 UTC Comment hidden (obsolete)
Comment 219 Charles Farmer 2020-03-03 21:23:33 UTC
Created attachment 100079 [details] [review]
Bug 17015: (follow-up) accept Koha::Library objects in place of a branchcode string, always use the syspref 'dateformat'