Use display_name for "username" instead of first_name, last_name (#1640) (#1638)

This commit is contained in:
Foo Bar
2025-03-03 19:03:49 +00:00
committed by GitHub
parent ec67ad2b42
commit a55111ca74
35 changed files with 237 additions and 246 deletions

View File

@@ -12,8 +12,7 @@ class EmailUserAdmin(UserAdmin):
_("Personal info"),
{
"fields": (
"first_name",
"last_name",
"display_name",
"github_username",
"valid_email",
"claimed",
@@ -54,13 +53,12 @@ class EmailUserAdmin(UserAdmin):
ordering = ("email",)
list_display = (
"email",
"first_name",
"last_name",
"display_name",
"is_staff",
"valid_email",
"claimed",
)
search_fields = ("email", "first_name", "last_name")
search_fields = ("email", "display_name")
admin.site.register(User, EmailUserAdmin)

View File

@@ -88,7 +88,24 @@ class PreferencesForm(forms.ModelForm):
class UserProfileForm(forms.ModelForm):
class Meta:
model = User
fields = ["email", "first_name", "last_name", "indicate_last_login_method"]
fields = [
"email",
"display_name",
"indicate_last_login_method",
"is_commit_author_name_overridden",
]
labels = {
"display_name": "Username",
"is_commit_author_name_overridden": "Override commit author name",
}
override_msg = (
"Globally replaces your git commit author name with Username "
"value set above."
)
help_texts = {
"display_name": "Your name as it will be displayed across the site.",
"is_commit_author_name_overridden": override_msg,
}
class CustomClearableFileInput(forms.ClearableFileInput):

View File

@@ -0,0 +1,14 @@
# Generated by Django 4.2.16 on 2025-02-14 22:40
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
("users", "0016_user_hq_image"),
]
operations = [
migrations.RunSQL("UPDATE users_user SET display_name = CONCAT(first_name, ' ', last_name) WHERE display_name IS NULL;"),
]

View File

@@ -0,0 +1,21 @@
# Generated by Django 4.2.16 on 2025-02-25 19:09
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
("users", "0017_populate_users_display_name"),
]
operations = [
migrations.AddField(
model_name="user",
name="is_commit_author_name_overridden",
field=models.BooleanField(
default=False,
help_text="Select to override the commit author with Username",
),
),
]

View File

@@ -73,7 +73,7 @@ class UserManager(BaseUserManager):
logger.info("Creating stub user with email='%s'", email)
return self._create_user(email, password, claimed=claimed, **extra_fields)
def find_contributor(self, email=None, first_name=None, last_name=None):
def find_contributor(self, email=None, display_name=None):
"""
Lazily finds a matching User record by email, or first name and last name.
@@ -88,8 +88,7 @@ class UserManager(BaseUserManager):
email (str, optional): The email address of the user to search for.
Assumes the email address is legitimate, and is not one we generated as
a placeholder.
first_name (str, optional): The first name of the user to search for.
last_name (str, optional): The last name of the user to search for.
display_name (str, optional): The display name of the user to search for.
Returns:
User object or None: If a user is found based on the provided criteria, the
@@ -104,10 +103,8 @@ class UserManager(BaseUserManager):
except self.model.DoesNotExist:
pass
if not user and first_name and last_name:
users = self.filter(
first_name__iexact=first_name, last_name__iexact=last_name
)
if not user and display_name:
users = self.filter(display_name__iexact=display_name)
authors_or_maintainers = users.filter(
models.Q(authors__isnull=False) | models.Q(maintainers__isnull=False)
).distinct()
@@ -144,6 +141,7 @@ class BaseUser(AbstractBaseUser, PermissionsMixin):
Our email for username user model
"""
# todo: remove first_name, last_name after May 2025
first_name = models.CharField(_("first name"), max_length=30, blank=True)
last_name = models.CharField(_("last name"), max_length=30, blank=True)
email = models.EmailField(_("email address"), unique=True, db_index=True)
@@ -175,17 +173,6 @@ class BaseUser(AbstractBaseUser, PermissionsMixin):
swappable = "AUTH_USER_MODEL"
abstract = True
def get_full_name(self):
"""
Returns the first_name plus the last_name, with a space in between.
"""
full_name = "%s %s" % (self.first_name, self.last_name)
return full_name.strip()
def get_short_name(self):
"""Returns the short name for the user."""
return self.first_name
def email_user(self, subject, message, from_email=None, **kwargs):
"""
Sends an email to this User.
@@ -212,7 +199,11 @@ class User(BaseUser):
"""
badges = models.ManyToManyField(Badge)
# todo: consider making this unique=True after checking user data for duplicates
github_username = models.CharField(_("github username"), max_length=100, blank=True)
is_commit_author_name_overridden = models.BooleanField(
default=False, help_text="Select to override the commit author with Username"
)
image = models.FileField(
upload_to="profile-images",
null=True,
@@ -269,7 +260,7 @@ class User(BaseUser):
# elapsed.
delete_permanently_at = models.DateTimeField(null=True, editable=False)
def save_image_from_github(self, avatar_url):
def save_image_from_provider(self, avatar_url):
response = requests.get(avatar_url)
filename = f"{self.profile_image_filename_root}.png"
os.path.join(settings.MEDIA_ROOT, "media", "profile-images", filename)
@@ -287,16 +278,6 @@ class User(BaseUser):
Does not include the file extension."""
return f"profile-{self.pk}"
@property
def get_display_name(self):
"""Returns the display name for the user."""
if self.display_name:
return self.display_name
elif self.first_name and self.last_name:
return f"{self.first_name} {self.last_name}"
else:
return self.first_name or self.last_name
def claim(self):
"""Claim the user"""
if not self.claimed:
@@ -314,6 +295,13 @@ class User(BaseUser):
return None
return f"https://github.com/{self.github_username}"
@staticmethod
def get_user_by_github_url(url: str):
if not url:
return None
github_user = url.rstrip("/").split("/")[-1]
return User.objects.filter(github_username=github_user).first()
@transaction.atomic
def delete_account(self):
from . import tasks
@@ -328,6 +316,7 @@ class User(BaseUser):
self.display_name = "John Doe"
self.first_name = "John"
self.last_name = "Doe"
self.display_name = "John Doe"
self.email = "deleted-{}@example.com".format(uuid.uuid4())
image = self.image
transaction.on_commit(lambda: image.delete())
@@ -337,7 +326,7 @@ class User(BaseUser):
self.save()
def __str__(self):
return f"{self.first_name} {self.last_name} <{self.email}>"
return f"{self.display_name} <{self.email}>"
class LastSeen(models.Model):

View File

@@ -13,13 +13,11 @@ class UserSerializer(serializers.ModelSerializer):
model = User
fields = (
"id",
"first_name",
"last_name",
"display_name",
)
read_only_fields = (
"id",
"first_name",
"last_name",
"display_name",
)
@@ -33,8 +31,7 @@ class CurrentUserSerializer(serializers.ModelSerializer):
fields = (
"id",
"email",
"first_name",
"last_name",
"display_name",
"image",
"date_joined",
"data",
@@ -53,8 +50,7 @@ class FullUserSerializer(serializers.ModelSerializer):
fields = (
"id",
"email",
"first_name",
"last_name",
"display_name",
"is_staff",
"is_active",
"is_superuser",

View File

@@ -7,6 +7,7 @@ from allauth.socialaccount.models import SocialAccount
from users.constants import LOGIN_METHOD_SESSION_FIELD_NAME
GITHUB = "github"
GOOGLE = "google"
@receiver(post_save, sender=SocialAccount)
@@ -16,19 +17,25 @@ def import_social_profile_data(sender, instance, created, **kwargs):
user's profile
"""
if not created:
# for display name to save this needs to be resaved after the redirect.
instance.user.save()
return
if instance.provider != GITHUB:
if instance.provider not in [GITHUB, GOOGLE]:
return
github_username = instance.extra_data.get("login")
avatar_url = instance.extra_data.get("avatar_url")
if github_username:
instance.user.github_username = github_username
instance.save()
avatar_url = None
if instance.provider == GITHUB:
instance.user.github_username = instance.extra_data.get("login")
avatar_url = instance.extra_data.get("avatar_url")
elif instance.provider == GOOGLE:
avatar_url = instance.extra_data.get("picture")
if avatar_url:
instance.user.save_image_from_github(avatar_url)
instance.user.save_image_from_provider(avatar_url)
instance.user.display_name = instance.extra_data.get("name")
instance.save()
@receiver(user_logged_in)

View File

@@ -45,6 +45,14 @@ def avatar(
contributor_label=None,
avatar_type=None,
):
def get_commit_author_attribute(commitauthor, attribute):
if isinstance(commitauthor, dict):
return commitauthor.get(attribute)
try:
return getattr(commitauthor, attribute, None)
except AttributeError:
return None
kwargs = {
"is_link": is_link,
"alt": alt,
@@ -54,33 +62,35 @@ def avatar(
"contributor_label": contributor_label,
"avatar_type": avatar_type,
}
if user and commitauthor:
image_url = user.get_thumbnail_url() or commitauthor.avatar_url
href = user.github_profile_url or commitauthor.github_profile_url
image_url = user.get_thumbnail_url() or get_commit_author_attribute(
commitauthor, "avatar_url"
)
href = user.github_profile_url or get_commit_author_attribute(
commitauthor, "github_profile_url"
)
return base_avatar(
user.get_full_name(),
commitauthor.display_name,
image_url,
href,
**kwargs,
)
elif user:
return base_avatar(
user.get_full_name(),
user.display_name,
user.get_thumbnail_url(),
user.github_profile_url,
**kwargs,
)
elif commitauthor:
if isinstance(commitauthor, dict):
name = commitauthor["name"]
avatar_url = commitauthor["avatar_url"]
github_profile_url = commitauthor["github_profile_url"]
else:
name = commitauthor.name
avatar_url = commitauthor.avatar_url
github_profile_url = commitauthor.github_profile_url
display_name = get_commit_author_attribute(commitauthor, "display_name")
avatar_url = get_commit_author_attribute(commitauthor, "avatar_url")
github_profile_url = get_commit_author_attribute(
commitauthor, "github_profile_url"
)
return base_avatar(
name,
display_name,
avatar_url,
github_profile_url,
**kwargs,

View File

@@ -13,9 +13,9 @@ def user(db):
user = baker.make(
"users.User",
email="user@example.com",
first_name="Regular",
last_name="User",
display_name="Regular User",
indicate_last_login_method=False,
is_commit_author_name_overridden=False,
last_login=timezone.now(),
image=None,
)
@@ -39,8 +39,7 @@ def staff_user(db):
user = baker.make(
"users.User",
email="staff@example.com",
first_name="Staff",
last_name="User",
display_name="Staff User",
last_login=timezone.now(),
is_staff=True,
image=None,
@@ -65,8 +64,7 @@ def super_user(db):
user = baker.make(
"users.User",
email="super@example.com",
first_name="Super",
last_name="User",
display_name="Super User",
last_login=timezone.now(),
is_staff=True,
is_superuser=True,

View File

@@ -157,16 +157,16 @@ def test_preferences_form_model_modifies_instance_empty_list_user_moderator(
def test_user_profile_form(user):
form = UserProfileForm(instance=user)
assert set(form.fields.keys()) == {
"first_name",
"last_name",
"display_name",
"email",
"indicate_last_login_method",
"is_commit_author_name_overridden",
}
assert form.initial == {
"first_name": user.first_name,
"last_name": user.last_name,
"display_name": user.display_name,
"email": user.email,
"indicate_last_login_method": user.indicate_last_login_method,
"is_commit_author_name_overridden": user.is_commit_author_name_overridden,
}
form = UserProfileForm(instance=user, data={"email": "test@example.com"})
assert form.is_valid()

View File

@@ -44,8 +44,7 @@ def test_create_stub_user(db):
u = User.objects.create_stub_user(
"t4@example.com",
None,
first_name="Tester",
last_name="Testerson",
display_name="Tester Testerson",
valid_email=False,
)
assert u.claimed is False
@@ -53,5 +52,4 @@ def test_create_stub_user(db):
assert u.is_active is True
assert u.is_staff is False
assert u.is_superuser is False
assert u.first_name == "Tester"
assert u.last_name == "Testerson"
assert u.display_name == "Tester Testerson"

View File

@@ -28,30 +28,6 @@ def test_super_user(super_user):
assert super_user.is_superuser is True
def test_get_display_name(user):
# Test case 1: Display name is set
user.display_name = "Display Name"
user.save()
assert user.get_display_name == "Display Name"
# Test case 2: First and last name are set, no display name
user.display_name = ""
user.save()
assert user.get_display_name == f"{user.first_name} {user.last_name}"
# Test case 3: Only first name is set, no display name
user.first_name = "First"
user.last_name = ""
user.save()
assert user.get_display_name == "First"
# Test case 4: Only last name is set, no display name
user.first_name = ""
user.last_name = "Last"
user.save()
assert user.get_display_name == "Last"
def test_profile_image_filename_root(user):
assert user.profile_image_filename_root == f"profile-{user.id}"
@@ -122,29 +98,21 @@ def test_find_contributor_by_email_not_found():
def test_find_contributor_not_author_or_maintainer(user: User):
found_user = User.objects.find_contributor(
first_name=user.first_name, last_name=user.last_name
)
found_user = User.objects.find_contributor(display_name=user.display_name)
assert found_user is None
def test_find_contributor_by_first_and_last_name_not_found():
non_existent_first_name = "Nonexistent"
non_existent_last_name = "User"
found_user = User.objects.find_contributor(
first_name=non_existent_first_name, last_name=non_existent_last_name
)
def test_find_contributor_by_display_name_not_found():
non_existent_name = "Nonexistent User"
found_user = User.objects.find_contributor(display_name=non_existent_name)
assert found_user is None
def test_find_contributor_by_first_and_last_name_multiple_results(user, staff_user):
staff_user.first_name = user.first_name
staff_user.last_name = user.last_name
def test_find_contributor_by_display_name_multiple_results(user, staff_user):
staff_user.display_name = user.display_name
staff_user.save()
found_user = User.objects.find_contributor(
first_name=user.first_name, last_name=user.last_name
)
found_user = User.objects.find_contributor(display_name=user.display_name)
assert found_user is None
@@ -157,9 +125,7 @@ def test_find_contributor_is_author(user, library):
library.authors.add(user)
library.save()
found_user = User.objects.find_contributor(
first_name=user.first_name, last_name=user.last_name
)
found_user = User.objects.find_contributor(display_name=user.display_name)
assert found_user == user
@@ -167,9 +133,7 @@ def test_find_contributor_is_maintainer(user, library_version):
library_version.maintainers.add(user)
library_version.save()
found_user = User.objects.find_contributor(
first_name=user.first_name, last_name=user.last_name
)
found_user = User.objects.find_contributor(display_name=user.display_name)
assert found_user == user

View File

@@ -96,16 +96,14 @@ def test_current_user_profile_view_update_name(user, tp):
tp.reverse("profile-account"),
data={
"email": user.email,
"first_name": new_first_name,
"last_name": new_last_name,
"display_name": f"{new_first_name} {new_last_name}",
"update_profile": "",
},
follow=True,
)
assert response.status_code == 200
user.refresh_from_db()
assert user.first_name == new_first_name
assert user.last_name == new_last_name
assert user.display_name == f"{new_first_name} {new_last_name}"
@pytest.mark.django_db