[UICHKOUT-451] Closed Library Due Date Management Created: 09/Oct/18 Updated: 17/Nov/20 Resolved: 18/Mar/19 |
|
| Status: | Closed |
| Project: | ui-checkout |
| Components: | None |
| Affects versions: | None |
| Fix versions: | None |
| Type: | Story | Priority: | P2 |
| Reporter: | sthomas (Inactive) | Assignee: | Unassigned |
| Resolution: | Done | Votes: | 0 |
| Labels: | check-out, core, epam-vega, loan_rules_policies, loans | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original estimate: | Not Specified | ||
| Attachments: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Issue links: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Sprint: | EPAM-Veg Sprint 11, EPAM-Veg Sprint 10, EPAM-Veg Sprint 7, EPAM-Veg Sprint 8, EPAM-Veg Sprint 5, EPAM-Veg Sprint 9 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Story Points: | 3 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Development Team: | Vega | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Description |
|
Purpose: As a library staff member who administers loan policies, I want to ensure that FOLIO can determine, at time of checkout, a revised due date timestamp (date+time) for a loan when the associated service point is scheduled to be closed at the system-determined due date originally defined by the associated loan policy. The system needs to calculate due dates and due times differently, depending upon the option chosen in the following scenarios, and in concert with other settings configured in the loan policy. In general, while the system needs to affix both a due date and due time to a loan, the logic behind the due time is different depending upon whether the loan is:
Short-term loans ('a' above) need to ensure that, for the pre-calculated due date/time, the service point schedule is considered both on the specific day and at the specific time in determining whether the service point is open and, if not, how to apply the Closed Library Due Date Management configurations appropriately in determining due date and due time. Long-term loans ('b' above) need only to ensure that the service point has any open hours for the pre-calculated due date and, if not, will apply the Closed Due Date Management configurations appropriately in determining due date. The Resource Access SIG has decided that the due time for long-term loans should be pre-set to 11:59:59PM* for the due date (for the purpose of calculating overdue statuses and applying fee/fine calculations). Scenarios: Long-term loans:
2. Scenario
3. Scenario
*Note: Should be the equivalent of 11:59:59PM for the tenant's local timezone configuration once that functionality is available. 4. Scenario
Short-term loans:
6. Scenario
7. Scenario
Additional Loan Policy Configurations which affect Due Date and Due Time Calculations:
2. Scenario (Closed Library / Fixed due date - Conflict)
(NOTE:
|
| Comments |
| Comment by Cate Boerema (Inactive) [ 10/Oct/18 ] |
|
Hi sthomas. This looks great! Just a few thoughts:
Thanks!! |
| Comment by sthomas (Inactive) [ 15/Oct/18 ] |
|
Removed exception condition for 'Skipped closed hours in intervening period' as RA-SIG requested this planned functionality be removed from the Loan Policy calculations for due date/time. |
| Comment by sthomas (Inactive) [ 01/Nov/18 ] |
|
Blocked on incorporating new select-list options for 'Closed library due date management' |
| Comment by sthomas (Inactive) [ 01/Nov/18 ] |
|
Updated description/scenarios to incorporate new requirements for context-based select-list for 'Closed library due date management' [UICHCKOUT-452] |
| Comment by Marc Johnson [ 12/Nov/18 ] |
|
sthomas Cate Boerema Jakub Skoczen These might sound like silly questions, but I want to confirm my understanding of this change. At the moment, the circulation API, which determines due date upon check out or renewal, is unaware of the tenant locale and so is timezone agnostic (
In effect this mean that long term loans are intended to end at the end of the day. Is this the end of the day in the chosen locale of the tenant? How are service point opening times defined, are they a tenant locale specific time (e.g. 08:00-5:00) or as text that needs to be interpreted in the relevant locale (e.g. 8:00 only}? |
| Comment by Cate Boerema (Inactive) [ 12/Nov/18 ] |
|
FYI, we have a separate timezone setting in FOLIO. It was implemented at the request of Jakub Skoczen and other developers because it was needed for logical consistency. Check out the scenarios on
|
| Comment by Marc Johnson [ 28/Dec/18 ] |
|
sthomas I'm starting to do a code review on
1. In the above scenarios. When it states:
Does that mean, when an item is checked out to a loanee at service point with ID SPID-1? And so all of these calculations are in reference to the service point where the item is checked out at? 2. Is the UI being changed to display the time part of the due date? 3. If so, given the decision:
and the scenarios steps similar too (example from scenario 2)
How has it been decided which time zone the due date should be relative to, or is it always UTC? (Oleksii Maksymov or Kostyantyn Khodarev maybe you help me with this question too) I'll try to expand upon this with a concrete example (based upon scenario 2 above): Given a library operating in eastern time (set in the reference UI in {{Settings ... Organization ... Language and localization }})
and the only defined rolling loan policy
states moving the due date to the previous open day when the library is closed
and applies to all loans (via a fallback policy in loan rules)
and a service point which is closed each Saturday and closes at 17:00 on Friday
When an item is checked out at that service point
and the calculated due date would be on a Saturday in eastern time
Then the due date of the loan should be moved to 17:00 on the previous Friday in eastern time
Is that example sensible? In particular, are the expectations about eastern time correct? Should it make reference to the chosen time zone of the organisation (and that all of these rules should be applied relative to that) or should it always be UTC? I'm aware that this likely sounds like I'm asking an obvious question. I think I've struggled previously to express why this is really important and now that I've been asked to review the implementation, I need to understand these decisions more fully to determine if it behaves in the expected way. I hope my expanded thoughts help with this. If my question does not make sense or does not seem relevant, please help me to understand better by providing me with some feedback. |
| Comment by Oleksii Maksymov [ 28/Dec/18 ] |
|
Marc Johnson During the calculation of the date (cast to UTС), I was guided by data from https://dev.folio.org/faqs/explain-dates-times/ |
| Comment by Marc Johnson [ 28/Dec/18 ] |
|
Oleksii Maksymov Ok, thank you. By cast, does that mean convert to UTC from other timezone if needed? So I can check my understanding Given a library has chosen to operate in America / New York time zone (which is currently Eastern Standard Time, -5:00 from UTC) From what you said above, that would mean that the calculated due date would be moved to 18:00 UTC (as it after the closing time of the service point where the item was checked out). That would then be presented as 13:00 EST in the UI. Is that a sensible understanding of what the changes would do if they were used now? (My concern is that developer guidance may have been written when the backend APIs could be ignorant of time zone or locale, as operations were not relative to times defined without a time zone, and this feature be the first where that is not the case, I'm hoping my expanded example above can clarify if that is the case) |
| Comment by sthomas (Inactive) [ 28/Dec/18 ] |
|
Marc Johnson Re: 1. In the above scenarios. When it states: Yes, that was the intent. The loan is service-point based and uses the closed library due date management (CLDDM) logic, in conjunction with the calendar API responses to determine open=Y/N and fetch appropriate previous/next open service point hours as appropriately set with CLDDM in the loan policy. |
| Comment by sthomas (Inactive) [ 28/Dec/18 ] |
|
Marc Johnson Re: 2. Is the UI being changed to display the time part of the due date? I'm not sure whether your question is about a) changing behavior or b) changing field values. It's my understanding that the UI currently includes a full timestamp for due dates that includes both date+time. As part of this issue, there is no change being made to affect this configuration. We will be changing the value stored based on due date/time response. The intent is, essentially, 'one minute before midnight' on the due date (for long term loans). How that is governed by (and display adjusted according to), system-defined time zone settings, I'll let Oleksii Maksymov and Kostyantyn Khodarev respond. There have been discussions in the RA-SIG about display of due time in patron-facing environments. The desire has been to display a due date +time for short-term loans (rolling loans with loan periods having 'hours' or 'minutes' intervals), but only a due date for long term loans (although they still wish for the 'one minute past midnight' due time to be associated with the loan). We've not covered any of this desired behavior in the scope of this story. |
| Comment by Marc Johnson [ 28/Dec/18 ] |
|
sthomas Thanks for your answers.
Apologies, that was my incorrect understanding that the UI didn't display the time part at the moment.
Is that one minute or one second before midnight, the example in the issue description the due time for long-term loans should be pre-set to 23:59:59 suggests to me it is one second? My understanding from that is one minute before midnight on the due date, in the tenant chosen time zone. For example, if a long term loan is due on the 10th February 2019 and the tenant chosen time zone is America / New York (and so EST), then the expected due date displayed in the UI would be 10th February 2019 23:59:59 EST. Is that a sensible interpretation? (this would be stored as 11th February 2019 04:59:59 as all date-time values are stored in UTC) This is not the current behaviour of the system (it uses the current time that the item was checked out, see
And short term loans that are subject to closed due date management should also respect the chosen timezone (e.g. due date time should be the closing time in the chosen time zone, and not necessarily UTC)? Would it be reasonable for me to summarise this as all loan due date and times should be relative to the chosen time zone of the organisation? My interpretation of Oleksii Maksymov earlier response is that the implemented decisions and calculations (in the pull request for
If both of those interpretations are sensible, does the behaviour need to change to respect the chosen timezone before this change can be accepted? |
| Comment by Oleksii Maksymov [ 28/Dec/18 ] |
|
Marc Johnson if I understand correctly, then to calculate the due date time should be the closing time in the chosen time zone of the library location. |
| Comment by sthomas (Inactive) [ 28/Dec/18 ] |
|
Marc Johnson Re: Is that one minute or one second before midnight, the example in the issue description the due time for long-term loans should be pre-set to 23:59:59 suggests to me it is one second? Yes, apologies, 1 second before midnight. |
| Comment by Marc Johnson [ 29/Dec/18 ] |
|
Oleksii Maksymov I don't think I know the answer to those questions yet, that is what I'm trying to understand. My understanding fits what you describe depending upon a couple of decisions. I'm aware that I'm likely labouring this topic and that could be frustrating to those involved. Apologies if that is the case, I am possibly overly conscious of the impact this could have on the scope of this feature (and others) and in how we conduct testing etc. I know that I've struggled to describe this simply. The easiest way I know how is something like: if we are checking a closing time of 18:00, and the chosen time zone is New York, then should we check that the time is after 18:00 UTC or 18:00 EST?. I'm aware the answer seems obvious (but has significant impact IMO). I think from sthomas comment above that the intent is for all due date calculations to respect the timezone an organisation has chosen, and so in the above statement, it would be 18:00 EST. I would like to confirm:
(I am assuming that distinguishing between short and long term loans and applying the one second before midnight rule are both in scope for this change already) After confirmation of those decisions, I think we can decide where the system defines the chosen timezone. At the moment, stripes-core and the organization settings pages in the reference UI use the configuration backend API to store this information. (Aditya matukumalli Michal Kuklis or Zak Burke please correct me if this is wrong). It appears that this defaults to UTC, and our reference environments do not define these settings (Wayne Schneider John Malconian please correct me if this is wrong). We need to decide if we want the source of truth for an organisations locale and timezone to be the configuration backend API. Jakub Skoczen what are your thoughts on this? If we choose to continue using this mechanism then I think there are a few other changes are involved:
We may also want to consider what mod-circulation should do if either the configuration or calendar APIs are not available. If that seems sensible, I'm happy to go into more detail about my understanding of what could be involved within mod-circulation. sthomas Oleksii Maksymov Kostyantyn Khodarev Jakub Skoczen Does that make sense? |
| Comment by Zak Burke [ 31/Dec/18 ] |
|
That's correct, Marc Johnson. ui-organization uses the configuration API to store the locale and timezone. As to whether, "the source of truth for an organisations locale and timezone [should] be the configuration backend API", what else could it be? In light of that, your follow up points sound correct to me as well:
|
| Comment by Marc Johnson [ 31/Dec/18 ] |
Agreed. Given we don't really have any alternatives at the moment and (from what I can tell) we don't really use it either (as the reference environments use a UI defined default of UTC, and the backend is ignorant). I think what I'm really trying to do is make that decision explicit (and acknowledge and plan the changes we need to enact that, e.g. add dependencies to reflect this, and maybe stop defaulting in the UI and fail instead). |
| Comment by Marc Johnson [ 01/Jan/19 ] |
|
sthomas and I had a call about this yesterday. Below is a summary of my understanding following that conversation. Situation
I believe the most important decision is whether we include timezone support in this change or do it separately soon after? Given the size of this change (~4000 lines) and the pre-requisites (and additional decisions) we likely need for the timezone support, my bias is towards clearing the decks of in-progress circulation changes first. Cate Boerema Jakub Skoczen Does this make sense and fit with your understanding? This likely impacts the 2019 Q1 planning conversations. (There are likely other backend calculations that need to respect the chosen timezone as well). |
| Comment by Kostyantyn Khodarev [ 02/Jan/19 ] |
|
Marc Johnson sthomas Jakub Skoczen Zak Burke |
| Comment by Marc Johnson [ 02/Jan/19 ] |
|
(Assuming my summary from yesterday is sensible) Kostyantyn Khodarev I think we are suggesting very much similar ideas.
Yes, the date time needs storing in UTC (or at least, it is, due to the tools RAML Module Builder uses). Does this mean your opinion is that these changes are intended to work as expected (e.g. end of day determination) when the chosen timezone is UTC (and are unlikely to in other circumstances)? And that the timezone support needs to be considered separately? If so, I think we are agreed that our suggestion is to complete the current work before trying to support choice of timezone.
Agreed. I believe this is the first place where we have needed to support / respect timezone in the backend API.
Agreed, there is a need for some technical analysis, spikes and decisions to decide how we want to do this. There will likely be impact across various areas of the system (multiple backend modules, front end and environment builds).
I believe the front end is already timezone aware, at least in terms of presentation (though our reference environments are unlikely to demonstrate this, as they use the default of UTC). For calculations or rules that care about specific points in a day (e.g. the end of the day, when a service point closes etc) I believe they need to respect the chosen timezone in order to behave as expected (otherwise the end of the day will be wrong, e.g. 23:59:59 UTC is 15:59:59 EST). Given that is has been decided that complex calculations should be performed in business logic modules, I think that means that they need to also support the choice of timezones. |
| Comment by Zak Burke [ 04/Jan/19 ] |
|
The internationalization guidelines state that front-end code should assume the time-related values it exchanges with the backend are in UTC. When a date or time is formatted for display using react-intl, the tenant's timezone will be automatically incorporated. As long as the timezone is provided with every value used in a comparison, any reputable comparison library ought to do The Right Thing and figure out that 2019-01-04T12:13:14+00:00 is before 2019-01-04T07:08:09-06:00. On the front-end I believe we do comparisons with moment, which uses helper functions like isBefore(), isAfter() etc. |
| Comment by Khalilah Gambrell [ 16/Jan/19 ] |
|
Oleksii Maksymov and Kostyantyn Khodarev - should I move this story to status = Blocked? |
| Comment by Khalilah Gambrell [ 29/Jan/19 ] |
|
Tetyana Afanasyeva and Oleksii Maksymov , what PR is tied to this user story? |
| Comment by Oleksii Maksymov [ 30/Jan/19 ] |
|
Khalilah Gambrell This is a basic user story for
|
| Comment by Oleksii Maksymov [ 08/Feb/19 ] |
|
sthomas implementation is available for testing on http://folio-testing.aws.indexdata.com/ |
| Comment by sthomas (Inactive) [ 11/Feb/19 ] |
|
Oleksii Maksymov Kostyantyn Khodarev I tried to do further testing on this story. (See attached long_term_loan_time-errors.png) The long-term loan scenarios define due time for 11:59PM. While I know that we're still working on the local timezone issue, most of the long-term scenarios, as implemented on http://folio-testing.aws.indexdata.com seem to be using the closing time of the service point as the due-time, rather than the 11:59PM time when long-term loans are due. I've moved this story back to In Progress until I can clarify what we should be seeing for these scenarios at this point. On another note, so far, the date component of the calculations has been correct. |
| Comment by Dmytro Tur [ 12/Feb/19 ] |
|
sthomas Given what is said in scenarios 2 and 3 ("due date timestamp should be changed to the latest SPID-1 endTime") and scenario 1 ("due date timestamp should remain unchanged from system calculated due date timestamp") current implementation seems to match the description. |
| Comment by Oleksii Maksymov [ 13/Feb/19 ] |
|
sthomas The diagram (See attached CDLS Long-Tern.pdf |
| Comment by Khalilah Gambrell [ 07/Mar/19 ] |
|
sthomas, what work remains on this story? |
| Comment by sthomas (Inactive) [ 18/Mar/19 ] |
|
This story is essentially a testing issue that acts as an umbrella to define the logic for Closed Library Due Date Management. All front- and back-end stories to implement this functionality have been built, tested and closed. Resolving this issue as done. |
| Comment by Erin Nettifee [ 13/Nov/20 ] |
|
Cheryl Malmborg and Emma Boettcher - it doesn't appear that the short-term loan options here were ever actually developed. If I'm right, do we need new features to get them in the queue? Tagging Andrea Loigman as well. |
| Comment by Cheryl Malmborg [ 13/Nov/20 ] |
|
I just re-tested the short-term scenarios 5, 6, and 7 in UChicago's Goldenrod test db. They all appear to work correctly. I used a 2 hour loan and each of the closed library due date options. They all worked as expected, including when the library closed and re-opened on the same day. Are you seeing some other behavior? |
| Comment by Erin Nettifee [ 16/Nov/20 ] |
|
Hi Cheryl Malmborg - it might have to do with how it's labeled, so I'll need to dig deeper. Duke was expecting to also see:
Andrea had these as four separate options, but I think they're really two, since moving to the end of the current day is moving to the end of the current service point hours and moving to the beginning of the next open service points hours is moving to the beginning of the next open day. So it appears #7, and #1 in the Additional section, weren't implemented. Does that make sense? |
| Comment by Erin Nettifee [ 17/Nov/20 ] |
|
I talked to Andrea Loigman about this yesterday, and it looks like #7 and #1 were implemented for rolling loan policies and not for fixed loan policies. I wonder if there was a reason we chose to do it that way? |