Merge pull request #1145 from umap-project/custom-username

Allow to customize user display name and URL slug

docs/custom.md

@@ -82,4 +82,22 @@ And so on!
 
 See also
 [https://github.com/etalab/cartes.data.gouv.fr](https://github.com/etalab/cartes.data.gouv.fr)
-for an example of customization.
+for an example of theme customization.
+
+
+## Custom user display name
+
+In some situation, you may want to customize the display name of users, which
+is by default the username.
+
+There are three settings you can play with to control that:
+
+    # The display name itself, could be for example "{first_name} {last_name}"
+    USER_DISPLAY_NAME = "{username}"
+    # Which field to search for when autocompleting users (for permissions)
+    # See https://django-agnocomplete.readthedocs.io/en/latest/autocomplete-definition.html#agnocompletemode
+    USER_AUTOCOMPLETE_FIELDS = ["^username"]
+    # Which field to use in the URL, may also be for example "pk" to use the
+    # primary key and not expose the username (which may be private or may change too
+    # often for URL persistance)
+    USER_URL_FIELD = "username"

umap/autocomplete.py

@@ -1,6 +1,5 @@
 from django.conf import settings
 from django.contrib.auth import get_user_model
-from django.urls import reverse
 
 
 from agnocomplete.register import register
@@ -10,10 +9,9 @@ from agnocomplete.core import AgnocompleteModel
 @register
 class AutocompleteUser(AgnocompleteModel):
     model = get_user_model()
-    fields = ['^username']
+    fields = settings.USER_AUTOCOMPLETE_FIELDS
 
     def item(self, current_item):
         data = super().item(current_item)
-        data['url'] = reverse(settings.USER_MAPS_URL,
-                              args=(current_item.get_username(), ))
+        data['url'] = current_item.get_url()
         return data

umap/models.py

@@ -1,6 +1,7 @@
 import os
 import time
 
+from django.contrib.auth.models import User
 from django.contrib.gis.db import models
 from django.conf import settings
 from django.urls import reverse
@@ -12,6 +13,29 @@ from django.core.files.base import File
 from .managers import PublicManager
 
 
+# Did not find a clean way to do this in Django
+# - creating a Proxy model would mean replacing get_user_model by this proxy model
+#   in every template
+# - extending User model woulc mean a non trivial migration
+def display_name(self):
+    return settings.USER_DISPLAY_NAME.format(**self.__dict__)
+
+
+def get_user_url(self):
+    identifier = getattr(self, settings.USER_URL_FIELD)
+    return reverse(settings.USER_MAPS_URL, kwargs={"identifier": identifier})
+
+
+def get_user_stars_url(self):
+    identifier = getattr(self, settings.USER_URL_FIELD)
+    return reverse("user_stars", kwargs={"identifier": identifier})
+
+
+User.add_to_class("__str__", display_name)
+User.add_to_class("get_url", get_user_url)
+User.add_to_class("get_stars_url", get_user_stars_url)
+
+
 class NamedModel(models.Model):
     name = models.CharField(max_length=200, verbose_name=_("name"))
 

umap/settings/base.py

@@ -211,6 +211,11 @@ MIDDLEWARE = (
 # Set to True if login into django account should be possible. Default is to
 # only use OAuth flow.
 ENABLE_ACCOUNT_LOGIN = env.bool("ENABLE_ACCOUNT_LOGIN", default=False)
+USER_DISPLAY_NAME = "{username}"
+# For use by Agnocomplete
+# See https://django-agnocomplete.readthedocs.io/en/latest/autocomplete-definition.html#agnocompletemode
+USER_AUTOCOMPLETE_FIELDS = ["^username"]
+USER_URL_FIELD = "username"
 
 # =============================================================================
 # Miscellaneous project settings
@@ -252,8 +257,6 @@ LEAFLET_ZOOM = env.int('LEAFLET_ZOOM', default=6)
 COMPRESS_ENABLED = True
 COMPRESS_OFFLINE = True
 
-SOCIAL_AUTH_DEFAULT_USERNAME = lambda u: slugify(u)
-SOCIAL_AUTH_ASSOCIATE_BY_EMAIL = True
 SOCIAL_AUTH_NO_DEFAULT_PROTECTED_USER_FIELDS = True
 SOCIAL_AUTH_PROTECTED_USER_FIELDS = ("id", )
 LOGIN_URL = "login"

umap/templates/umap/login_popup_end.html

@@ -9,7 +9,7 @@
             window.opener.umap_proceed();
         } else {
             // Trade off as Twitter does not allow us to access window.opener
-            window.location.href = '{% url "user_maps" request.user.username %}'
+            window.location.href = '{{ request.user.get_url }}'
         }
     }
 

umap/templates/umap/map_list.html

@@ -4,7 +4,7 @@
 <hr />
 <div class="col wide">
     {% map_fragment map_inst prefix=prefix page=request.GET.p %}
