Bug 6796 - Overnight checkouts taking into account opening and closing hours
Summary: Overnight checkouts taking into account opening and closing hours
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Aleisha Amohia
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 8133
Blocks: 35937 36079
  Show dependency treegraph
 
Reported: 2011-08-27 05:23 UTC by vimal kumar
Modified: 2024-02-29 22:21 UTC (History)
19 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This feature adds the ability to set opening and closing hours for your library and for these hours to be considered when calculating due dates for hourly loans. If the due date for an hourly loan falls after the library closes, the library can choose for the due date to be shortened to meet the close time, or extended to meet the open time the next day. This feature adds a new table 'branch_hours' for storing the open and close times per day for each library, and a new system preference 'ConsiderLibraryHoursWhenIssuing' to choose which behaviour should be followed when calculating due dates.
Version(s) released in:


Attachments
Bug 6796: Add branch_hours table and set opening hours for library (12.56 KB, patch)
2021-09-09 03:51 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Updated schema files (3.33 KB, patch)
2021-09-09 03:51 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: ConsiderLibraryHoursWhenIssuing system preference (3.76 KB, patch)
2021-09-09 03:51 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Consider library hours when calculating due date + tests (12.16 KB, patch)
2021-09-09 03:51 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Add branch_hours table and set opening hours for library (12.97 KB, patch)
2022-04-29 00:44 UTC, Hayley Pelham
Details | Diff | Splinter Review
Bug 6796: Updated schema files (2.90 KB, patch)
2022-04-29 00:46 UTC, Hayley Pelham
Details | Diff | Splinter Review
Bug 6796: ConsiderLibraryHoursWhenIssuing system preference (3.75 KB, patch)
2022-04-29 00:46 UTC, Hayley Pelham
Details | Diff | Splinter Review
Bug 6796: Consider library hours when calculating due date + tests (12.13 KB, patch)
2022-04-29 00:47 UTC, Hayley Pelham
Details | Diff | Splinter Review
Bug 6796: (follow-up) Fix logic for calculating following day's open hours (2.04 KB, patch)
2022-04-29 00:47 UTC, Hayley Pelham
Details | Diff | Splinter Review
Bug 6796: Add branch_hours table and set opening hours for library (13.02 KB, patch)
2023-01-25 21:09 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: ConsiderLibraryHoursWhenIssuing system preference (3.69 KB, patch)
2023-01-25 21:09 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Consider library hours when calculating due date + tests (12.35 KB, patch)
2023-01-25 21:09 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: (follow-up) Fix logic for calculating following day's open hours (2.04 KB, patch)
2023-01-25 21:09 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Updated schema files (3.33 KB, patch)
2023-01-25 21:09 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Add branch_hours table and set opening hours for library (12.72 KB, patch)
2023-02-20 23:45 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: ConsiderLibraryHoursWhenIssuing system preference (3.66 KB, patch)
2023-02-20 23:46 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Consider library hours when calculating due date + tests (12.33 KB, patch)
2023-02-20 23:46 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: (follow-up) Fix logic for calculating following day's open hours (2.04 KB, patch)
2023-02-20 23:46 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Updated schema files (3.32 KB, patch)
2023-02-20 23:46 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Add branch_hours table and set opening hours for library (12.73 KB, patch)
2023-05-01 03:00 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: ConsiderLibraryHoursWhenIssuing system preference (3.77 KB, patch)
2023-05-01 03:00 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Consider library hours when calculating due date + tests (12.33 KB, patch)
2023-05-01 03:00 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: (follow-up) Fix logic for calculating following day's open hours (2.04 KB, patch)
2023-05-01 03:01 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Updated schema files (3.33 KB, patch)
2023-05-01 03:01 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Fix atomic updates (5.49 KB, patch)
2023-05-01 03:01 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Fix saving of libraries and tests (5.22 KB, patch)
2023-05-01 03:01 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: (follow-up) Fix branch_hours.day to enum and add filters (5.46 KB, patch)
2023-05-01 22:32 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: (follow-up) Schema changes (1.57 KB, patch)
2023-05-01 22:32 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Add branch_hours table and set opening hours for library (12.73 KB, patch)
2023-07-10 04:29 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: ConsiderLibraryHoursWhenIssuing system preference (3.77 KB, patch)
2023-07-10 04:29 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Consider library hours when calculating due date + tests (12.34 KB, patch)
2023-07-10 04:29 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: (follow-up) Fix logic for calculating following day's open hours (2.04 KB, patch)
2023-07-10 04:29 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Updated schema files (3.33 KB, patch)
2023-07-10 04:29 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Fix atomic updates (5.49 KB, patch)
2023-07-10 04:30 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Fix saving of libraries and tests (5.22 KB, patch)
2023-07-10 04:30 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: (follow-up) Fix branch_hours.day to enum and add filters (5.46 KB, patch)
2023-07-10 04:30 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: (follow-up) Schema changes (1.57 KB, patch)
2023-07-10 04:30 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Don't add a day if hourly loan period pushes due date (3.32 KB, patch)
2023-07-10 04:30 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Add branch_hours table and set opening hours for library (12.76 KB, patch)
2023-07-10 14:26 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 6796: ConsiderLibraryHoursWhenIssuing system preference (3.81 KB, patch)
2023-07-10 14:26 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 6796: Consider library hours when calculating due date + tests (12.38 KB, patch)
2023-07-10 14:26 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 6796: (follow-up) Fix logic for calculating following day's open hours (2.08 KB, patch)
2023-07-10 14:26 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 6796: Updated schema files (3.36 KB, patch)
2023-07-10 14:26 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 6796: Fix atomic updates (5.52 KB, patch)
2023-07-10 14:26 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 6796: Fix saving of libraries and tests (5.26 KB, patch)
2023-07-10 14:27 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 6796: (follow-up) Fix branch_hours.day to enum and add filters (5.49 KB, patch)
2023-07-10 14:27 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 6796: (follow-up) Schema changes (1.61 KB, patch)
2023-07-10 14:27 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 6796: Don't add a day if hourly loan period pushes due date (3.36 KB, patch)
2023-07-10 14:27 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 6796: Add library_hours table and set opening hours for library (13.30 KB, patch)
2023-10-10 04:08 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: ConsiderLibraryHoursInCirculation system preference (3.96 KB, patch)
2023-10-10 04:08 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Consider library hours when calculating due date + tests (12.86 KB, patch)
2023-10-10 04:08 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: (follow-up) Fix logic for calculating following day's open hours (1.80 KB, patch)
2023-10-10 04:08 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Fix saving of libraries and tests (5.28 KB, patch)
2023-10-10 04:09 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Don't add a day if hourly loan period pushes due date (3.36 KB, patch)
2023-10-10 04:09 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Updated schema files (3.42 KB, patch)
2023-10-10 04:09 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: (follow-up) QA test tool fixes (17.99 KB, patch)
2023-10-10 04:09 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: [WIP] (follow-up) Displaying library hours (9.72 KB, patch)
2023-10-10 04:09 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 6796: Add library_hours table and set opening hours for library (13.74 KB, patch)
2024-02-09 16:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: ConsiderLibraryHoursInCirculation system preference (3.96 KB, patch)
2024-02-09 16:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Consider library hours when calculating due date + tests (12.86 KB, patch)
2024-02-09 16:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: (follow-up) Fix logic for calculating following day's open hours (1.80 KB, patch)
2024-02-09 16:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Fix saving of libraries and tests (5.28 KB, patch)
2024-02-09 16:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Don't add a day if hourly loan period pushes due date (3.36 KB, patch)
2024-02-09 16:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Updated schema files (3.42 KB, patch)
2024-02-09 16:03 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: (follow-up) QA test tool fixes (17.99 KB, patch)
2024-02-09 16:03 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: [WIP] (follow-up) Displaying library hours (9.72 KB, patch)
2024-02-09 16:03 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Fix api specifications and embed hours in librarly management (2.36 KB, patch)
2024-02-09 16:03 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Fix display of libraries table (2.25 KB, patch)
2024-02-09 16:03 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: (follow-up) Displaying library hours (9.71 KB, patch)
2024-02-09 17:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Fix api specifications and embed hours in librarly management (2.81 KB, patch)
2024-02-09 17:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Fix display of libraries table (2.25 KB, patch)
2024-02-09 17:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Take CalenderFirstDayOfWeek into account (3.11 KB, patch)
2024-02-09 17:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Add library_hours table and set opening hours for library (13.74 KB, patch)
2024-02-12 15:49 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: ConsiderLibraryHoursInCirculation system preference (3.96 KB, patch)
2024-02-12 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Consider library hours when calculating due date + tests (12.86 KB, patch)
2024-02-12 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: (follow-up) Fix logic for calculating following day's open hours (1.80 KB, patch)
2024-02-12 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Fix saving of libraries and tests (5.28 KB, patch)
2024-02-12 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Don't add a day if hourly loan period pushes due date (3.36 KB, patch)
2024-02-12 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Updated schema files (3.42 KB, patch)
2024-02-12 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: (follow-up) QA test tool fixes (18.35 KB, patch)
2024-02-12 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: (follow-up) Displaying library hours (9.71 KB, patch)
2024-02-12 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Fix api specifications and embed hours in librarly management (2.81 KB, patch)
2024-02-12 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Fix display of libraries table (2.25 KB, patch)
2024-02-12 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Take CalenderFirstDayOfWeek and TimeFormat into account (18.23 KB, patch)
2024-02-12 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Merge and correct database update (3.15 KB, patch)
2024-02-12 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: day_of_week representation fix (1.62 KB, patch)
2024-02-12 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Add library_hours table and set opening hours for library (13.87 KB, patch)
2024-02-13 10:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: ConsiderLibraryHoursInCirculation system preference (4.09 KB, patch)
2024-02-13 10:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Consider library hours when calculating due date + tests (12.92 KB, patch)
2024-02-13 10:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: (follow-up) Fix logic for calculating following day's open hours (1.87 KB, patch)
2024-02-13 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Fix saving of libraries and tests (5.41 KB, patch)
2024-02-13 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Don't add a day if hourly loan period pushes due date (3.50 KB, patch)
2024-02-13 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Updated schema files (3.59 KB, patch)
2024-02-13 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: (follow-up) QA test tool fixes (18.52 KB, patch)
2024-02-13 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: (follow-up) Displaying library hours (9.92 KB, patch)
2024-02-13 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Fix api specifications and embed hours in librarly management (2.90 KB, patch)
2024-02-13 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Fix display of libraries table (2.34 KB, patch)
2024-02-13 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Take CalenderFirstDayOfWeek and TimeFormat into account (18.32 KB, patch)
2024-02-13 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Merge and correct database update (3.31 KB, patch)
2024-02-13 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: day_of_week representation fix (1.71 KB, patch)
2024-02-13 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6796: Code defensively for if library hours are not set (1.08 KB, patch)
2024-02-13 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description vimal kumar 2011-08-27 05:23:07 UTC
Most of the libraries check out reference books (Not for loan) for over night reading. Reference book check out on evening and the patron must return on next day morning itself. In present condition, reference books are not for loan in Koha. But, libraries need to check out for over night reading. Its nice, if there is a provision to check out Reference books not for loan.
Comment 1 Katrin Fischer 2013-04-13 22:14:24 UTC
You can allow librarians to override the not for loan setting via a system preference. Also the configuration can be changed from not for loan to an overnight (1 day loan period) loan. I am not sure, if that fixes the problem, so please reopen if there is more functionality needed.
Comment 2 Nicole C. Engard 2013-04-26 00:27:03 UTC
I think this needs to be reopened and I have another example.  Basically if you have a 1 day loan then the item is due anytime the next day, but we need it to be due in the morning - at a specific time.

