-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Feature/configurable profile inline #1572
base: master
Are you sure you want to change the base?
Changes from 5 commits
8acbb13
aa7657a
aeff98f
4aa27e6
f95b86c
6c1392b
200a7f4
55c0b01
5b91ebe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,8 @@ | |
|
||
from django.contrib import admin | ||
from django.contrib.auth import get_user_model | ||
from mezzanine.accounts import get_profile_model, ProfileNotConfigured | ||
from mezzanine.accounts import get_profile_model, ProfileNotConfigured, \ | ||
get_profile_inline_form | ||
|
||
from mezzanine.core.admin import SitePermissionUserAdmin | ||
from mezzanine.conf import settings | ||
|
@@ -45,20 +46,15 @@ def save_model(self, request, obj, form, change): | |
send_verification_mail(request, user, "signup_verify") | ||
|
||
|
||
try: | ||
class ProfileInline(admin.StackedInline): | ||
model = get_profile_model() | ||
can_delete = False | ||
template = "admin/profile_inline.html" | ||
extra = 0 | ||
class ProfileInline(admin.StackedInline): | ||
model = get_profile_model() | ||
can_delete = False | ||
template = "admin/profile_inline.html" | ||
extra = 0 | ||
|
||
def get_min_num(self, request, obj=None, **kwargs): | ||
"""This causes profile forms to be shown when editing but hidden | ||
when creating. If min_num is fixed at 1, Django's initial user | ||
creation form fails if the profile model has a required field.""" | ||
return 0 if obj is None else 1 | ||
|
||
UserProfileAdmin.inlines += (ProfileInline,) | ||
try: | ||
UserProfileAdmin.inlines += (get_profile_inline_form(),) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wondering if we even need to create a wrapper function for this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems reasonable to me. I don't see a reason we need trap and log for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool, let's see what @stephenmcd has to say. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with @jerivas, function seems unnecessary, especially since the default should never fail since the setting will default to something that always works There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also it only gets called in one spot, so there's no notion of reusability There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool, thanks. Removed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like it's still there. |
||
except ProfileNotConfigured: | ||
pass | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,6 +55,14 @@ | |
default=False, | ||
) | ||
|
||
register_setting( | ||
name="ACCOUNTS_PROFILE_INLINE_CLASS", | ||
description=_("Dotted package path of the profile inline " | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMHO this could explain a bit better that this appears in the User admin. Something like "Dotted package path and class name of the profile inline admin that will be added to the User admin, when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
"class to use when ``mezzanine.accounts`` is installed."), | ||
editable=False, | ||
default="mezzanine.accounts.admin.ProfileInline", | ||
) | ||
|
||
register_setting( | ||
name="ACCOUNTS_VERIFICATION_REQUIRED", | ||
description=_("If ``True``, when users create an account, they will be " | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find the name of this function misleading. It currently implies that a form class is returned, not an admin class. I suggest we rename it to
get_profile_inline_admin()
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. Done