Refactor reminders for draft challenge requests

[chrisvanrun]

Closes #4721

Unlike the follow-up date, the draft ChallengeRequest reminders is operating on a time window which doesn't quite work.

Thanks to Miriam for spotting this:

If the request was made on 21-4 then there is a check on 1-5 (too young: 10 days), on 14-5 (23 days: reminder), and then on 1-6 (40 days: too old).

Instead of breaking my head over a window date range that might work, I simplified things with a straight forward counter.

Frequency of them being sent is the frequency of the task, unchanged and is every 1st and 14th of the month.

It also reduces the age at which reminders starting to get sent to a minimum of 7 days.

Any current requests still in draft will receive a few extra reminders for the next few weeks. Which IMHO is fine. I intentionally did not add a migration step to counter that.

[jmsmkn]

This field should not be editable.

[jmsmkn]

The task is not a singleton so it would be more robust to do this with a single update:

requests = ChallengeRequest.objects.filter(
        status=ChallengeRequest.ChallengeRequestStatusChoices.DRAFT,
        created__lte=now()
        - settings.CHALLENGE_REQUEST_AGE_START_DRAFT_REMINDER_CUTOFF,
        draft_reminder_count__lt=settings.CHALLENGE_REQUEST_MAX_DRAFT_REMINDERS,Expand commentComment on line R248Resolved
    )

for request in requests:
    send_challenge_requests_draft_reminder(challenge_request=request)

requests.update(draft_reminder_count=F("draft_reminder_count") + 1)

[chrisvanrun]

It is superior in that it would prevent an additional DB query per reminder! I am not sure why you would say it is more robust? Any failure would be caught by the transaction wrapper, and doing the update in a single call wouldn't catch other spawned (same) tasks to also send the email afaik.

[jmsmkn]

The task is not a singleton, so n-tasks can run at the same time. Updating the value in python takes the read value and increments it. If n tasks run at the same time, they read draft_reminder_count as 0, and then all set it to 1. But n emails have been sent. Using F(...) does the update in the database, so the counter gets updated correctly. See https://docs.djangoproject.com/en/6.0/ref/models/expressions/#f-expressions

[chrisvanrun]

Aaaaah! The increment is applied at the commit of a transaction if using an F() construct, and a pure increment in that it's not calculating the new value during the transaction but only at the time of commit in the DB. Yep, way robuster!

Today I learned something new! Thank you!