Setting custom start of the day

[KristianTashkov]

Implements #607.

1. Instead of hard-coding 3 AM like the last time this was implemented, I decided to add it as a setting so everyone can set whatever time they desire to start the new day.
2. Tested it through the app, widgets and notifications and checks go to the appropriate day, UI doesn't move to the next day until you pass the custom mark and notifications arrive as usual so they aren't affected by the offset.
3. It was bugging me that widgets don't automatically refresh when the new day starts, so also implemented that in this PR (even if we don't merge this, we should add this to the main app).

As usual waiting for feedback from @iSoron before I start writing tests for the functionality and will be dog fooding this version in the mean time.

[iSoron]

Thank you for the pull request, @KristianTashkov. Unfortunately, this pull request tries to accomplish too many things at the same time, which makes it very hard for me to review it. Could you please:

1. Revert all class and method renames (for example, from MidnightTimer to DayStartTimer and atMidnight to atDayStart). This refactoring can be submitted as a separate pull request after this one is merged.
2. Revert the automatic update at midnight. This can also be submitted as a separate pull request.
3. Revert the changes to the settings screen. This can also be submitted as a separate pull request.
4. I noticed that you replaced most getToday() calls by getToday(true). Instead of doing this, please modify getToday() so that it does whatever getToday(true) is currently doing, then create another method getTodayWithoutOffset() that does whatever getToday(false) does. In general, please try to avoid boolean arguments; they are usually considered a code smell.

[KristianTashkov]

@iSoron Thanks for the feedback!
Reverted anything not required to make a fixed custom start of the day work (hard coded at 3 AM like before).
I opted to name the new methods getTodayWithOffset instead of making current ones getTodayWIthoutOffset. It feels like this is less confusing if someone is trying to work on generic time functions through DateUtil. Technically the opposite argument is true that someone might introduce a new function that doesn't account for offset if working on habit logic, so I'm not sure which one is better. If you disagree with the current variant I can global replace the names easily.

[iSoron]

I opted to name the new methods getTodayWithOffset instead of making current ones getTodayWIthoutOffset.

Thanks, that's fine. I will review it soon.

[iSoron]

I am getting an exception after the following steps:

1. Create a daily habit with a reminder at any time of the day
2. Set your system time to 1:30 AM, a day in the future; make sure that the notification appears
3. Click "YES" on the notification

The exception is below:

    java.lang.IllegalArgumentException: timestamp is not valid
        at org.isoron.uhabits.intents.IntentParser.parseTimestamp(IntentParser.kt:56)
        at org.isoron.uhabits.intents.IntentParser.parseCheckmarkIntent(IntentParser.kt:36)
        at org.isoron.uhabits.receivers.WidgetReceiver.onReceive(WidgetReceiver.java:77)
        at android.app.ActivityThread.handleReceiver(ActivityThread.java:3379)
        at android.app.ActivityThread.access$1200(ActivityThread.java:199)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1661)
        at android.os.Handler.dispatchMessage(Handler.java:106)
        at android.os.Looper.loop(Looper.java:193)
        at android.app.ActivityThread.main(ActivityThread.java:6669)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)

If we remove the check that causes the exception, the app does not crash, but the checkmark is added to the incorrect date. Before fixing this crash, could you try to write an automated test to detect it?

[KristianTashkov]

Hey @iSoron did you change anything before getting this exception, because I can't reproduce it.
I hit this problem while developing the branch but have since accounted for it. It happens when you apply two WithOffset functions to the same timestamp, essentially moving it back two days. For notifications there are two possible places where that can happen and I left one of them intentionally to use the regular without offset function to account for that:

1. ReminderScheduler.java:124 where we do use with offset function to set the correct timestamp for the notification
2. CheckmarkIntentData:53 where we don't use with offset because the timestamp already shows the correct day it should be used for.

To get the behavior you explained it means the second location also applies the offset and it shouldn't.

[iSoron]

@KristianTashkov Never mind, it seems like I accidentally showed the notification, then set the clock to a time before the notification was supposed to appear. This is an issue, but it's not related to your patch. The PR looks fine now, but I would like to test it a bit more around changes in daylight saving times, just to make sure nothing weird happens. I will keep you updated.

[iSoron]

Merged! Thanks again, @KristianTashkov. Regarding the changes that I asked you to revert:

1. Please feel free to submit a PR to rename MidnightTimer to DayStartTimer
2. Regarding the settings, I don't think that level of granularity is necessary. I would prefer a simple boolean setting "delay start of the day" which sets the offset to something reasonable, such as 3am.

[KristianTashkov]

2. Reasonable is subjective. if we are going to show a setting at all, might as well let people set it to whatever they want, the code already supports it. If you are interested in that PR I can open it, otherwise I'm going to focus on some other features.

[Mrnofish]

2. Reasonable is subjective. if we are going to show a setting at all, might as well let people set it to whatever they want, the code already supports it. If you are interested in that PR I can open it, otherwise I'm going to focus on some other features.

Definitely.

I'd say e.g. people who work at night may have a different idea of what is reasonable compared to most.

[kmantel]

2. Reasonable is subjective. if we are going to show a setting at all, might as well let people set it to whatever they want, the code already supports it. If you are interested in that PR I can open it, otherwise I'm going to focus on some other features.

Definitely.

I'd say e.g. people who work at night may have a different idea of what is reasonable compared to most.

Strongly agree, considering I'm posting this at about 4:30 AM. Some other day-based software (the example I'm aware of is Anki https://github.com/ankitects/anki) have the option to reset a day at any hour, which lets me use them effectively.

A cursory glance at the code makes it seem like it wouldn't require major changes to support any time

uhabits/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/utils/DateUtils.kt

Lines 37 to 38 in 856a072

	private var startDayHourOffset: Int = 0
	private var startDayMinuteOffset: Int = 0

and that it may only take an interface/preference change

uhabits/uhabits-android/src/main/java/org/isoron/uhabits/HabitsApplication.kt

Lines 73 to 77 in 856a072

	if (prefs.isMidnightDelayEnabled) {
	setStartDayOffset(3, 0)
	} else {
	setStartDayOffset(0, 0)
	}

I'm not familiar enough with Android development to know, but is that correct?