For example let's say you check out a 2 hour loan 1 hour before closing.  It would be overdue if returned the next day, but most libraries allow for the item to be returned at opening the next morning.  This is a big issue - bigger than the circ matrix - but it is a desired feature. Maybe the addition of library hours to the calendar and a check of that to figure out if the items should be due an hour after the library closes (which is silly) or the next morning during the first open hour.
Comment 3 SiobhanC 2019-08-05 15:27:28 UTC
Having an Opening Hours element to the Calendar would be useful for us, we have a few lapsafe machines that show the date and time of return on the machine - it currently says that laptops are due back at 23:59 because the circ rules say that they are loaned for 0 days (i.e. same day return). This is misleading as the libraries are not open at that time. 

In our scenario, the item would have to be due back before the library closes, which is different from what Nicole mentions. This could work in a similar way to the hard due date in the circ rules, it would be a hard due time for a particular item type. 

Lapsafe are not able to alter the display, and I'm not aware of a way in which Koha can exclude a time element from the issue transaction.
Comment 4 Aleisha Amohia 2021-09-02 04:29:39 UTC
Hi all, this is a required feature for one of our libraries so I will give it a go!

Just wanting to clarify some behaviour - would we prefer for the item to be due at closing time, or does their loan period get extended to opening time the following open day?

For example, my library closes at midnight (12am) and opens at 8am tomorrow. If I issue a 2 hour loan at 11pm, should the item be due in one hour at 12am (closing time), or at 8am tomorrow (opening time)?
Comment 5 Aleisha Amohia 2021-09-09 03:51:15 UTC Comment hidden (obsolete)
Comment 6 Aleisha Amohia 2021-09-09 03:51:20 UTC Comment hidden (obsolete)
Comment 7 Aleisha Amohia 2021-09-09 03:51:24 UTC Comment hidden (obsolete)
Comment 8 Aleisha Amohia 2021-09-09 03:51:29 UTC Comment hidden (obsolete)
Comment 9 Hayley Pelham 2022-04-29 00:44:15 UTC Comment hidden (obsolete)
Comment 10 Hayley Pelham 2022-04-29 00:46:00 UTC Comment hidden (obsolete)
Comment 11 Hayley Pelham 2022-04-29 00:46:57 UTC Comment hidden (obsolete)
Comment 12 Hayley Pelham 2022-04-29 00:47:29 UTC Comment hidden (obsolete)
Comment 13 Hayley Pelham 2022-04-29 00:47:55 UTC Comment hidden (obsolete)
Comment 14 Catrina Berka 2022-12-05 19:20:43 UTC
Could not get patches to apply in sandbox
Comment 15 Aleisha Amohia 2023-01-25 21:09:20 UTC Comment hidden (obsolete)
Comment 16 Aleisha Amohia 2023-01-25 21:09:26 UTC Comment hidden (obsolete)
Comment 17 Aleisha Amohia 2023-01-25 21:09:33 UTC Comment hidden (obsolete)
Comment 18 Aleisha Amohia 2023-01-25 21:09:39 UTC Comment hidden (obsolete)
Comment 19 Aleisha Amohia 2023-01-25 21:09:45 UTC Comment hidden (obsolete)
Comment 20 Emily Lamancusa 2023-01-26 21:48:24 UTC
> 2. Go to Administration -> Libraries. Edit a library and scroll to the
> bottom to find the 'opening hours' section. Test adding and removing
> open and close times on various days. Confirm saving works as expected.

When I clicked the button to edit a library, I got the error: Template process failed: undef error - The method Koha::SMTP::Servers->id is not covered by tests!

These are the first few lines of the stack trace, in case it is helpful:
Trace begun at /kohadevbox/koha/Koha/Objects.pm line 572
Koha::Objects::AUTOLOAD('Koha::SMTP::Servers=HASH(0x55588e5e86a0)') called at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt line 237
eval {...} at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt line 237
eval {...} at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt line 252
eval {...} at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt line 18
Comment 21 Aleisha Amohia 2023-02-07 01:35:21 UTC
Thanks Emily, please mark bug reports as Failed QA if they don't pass the test plan :)
Comment 22 Aleisha Amohia 2023-02-20 23:45:59 UTC Comment hidden (obsolete)
Comment 23 Aleisha Amohia 2023-02-20 23:46:03 UTC Comment hidden (obsolete)
Comment 24 Aleisha Amohia 2023-02-20 23:46:07 UTC Comment hidden (obsolete)
Comment 25 Aleisha Amohia 2023-02-20 23:46:10 UTC Comment hidden (obsolete)
Comment 26 Aleisha Amohia 2023-02-20 23:46:14 UTC Comment hidden (obsolete)
Comment 27 Emily Lamancusa 2023-02-27 19:40:41 UTC
Thanks for the follow-up Aleisha! Unfortunately, I'm still getting some errors when trying to edit a library (testing on KTD).

When editing an existing library, upon saving:
"An error occurred when updating this library. Perhaps it already exists."

When creating a new library, upon saving:
"An error occurred when adding this library. The branchcode might already exist."

