Problem/Motivation

Since on the Track existing entities form and --since on cn:track-existing move the cutoff of the tracking run. Maximum lateness governs what cron still sends. The two are separate, which is defensible — but the UI text says otherwise, and the README's rationale for the cutoff carries the pre-lateness model.

1. The form description is wrong. TrackingBackfillForm line 85:

Also track combinations whose moment lies at or after this time, even where cron would consider them too late - they are sent on the next cron run. Leave empty to track only what cron would still send: moments from now on, less the template's maximum lateness.

They are not sent. NotificationDispatcher::isTooLate() reads $effective->maxLateness and nothing else — it has no way of knowing that the row was created by a run with an explicit since — so every row the catch-up created beyond the lateness is closed on the first cron run that reaches it, marked like a sent one, with the "is too late for" notice.

2. The Drush option gets it right, which is what makes the pair confusing. TrackingCommands line 40 says only "even where cron would consider them too late" and stops there. Two descriptions of one parameter, one of which promises a send.

3. The README's reason for the cutoff is the pre-lateness one. Line 139:

A moment further back is skipped rather than tracked, because a tracked row that is due is sent on the next run however old its moment is.

That was true before Maximum lateness existed. It is still true for a template that sets none — where the cutoff is now anyway — and false for every template that does, which is exactly the configuration where a catch-up is interesting. So the sentence explains the cutoff with a consequence the same release removed.

The practical damage is a silent no-op that reads as success. Someone catching up through the UI sees "Tracked 13 existing entities", expects the mails, and gets none; the only trace is a notice per row in the log, which is not where they are looking.

Steps to reproduce

  1. Give a template with a time-based condition a Maximum lateness of 1 day, and make sure entities exist whose moment came due weeks ago.
  2. Open Tracking › Track existing entities, pick that template, set Since to three months back, submit.
  3. The form reports how many combinations were tracked — 13 in the run measured here.
  4. Run cron, then look for mails.

Measured: 13 rows tracked, 11 "is too late for" notices on the next cron run, zero mails. The two rows left unsent were the ones with a moment in the future.

The same through Drush, whose help does not promise the send:

drush cn:track-existing <id> --since="-100 days"
# Template N: 17 tracked; 4 skipped as before the cutoff, …
drush cron
# 6 further "is too late" notices, nothing sent

Proposed resolution

The straightforward fix is to make all three texts say the same true thing, and to say what the user then has to do:

  • The form description: replace "they are sent on the next cron run" with what actually happens — that the template's Maximum lateness still decides whether they go out, so a catch-up that is meant to deliver needs the lateness raised as well.
  • The README: give the cutoff its real reason. It is not "a due row is sent however old it is" any more; it is that a moment before the cutoff is a moment the template has already declared not worth sending, so tracking it would only create a row for cron to close.
  • Optionally the form itself: a hint next to Since when the selected template has a lateness shorter than the chosen Since reaches back — the one moment where the mismatch is visible before it costs anything.

Remaining tasks

  • Decide whether the optional form-level hint is wanted.
  • Patch the three texts.
  • A test asserting that a run with an explicit since reaching past the lateness produces rows that the next cron run closes without sending — the behaviour is right and currently rests on nothing.

User interface changes

The description under Since on the Track existing entities form, and possibly a warning there when the chosen Since reaches further back than the template's Maximum lateness allows.

API changes

None.

Data model changes

None

Comments

tkasecker created an issue.

daniel.pernold’s picture

daniel.pernold commented

Checked against the current 2.0.x (after alpha28): the analysis still holds — the description under Since, the Drush option and the README read as described. Two findings on top:

  • The catch-up does not only do nothing, it closes the combinations. Cron closes a row that is too late with markSent(), and trackExisting() upserts with $reset_sent = FALSE. A second catch-up after raising the Maximum lateness therefore no longer reaches these rows; they can only be recovered one by one on the Tracking tab, by clearing the sent flag or deleting the row.
  • Two more texts carry the same promise: the README section on Track existing entities ("knowing they go out on the next run") and the help topic conditional_notification.conditions ("a moment further back would be sent stale on the next run" — the rationale of README line 139 again).

Also, the lateness is not a property of the template alone: overrides carry their own max_lateness, so the effective lateness differs per entity. A hint on the form can only approximate it, and it does nothing for Drush.

The options as I see them:

  1. Fix the texts and add the test, as proposed — including the two places above and a line on how to recover rows that were already closed. The form description and the help topic are translatable, so their strings change on localize.drupal.org as well.
  2. The optional hint on the form — not needed with 3.
  3. Let the backfill respect the lateness with an explicit since as well. trackExisting() takes the later of since and now less the effective lateness, per entity, overrides included; a template without a lateness behaves as today. What this skips becomes a TrackingOutcome of its own (e.g. "beyond the maximum lateness"), counted and reported by the form, Drush and the log line with "raise the maximum lateness to catch these up". No row is created any more that cron would close and mark as sent, and the result is visible right away instead of only in the log. TrackingBackfill::cutoffOf() should use the same cutoff, so the decision between a full and an incremental run stays consistent. No schema change; the new enum case goes under "Changed".
  4. Make the promise true instead: an explicit since waives the lateness for the rows its run creates. That needs a marker per row — a column in the tracking table plus hook_update_N() — that isTooLate() reads and that a moved watched date clears. It is the bigger change and touches the lateness model, but it spares the detour 1 + 3 leave: raise the lateness, wait for cron, lower it again, while the higher lateness also applies to every ordinary save in between.

Would 1 + 3 be a viable way for you? Or do you see a use case that calls for 4 — catching up beyond the lateness without touching the template?