-    <div class="legend"><a href="{{ map_inst.get_absolute_url }}">{{ map_inst.name }}</a>{% if map_inst.owner %} <em>{% trans "by" %} <a href="{% url 'user_maps' map_inst.owner.username %}">{{ map_inst.owner }}</a></em>{% endif %}</div>
+    <div class="legend"><a href="{{ map_inst.get_absolute_url }}">{{ map_inst.name }}</a>{% if map_inst.owner %} <em>{% trans "by" %} <a href="{{ map_inst.owner.get_url }}">{{ map_inst.owner }}</a></em>{% endif %}</div>
 </div>
 {% endfor %}
 {% if maps.has_next %}

umap/templates/umap/navigation.html

@@ -7,8 +7,8 @@
   <section>
     <ul>
       {% if user.is_authenticated %}
-        <li><a href="{% url 'user_maps' user.username %}">{% trans "My maps" %} ({{ user }})</a></li>
-        <li><a href="{% url 'user_stars' user.username %}">{% trans "Starred maps" %}</a></li>
+        <li><a href="{{ user.get_url }}">{% trans "My maps" %} ({{ user }})</a></li>
+        <li><a href="{{ user.get_stars_url %}">{% trans "Starred maps" %}</a></li>
       {% else %}
         <li><a href="{% url 'login' %}" class="login">{% trans "Log in" %} / {% trans "Sign in" %}</a></li>
       {% endif %}

umap/tests/test_views.py

@@ -207,3 +207,33 @@ def test_read_only_shows_create_buttons_if_disabled(client, settings):
     settings.UMAP_READONLY = False
     response = client.get(reverse("home"))
     assert "Create a map" in response.content.decode()
+
+
+@pytest.mark.django_db
+def test_change_user_display_name(client, user, settings):
+    username = "MyUserFooName"
+    first_name = "Ezekiel"
+    user.username = username
+    user.first_name = first_name
+    user.save()
+    client.login(username=username, password="123123")
+    response = client.get(reverse("home"))
+    assert username in response.content.decode()
+    assert first_name not in response.content.decode()
+    settings.USER_DISPLAY_NAME = "{first_name}"
+    response = client.get(reverse("home"))
+    assert first_name in response.content.decode()
+    # username will still be in the contant as it's in the "my maps" URL path.
+
+
+@pytest.mark.django_db
+def test_change_user_slug(client, user, settings):
+    username = "MyUserFooName"
+    user.username = username
+    user.save()
+    client.login(username=username, password="123123")
+    response = client.get(reverse("home"))
+    assert f"/en/user/{username}/" in response.content.decode()
+    settings.USER_URL_FIELD = "pk"
+    response = client.get(reverse("home"))
+    assert f"/en/user/{user.pk}/" in response.content.decode()

umap/urls.py

@@ -157,8 +157,8 @@ urlpatterns += i18n_patterns(
     ),
     re_path(r"^search/$", views.search, name="search"),
     re_path(r"^about/$", views.about, name="about"),
-    re_path(r"^user/(?P<username>.+)/stars/$", views.user_stars, name="user_stars"),
-    re_path(r"^user/(?P<username>.+)/$", views.user_maps, name="user_maps"),
+    re_path(r"^user/(?P<identifier>.+)/stars/$", views.user_stars, name="user_stars"),
+    re_path(r"^user/(?P<identifier>.+)/$", views.user_maps, name="user_maps"),
     re_path(r"", include(i18n_urls)),
 )
 urlpatterns += (path("stats/", cache_page(60 * 60)(views.stats), name="stats"),)

umap/views.py

@@ -154,8 +154,8 @@ about = About.as_view()
 
 class UserMaps(DetailView, PaginatorMixin):
     model = User
-    slug_url_kwarg = "username"
-    slug_field = "username"
+    slug_url_kwarg = "identifier"
+    slug_field = settings.USER_URL_FIELD
     list_template_name = "umap/map_list.html"
     context_object_name = "current_user"
 
@@ -250,7 +250,7 @@ class MapsShowCase(View):
                 description = "{description}\n{by} [[{url}|{name}]]".format(
                     description=description,
                     by=_("by"),
-                    url=reverse("user_maps", kwargs={"username": m.owner.username}),
+                    url=m.owner.get_url(),
                     name=m.owner,
                 )
             description = "{}\n[[{}|{}]]".format(
@@ -418,8 +418,8 @@ class MapDetailMixin:
         if not user.is_anonymous:
             properties["user"] = {
                 "id": user.pk,
-                "name": user.get_username(),
-                "url": reverse(settings.USER_MAPS_URL, args=(user.get_username(),)),
+                "name": str(user),
+                "url": user.get_url(),
             }
         map_settings = self.get_geojson()
         if "properties" not in map_settings:
@@ -465,16 +465,11 @@ class PermissionsMixin:
         if self.object.owner:
             permissions["owner"] = {
                 "id": self.object.owner.pk,
-                "name": self.object.owner.get_username(),
-                "url": reverse(
-                    settings.USER_MAPS_URL, args=(self.object.owner.get_username(),)
-                ),
+                "name": str(self.object.owner),
+                "url": self.object.owner.get_url(),
             }
             permissions["editors"] = [
-                {
-                    "id": editor.pk,
-                    "name": editor.get_username(),
-                }
+                {"id": editor.pk, "name": str(editor)}
                 for editor in self.object.editors.all()
             ]
         if not self.object.owner and self.object.is_anonymous_owner(self.request):