Closed matthijskooijman closed 3 years ago
Here's an overview of all the possible text that can be shown. Note that this includes situations that cannot occur with the current code or even do not make real sense (i.e. article content will probably always be required), but I've added them all to allow future expansion.
Fixed all comments, I think this should be ready to merge now (after squashing the fixup commits).
I changed the checks slightly, so an article body with tags, but with an element like img
or iframe
is also acceptable. For the other fields (excerpt, sources and method), text is still required.
I squashed the fixups and rebased on master, merging next.
In the DRAFT, REVIEW and APPROVED stages, this shows a checklist of things to do before progressing to the next stage, which helps to quickly see if anything is forgotten.
Some of these things are marked as required, meaning they prevent progress (i.e. starting review or publishing). Others are marked as recommended, meaning they are shown but not enforced. Whether something is required or recommended is hardcoded for now, but the code is writting to allow making this configurable later.
This additionally does some more or less related factorings and small cleanups.
The new functions have very verbose type definitions (intended to be consumed by Psalm), we will have to see if this is workable or simpler types should be used (which allow less checking).
Originally, I had intended for
updateToNewStatus
to also check the checklist and prevent state changes when the checklist was not completed, which would allow removingcanBeSetAsApproved
entirely (just try to set the status toAPPROVED
and if it fails, e.g. because the checklist says more approvals are needed, just ignore that failure). However, it turns out this could create a corner case where removing e.g. all tags after review start would prevent the status from progressing toAPPROVED
, so I ended up only checking the checklist instatusChangeAction
. This results in code that is a bit more messy, but should be correct. @laurensmartina, any suggestions on this?This fixes #304.