The above two errors occurred consistently regardless of which (if any) fields I edited or what values I entered in those fields.

---

It's also failing tests:
#   Failed test 'Loan period was shortened (but considers the holiday) because ConsiderLibraryHoursWhenIssuing is set to close time'
#   at t/db_dependent/Circulation/CalcDateDue.t line 413.
#          got: '2023-02-27T20:35:41'
#     expected: '2023-03-01T20:35:41'

#   Failed test 'Loan period was extended (but considers the holiday) because ConsiderLibraryHoursWhenIssuing is set to open time'
#   at t/db_dependent/Circulation/CalcDateDue.t line 420.
#          got: '2023-02-28T14:35:41'
#     expected: '2023-03-01T14:35:41'
Comment 28 Aleisha Amohia 2023-05-01 03:00:52 UTC Comment hidden (obsolete)
Comment 29 Aleisha Amohia 2023-05-01 03:00:55 UTC Comment hidden (obsolete)
Comment 30 Aleisha Amohia 2023-05-01 03:00:59 UTC Comment hidden (obsolete)
Comment 31 Aleisha Amohia 2023-05-01 03:01:02 UTC Comment hidden (obsolete)
Comment 32 Aleisha Amohia 2023-05-01 03:01:06 UTC Comment hidden (obsolete)
Comment 33 Aleisha Amohia 2023-05-01 03:01:09 UTC Comment hidden (obsolete)
Comment 34 Aleisha Amohia 2023-05-01 03:01:13 UTC Comment hidden (obsolete)
Comment 35 Lucas Gass 2023-05-01 17:35:51 UTC
Aleisha,

There are some things the QA script caught that need to be cleaned up:

Missing filters:

 OK	installer/data/mysql/mandatory/sysprefs.sql
 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt
   FAIL	  filters
		missing_filter at line 292 (                                <tr id="hours_[% daycount %]">)
		missing_filter at line 295 (                                        <input type="hidden" value="[% daycount %]" name="day">)
		missing_filter at line 299 (                                        <input type="time" value="[% hr.open_time %]" name="open_time">)
		missing_filter at line 306 (                                        <input type="time" value="[% hr.close_time %]" name="close_time">)
		missing_filter at line 316 (                                <tr id="hours_[% day %]">)
		missing_filter at line 319 (                                        <input type="hidden" value="[% daycount %]" name="day">)


DB columns:
 FAIL	installer/data/mysql/kohastructure.sql
   FAIL	  boolean_vs_tinyint
		WARNING - The new column (day) for table branch_hours is using INT(1) as type, must be TINYINT(1) if it has a boolean purpose, see the SQL12 coding guideline
Comment 36 Aleisha Amohia 2023-05-01 22:32:25 UTC Comment hidden (obsolete)
Comment 37 Aleisha Amohia 2023-05-01 22:32:29 UTC Comment hidden (obsolete)
Comment 38 Sam Lau 2023-06-01 19:28:52 UTC
I'm in the process of testing this and so far shortening the loan period seems to be working for me, but extending the loan period isn't working as expected.


Steps taken to replicate my issue
1) Set library opening time to 8:30am closing time to 9pm
2) set loan period to be 3 hours.
3) Set the "ConsiderLibraryHoursWhenIssuing" preference to the extend option.
4) checked out an item. My checkout time and date was 06/01/2023 19:07

  - with the loan period I selected (3 hours), the book would be due on 06/01/2023 @ 22:07, but since the library closes at 9pm (21:00), the due date should be extended to the next day at opening time. 

5) The shown due date is 06/03/2023 at 08:30

   - The due date that was expected was 06/02/2023 at 08:30 so it skipped over a day for some reason.
Comment 39 Aleisha Amohia 2023-07-10 04:29:41 UTC Comment hidden (obsolete)
Comment 40 Aleisha Amohia 2023-07-10 04:29:46 UTC Comment hidden (obsolete)
Comment 41 Aleisha Amohia 2023-07-10 04:29:50 UTC Comment hidden (obsolete)
Comment 42 Aleisha Amohia 2023-07-10 04:29:54 UTC Comment hidden (obsolete)
Comment 43 Aleisha Amohia 2023-07-10 04:29:59 UTC Comment hidden (obsolete)
Comment 44 Aleisha Amohia 2023-07-10 04:30:03 UTC Comment hidden (obsolete)
Comment 45 Aleisha Amohia 2023-07-10 04:30:08 UTC Comment hidden (obsolete)
Comment 46 Aleisha Amohia 2023-07-10 04:30:12 UTC Comment hidden (obsolete)
Comment 47 Aleisha Amohia 2023-07-10 04:30:16 UTC Comment hidden (obsolete)
Comment 48 Aleisha Amohia 2023-07-10 04:30:21 UTC Comment hidden (obsolete)
Comment 49 Sam Lau 2023-07-10 14:26:42 UTC Comment hidden (obsolete)
Comment 50 Sam Lau 2023-07-10 14:26:45 UTC Comment hidden (obsolete)
Comment 51 Sam Lau 2023-07-10 14:26:49 UTC Comment hidden (obsolete)
Comment 52 Sam Lau 2023-07-10 14:26:52 UTC Comment hidden (obsolete)
Comment 53 Sam Lau 2023-07-10 14:26:56 UTC Comment hidden (obsolete)
Comment 54 Sam Lau 2023-07-10 14:26:59 UTC Comment hidden (obsolete)
Comment 55 Sam Lau 2023-07-10 14:27:03 UTC Comment hidden (obsolete)
Comment 56 Sam Lau 2023-07-10 14:27:07 UTC Comment hidden (obsolete)
Comment 57 Sam Lau 2023-07-10 14:27:10 UTC Comment hidden (obsolete)
Comment 58 Sam Lau 2023-07-10 14:27:14 UTC Comment hidden (obsolete)
Comment 59 Katrin Fischer 2023-10-08 15:00:28 UTC
Trying to do a first run through here, but I feel like we might want another set of eyes maybe on the date calculations later on:

1) QA Test Tools

 FAIL	C4/Circulation.pm
   FAIL	  critic
		# Variables::ProhibitUnusedVariables: Got 1 violation(s).  
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 1100, now: 1106)

 WARN	Koha/Schema/Result/Branch.pm
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 222, now: 226)

 FAIL	Koha/Schema/Result/BranchHour.pm
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 0, now: 18)

 WARN	admin/branches.pl
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 47, now: 58)

 WARN	installer/data/mysql/atomicupdate/bug_6796_-_add_ConsiderLibraryHoursWhenIssuing_syspref.pl
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 0, now: 4)

 WARN	installer/data/mysql/atomicupdate/bug_6796_-_add_branch_hours_table.pl
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 0, now: 8)

 FAIL	t/db_dependent/Circulation/CalcDateDue.t
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 138, now: 164)
   FAIL	  valid
		Global symbol "$categorycode" requires explicit package name (did you forget to declare "my $categorycode"?)
		Global symbol "$categorycode" requires explicit package name (did you forget to declare "my $categorycode"?)
		t/db_dependent/Circulation/CalcDateDue.t had compilation errors.


The last one I believe might be responsible for the unit tests not passing. Please make always sure to run QA tests and unit tests on rebase:

Global symbol "$categorycode" requires explicit package name (did you forget to declare "my $categorycode"?) at t/db_dependent/Circulation/CalcDateDue.t line 414.
Execution of t/db_dependent/Circulation/CalcDateDue.t aborted due to compilation errors.
# Looks like your test exited with 255 before it could output anything.
t/db_dependent/Circulation/CalcDateDue.t .. Dubious, test returned 255 (wstat 65280, 0xff00)


2) Terminology

a) ConsiderLibraryHoursWhenIssuing = instead of issue we usually use checkout, but maybe ConsiderLibraryHoursInCirculation would be a good fit?

b) We have Koha::Library::Hours.pm, but Koha::Schema::Results::BranchHour - why not use LibraryHour and also library_hours for the table name?

c) Issuing should be 'checking out' in system preference description: 
Take library opening hours into consideration to calculate due date when issuing.

3) Database update

The 2 separate database updates for adding the pref and the table could have been combined (not blocker).

a) The code comments should be removed form the kohastructure.sql file.

+/*!40101 SET @saved_cs_client     = @@character_set_client */;
+/*!40101 SET character_set_client = utf8 */;
+/*!40101 SET character_set_client = @saved_cs_client */;

4) Translatability

Our scripts always look for strings inside of HTML tags. Wrapping these in spans will make it much easier on translators and possibly allow to 'reuse' the existing translations for week days from other parts of Koha.

+[% BLOCK dayname %]
+    [% IF day == 0 %]
+    Monday
+    [% ELSIF day == 1 %]
+    Tuesday
+    [% ELSIF day == 2 %]
+    Wednesday
+    [% ELSIF day == 3 %]
+    Thursday
+    [% ELSIF day == 4 %]
+    Friday
+    [% ELSIF day == 5 %]
+    Saturday
+    [% ELSE %]
+    Sunday
+    [% END %]
+[% END %]


4) Library administration

a) The opening and closing times should also appear in the 'summary' table of all libraries. (Don't forget to add to add to column configuration as well!)

b) It should also be present in the library 'view' page, that is accessible if you don't have admin permissions.
http://localhost:8081/cgi-bin/koha/admin/branches.pl?op=view&branchcode=CPL

c) TimeFormat is set to 24h format, but I still get presented with AM/PM on data entry. It also doesn't follow CalendarFirstDayOfWeek. I'd think the first would be more important. Maybe the curbside pickup GUI could give some ideas here as I think we did fix the TimeFormat issue there.
Comment 60 Aleisha Amohia 2023-10-10 04:08:46 UTC Comment hidden (obsolete)
Comment 61 Aleisha Amohia 2023-10-10 04:08:50 UTC Comment hidden (obsolete)
Comment 62 Aleisha Amohia 2023-10-10 04:08:54 UTC Comment hidden (obsolete)
Comment 63 Aleisha Amohia 2023-10-10 04:08:58 UTC Comment hidden (obsolete)
Comment 64 Aleisha Amohia 2023-10-10 04:09:02 UTC Comment hidden (obsolete)
Comment 65 Aleisha Amohia 2023-10-10 04:09:06 UTC Comment hidden (obsolete)
Comment 66 Aleisha Amohia 2023-10-10 04:09:10 UTC Comment hidden (obsolete)
Comment 67 Aleisha Amohia 2023-10-10 04:09:15 UTC Comment hidden (obsolete)
Comment 68 Aleisha Amohia 2023-10-10 04:09:19 UTC Comment hidden (obsolete)
Comment 69 Aleisha Amohia 2023-10-10 04:14:50 UTC
The attached patchset addresses the QA test tools and fixes the erroring test. I could not fix the POD and perltidy warnings about Koha/Schema/Result/Library.pm, Koha/Schema/Result/LibraryHour.pm, and Koha/Library/Hour.pm - any guidance on that would be appreciated.

(In reply to Katrin Fischer from comment #59)

> 
> 2) Terminology
> 
> a) ConsiderLibraryHoursWhenIssuing = instead of issue we usually use
> checkout, but maybe ConsiderLibraryHoursInCirculation would be a good fit?

