Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Period starting date can be set to the past #10

Open
olli-suutari-jkl opened this issue May 3, 2019 · 1 comment
Open

Period starting date can be set to the past #10

olli-suutari-jkl opened this issue May 3, 2019 · 1 comment

Comments

@olli-suutari-jkl
Copy link

When creating a period, it is possible to change the starting date to a past date.
Saving the period also saves it to the database refs.periods.validFrom.

From: libraries-fi/kirkanta-api#4

Changing past schedules is not allowed because users would most likely only screw them up unintentionally. There is no need to create schedules into the past as there is no use-case for that. The only supported case would be for statistics and this is also the reason why users are blocked from modifying the past.

Right now schedule modification apply starting Monday of the running week so that weekly tables could be fixed if there has been an error.

This restriction applies to API v3 as well;
https://github.com/libraries-fi/legacy-kirkanta/blob/master/module/ScheduleGenerator/src/Subscriber/GenerateSchedules.php#L54
https://github.com/libraries-fi/legacy-kirkanta/blob/master/module/ScheduleGenerator/src/Controller/ScheduleController.php#L67

Originally posted by @sjuvonen in libraries-fi/kirkanta-api#4 (comment)

Just accept that for some libraries there will be no schedules into the past, there is no other solution to it. For reference, in Kirjastohakemisto it is not possible to navigate into the past weeks at all.

Originally posted by @sjuvonen in libraries-fi/kirkanta-api#4 (comment)

olli-suutari-jkl added a commit to olli-suutari-jkl/jyvaskyla.fi.kirjasto that referenced this issue May 3, 2019
- Fix the issue where schedules would be initially hidden because it is possible that there are none available for the selected week in kirkanta: libraries-fi/kirkanta#10
@sjuvonen
Copy link
Contributor

sjuvonen commented May 25, 2019

Yea, this is just something not taken into account after fiddling with the schedule generation rules.

But I think TRYING TO validate and block this user misbehavior is again not worth the pain it requires to make everything work properly.

For starters users have to be able to edit an existing period whose start date is (at the time) in the past. The framework we're using does not have any convenient way to access old and new values during validation so it'd be hack-ish to verify that the start date was not changed.

Users also should be able to correct the start date back into the original value if they accidentally change it to a wrong date. Past schedules won't be deleted if users change the start date so this would lead to a scenario where the API reports schedules from past dates belonging to a period X although that period reports its validity to be something else. (This is a gotcha in current implementation as well but more of a documentational issue than operational fault.) This could also lead to confused developers and lots of pointless debugging to chase a bug that never existed.

Period validity is considered public information so users have to be able to fix the dates to what they actually are, not just what kinda-but-maybe-not makes sense on the technical side of compiling period definitions into schedules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants