using Daystamp when considering deadline
#510
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
theospears
approved these changes
Nov 10, 2024
As documented in beeminder#17, prior to Commit 5f3278c the app's built-in Timer did not account for deadlines when submitting datapoints. While the fix covered many of the possible deadlines, it missed out on the deadlines extending 'end of day' by up to an hour. Rather than duplicating the logic to determine the daystamp on the timer, this fix uses the Daystamp entity introduced in beeminder#430, which is meant to deal with this very thing - given a submission date and a deadline, calculate which calendar date shall be reported when adding the datapoint. Fixes beeminder#228
The add datapoint section of the goal screen contained logic for determining the stepper's initial value for a goal. It defaults to today with adjustments made for a goal's deadline. It contained the same logic as found on the Timer screen and contained the same logic error of not accounting for deadlines within the first hour after midnight. This fix replaces the Goal screen's duplicated logic accounting for deadlines with logic already present in the dedicated entity Daystamp. Possibly related: beeminder#473
the client has the data and need not rely on the backend to use its magic around which year and month might be associated with a lone day
Today View extensions have been deprecated in iOS with iOS 14 and the one in this app is expected to be removed before long. Nonetheless, for completeness and consistency, the fix has been applied to the extension of Today View as well: using Daystamp to calculate the day to be reported having accounted for the deadline
krugerk
force-pushed
the
bugfix/17-built-in-timer-does-not-respect-beeminder-end-of-day
branch
from
7f2326c to
71b4f52
Compare
theospears
approved these changes
Nov 11, 2024
krugerk
deleted the
bugfix/17-built-in-timer-does-not-respect-beeminder-end-of-day
branch
krugerk
commented
Nov 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Daystampcontains the logic for calculating the date (all of year, month, and day) that should be reported as the daystamp of a newly created datapoint, including accounting for the goal's deadline.This merge request uses Daystamp and this feature of it to replace the three instances of other areas in the app tackling this same calculation: in Today View extension, in Goal / add datapoint, and in Timer.
Furthermore, three places with their own implementations all suffered the same logic error of not accounting for certain deadlines correctly - in particular deadlines just after midnight (midnight until 1 in the morning):
If there was not a whole hour of difference, then the deadline was effectively ignored.
Fixes #17
Fixes #228
--
There have been many reports of the app not handling deadlines well or properly. Commit 5f3278c introduces logic for handling the deadline in the Goal/add and Timer screens.
Other, possibly related issues: #17, #35, #228, #450