done

> 
> b) We have Koha::Library::Hours.pm, but Koha::Schema::Results::BranchHour -
> why not use LibraryHour and also library_hours for the table name?

done

> 
> c) Issuing should be 'checking out' in system preference description: 
> Take library opening hours into consideration to calculate due date when
> issuing.

changed to 'when circulating' to match the syspref name

> 
> 3) Database update
> 
> The 2 separate database updates for adding the pref and the table could have
> been combined (not blocker).

Done, QA tools complained about this too

> 
> a) The code comments should be removed form the kohastructure.sql file.
> 
> +/*!40101 SET @saved_cs_client     = @@character_set_client */;
> +/*!40101 SET character_set_client = utf8 */;
> +/*!40101 SET character_set_client = @saved_cs_client */;

done

> 
> 4) Translatability
> 
> Our scripts always look for strings inside of HTML tags. Wrapping these in
> spans will make it much easier on translators and possibly allow to 'reuse'
> the existing translations for week days from other parts of Koha.
> 
> +[% BLOCK dayname %]
> +    [% IF day == 0 %]
> +    Monday
> +    [% ELSIF day == 1 %]
> +    Tuesday
> +    [% ELSIF day == 2 %]
> +    Wednesday
> +    [% ELSIF day == 3 %]
> +    Thursday
> +    [% ELSIF day == 4 %]
> +    Friday
> +    [% ELSIF day == 5 %]
> +    Saturday
> +    [% ELSE %]
> +    Sunday
> +    [% END %]
> +[% END %]
> 

done ... still WIP for the libraries summary page

> 
> 5) Library administration
> 
> b) It should also be present in the library 'view' page, that is accessible
> if you don't have admin permissions.
> http://localhost:8081/cgi-bin/koha/admin/branches.pl?op=view&branchcode=CPL
> 

Done

> 
> a) The opening and closing times should also appear in the 'summary' table
> of all libraries. (Don't forget to add to add to column configuration as
> well!)
> c) TimeFormat is set to 24h format, but I still get presented with AM/PM on
> data entry. It also doesn't follow CalendarFirstDayOfWeek. I'd think the
> first would be more important. Maybe the curbside pickup GUI could give some
> ideas here as I think we did fix the TimeFormat issue there.

These two requests have added a lot of complexity to this - adding new columns to an API generated table is not documented anywhere. I don't have capacity to work on this for a while, but maybe someone can build upon what I've done in the most recent patch.
Comment 70 Martin Renvoize 2024-02-09 13:46:03 UTC
Are you still intending to follow-up here Aleisha?
Comment 71 Martin Renvoize 2024-02-09 16:02:38 UTC Comment hidden (obsolete)
Comment 72 Martin Renvoize 2024-02-09 16:02:42 UTC Comment hidden (obsolete)
Comment 73 Martin Renvoize 2024-02-09 16:02:47 UTC Comment hidden (obsolete)
Comment 74 Martin Renvoize 2024-02-09 16:02:51 UTC Comment hidden (obsolete)
Comment 75 Martin Renvoize 2024-02-09 16:02:55 UTC Comment hidden (obsolete)
Comment 76 Martin Renvoize 2024-02-09 16:02:59 UTC Comment hidden (obsolete)
Comment 77 Martin Renvoize 2024-02-09 16:03:04 UTC Comment hidden (obsolete)
Comment 78 Martin Renvoize 2024-02-09 16:03:08 UTC Comment hidden (obsolete)
Comment 79 Martin Renvoize 2024-02-09 16:03:12 UTC Comment hidden (obsolete)
Comment 80 Martin Renvoize 2024-02-09 16:03:16 UTC Comment hidden (obsolete)
Comment 81 Martin Renvoize 2024-02-09 16:03:20 UTC Comment hidden (obsolete)
Comment 82 Martin Renvoize 2024-02-09 16:06:07 UTC
I rebased it for you and added a couple fo fixes to get it to work.. now I'm working through testing.
Comment 83 Martin Renvoize 2024-02-09 17:02:07 UTC Comment hidden (obsolete)
Comment 84 Martin Renvoize 2024-02-09 17:02:11 UTC Comment hidden (obsolete)
Comment 85 Martin Renvoize 2024-02-09 17:02:15 UTC Comment hidden (obsolete)
Comment 86 Martin Renvoize 2024-02-09 17:02:20 UTC Comment hidden (obsolete)
Comment 87 Martin Renvoize 2024-02-09 17:04:01 UTC
OK, I'm grabbing QA here as Katrin is unlikely to get back to it as the current RM.

I'm leaving it at assigned whilst I work through the final touches to your last WIP followup.. those are mostly working now.. so I'm going to now QA.
Comment 88 Martin Renvoize 2024-02-09 17:05:33 UTC
Note to self.. I'd love to display the 'closed days' inline here too.. and it would be great to have a fallback to default for opening hours so you could set at the global level and override at the branch level.. like many of our other options of this style.
Comment 89 Aleisha Amohia 2024-02-12 06:45:20 UTC
(In reply to Martin Renvoize from comment #87)
> OK, I'm grabbing QA here as Katrin is unlikely to get back to it as the
> current RM.
> 
> I'm leaving it at assigned whilst I work through the final touches to your
> last WIP followup.. those are mostly working now.. so I'm going to now QA.

Thanks Martin. This is something I've had to deprioritise and I'm not sure when I can get back to it, but please let me know if there are specific fixes/problems you want me to look at for you.
Comment 90 Martin Renvoize 2024-02-12 15:49:58 UTC Comment hidden (obsolete)
Comment 91 Martin Renvoize 2024-02-12 15:50:02 UTC Comment hidden (obsolete)
Comment 92 Martin Renvoize 2024-02-12 15:50:06 UTC Comment hidden (obsolete)
Comment 93 Martin Renvoize 2024-02-12 15:50:11 UTC Comment hidden (obsolete)
Comment 94 Martin Renvoize 2024-02-12 15:50:15 UTC Comment hidden (obsolete)
Comment 95 Martin Renvoize 2024-02-12 15:50:19 UTC Comment hidden (obsolete)
Comment 96 Martin Renvoize 2024-02-12 15:50:23 UTC Comment hidden (obsolete)
Comment 97 Martin Renvoize 2024-02-12 15:50:27 UTC Comment hidden (obsolete)
Comment 98 Martin Renvoize 2024-02-12 15:50:31 UTC Comment hidden (obsolete)
Comment 99 Martin Renvoize 2024-02-12 15:50:35 UTC Comment hidden (obsolete)
Comment 100 Martin Renvoize 2024-02-12 15:50:39 UTC Comment hidden (obsolete)
Comment 101 Martin Renvoize 2024-02-12 15:50:44 UTC Comment hidden (obsolete)
Comment 102 Martin Renvoize 2024-02-12 15:50:48 UTC Comment hidden (obsolete)
Comment 103 Martin Renvoize 2024-02-12 15:50:52 UTC Comment hidden (obsolete)
Comment 104 Martin Renvoize 2024-02-12 15:56:17 UTC
I've fixed a whole bunch of little issues to get this vaguely working now, but as I run the test suit against it I see there are more issues hidden.

I've fixed some array indexing issues where your code worked in opposition to how Koha's calendar indexes days of the week already.. but Circulation tests still fail. This is because, as far as I can tell, your code expects every branch to always have opening hours set.. i.e. there MUST be rows for days 0 to 6 for every branch, even if those rows have hours of null-null.  I've fixed the database update to handle this, however the failing tests expose the lack of treatment for this in t/lib/TestBuilder. We'll need that resolving before this can work it's way through QA.

I'm not sure whether we should be adding this handling to TestBuilder (i.e. to always ensure there's 7 rows per branch for any time a branch is added in testing), or whether really we should not be making this assumption in the first place and instead we should be allowing for missing rows in the library_hours table.
Comment 105 Martin Renvoize 2024-02-12 16:31:41 UTC
Coming back to comment #88

> and it would be great to have a fallback to default for opening hours so you could set at the global level and override at the branch level.. like many of our other options of this style.

I think this is the resolution we need for tests.. if we allow library_id to be nullable in library_hours and add a row for each day with default opening hours to fall back to, then handling this in code should mean we can get tests passing more easily.
Comment 106 Martin Renvoize 2024-02-13 09:23:46 UTC
A restless night later and I did a complete about turn.. I came up with a way to defensively code around this instead to protect from bad data ended up in the database somehow.
Comment 107 Martin Renvoize 2024-02-13 10:39:46 UTC
Created attachment 162090 [details] [review]
Bug 6796: Add library_hours table and set opening hours for library

Sponsored-by: Catalyst IT
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 108 Martin Renvoize 2024-02-13 10:39:50 UTC
Created attachment 162091 [details] [review]
Bug 6796: ConsiderLibraryHoursInCirculation system preference

Sponsored-by: Catalyst IT
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 109 Martin Renvoize 2024-02-13 10:39:55 UTC
Created attachment 162092 [details] [review]
Bug 6796: Consider library hours when calculating due date + tests

This feature adds the ability to set opening and closing hours for your
library and for these hours to be considered when calculating due dates
for hourly loans. If the due date for an hourly loan falls after the
library closes, the library can choose for the due date to be shortened
to meet the close time, or extended to meet the open time the next day.
This feature adds a new table 'branch_hours' for storing the open and
close times per day for each library, and a new system preference
'ConsiderLibraryHoursInCirculation' to choose which behaviour should be
followed when calculating due dates.

To test:
1. Apply patches and update database. Upgrade schema if not applying
patch with schema changes. Restart services.
2. Go to Administration -> Libraries. Edit a library and scroll to the
bottom to find the 'opening hours' section. Test adding and removing
open and close times on various days. Confirm saving works as expected.
3. Add a new library and test adding open and close times works as
expected.
4. Edit your default library and save open and close times for each day.
5. Go to Administration -> Circulation and fine rules. Edit a rule, set
the unit to 'hours' and set the loan period to a number that would cause
a checkout to be due after the close time you just set, i.e. if you set
your close time to be 5pm and your system time is currently 1pm, set the
loan period to be 5 (5 hours) so the calculated due date would be 6pm.
6. Go to Administration -> system preferences. Search for
ConsiderLibraryHoursInCirculation. It should be under 'Checkout policy' in
the Circulation system preferences. Confirm the pre-selected option is
'ignore'. Keep this tab open.
6. In a new tab, get the barcode for an item that has an itemtype
matching the circulation rule you just set.
7. Go to the checkouts for a patron that has a categorycode matching the
circulation rule you just set.
8. Check out your item. Confirm that the checkout is due at the end of
the loan period, not taking closing hours into consideration. Return the
item.
9. Back in your other tab, set ConsiderLibraryHoursInCirculation to
'close', so the due date should be shortened to meet the close time.
10. Check out your item. Confirm the checkout is due when the library
closes. Return the item.
11. Back in your other tab, set ConsiderLibraryHoursInCirculation to
'open', so the due date should be extended to meet the opening time.
12. Check out your item. Confirm the checkout is due the next day when
the library opens.
13. Confirm tests pass t/db_dependent/Circulation/CalcDateDue.t

