Auth: make first_name and last_name optional, add plugin compatibility check #949
Conversation
|
This adds a lot of complexity. When prevents a developer today from deleting the last_name field and renaming the first_name as display_name at the app level? I am afraid we will never be able to cover use cases and we add complexity. Is there perhaps a more minimal change we can make to enables developers to do this at the app level? |
|
what is the complexity being added here? a Virtual field, or the plugin stuff? the core is just the boolean param toggling the first_name and last_name field. I also restructured how the auth_user table is sorted because i think this way is more explicit with the order being consistent across different permutations of the optional it fields. display_name is intended to mitigate confusion when disabling first_name and last_name while all the docs, examples etc assume its there. the plugin stuff is something i noticed would already cause issues, as some plugins seem to assume auth_user.username exists and it doesn't have to. i really don't see much complexity here - well, except allowing plugins to fail with a useful error if they need something from auth... |
|
I will take a second look. :-)
…On Sat, 8 Feb 2025 at 18:39, laund ***@***.***> wrote:
what is the complexity being added here? a Virtual field, or the plugin
stuff?
the core is just the boolean param toggling the first_name and last_name
field. I also restructured how the auth_user table is sorted because i
think this way is more explicit with the order being consistent across
different permutations of the optional it fields.
display_name is intended to mitigate confusion when disabling first_name
and last_name while all the docs, examples etc assume its there.
the plugin stuff is something i noticed would *already* cause issues, as
some plugins seem to assume auth_user.username exists and it doesn't have
to.
i really don't see much complexity here - well, except allowing plugins to
fail with a *useful* error if they need something from auth...
—
Reply to this email directly, view it on GitHub
<#949 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHLZTYNTDJW3MJKNAQPQVD2O25XDAVCNFSM6AAAAABWWRXEEKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNBWGAZTMNZVGM>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
I am going to take most of this but I am considering a minor change. Basically if the the auth_user table exists and has a display_name field, then use it. If the auth_user is not provided, or if it is provided but does not have a display_name field, than add the display_name as a virtual field which is an alias of the username if exists or first_name otherwise. If users want to customize this they can edit the properties of the display_name virtual field. I think this is general, simple, and does not break users' code that may use a custom auth_user table. |
|
sounds good to me. i can try changing this tomorrow if you want, but i have "allow edits from maintainers" enabled so you should have push permission to my fork |
This PR makes
first_nameandlast_namefields optional, similar to username being optional.This is useful for:
auth_userInstead, this introduces 2 Virtual fields:
display_nameanddisplay_name_full, which contain whichever name is enabled, with this preference order:display_name:first_name>username>emaildisplay_name_full:first_name last_name>emailThis also adds a compatibility check to the Auth plugins.
To do this, all Auth plugins now inherit from
BasePlugin, which defines the__str__andis_auth_compatiblemethods. Auth now calls theis_auth_compatiblemethod when a plugin is registered, and raises a error if its incompatible (due to username or first_name+last_name being disabled)I'm not sure if i set all the compatibility checks properly, since i didn't test all plugins.