Problem/Motivation
The logic in Registration::preSave() executes before hook_ENTITY_TYPE_presave(), which means if you set the state to 'complete' in a presave hook, then no completed timestamp will ever be saved, and RegisterForm::useAdvancedForm will throw an error when it tries to format a NULL datetime.
It's debatable whether the problem here is that it's too easy to end up with no completed timestamp, or that RegisterForm is not defensive enough.
I can see an argument that setting the completed time in Registration::preSave() is not optimal, and it would be better to have a Registration::setState() method and make that responsible for the completed time logic (the setter could be called from ::preSave() as a backup too).
Proposed resolution
Make RegisterForm more defensive.
Remaining tasks
None
User interface changes
None
API changes
None
Data model changes
None
Issue fork registration-3412904
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
jonathanshawComment #8
john.oltman commentedThanks for the post, another good catch @jonathonshaw! I could not take your MR because it is possible for a canceled registration to have a completed timestamp if it was completed and then subsequently canceled. Also, it is not valid for a registration to be completed and not have a completed timestamp. As you point out in your issue description, entity Presave is not a good place for the timestamp logic because other modules may have presave hooks or presave subscribers that set the state after it. So I moved the logic into a low priority presave subscriber - this event is triggered by registration storage, right before the actual db save is going to occur, so it runs after any presave hooks. I added a test and confirmed it failed when the logic was in the entity preSave function.
I committed the fix to dev branch - since I just did a release recently, I'm going to see what other issues arise and then do a release in early February.