Sponsored-by: Catalyst IT
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 110 Martin Renvoize 2024-02-13 10:40:00 UTC
Created attachment 162093 [details] [review]
Bug 6796: (follow-up) Fix logic for calculating following day's open hours

Since days for branch hours are stored as 0-6 in the database, when it's
a Saturday (6) incrementing the date leads to an error when issuing an
hourly loan because no opening hours are found for the non-existent day
(7).

This patch fixes this by calculating the tomorrow day and setting it to
0 if it's greater than 6.

This patch also corrects the mappings for days, where local_day_of_week
caluclates the date with Sunday first, which put it out of sync with the
database opening hours days.

Sponsored-by: Catalyst IT
Sponsored-by: Auckland University of Technology
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 111 Martin Renvoize 2024-02-13 10:40:04 UTC
Created attachment 162094 [details] [review]
Bug 6796: Fix saving of libraries and tests

Sponsored-by: Catalyst IT
Sponsored-by: Auckland University of Technology
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 112 Martin Renvoize 2024-02-13 10:40:09 UTC
Created attachment 162095 [details] [review]
Bug 6796: Don't add a day if hourly loan period pushes due date

If ConsiderLibraryHoursWhenIssuing is set to shorten the loan period to
the closing time, if the loan period initially pushes the due date to
the following day, the day still gets added when calculating the due
date. We simply need to hardcode the due time here as the due day is the
same as the issue day.

We only need to calculate a due date if ConsiderLibraryHoursWhenIssuing
is set to extend the loan period to the next opening day, as we'll need
to consider holidays/closed days.

Sponsored-by: Catalyst IT
Sponsored-by: Auckland University of Technology
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 113 Martin Renvoize 2024-02-13 10:40:13 UTC
Created attachment 162096 [details] [review]
Bug 6796: Updated schema files

Sponsored-by: Catalyst IT
Sponsored-by: Auckland University of Technology
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 114 Martin Renvoize 2024-02-13 10:40:18 UTC
Created attachment 162097 [details] [review]
Bug 6796: (follow-up) QA test tool fixes

Sponsored-by: Catalyst IT
Sponsored-by: Auckland University of Technology
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 115 Martin Renvoize 2024-02-13 10:40:22 UTC
Created attachment 162098 [details] [review]
Bug 6796: (follow-up) Displaying library hours

This patch shows the set library hours on a library's view page.
Still to be done:
- show on the Libraries summary page
- write tests
- consider time format settings
- fix translateability of day names in Libraries summary page

Sponsored-by: Catalyst IT
Sponsored-by: Auckland University of Technology
Sponsored-by: PTFS Europe
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 116 Martin Renvoize 2024-02-13 10:40:26 UTC
Created attachment 162099 [details] [review]
Bug 6796: Fix api specifications and embed hours in librarly management

Sponsored-by: PTFS Europe
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 117 Martin Renvoize 2024-02-13 10:40:30 UTC
Created attachment 162100 [details] [review]
Bug 6796: Fix display of libraries table

Sponsored-by: PTFS Europe
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 118 Martin Renvoize 2024-02-13 10:40:35 UTC
Created attachment 162101 [details] [review]
Bug 6796: Take CalenderFirstDayOfWeek and TimeFormat into account

This patch adds accounting for the `CalendarFirstDayOfWeek` and
`TimeFormat` preferences in the libraries management pages.

We respect CalendarFirstDayOfWeek for both input and display, but we
only respect TimeFormat for input at this time. We need a new template
helper and js helper for formatting just time in the appropriate format,
currently we only have such formatters for full datetimes.

Sponsored-by: PTFS Europe
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 119 Martin Renvoize 2024-02-13 10:40:39 UTC
Created attachment 162102 [details] [review]
Bug 6796: Merge and correct database update

Sponsored-by: PTFS Europe
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 120 Martin Renvoize 2024-02-13 10:40:43 UTC
Created attachment 162103 [details] [review]
Bug 6796: day_of_week representation fix

The code here was flawed, DateTime returns a 1-7 indexed array with the
first day being Monday, Koha expects an 0-6 indexed array with the first
day being Sunday.

Sponsored-by: PTFS Europe
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 121 Martin Renvoize 2024-02-13 10:40:47 UTC
Created attachment 162104 [details] [review]
Bug 6796: Code defensively for if library hours are not set

Sponsored-by: PTFS Europe
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 122 Martin Renvoize 2024-02-13 10:41:49 UTC
OK.. I think this is 'good enough' at this point.  There are follow-ups I'd like to see so I'm going to post bugs for them and link them to here.. but I think the basic feature all works and this is a long long overdue piece of work.  We have a foundation to build upon now.
Comment 123 Ray Delahunty 2024-02-13 10:55:18 UTC
I see the status is Passed QA but I'd love to see the changes this bugfix offers... in action. Is a test plan possible so a signoff can be tried in a sandbox? I note the test plan in Comment 109 has 'Apply patches and update database. Upgrade schema if not applying patch with schema changes. Restart services'. I don't see an 'Update database' as an Action. I see Apply patches, Refresh schema and Restart services. Is this one not appropriate for a newbie Signoff-er?
Comment 124 Martin Renvoize 2024-02-13 11:05:29 UTC
I don't believe this should be pushed without the follow-up bug worked on.. it is confusing to the end user as it stands without any meaningful link the the UI to closed days from the calendar.

There is a proposal on how to achieve such a link at the data level such that such display and manipulation would be more future-proof and easy to implement in my opinion.. however the mere mention of that idea was immediately dismissed in chat so I'm stepping away again.