Problem/Motivation

A declaration progresses through various states depending on certain conditions in a fairly complex way, however the mechanism has various gaps. The list below could be split into separate issues, however I've grouped them as one for now as they are related, plus perhaps we're not going to do the work, so it's more time efficient for now just to raise one issue.

  1. We allow staff to edit a recent declaration for the duration of STABILITY_WINDOW, but only if no confirmation was needed. We could wait for the stability window to expire before sending the confirmation using cron, so the stability window is available in all cases.
  2. Create a way to tell which declarations might have been used for claims. Previously I thought that we could use isClaimable(), on the basis that once it became TRUE, it never went back to FALSE. However now we have confirmations, that assumption no longer applies.
  3. Create an intelligent retry for sending confirmations using cron. Likely we currently retry every time the declaration is edited without limit. Realistically we'd rather something like a retry based on elapsed times, with a limit. Ideally we would distinguish temporary vs permanent error responses, but the Symfony Mailer library doesn't easily expose that.
  4. Create events so custom code can respond to key transitions, e.g. send a reminder email when a declaration ends. Currently perhaps code could use cron and try to find an SQL query that did the job, but that seems a horrible, inefficient and possibly totally flawed way of substituting for an event😃.
  5. Store the state/status in the database to allow efficient bulk querying. The isXXX() helpers, plus getState() are fine to evaluate the state of a single declaration. They are highly inefficient to search in bulk for declarations that much certain conditions - it means loading all declarations then calling the functions on each. In some cases we can fix the state in postSave(), but in other cases the state is date-dependent, so we would need to use cron to fix it. (Before we tried a computed field, but there were concerns over caching - or concerns over performance if we disable caching).

Proposed resolution

Several of the solutions above require cron. Before I was resisting that, as I felt it was complicated. Now I becoming attracted to it - true it takes a little code to set up, but once we have it we can eliminate various other difficulties.

Remaining tasks

User interface changes

API changes

Data model changes

Comments

adamps created an issue. See original summary.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented

I've worked with state a lot in commerce and registration. And I've come to hate it. I could rant at some length about it's evils. The only time when it's merited I think is it's really important to allow humans to edit it, and even then I'd rather seperate human choices, system known facts, and calculated state built from combining the two.

I absolutely love the stacked, atomic, deterministic, precisely articulated, dynamically calculated so always up to date, logic we've built here. It's so much easier to reason about and have confidence in. For this compliance-sensitive complex logic we're building it seems awesome to me.

The mechanism has various gaps

Let's fix them.

we send a confirmation before the declaration is locked, so staff can alter critical fields

Good catch, I overlooked that isConfirmationSent() had got dropped from isLocked(), so I added a review point on that to #3537036: Written confirmation. I think that solves the problem well enough.

we don't know which declarations might have been used for claims in some cases (if multiple confirmations were sent)

We have discussed having a mayHaveClaimed() method on DeclarationInterface. You suggested to leave it off for now as not yet needed, better to add later when we had a real need for it. My basic idea was that if a declaration ever got into a state where it was claimable, it was permanetly marked as mayhaveclaimed. But I was also interested in the idea that sites could adopt a more sophisticated approach, actually marking declarations when they ran reports to build claims. Al out of scope for now.

we don't have an intelligent retry for sending confirmations (likely we retry every time the declaration is edited without limit);

Fair, but a hard problem to know the right solution for and I don't see how state helps much. #3568837: Create a UI to allow staff to override whether to send a confirmation seems pertinent.

we don't have convenient way for custom code to respond to key transitions (e.g. send a reminder email when a declaration ends).

Create a true database field for "state", which is therefore highly efficient for database queries

I agree that these are both valid concerns.

The querying one falls into the whole basket of 'reporting', which I know will have to have a least a little work done for it, but so far I've been happy to cross that bridge when we come to it. It may be true that one or more state-like fields in the database become necessary. But I'd prefer these to be crystallisations of the real calculated truth from the methods, precalculated for convenience and updated in preSave, without getters of their own, just query helpers really.

If someone wants to respond to time-based transitions, I'm happy for them to do the work of querying for what might have recently changed, it's way outside the 90% use case.

adamps’s picture

adamps commented

Thanks. OK, then let me rephrase this without reference to state_machine. This issue contains some potential problems that still exist even if my first idea for a solution isn't good.

adamps’s picture

adamps commented
Issue summary: View changes
jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented

It's good you captured the issues here. Let's solve them when/if we need to, but having them all here in one place will help us find the right solution when/if we need to.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented
Status: Active » Postponed