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

Add the ability to customize the UserViewSet via the app config #11952

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

laymonage
Copy link
Member

@laymonage laymonage commented May 13, 2024

Fixes #11954. Somewhat "solves" the step 2 in #11915 indirectly, albeit not yet documented (via IndexView.get_list_buttons/IndexView.get_list_more_buttons).

The mechanism follows what we already have for the GroupViewSet. We could go a step further by also deprecating the WAGTAIL_USER_EDIT_FORM, WAGTAIL_USER_CREATION_FORM, and WAGTAIL_USER_CUSTOM_FIELDS settings, but I thought I'd test the waters with a smaller-scoped PR for now.

Copy link

squash-labs bot commented May 13, 2024

Manage this branch in Squash

Test this branch here: https://laymonagecustom-user-viewset-w91r5.squash.io

Comment on lines 82 to +87
WAGTAIL_USER_CUSTOM_FIELDS = ['country', 'status']
```

```{versionadded} 6.2
The ability to customize the `UserViewSet` was added. Instead of customizing the forms via the `WAGTAIL_USER_EDIT_FORM`, `WAGTAIL_USER_CREATION_FORM`, and `WAGTAIL_USER_CUSTOM_FIELDS` settings, we recommend customizing them via the viewset instead, as explained below. The aforementioned settings might be deprecated in a future release.
```
Copy link
Member Author

@laymonage laymonage May 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The currently documented way to customise the user forms (added in #2414) is a bit iffy. For one, I don't understand why we suggest specifying the custom fields in WAGTAIL_USER_CUSTOM_FIELDS, if the fields are already added declaratively in the custom forms.

With the example in https://docs.wagtail.org/en/stable/advanced_topics/customisation/custom_user_models.html, adding WAGTAIL_USER_CUSTOM_FIELDS = ['country', 'status'] really has no effect, as the fields are already specified declaratively on the form class. The setting is used here,

# The standard fields each user model is expected to have, as a minimum.
standard_fields = {"email", "first_name", "last_name", "is_superuser", "groups"}
# Custom fields
if hasattr(settings, "WAGTAIL_USER_CUSTOM_FIELDS"):
custom_fields = set(settings.WAGTAIL_USER_CUSTOM_FIELDS)
else:
custom_fields = set()

which is then combined with the default fields in the form's Meta.fields:

class UserCreationForm(UserForm):
class Meta:
model = User
fields = {User.USERNAME_FIELD} | standard_fields | custom_fields
widgets = {"groups": forms.CheckboxSelectMultiple}

However, as per Django docs,

ModelForm is a regular Form which can automatically generate certain fields. The fields that are automatically generated depend on the content of the Meta class and on which fields have already been defined declaratively. Basically, ModelForm will only generate fields that are missing from the form, or in other words, fields that weren’t defined declaratively.

If there are fields not specified declaratively on the form class, then yes, that setting could be useful. But that's not how the docs explain it...

I'm inclined to deprecate the settings as soon as we have this UserViewSet customisation publicly documented, for reasons mentioned in #6477 (comment), but I also know that the documentation could do a bit of rework, so I left it out for now. If you think that's a good idea, I'd be happy to add the deprecations and make a draft for the docs, though. Perhaps merging it with the GroupViewSet documentation https://docs.wagtail.org/en/stable/extending/customizing_group_views.html would also be a good idea...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

Allow to customize UserViewset like GroupViewset
1 participant