Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default to no formsets when 'formsets' / 'exclude_formsets' not specified #158

Merged
merged 3 commits into from
Mar 14, 2022

Conversation

gasman
Copy link
Contributor

@gasman gasman commented Mar 10, 2022

Previously, if both formsets and exclude_formsets were left unspecified in the class Meta of a ClusterForm, it would default to creating formsets for all child relations defined on the model - this was done for consistency with the pre-Django-1.8 behaviour of ModelForm's fields / exclude options, and was never changed to match the newer Django behaviour. This PR changes this to creating no child formsets instead. This way, a ClusterForm omitting these options is functionally equivalent to a ModelForm with the same set of Meta options.

As a pre-requisite to this, we need to implement the ability to pass nested formsets definitions to be handled by childformset_factory, which was previously left out of #106:

    class BandForm(ClusterForm):
        class Meta:
            model = Band
            fields = ['name']
            formsets={
                'members': {},
                'albums': {
                    'fields': ['name'],
                    'formsets': ['songs'],
                }
            }

Without this, the only way to build multi-level formsets was to leave the inner ClusterForm to automatically 'discover' its child formsets

This is a breaking change and will need an update in Wagtail - standard EditHandler/InlinePanel-based forms are OK (because they specify the formsets explicitly via the EditHandler.required_formsets method), but nested InlinePanels now need to propagate their formsets arguments up the tree via this newly implemented mechanism.

This then makes it possible to specify these in a nested 'formsets' dict in ClusterForm.Meta.
gasman added a commit to gasman/wagtail that referenced this pull request Mar 11, 2022
…s semantics

django-modelcluster 6.0 is planned to include wagtail/django-modelcluster#158, which changes the semantics of formsets/exclude_formsets so that a ClusterForm that specifies neither of them will have no formsets defined, rather than formsets for all child relations. (This corrects an implementation wart in the upcoming EditHandler refactor.) This doesn't affect Wagtail for the most part, because EditHandler passes an explicit formsets option - but nested InlinePanels currently rely on the old behaviour, because modelcluster <=5 provides no way to pass the inner panel's formset definition inside the outer one. We fix this by adding the necessary 'formsets' key introduced in wagtail/django-modelcluster#158.
gasman added a commit to gasman/wagtail that referenced this pull request Mar 11, 2022
…s semantics

django-modelcluster 6.0 is planned to include wagtail/django-modelcluster#158, which changes the semantics of formsets/exclude_formsets so that a ClusterForm that specifies neither of them will have no formsets defined, rather than formsets for all child relations. (This corrects an implementation wart in the upcoming EditHandler refactor.) This doesn't affect Wagtail for the most part, because EditHandler passes an explicit formsets option - but nested InlinePanels currently rely on the old behaviour, because modelcluster <=5 provides no way to pass the inner panel's formset definition inside the outer one. We fix this by adding the necessary 'formsets' key introduced in wagtail/django-modelcluster#158.
gasman added a commit to gasman/wagtail that referenced this pull request Mar 11, 2022
…s semantics

django-modelcluster 6.0 is planned to include wagtail/django-modelcluster#158, which changes the semantics of formsets/exclude_formsets so that a ClusterForm that specifies neither of them will have no formsets defined, rather than formsets for all child relations. (This corrects an implementation wart in the upcoming EditHandler refactor.) This doesn't affect Wagtail for the most part, because EditHandler passes an explicit formsets option - but nested InlinePanels currently rely on the old behaviour, because modelcluster <=5 provides no way to pass the inner panel's formset definition inside the outer one. We fix this by adding the necessary 'formsets' key introduced in wagtail/django-modelcluster#158.
gasman added a commit to gasman/wagtail that referenced this pull request Mar 11, 2022
…s semantics

django-modelcluster 6.0 is planned to include wagtail/django-modelcluster#158, which changes the semantics of formsets/exclude_formsets so that a ClusterForm that specifies neither of them will have no formsets defined, rather than formsets for all child relations. (This corrects an implementation wart in the upcoming EditHandler refactor.) This doesn't affect Wagtail for the most part, because EditHandler passes an explicit formsets option - but nested InlinePanels currently rely on the old behaviour, because modelcluster <=5 provides no way to pass the inner panel's formset definition inside the outer one. We fix this by adding the necessary 'formsets' key introduced in wagtail/django-modelcluster#158.
@gasman gasman merged commit 5e78f77 into wagtail:main Mar 14, 2022
gasman added a commit that referenced this pull request Mar 14, 2022
gasman added a commit to gasman/wagtail that referenced this pull request Mar 14, 2022
…s semantics

django-modelcluster 6.0 includes wagtail/django-modelcluster#158, which changes the semantics of formsets/exclude_formsets so that a ClusterForm that specifies neither of them will have no formsets defined, rather than formsets for all child relations. (This corrects an implementation wart in the upcoming EditHandler refactor.) This doesn't affect Wagtail for the most part, because EditHandler passes an explicit formsets option - but nested InlinePanels currently rely on the old behaviour, because modelcluster <=5 provides no way to pass the inner panel's formset definition inside the outer one. We fix this by adding the necessary 'formsets' key introduced in wagtail/django-modelcluster#158.
gasman added a commit to gasman/wagtail that referenced this pull request Mar 14, 2022
…s semantics

django-modelcluster 6.0 includes wagtail/django-modelcluster#158, which changes the semantics of formsets/exclude_formsets so that a ClusterForm that specifies neither of them will have no formsets defined, rather than formsets for all child relations. (This corrects an implementation wart in the upcoming EditHandler refactor.) This doesn't affect Wagtail for the most part, because EditHandler passes an explicit formsets option - but nested InlinePanels currently rely on the old behaviour, because modelcluster <=5 provides no way to pass the inner panel's formset definition inside the outer one. We fix this by adding the necessary 'formsets' key introduced in wagtail/django-modelcluster#158.
gasman added a commit to gasman/wagtail that referenced this pull request Mar 14, 2022
…s semantics

django-modelcluster 6.0 includes wagtail/django-modelcluster#158, which changes the semantics of formsets/exclude_formsets so that a ClusterForm that specifies neither of them will have no formsets defined, rather than formsets for all child relations. (This corrects an implementation wart in the upcoming EditHandler refactor.) This doesn't affect Wagtail for the most part, because EditHandler passes an explicit formsets option - but nested InlinePanels currently rely on the old behaviour, because modelcluster <=5 provides no way to pass the inner panel's formset definition inside the outer one. We fix this by adding the necessary 'formsets' key introduced in wagtail/django-modelcluster#158.
gasman added a commit to wagtail/wagtail that referenced this pull request Mar 16, 2022
…s semantics

django-modelcluster 6.0 includes wagtail/django-modelcluster#158, which changes the semantics of formsets/exclude_formsets so that a ClusterForm that specifies neither of them will have no formsets defined, rather than formsets for all child relations. (This corrects an implementation wart in the upcoming EditHandler refactor.) This doesn't affect Wagtail for the most part, because EditHandler passes an explicit formsets option - but nested InlinePanels currently rely on the old behaviour, because modelcluster <=5 provides no way to pass the inner panel's formset definition inside the outer one. We fix this by adding the necessary 'formsets' key introduced in wagtail/django-modelcluster#158.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

1 participant