user: allow customization of User.get_full_name() through templates (#72945) #2
|
@ -288,6 +288,8 @@ if PROJECT_NAME != 'wcs' and 'authentic2' not in INSTALLED_APPS:
|
|||
'mellon.middleware.PassiveAuthenticationMiddleware',
|
||||
'hobo.provisionning.middleware.ProvisionningMiddleware',
|
||||
)
|
||||
if PROJECT_NAME != 'wcs':
|
||||
INSTALLED_APPS += ('hobo.user_name.apps.UserNameConfig',)
|
||||
|
||||
|
||||
if 'authentic2' in INSTALLED_APPS:
|
||||
MIDDLEWARE = MIDDLEWARE + ('hobo.agent.authentic2.middleware.ProvisionningMiddleware',)
|
||||
|
|
|
@ -106,8 +106,9 @@ class RequestContextFilter(logging.Filter):
|
|||
if saml_identifier:
|
||||
record.user_uuid = saml_identifier.name_id
|
||||
record.user = record.user_uuid[:6]
|
||||
if hasattr(user, 'get_full_name') and user.get_full_name():
|
||||
record.user = record.user_display_name = user.get_full_name()
|
||||
if hasattr(user, 'original_get_full_name') and user.original_get_full_name():
|
||||
# record original full name, not templated version from user_name app
|
||||
record.user = record.user_display_name = user.original_get_full_name()
|
||||
if getattr(user, 'email', None):
|
||||
record.user = record.user_email = user.email
|
||||
if getattr(user, 'username', None):
|
||||
|
|
|
@ -0,0 +1,61 @@
|
|||
import functools
|
||||
import logging
|
||||
|
||||
import django.db
|
||||
import django.template.exceptions
|
||||
from django.apps import AppConfig
|
||||
from django.conf import settings
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.template import engines
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def get_full_name(user):
|
||||
from hobo.agent.common.models import UserExtraAttributes
|
||||
|
||||
context = {}
|
||||
context['user'] = user
|
||||
template_vars = getattr(settings, 'TEMPLATE_VARS', {})
|
||||
if 'user_full_name_template' in template_vars:
|
||||
try:
|
||||
template = engines['django'].from_string(template_vars['user_full_name_template'])
|
||||
return template.render(context)
|
||||
except django.template.exceptions.TemplateSyntaxError:
|
||||
logger.exception('hobo.user_name: syntax error in inline user name template var')
|
||||
except UserExtraAttributes.DoesNotExist:
|
||||
fpeters
commented
Ça ne me semble pas adéquat ça va spammer d'exceptions partout alors que ne pas avoir de gabarit personnalisé reste le cas normal. C'est peut-être plus simple de ne pas découper autant, ramener le code de get_full_name_from_template dans cette fonction, et si (désolé de ne pas avoir pu tout noter d'un coup). Ça ne me semble pas adéquat ça va spammer d'exceptions partout alors que ne pas avoir de gabarit personnalisé reste le cas normal.
C'est peut-être plus simple de ne pas découper autant, ramener le code de get_full_name_from_template dans cette fonction, et si `if 'user_full_name_template' not in template_vars` alors `return user.original_get_full_name()`, et laisser alors la gestion des exceptions pour les vrais cas exceptionnels.
(désolé de ne pas avoir pu tout noter d'un coup).
pmarillonnet
commented
Oui complètement, j’avais loupé ça, merci.
Ok, c’est modifié dans la branche.
(absolument aucun souci) > Ça ne me semble pas adéquat ça va spammer d'exceptions partout alors que ne pas avoir de gabarit personnalisé reste le cas normal.
Oui complètement, j’avais loupé ça, merci.
> C'est peut-être plus simple de ne pas découper autant, ramener le code de get_full_name_from_template dans cette fonction, et si if 'user_full_name_template' not in template_vars alors return user.original_get_full_name(), et laisser alors la gestion des exceptions pour les vrais cas exceptionnels.
Ok, c’est modifié dans la branche.
J’ai laissé le ´except UserExtraAttributes.DoesNotExist´ dans le code bien qu’il ne me semble pas nécessaire, mais je me dis que peut-être j’ai loupé un cas limite qu’Agate aurait identifié pour certaines briques (?)
> (désolé de ne pas avoir pu tout noter d'un coup).
(absolument aucun souci)
|
||||
logger.exception('hobo.user_name: inline user name template refers to nonexistent attribute')
|
||||
return user.original_get_full_name()
|
||||
|
||||
|
||||
def cached_extra_attributes(user):
|
||||
try:
|
||||
return user.extra_attributes.data
|
||||
except django.db.models.ObjectDoesNotExist:
|
||||
aberriot
commented
Pas sûre de la meilleure façon de faire ça. Idéalement j'aimerai avoir la même payload que ce qui est envoyé lors du Pas sûre de la meilleure façon de faire ça. Idéalement j'aimerai avoir la même payload que ce qui est envoyé lors du `GET /__provision__` sur chaque service mais je n'ai pas trouvé où elle est générée.
pmarillonnet
commented
Oui en effet il me semble que ça ne va pas fonctionner pour les attributs multiples dans authentic. Je regarde. Oui en effet il me semble que ça ne va pas fonctionner pour les attributs multiples dans authentic. Je regarde.
|
||||
return {}
|
||||
|
||||
|
||||
class UserNameConfig(AppConfig):
|
||||
name = 'hobo.user_name'
|
||||
label = 'hobo_user_name'
|
||||
verbose_name = 'Hobo User Name'
|
||||
|
||||
def ready(self):
|
||||
"""
|
||||
We monkey-patch AUTH_USER_MODEL()
|
||||
to ensure consistency in the rendering of user name
|
||||
in the front-office, backoffice, emails, etc.
|
||||
"""
|
||||
logger.info('hobo.user_name: installing User.get_full_name customization…')
|
||||
User = get_user_model()
|
||||
|
||||
# to have a fallback when necessary if the new method crashes during render
|
||||
User.original_get_full_name = User.get_full_name
|
||||
# to replace the rendering everywhere in a consistent manner
|
||||
User.get_full_name = get_full_name
|
||||
# to avoid performance/recursion issues
|
||||
User.__str__ = User.original_get_full_name
|
||||
|
||||
if 'attributes' not in User.__dict__:
|
||||
User.attributes = functools.cached_property(cached_extra_attributes)
|
||||
User.attributes.__set_name__(User, 'attributes')
|
|
@ -5,7 +5,7 @@ import hobo.test_utils
|
|||
LANGUAGE_CODE = 'en-us'
|
||||
BROKER_URL = 'memory://'
|
||||
|
||||
INSTALLED_APPS += ('hobo.agent.common',)
|
||||
INSTALLED_APPS += ('hobo.agent.common', 'hobo.user_name.apps.UserNameConfig')
|
||||
|
||||
ALLOWED_HOSTS.append('localhost')
|
||||
|
||||
|
|
|
@ -0,0 +1,44 @@
|
|||
import pytest
|
||||
from django.contrib.auth.models import User
|
||||
from django.test import override_settings
|
||||
|
||||
from hobo.agent.common.models import UserExtraAttributes
|
||||
from hobo.user_name.apps import get_full_name
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def user(db):
|
||||
u = User.objects.create(
|
||||
first_name='Jane',
|
||||
last_name='Doe',
|
||||
)
|
||||
UserExtraAttributes.objects.create(user=u, data={'foo': 'bar'})
|
||||
return u
|
||||
|
||||
|
||||
def test_user_original_get_full_name(user):
|
||||
assert user.original_get_full_name() == 'Jane Doe'
|
||||
|
||||
|
||||
def test_user_cached_extra_attributes(user):
|
||||
assert user.attributes == {'foo': 'bar'}
|
||||
|
||||
|
||||
def test_user_cached_extra_attributes_missing_fallbacks_to_empty_dict(user):
|
||||
user.extra_attributes.delete()
|
||||
user.refresh_from_db()
|
||||
assert user.attributes == {}
|
||||
|
||||
|
||||
def test_user_get_full_name_from_template(user):
|
||||
with override_settings(
|
||||
TEMPLATE_VARS={'user_full_name_template': '{{ user.first_name }} {{ user.attributes.foo }}'}
|
||||
):
|
||||
assert get_full_name(user) == 'Jane bar'
|
||||
|
||||
|
||||
def test_user_get_full_name(user):
|
||||
with override_settings(
|
||||
TEMPLATE_VARS={'user_full_name_template': '{{ user.first_name }} {{ user.attributes.foo }}'}
|
||||
):
|
||||
assert user.get_full_name() == 'Jane bar'
|
|
@ -0,0 +1,92 @@
|
|||
from authentic2.models import Attribute
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.test import override_settings
|
||||
from tenant_schemas.utils import tenant_context
|
||||
|
||||
from hobo.user_name.apps import get_full_name
|
||||
|
||||
User = get_user_model()
|
||||
|
||||
|
||||
def test_get_full_name_from_template_utils_from_multiple_attrs(db, tenant, settings):
|
||||
with tenant_context(tenant):
|
||||
user = User.objects.create(
|
||||
first_name='Jane',
|
||||
last_name='Doe',
|
||||
)
|
||||
Attribute.objects.create(
|
||||
name='foo',
|
||||
label='Foo',
|
||||
kind='string',
|
||||
required=False,
|
||||
multiple=False,
|
||||
user_visible=True,
|
||||
user_editable=True,
|
||||
)
|
||||
Attribute.objects.create(
|
||||
name='nicknames',
|
||||
label='Nicknames',
|
||||
kind='string',
|
||||
required=False,
|
||||
multiple=True,
|
||||
user_visible=True,
|
||||
user_editable=True,
|
||||
)
|
||||
user.attributes.nicknames = ['Milly', 'Molly', 'Minnie']
|
||||
user.attributes.foo = 'bar'
|
||||
user.save()
|
||||
user.refresh_from_db()
|
||||
|
||||
with override_settings(
|
||||
TEMPLATE_VARS={'user_full_name_template': '{{ user.first_name }} {{ user.attributes.foo }}'}
|
||||
):
|
||||
assert get_full_name(user) == 'Jane bar'
|
||||
|
||||
with override_settings(
|
||||
TEMPLATE_VARS={
|
||||
'user_full_name_template': '{{ user.first_name }} {{ user.attributes.nicknames.0 }} {{ user.attributes.nicknames.2 }}'
|
||||
}
|
||||
):
|
||||
assert get_full_name(user) == 'Jane Milly Minnie'
|
||||
|
||||
|
||||
def test_get_full_name_from_template_accessor_from_multiple_attrs(db, tenant, settings):
|
||||
with tenant_context(tenant):
|
||||
user = User.objects.create(
|
||||
first_name='Jane',
|
||||
last_name='Doe',
|
||||
)
|
||||
Attribute.objects.create(
|
||||
name='foo',
|
||||
label='Foo',
|
||||
kind='string',
|
||||
required=False,
|
||||
multiple=False,
|
||||
user_visible=True,
|
||||
user_editable=True,
|
||||
)
|
||||
Attribute.objects.create(
|
||||
name='nicknames',
|
||||
label='Nicknames',
|
||||
kind='string',
|
||||
required=False,
|
||||
multiple=True,
|
||||
user_visible=True,
|
||||
user_editable=True,
|
||||
)
|
||||
user.attributes.nicknames = ['Milly', 'Molly', 'Minnie']
|
||||
user.attributes.foo = 'bar'
|
||||
user.save()
|
||||
user.refresh_from_db()
|
||||
|
||||
with override_settings(
|
||||
TEMPLATE_VARS={'user_full_name_template': '{{ user.first_name }} {{ user.attributes.foo }}'}
|
||||
):
|
||||
assert user.get_full_name() == 'Jane bar'
|
||||
|
||||
with override_settings(
|
||||
TEMPLATE_VARS={
|
||||
'user_full_name_template': '{{ user.first_name }} {{ user.attributes.nicknames.0 }} {{ user.attributes.nicknames.2 }}'
|
||||
}
|
||||
):
|
||||
assert user.get_full_name() == 'Jane Milly Minnie'
|
@fpeters ça te parait okay ? J'ai modifié la logique dans le
utils.py
pour avoir un comportement distinct côté authentic.