improvement(set-due-date): simplify for FSRS by removing "set interval to same value"

[david-allison]

Purpose / Description

I wanted to simplify the dialog, especially explaining what "set interval to same value" due to user feedback.

But, it didn't make sense to me for FSRS.

After a sufficiently long discussion with Expertium & Jarrett, we discovered:

• Expertium previously wasn't aware of the ! option
• This was raised as an issue previously: https://forums.ankiweb.net/t/set-due-date-doesnt-update-the-interval-of-card/52646

Fixes

• Fixes Set Due Date: Hide 'set interval to same value' if FSRS is enabled #17707

Approach

When FSRS is enabled, there is no reason for due date to be inconsistent with interval

So, hide 'set interval to same value' and use true

Confirmed by Jarrett and Expertium, paraphrased:

Expertium: when FSRS is enabled, is there any reason for keeping ivl not consistent with due date? Because I'm pretty sure there isn't
Jarrett: Yeah, there isn’t.

https://discord.com/channels/368267295601983490/1282005522513530952/1324758732122624020

How Has This Been Tested?

Logs when FSRS enabled:

16:48:23.999 Scheduler               com.ichi2.anki.debug                 I  updating due date of 1 card(s) to '12!'

When disabled and unticked

16:49:50.330 Scheduler               com.ichi2.anki.debug                 I  updating due date of 1 card(s) to '12'

Ticking the checkbox still works

Unit tests have been added

Screenshots:

Learning (optional, can help others)

Deep dives can improve the UX

Checklist

• You have a descriptive commit message with a short title (first line, max 50 chars).
• You have commented your code, particularly in hard-to-understand areas
• You have performed a self-review of your own code
• UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
• UI Changes: You have tested your change using the Google Accessibility Scanner

[david-allison]

I would like to pay attention to comments on the issue, especially

Regarding the value of the checkbox:

FSRS user:

• No effect on scheduling
• Affects searching/sorting. But neither true nor false would result in correct behaviour. Correcting the sorting/searching requires either
  • changing value of ivl to the exact number of days between the due date and the last review date (43 days in my example above)
  • storing the value of LRD in the cards table, which dae has already agreed to.

I do believe that giving the user no choice, vs giving the user a choice where either outcome is incorrect is the best result

Jarrett agreed

In future, this should be fixed upstream. Hopefully in a manner where there is no need for the checkbox

[mikehardy]

LGTM, +1 on the tests, awesome

[lukstbit]

LGTM
Left one minor nitpick if you agree, merge at will otherwise.

[lukstbit]

I don't see why this method needs to be suspended. If the dialog requires the fsrs status(and it does) then I think this should be made obvious in the api by having the method receiving the value as a parameter. Call sites can do the required work to properly setup this dialog instead of hiding it here.

Would probably be useful for tests as well because we could avoid having to mock getFSRSStatus().

[david-allison]

Maybe it shouldn't be here, but it shouldn't be directly at the call site.

I wouldn't mind a new call, but it feels verbose.

If we moved to the call site:

• Each call site would be executing the same code
• FSRS status is system-supplied. The list of IDs is determined by the caller. The system supplied data isn't useful for the understanding of the code intent: "launch the dialog"

[david-allison]

In terms of tests: we use the arguments, so no mocks are needed