wagtail
RFC 104: Validation on publish
A frequently-requested feature (see Wagtail issue #12505) is the ability to save draft versions of pages in an incomplete state that would not pass validation, but still enforce validation at the point that the page is published or submitted for approval. This is also a prerequisite for an effective auto-save implementation, as noted in RFC 99. This RFC sets out the requirements for such a feature, and an approach to implementing it.
@tm-kn has raised the important question of whether this only covers text fields, or whether for example it would be possible to leave a foreign key field blank and still save as a draft.
The intention was very much for it to work on all field types, but I now realise there's one aspect where I've fallen into the trap of text-centric thinking: for non-text fields, this fails to satisfy the goal of "it should just work without the site implementor having to specifically accommodate it in their code". When bypassing the Django-level validation, a text field that's been left blank can still be saved to the database (and will be written as an empty string) but this is not the case for non-text fields unless null=True is specified. In other words, we're reliant on developers to remember to define a field as IntegerField(null=True) if they want it to be a required field on publish but still allow saving incomplete drafts (as distinct from IntegerField(null=True, blank=True) which would be a non-required field on publish). I don't think there's a good way around this, though.
@gasman Yeah, I think there’s going to be a lot of edge cases to cover if we went down that path, e.g. database constraints.
Hence I think we should treat the form state as something else entirely and save it to the database as-is, skipping the entire validation chain for the model/form (until the form is actually saved).
How might this impact scheduling publication for a future date? Would validation be run at the moment of scheduling? Apologies in advance as I haven't had a chance to checkout the draft PR to test this use case and don't have the relevant internals loaded into my working memory.
I recently had a situation where I had to programmatically add related fields to a page and schedule publication for a future date (page.save_revision(user=user, log_action=True, approved_go_live_at=future_date)), and it isn't clear to me from this RFC how "validation on publish" might impact such a workflow (whether done programmatically or via the UI).
How might this impact scheduling publication for a future date? Would validation be run at the moment of scheduling? Apologies in advance as I haven't had a chance to checkout the draft PR to test this use case and don't have the relevant internals loaded into my working memory.
I recently had a situation where I had to programmatically add related fields to a page and schedule publication for a future date (
page.save_revision(user=user, log_action=True, approved_go_live_at=future_date)), and it isn't clear to me from this RFC how "validation on publish" might impact such a workflow (whether done programmatically or via the UI).
@jsma That's a good point - within the admin interface, scheduling a page for future publication is done through the "publish" action (even though the button may be relabelled if https://github.com/wagtail/wagtail/pull/12424 goes ahead) and so the validation rules applicable to publishing will be enforced at that point.
If you're scheduling them programmatically instead, things are a bit more subtle since it's bypassing the form validation and entirely reliant on model-level validation - as it stands, Page.save_revision does enforce full validation unless you specifically pass clean=False, so I believe this is handled correctly. It would definitely be worth us adding a unit test to ensure that stays in force, though.
A user experience issue that has arisen while implementing this for snippets: if the field (or fields) used for the model's __str__ representation has been defined in the conventional Django way (blank=False but no required_on_save flag set), it will be possible to save a draft instance with a blank string representation, which will then show up as blank in listings and other places.
I don't think there's any way we can identify those fields to be singled out for special-case treatment, and while it might be possible to implement some heuristics (such as requiring validation on draft if the field is called title or is the only field defined directly on the model), that seems like too much magic.
At least it's an easy problem to spot and correct when it does come up...
We should definitely ensure that anything making use of the string representation (or get_admin_display_title) provides a fallback such as [Foo object]. This would include listings, and log entries (the latter being where this was spotted, since the BaseLogEntry model defaults to populating the label field with this, via the get_instance_title method - and this is a required field that gets validated on save).
I'm also considering the possibility that this might be a big enough user-facing change to justify bumping the version to 7.0, especially if we can get an MVP of auto-save at the same time. (We're probably about due for another major release to clear out all the deprecated features, anyhow.)
Now updated to document additional details encountered while working on the final implementation in Wagtail 7.0: https://github.com/wagtail/rfcs/pull/104#discussion_r1975610395, https://github.com/wagtail/rfcs/pull/104#issuecomment-2695046184. This brings the RFC in line with the final implementation, and so I'll go ahead and merge now.