From f05f7ce31f04d1aa87946bf19597c8beff2d3835 Mon Sep 17 00:00:00 2001 From: Adrian D'Alessandro Date: Mon, 26 Oct 2026 17:14:55 +0000 Subject: [PATCH 1/6] Add Profile model --- ...serskill_user_profile_userskill_profile.py | 32 +++++++++++++++++++ main/models/user_models.py | 13 +++++++- 2 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 main/migrations/0032_alter_userskill_user_profile_userskill_profile.py diff --git a/main/migrations/0032_alter_userskill_user_profile_userskill_profile.py b/main/migrations/0032_alter_userskill_user_profile_userskill_profile.py new file mode 100644 index 00000000..9162acc9 --- /dev/null +++ b/main/migrations/0032_alter_userskill_user_profile_userskill_profile.py @@ -0,0 +1,32 @@ +# Generated by Django 6.1.1 on 2026-10-27 08:16 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0031_team_teammembership'), + ] + + operations = [ + migrations.AlterField( + model_name='userskill', + name='user', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL), + ), + migrations.CreateModel( + name='Profile', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('user', models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + ), + migrations.AddField( + model_name='userskill', + name='profile', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to='main.profile'), + ), + ] diff --git a/main/models/user_models.py b/main/models/user_models.py index 9150edfd..65e833e8 100644 --- a/main/models/user_models.py +++ b/main/models/user_models.py @@ -17,10 +17,21 @@ class User(AbstractUser): date_agreed = models.DateTimeField(_("date agreed"), blank=True, null=True) +class Profile(models.Model): + """Profile model for individual user profiles and anonymous career profiles.""" + + user = models.OneToOneField( + settings.AUTH_USER_MODEL, on_delete=models.CASCADE, blank=True, null=True + ) + + class UserSkill(models.Model): """Model for mapping users to skills and skill levels.""" - user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE) + user = models.ForeignKey( + settings.AUTH_USER_MODEL, on_delete=models.CASCADE, null=True + ) + profile = models.ForeignKey(Profile, on_delete=models.CASCADE, null=True) skill = models.ForeignKey(Skill, on_delete=models.CASCADE) skill_level = models.ForeignKey(SkillLevel, on_delete=models.CASCADE) From dda007d88ee9e25a406474b4d5c59f2a8e9d1073 Mon Sep 17 00:00:00 2001 From: Adrian D'Alessandro Date: Tue, 27 Oct 2026 10:04:50 +0000 Subject: [PATCH 2/6] Populate profile skills --- .../0033_populate_userskill_profiles.py | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 main/migrations/0033_populate_userskill_profiles.py diff --git a/main/migrations/0033_populate_userskill_profiles.py b/main/migrations/0033_populate_userskill_profiles.py new file mode 100644 index 00000000..37d324b9 --- /dev/null +++ b/main/migrations/0033_populate_userskill_profiles.py @@ -0,0 +1,62 @@ +# Generated by Django 6.1.1 on 2026-10-27 08:16 + +from django.conf import settings +from django.db import migrations +from django.db.backends.base.schema import BaseDatabaseSchemaEditor +from django.db.migrations.state import StateApps + + +def populate_profiles(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor) -> None: + """Populate the profile field in the user skills models based off the user field.""" + + User = apps.get_model(settings.AUTH_USER_MODEL) + Profile = apps.get_model("main", "Profile") + UserSkill = apps.get_model("main", "UserSkill") + db_alias = schema_editor.connection.alias + + user_ids = ( + User.objects.using(db_alias) + .order_by() + .values_list("pk", flat=True) + .distinct() + ) + + for user_id in user_ids.iterator(): + profile, _ = Profile.objects.using(db_alias).get_or_create(user_id=user_id) + UserSkill.objects.using(db_alias).filter(user_id=user_id).update( + profile_id=profile.pk, + user_id=None + ) + + +def restore_users(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor) -> None: + """Populate the user field in the user skills models based off the profile field.""" + + Profile = apps.get_model("main", "Profile") + UserSkill = apps.get_model("main", "UserSkill") + db_alias = schema_editor.connection.alias + + user_skills = UserSkill.objects.using(db_alias) + + profiles = ( + Profile.objects.using(db_alias) + .filter(pk__in=user_skills.values("profile_id")) + .values_list("pk", "user_id") + ) + + for profile_id, user_id in profiles.iterator(): + user_skills.filter(profile_id=profile_id).update( + user_id=user_id, + profile_id=None, + ) + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0032_alter_userskill_user_profile_userskill_profile'), + ] + + operations = [ + migrations.RunPython(populate_profiles, reverse_code=restore_users), + ] From e0114a644225b889704e093e7465747e9da7688e Mon Sep 17 00:00:00 2001 From: Adrian D'Alessandro Date: Tue, 27 Oct 2026 10:51:16 +0000 Subject: [PATCH 3/6] Remove link from user to userskill --- main/admin.py | 110 ++++++++++-------- ...ve_userskill_unique_user_skill_and_more.py | 34 ++++++ main/models/user_models.py | 11 +- 3 files changed, 98 insertions(+), 57 deletions(-) create mode 100644 main/migrations/0034_delete_userproxy_remove_userskill_unique_user_skill_and_more.py diff --git a/main/admin.py b/main/admin.py index 42c52f22..fa716b3b 100644 --- a/main/admin.py +++ b/main/admin.py @@ -4,7 +4,6 @@ from django.contrib import admin from django.contrib.auth.admin import UserAdmin -from django.http import HttpRequest from import_export.admin import ImportExportModelAdmin from .io_resources import ( @@ -22,6 +21,7 @@ CompetencyDomain, Event, LearningResource, + Profile, Provider, Skill, SkillLevel, @@ -159,13 +159,6 @@ class SkillLevelAdmin(ImportExportModelAdmin[SkillLevel]): resource_classes = (SkillLevelResource,) -class UserSkillInline(admin.TabularInline[UserSkill, User]): - """Inline admin class for the UserSkill model.""" - - model = UserSkill - extra = 1 - - @admin.register(Event) class EventAdmin(ImportExportModelAdmin[Event]): """Admin class for the Event model.""" @@ -176,54 +169,69 @@ class EventAdmin(ImportExportModelAdmin[Event]): resource_classes = (EventResource,) -class UserProxy(User): - """A proxy model of the User model to create a User Skills admin view. - - This is required to make a second admin view tied to the User model. - """ +class SkillProfileInline(admin.TabularInline[UserSkill, Profile]): + """Inline admin class for the SkillProfile model.""" - class Meta: - """Make model a proxy and set verbose names.""" - - proxy = True - verbose_name = "User skills profile" + model = UserSkill + extra = 1 -@admin.register(UserProxy) -class CustomUserSkillsProfilesAdmin(admin.ModelAdmin[UserProxy]): - """Admin class for adding UserSkills in bulk to individual Users.""" +@admin.register(Profile) +class ProfileAdmin(admin.ModelAdmin[Profile]): + """Admin class for the Profile model.""" - exclude = ( - "username", - "password", - "first_name", - "last_name", - "email", - "last_login", - "is_superuser", - "is_staff", - "is_active", - "date_joined", - "groups", - "user_permissions", - "agreed_to_tos", - "date_agreed", + fields = ( + "user", + "user_first_name", + "user_last_name", + "user_email", + "user_is_active", + ) + list_display = ( + "user", + "user__first_name", + "user__last_name", + "user__email", + "user__is_active", + ) + search_fields = ( + "user__username", + "user__first_name", + "user__last_name", + "user__email", + "user__is_active", ) - readonly_fields = ("username", "first_name", "last_name", "email", "is_active") - list_display = ("username", "is_active") - search_fields = ("username", "email") - list_filter = ("is_active",) - inlines = (UserSkillInline,) - - def has_add_permission(self, request: HttpRequest) -> bool: - """Do not allow adding new users from this view.""" - return False - - def has_delete_permission( - self, request: HttpRequest, obj: UserProxy | None = None - ) -> bool: - """Do not allow deleting users from this view.""" - return False + list_filter = ("user__is_active",) + readonly_fields = fields + inlines = (SkillProfileInline,) + + @admin.display(description="First name", ordering="user__first_name") + def user_first_name(self, obj: Profile) -> str: + """Render user first name in the detail/edit view.""" + if obj.user is None: + return "" + return obj.user.first_name + + @admin.display(description="Last name", ordering="user__last_name") + def user_last_name(self, obj: Profile) -> str: + """Render user last name in the detail/edit view.""" + if obj.user is None: + return "" + return obj.user.last_name + + @admin.display(description="Email", ordering="user__email") + def user_email(self, obj: Profile) -> str: + """Render user email in the detail/edit view.""" + if obj.user is None: + return "" + return obj.user.email + + @admin.display(description="Active", boolean=True, ordering="user__is_active") + def user_is_active(self, obj: Profile) -> bool: + """Render user is_active status in the detail/edit view.""" + if obj.user is None: + return False + return obj.user.is_active class TeamMembershipInline(admin.TabularInline[TeamMembership, Team]): diff --git a/main/migrations/0034_delete_userproxy_remove_userskill_unique_user_skill_and_more.py b/main/migrations/0034_delete_userproxy_remove_userskill_unique_user_skill_and_more.py new file mode 100644 index 00000000..b644de09 --- /dev/null +++ b/main/migrations/0034_delete_userproxy_remove_userskill_unique_user_skill_and_more.py @@ -0,0 +1,34 @@ +# Generated by Django 6.1.1 on 2026-10-27 10:14 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0033_populate_userskill_profiles'), + ] + + operations = [ + migrations.DeleteModel( + name='UserProxy', + ), + migrations.RemoveConstraint( + model_name='userskill', + name='unique_user_skill', + ), + migrations.AlterField( + model_name='userskill', + name='profile', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='main.profile'), + ), + migrations.AddConstraint( + model_name='userskill', + constraint=models.UniqueConstraint(fields=('profile', 'skill'), name='unique_skill_profile'), + ), + migrations.RemoveField( + model_name='userskill', + name='user', + ), + ] diff --git a/main/models/user_models.py b/main/models/user_models.py index 65e833e8..a91c04bb 100644 --- a/main/models/user_models.py +++ b/main/models/user_models.py @@ -26,12 +26,9 @@ class Profile(models.Model): class UserSkill(models.Model): - """Model for mapping users to skills and skill levels.""" + """Model for mapping profiles to skills and skill levels.""" - user = models.ForeignKey( - settings.AUTH_USER_MODEL, on_delete=models.CASCADE, null=True - ) - profile = models.ForeignKey(Profile, on_delete=models.CASCADE, null=True) + profile = models.ForeignKey(Profile, on_delete=models.CASCADE) skill = models.ForeignKey(Skill, on_delete=models.CASCADE) skill_level = models.ForeignKey(SkillLevel, on_delete=models.CASCADE) @@ -39,5 +36,7 @@ class Meta: """Meta options for UserSkill model.""" constraints = ( - models.UniqueConstraint(fields=["user", "skill"], name="unique_user_skill"), + models.UniqueConstraint( + fields=["profile", "skill"], name="unique_skill_profile" + ), ) From a90d37d200ce5e97c636325aee821e7e1709ed2b Mon Sep 17 00:00:00 2001 From: Adrian D'Alessandro Date: Tue, 27 Oct 2026 14:07:42 +0000 Subject: [PATCH 4/6] Rename UserSkill to Skill Profile --- main/admin.py | 6 +- main/forms.py | 60 ++++++-------- .../0035_rename_userskill_skillprofile.py | 17 ++++ main/models/user_models.py | 4 +- .../assets/sample_data/sample_profile_1.json | 2 +- .../assets/sample_data/sample_profile_41.json | 2 +- .../assets/sample_data/sample_profile_59.json | 2 +- ...nk.html => create_skill_profile_link.html} | 2 +- ...profile.html => export_skill_profile.html} | 10 +-- .../share_skill_profile_dropdown.html | 2 +- main/templates/main/snippets/skill-wheel.html | 2 +- main/urls.py | 4 +- main/views/account_data_views.py | 54 ++++++------ main/views/account_views.py | 36 ++++---- tests/data/example_role.json | 2 +- tests/main/conftest.py | 12 ++- tests/main/test_account_data_views.py | 34 ++++---- tests/main/test_admin_views.py | 23 ++---- tests/main/test_main_views.py | 82 +++++++++---------- 19 files changed, 185 insertions(+), 171 deletions(-) create mode 100644 main/migrations/0035_rename_userskill_skillprofile.py rename main/templates/main/snippets/{create_user_skill_profile_link.html => create_skill_profile_link.html} (96%) rename main/templates/main/snippets/{export_user_skill_profile.html => export_skill_profile.html} (86%) diff --git a/main/admin.py b/main/admin.py index fa716b3b..1dae2b19 100644 --- a/main/admin.py +++ b/main/admin.py @@ -25,11 +25,11 @@ Provider, Skill, SkillLevel, + SkillProfile, Team, TeamMembership, ToolLanguageMethodology, User, - UserSkill, ) @@ -169,10 +169,10 @@ class EventAdmin(ImportExportModelAdmin[Event]): resource_classes = (EventResource,) -class SkillProfileInline(admin.TabularInline[UserSkill, Profile]): +class SkillProfileInline(admin.TabularInline[SkillProfile, Profile]): """Inline admin class for the SkillProfile model.""" - model = UserSkill + model = SkillProfile extra = 1 diff --git a/main/forms.py b/main/forms.py index b9c4fae1..1ba9800b 100644 --- a/main/forms.py +++ b/main/forms.py @@ -1,12 +1,11 @@ """Forms module for the main app.""" -from typing import TYPE_CHECKING, Any +from typing import Any from crispy_forms.helper import FormHelper from crispy_forms.layout import HTML, Div, Layout, Submit from django import forms from django.core.exceptions import ValidationError -from django.forms import ModelForm from django.urls import reverse from django.utils import timezone from django.utils.html import format_html @@ -16,10 +15,7 @@ RegistrationFormUniqueEmail, ) -from .models import Skill, SkillLevel, UserSkill - -if TYPE_CHECKING: # pragma: no cover - from .models import User as UserType +from .models import Profile, Skill, SkillLevel, SkillProfile def _build_tos_form_label() -> str: @@ -73,20 +69,12 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: self.fields["tos"].label = _(_build_tos_form_label()) -class UserSkillForm(ModelForm[UserSkill]): - """Form for user skills.""" - - class Meta: # noqa: D106 - model = UserSkill - fields = ("skill", "skill_level") - - -class UserSkillsForm(forms.Form): - """Form for creating UserSkills for all skills in the database.""" +class SkillProfileForm(forms.Form): + """Form for creating SkillProfiles for all skills in the database.""" - def __init__(self, *args: Any, user: "UserType", **kwargs: Any) -> None: + def __init__(self, *args: Any, profile: Profile, **kwargs: Any) -> None: """Initialize the form with a field for each skill.""" - self.user = user + self.profile = profile super().__init__(*args, **kwargs) # Get all skills, ordered by competency for better organization @@ -107,19 +95,21 @@ def __init__(self, *args: Any, user: "UserType", **kwargs: Any) -> None: for skill in skills: field_name = f"skill_{skill.id}" - # Check if user already has this skill - existing_user_skill = None - if self.user: + # Check if profile already has this skill + existing_skill_profile = None + if self.profile: try: - existing_user_skill = UserSkill.objects.get( - user=self.user, skill=skill + existing_skill_profile = SkillProfile.objects.get( + profile=self.profile, skill=skill ) - except UserSkill.DoesNotExist: + except SkillProfile.DoesNotExist: pass # Set initial value if user already has this skill initial_value = ( - existing_user_skill.skill_level.id if existing_user_skill else None + existing_skill_profile.skill_level.id + if existing_skill_profile + else None ) # Organize skills by competency hierarchy for layout @@ -244,8 +234,8 @@ def __init__(self, *args: Any, user: "UserType", **kwargs: Any) -> None: self.helper.layout = Layout(*layout_elements) - def save(self, user: "UserType") -> tuple[list[UserSkill], list[UserSkill]]: - """Save the form data as UserSkill instances.""" + def save(self, profile: Profile) -> tuple[list[SkillProfile], list[SkillProfile]]: + """Save the form data as SkillProfile instances.""" created_skills = [] updated_skills = [] @@ -255,17 +245,17 @@ def save(self, user: "UserType") -> tuple[list[UserSkill], list[UserSkill]]: skill = Skill.objects.get(id=skill_id) skill_level = SkillLevel.objects.get(id=skill_level_id) - # Check if UserSkill already exists - user_skill, created = UserSkill.objects.get_or_create( - user=user, skill=skill, defaults={"skill_level": skill_level} + # Check if SkillProfile already exists + skill_profile, created = SkillProfile.objects.get_or_create( + profile=profile, skill=skill, defaults={"skill_level": skill_level} ) if not created: - # Update existing UserSkill - user_skill.skill_level = skill_level - user_skill.save() - updated_skills.append(user_skill) + # Update existing SkillProfile + skill_profile.skill_level = skill_level + skill_profile.save() + updated_skills.append(skill_profile) else: - created_skills.append(user_skill) + created_skills.append(skill_profile) return created_skills, updated_skills diff --git a/main/migrations/0035_rename_userskill_skillprofile.py b/main/migrations/0035_rename_userskill_skillprofile.py new file mode 100644 index 00000000..71d45161 --- /dev/null +++ b/main/migrations/0035_rename_userskill_skillprofile.py @@ -0,0 +1,17 @@ +# Generated by Django 6.1.1 on 2026-10-27 10:18 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0034_delete_userproxy_remove_userskill_unique_user_skill_and_more'), + ] + + operations = [ + migrations.RenameModel( + old_name='UserSkill', + new_name='SkillProfile', + ), + ] diff --git a/main/models/user_models.py b/main/models/user_models.py index a91c04bb..7f8df241 100644 --- a/main/models/user_models.py +++ b/main/models/user_models.py @@ -25,7 +25,7 @@ class Profile(models.Model): ) -class UserSkill(models.Model): +class SkillProfile(models.Model): """Model for mapping profiles to skills and skill levels.""" profile = models.ForeignKey(Profile, on_delete=models.CASCADE) @@ -33,7 +33,7 @@ class UserSkill(models.Model): skill_level = models.ForeignKey(SkillLevel, on_delete=models.CASCADE) class Meta: - """Meta options for UserSkill model.""" + """Meta options for SkillProfile model.""" constraints = ( models.UniqueConstraint( diff --git a/main/static/assets/sample_data/sample_profile_1.json b/main/static/assets/sample_data/sample_profile_1.json index 0eeb37c1..6a138695 100644 --- a/main/static/assets/sample_data/sample_profile_1.json +++ b/main/static/assets/sample_data/sample_profile_1.json @@ -8,7 +8,7 @@ "Her team works on developing software tools and platforms to support research in the humanities, including text analysis, data visualisation, and digital archiving. She is passionate about using technology to enhance our understanding of the past and to make historical data more accessible to researchers and the public.", "As an RSE, Kirti collaborates closely with researchers, helping them to leverage computational methods in their work. She is skilled in programming, data management, and software development, and is committed to promoting best practices in research software engineering." ], - "user_data": [ + "skill_profile_data": [ { "skill": "Programming", "category": "Software design and development", diff --git a/main/static/assets/sample_data/sample_profile_41.json b/main/static/assets/sample_data/sample_profile_41.json index 73e09bf1..c667a3db 100644 --- a/main/static/assets/sample_data/sample_profile_41.json +++ b/main/static/assets/sample_data/sample_profile_41.json @@ -8,7 +8,7 @@ "Her team supports projects spanning genomics, climate modelling, digital humanities, and AI, ensuring that computational research is efficient, reproducible, and sustainable. Maya is passionate about helping researchers adopt best practices in cloud/HPC use, while also advocating for recognition of RSEs as vital contributors to the research ecosystem.", "As RSE Team Lead, Maya provides strategic direction for the group, ensuring that their work aligns with institutional research priorities while promoting sustainable and scalable computing practices. She oversees the delivery of HPC and cloud-based projects, guiding teams to develop optimised, cost-efficient, and reproducible workflows. Alongside project management, she mentors both RSEs and researchers, helping them build expertise in containers, workflow automation, and advanced computing methods. Maya works closely with principal investigators, IT services, and national computing centres to design and implement research solutions at scale, and she actively advocates for the adoption of FAIR, open, and environmentally responsible approaches to cloud and HPC technologies." ], - "user_data":[ + "skill_profile_data":[ { "category":"Professional and people skills", "subcategory":"Professional and people skills", diff --git a/main/static/assets/sample_data/sample_profile_59.json b/main/static/assets/sample_data/sample_profile_59.json index 4f030643..d924acd4 100644 --- a/main/static/assets/sample_data/sample_profile_59.json +++ b/main/static/assets/sample_data/sample_profile_59.json @@ -7,7 +7,7 @@ "Riley is the Community Manager for the Research Software Engineering (RSE) group at Kingsmere University. With a background in digital communications and research support, Riley specialises in building and sustaining communities around research software. They focus on connecting researchers, developers, and technical staff, ensuring knowledge flows effectively and that the RSE team’s work is visible, accessible, and impactful.", "Riley’s role blends facilitation, communication, and advocacy. They design engagement strategies, run events and workshops, and create resources that help researchers adopt best practices in software development and reproducibility. Riley also acts as a bridge between the RSE team, research groups, and external partners, promoting collaboration and inclusivity across projects." ], - "user_data":[ + "skill_profile_data":[ { "category":"Professional and people skills", "subcategory":"Professional and people skills", diff --git a/main/templates/main/snippets/create_user_skill_profile_link.html b/main/templates/main/snippets/create_skill_profile_link.html similarity index 96% rename from main/templates/main/snippets/create_user_skill_profile_link.html rename to main/templates/main/snippets/create_skill_profile_link.html index 7f60e10d..def17ef8 100644 --- a/main/templates/main/snippets/create_user_skill_profile_link.html +++ b/main/templates/main/snippets/create_skill_profile_link.html @@ -5,7 +5,7 @@ To use add a button with the class "export-skill-profile-link" to the page and include the following script at the end of the page. -"{ % include "main/snippets/create_user_skill_profile_link.html" %}" +"{ % include "main/snippets/create_skill_profile_link.html" %}" --> diff --git a/main/urls.py b/main/urls.py index b4ccf6d9..b45953c3 100644 --- a/main/urls.py +++ b/main/urls.py @@ -48,12 +48,12 @@ path("overview/", views.AccountOverviewView.as_view(), name="account-overview"), path( "download-skills-data/csv/", - views.DownloadUserSkillDataCsvView.as_view(), + views.DownloadSkillProfileDataCsvView.as_view(), name="download-csv-data-link", ), path( "download-skills-data/json/", - views.DownloadUserSkillDataJSONView.as_view(), + views.DownloadSkillProfileDataJSONView.as_view(), name="download-json-data-link", ), ] diff --git a/main/views/account_data_views.py b/main/views/account_data_views.py index 3dcc5409..bfb4001c 100644 --- a/main/views/account_data_views.py +++ b/main/views/account_data_views.py @@ -8,57 +8,61 @@ ) from django.views import View -from ..models import User, UserSkill +from ..models import Profile, SkillProfile from .account_views import AuthenticatedHttpRequest, TermsAcceptedMixin -def format_user_skill_data_for_download(user: User) -> list[dict[str, str | int]]: - """Format the user skill data for download.""" - user_skills = UserSkill.objects.filter(user=user.pk) +def format_skill_profile_data_for_download( + profile: Profile, +) -> list[dict[str, str | int]]: + """Format the skill profile data for download.""" + skill_profiles = SkillProfile.objects.filter(profile=profile) return [ { - "skill": user_skill.skill.name, - "competency_domain": user_skill.skill.competency.competency_domain.name, - "competency": user_skill.skill.competency.name, - "skill_level": user_skill.skill_level.level, + "skill": skill_profile.skill.name, + "competency_domain": skill_profile.skill.competency.competency_domain.name, + "competency": skill_profile.skill.competency.name, + "skill_level": skill_profile.skill_level.level, } - for user_skill in user_skills + for skill_profile in skill_profiles ] -class DownloadUserSkillDataCsvView(TermsAcceptedMixin, View): - """A view that returns the user skill data as a CSV file.""" +class DownloadSkillProfileDataCsvView(TermsAcceptedMixin, View): + """A view that returns the skill profile data as a CSV file.""" def get(self, request: AuthenticatedHttpRequest) -> HttpResponse: - """Handle GET request to download user skill data as CSV. + """Handle GET request to download skill profile data as CSV. Args: request: The HTTP request object. Returns: - An HttpResponse containing the user skill data as a CSV file + An HttpResponse containing the skill profile data as a CSV file """ - user_skills_data = format_user_skill_data_for_download(request.user) + skill_profile_data = format_skill_profile_data_for_download( + request.user.profile + ) response = HttpResponse( content_type="text/csv", headers={"Content-Disposition": 'attachment; filename="skill_profile.csv"'}, ) writer = csv.writer(response) writer.writerow(["skill", "competency_domain", "competency", "skill_level"]) - for user_skill in user_skills_data: + for skill_profile in skill_profile_data: writer.writerow( [ - user_skill["skill"], - user_skill["competency_domain"], - user_skill["competency"], - user_skill["skill_level"], + skill_profile["skill"], + skill_profile["competency_domain"], + skill_profile["competency"], + skill_profile["skill_level"], ] ) return response -class DownloadUserSkillDataJSONView(TermsAcceptedMixin, View): - """A view that returns the user skill data as a JSON file.""" +class DownloadSkillProfileDataJSONView(TermsAcceptedMixin, View): + """A view that returns the skill profile data as a JSON file.""" def get(self, request: AuthenticatedHttpRequest) -> JsonResponse: """Define the GET response. @@ -67,13 +71,15 @@ def get(self, request: AuthenticatedHttpRequest) -> JsonResponse: request: The HTTP request object. Returns: - A JsonResponse containing the user skill data in JSON format. + A JsonResponse containing the skill profile data in JSON format. """ - user_skills_data = format_user_skill_data_for_download(request.user) + skill_profile_data = format_skill_profile_data_for_download( + request.user.profile + ) get_anonymous = request.GET.get("anonymous", "false").lower() == "true" returned_data = { "username": "anonymous" if get_anonymous else request.user.username, - "user_data": user_skills_data, + "skill_profile_data": skill_profile_data, } return JsonResponse( returned_data, diff --git a/main/views/account_views.py b/main/views/account_views.py index 706413bc..47a07b5a 100644 --- a/main/views/account_views.py +++ b/main/views/account_views.py @@ -15,8 +15,8 @@ from django.views.generic.base import RedirectView, TemplateView from django.views.generic.edit import FormView, UpdateView -from ..forms import TermsAcceptanceForm, UserSkillsForm -from ..models import SkillLevel, UserSkill +from ..forms import SkillProfileForm, TermsAcceptanceForm +from ..models import SkillLevel, SkillProfile logger = logging.getLogger(__name__) @@ -78,26 +78,28 @@ class SkillProfileView(TermsAcceptedMixin, TemplateView): template_name = "main/user-skills-profile.html" def get_context_data(self, **kwargs: Any) -> dict[str, Any]: - """Add user skills and skill levels to the template context.""" + """Add skill profiles and skill levels to the template context.""" context = super().get_context_data(**kwargs) logger.info("Rendering skills-profile page.") - user_skills = UserSkill.objects.filter(user=self.request.user.pk) - user_skills_data = [ + skill_profiles = SkillProfile.objects.filter( + profile__user_id=self.request.user.pk + ) + skill_profile_data = [ { - "skill": user_skill.skill.name, - "category": user_skill.skill.competency.competency_domain.name, - "subcategory": user_skill.skill.competency.name, - "skill_level": user_skill.skill_level.level, + "skill": skill_profile.skill.name, + "category": skill_profile.skill.competency.competency_domain.name, + "subcategory": skill_profile.skill.competency.name, + "skill_level": skill_profile.skill_level.level, } - for user_skill in user_skills + for skill_profile in skill_profiles ] context["chart_data"] = dumps( [ { "user_id": "root", - "user_data": user_skills_data, + "skill_profile_data": skill_profile_data, } ] ) @@ -118,7 +120,7 @@ class AccountOverviewView(TermsAcceptedMixin, RedirectView): def get_redirect_url(self, *args: Any, **kwargs: Any) -> str: """Redirect to skill profile if skills exist, otherwise self-assess.""" - if UserSkill.objects.filter(user=self.request.user).exists(): + if SkillProfile.objects.filter(profile__user=self.request.user).exists(): return reverse("skills_profile") return reverse("self_assess") @@ -137,23 +139,23 @@ def get_object(self, queryset: Any | None = None) -> "UserType": return self.request.user -class SelfAssessPageView(TermsAcceptedMixin, FormView[UserSkillsForm]): +class SelfAssessPageView(TermsAcceptedMixin, FormView[SkillProfileForm]): """View that renders the self-assessment questionnaire page.""" request: AuthenticatedHttpRequest template_name = "main/user-self-assess.html" - form_class = UserSkillsForm + form_class = SkillProfileForm success_url = reverse_lazy("self_assess") def get_form_kwargs(self) -> dict[str, Any]: """Return the keyword arguments for instantiating the form.""" kwargs = super().get_form_kwargs() - kwargs["user"] = self.request.user + kwargs["profile"] = self.request.user.profile return kwargs - def form_valid(self, form: UserSkillsForm) -> HttpResponse: + def form_valid(self, form: SkillProfileForm) -> HttpResponse: """Handle valid form submission.""" - created_skills, updated_skills = form.save(self.request.user) + created_skills, updated_skills = form.save(self.request.user.profile) # Add success messages if created_skills: diff --git a/tests/data/example_role.json b/tests/data/example_role.json index e8d89ab6..e61ac11b 100644 --- a/tests/data/example_role.json +++ b/tests/data/example_role.json @@ -6,7 +6,7 @@ "description": [ "Description of the example role" ], - "user_data": [ + "skill_profile_data": [ { "skill": "Skill 1", "category": "Category 1", diff --git a/tests/main/conftest.py b/tests/main/conftest.py index d50340cb..e815bb7e 100644 --- a/tests/main/conftest.py +++ b/tests/main/conftest.py @@ -7,11 +7,12 @@ CompetencyDomain, Event, LearningResource, + Profile, Provider, Skill, SkillLevel, + SkillProfile, ToolLanguageMethodology, - UserSkill, ) @@ -101,9 +102,12 @@ def skill_level() -> SkillLevel: @pytest.fixture -def user_skill(user, skill: Skill, skill_level: SkillLevel) -> UserSkill: - """Fixture for creating a UserSkill instance.""" - return UserSkill.objects.create(user=user, skill=skill, skill_level=skill_level) +def skill_profile(user, skill: Skill, skill_level: SkillLevel) -> SkillProfile: + """Fixture for creating a SkillProfile instance.""" + profile, _ = Profile.objects.get_or_create(user=user) + return SkillProfile.objects.create( + profile=profile, skill=skill, skill_level=skill_level + ) @pytest.fixture diff --git a/tests/main/test_account_data_views.py b/tests/main/test_account_data_views.py index f5094b8e..e9246a12 100644 --- a/tests/main/test_account_data_views.py +++ b/tests/main/test_account_data_views.py @@ -10,8 +10,8 @@ from .test_main_views import LoginRequiredMixin -class DownloadUserSkillDataMixin(LoginRequiredMixin): - """Test suite for the GetUserSkillsDataMixin.""" +class DownloadSkillProfileDataMixin(LoginRequiredMixin): + """Test suite mixin for the views that download SkillProfile data.""" _url: str _content_type: str @@ -38,14 +38,14 @@ def _get_url(self): return reverse(self._url) -class TestDownloadUserSkillDataCsvView(DownloadUserSkillDataMixin): - """Test suite for the DownloadCSVDataLinkView.""" +class TestDownloadSkillProfileDataCsvView(DownloadSkillProfileDataMixin): + """Test suite for the DownloadSkillProfileDataCsvView.""" _url = "download-csv-data-link" _content_type = "text/csv" def test_download_csv_data_link_view_csv_content( - self, client_logged_in, user_skill + self, client_logged_in, skill_profile ): """Test that the CSV content is correct.""" response = client_logged_in.get(self._get_url()) @@ -54,14 +54,14 @@ def test_download_csv_data_link_view_csv_content( assert "competency_domain" in content assert "competency" in content assert "skill_level" in content - assert user_skill.skill.name in content + assert skill_profile.skill.name in content assert ( content.count("\n") > 1 ) # Ensure there is at least one row of data besides the header -class TestDownloadUserSkillDataJSONView(DownloadUserSkillDataMixin): - """Test suite for the DownloadUserSkillDataJSONView.""" +class TestDownloadSkillProfileDataJSONView(DownloadSkillProfileDataMixin): + """Test suite for the DownloadSkillProfileDataJSONView.""" _url = "download-json-data-link" _content_type = "application/json" @@ -73,13 +73,13 @@ def test_download_json_data_link_view_json_content(self, client_logged_in): content = response.json() assert isinstance(content, dict) assert "username" in content - assert "user_data" in content - assert isinstance(content["user_data"], list) - if content["user_data"]: - assert "skill" in content["user_data"][0] - assert "competency_domain" in content["user_data"][0] - assert "competency" in content["user_data"][0] - assert "skill_level" in content["user_data"][0] + assert "skill_profile_data" in content + assert isinstance(content["skill_profile_data"], list) + if content["skill_profile_data"]: + assert "skill" in content["skill_profile_data"][0] + assert "competency_domain" in content["skill_profile_data"][0] + assert "competency" in content["skill_profile_data"][0] + assert "skill_level" in content["skill_profile_data"][0] def test_returns_anonymous_username_when_anonymous_param_is_true( self, client_logged_in @@ -89,5 +89,5 @@ def test_returns_anonymous_username_when_anonymous_param_is_true( assert response.status_code == HTTPStatus.OK content = response.json() assert content["username"] == "anonymous" - # Also check that the user_data is still present - assert "user_data" in content + # Also check that the skill_profile_data is still present + assert "skill_profile_data" in content diff --git a/tests/main/test_admin_views.py b/tests/main/test_admin_views.py index f54fb203..25c1ef1a 100644 --- a/tests/main/test_admin_views.py +++ b/tests/main/test_admin_views.py @@ -7,11 +7,11 @@ from django.contrib.auth import get_user_model from django.db.models import Model -from main.admin import UserProxy from main.models import ( Competency, CompetencyDomain, LearningResource, + Profile, Provider, Skill, SkillLevel, @@ -160,19 +160,14 @@ class TestSkillLevelAdmin(AdminMixin): } -class TestCustomUserSkillAdmin(AdminMixin): - """Test suite for the CustomUserSkill admin views. +class TestProfileAdmin(AdminMixin): + """Test suite for the SkillProfile admin views.""" - This Model should not have an add view. - """ + _model = Profile - _model = UserProxy - _data: ClassVar[dict[str, Any]] = { - "username": "TestUser", - "password": "testpassword", - } + def _create_object(self): + """Overwrite to also create the related user for the profile being changed.""" + user = TestUserAdmin()._create_object() + self._data["user"] = user - def test_add_get(self, admin_client): - """Overwrite to test the GET request to add an object is forbidden.""" - response = admin_client.get(f"/admin/main/{self._model._meta.model_name}/add/") - assert response.status_code == HTTPStatus.FORBIDDEN + return super()._create_object() diff --git a/tests/main/test_main_views.py b/tests/main/test_main_views.py index 805bff6d..030b39cc 100644 --- a/tests/main/test_main_views.py +++ b/tests/main/test_main_views.py @@ -15,7 +15,7 @@ from django.urls import reverse from pytest_django.asserts import assertTemplateUsed -from main.models import LearningResource, Provider, Skill, SkillLevel, UserSkill +from main.models import LearningResource, Provider, Skill, SkillLevel, SkillProfile from main.views.page_views import _extract_and_combine_roles from .view_utils import ( @@ -265,7 +265,7 @@ def _get_url(self): @pytest.mark.django_db def test_post(self, client, user, competency, skill, skill_level): - """Test the view POST request creates new user skills and redirects.""" + """Test the view POST request creates new skill profiles and redirects.""" client.force_login(user) skill2 = Skill.objects.create( @@ -283,17 +283,17 @@ def test_post(self, client, user, competency, skill, skill_level): # Make POST request response = client.post(self._get_url(), data=form_data) - # Assert user skills are created - user_skills = UserSkill.objects.filter(user=user) - assert user_skills.count() == 2 + # Assert skill profiles are created + skill_profiles = SkillProfile.objects.filter(user=user) + assert skill_profiles.count() == 2 # Assert the skills are correctly associated - skill_ids = set(us.skill.id for us in user_skills) + skill_ids = set(us.skill.id for us in skill_profiles) assert skill_ids == {skill.id, skill2.id} - # Assert all user skills have the correct level - for user_skill in user_skills: - assert user_skill.skill_level == skill_level + # Assert all skill profiles have the correct level + for skill_profile in skill_profiles: + assert skill_profile.skill_level == skill_level # Assert redirects to self_assess (success_url) assert response.status_code == HTTPStatus.FOUND @@ -317,8 +317,8 @@ def test_unaccepted_users_are_redirected_to_terms_acceptance(client, user, url_n assert response.url == reverse("terms_acceptance") -class TestUserSkillProfile(TemplateOkMixin, BS4Mixin): - """Test suite for the user skill profile view.""" +class TestSkillProfile(TemplateOkMixin, BS4Mixin): + """Test suite for the skill profile view.""" _template_name = "main/user-skills-profile.html" @@ -335,7 +335,7 @@ def test_provides_required_context(self, admin_client, skill_level): [{"level": skill_level.level, "name": skill_level.name}] ) - def test_skill_wheel_script(self, user_skill, auth_soup): + def test_skill_wheel_script(self, skill_profile, auth_soup): """Test that the skill profile view contains the correct script.""" card = auth_soup.find("div", class_="card-body") @@ -343,13 +343,13 @@ def test_skill_wheel_script(self, user_skill, auth_soup): assert card.find("div", id="dataviz_root") skill_level_list = list(SkillLevel.objects.values("level", "name")) - user_skill_dict = { - "skill": user_skill.skill.name, - "category": user_skill.skill.competency.competency_domain.name, - "subcategory": user_skill.skill.competency.name, - "skill_level": user_skill.skill_level.level, + skill_profile_dict = { + "skill": skill_profile.skill.name, + "category": skill_profile.skill.competency.competency_domain.name, + "subcategory": skill_profile.skill.competency.name, + "skill_level": skill_profile.skill_level.level, } - chart_data = [{"user_id": "root", "user_data": [user_skill_dict]}] + chart_data = [{"user_id": "root", "skill_profile_data": [skill_profile_dict]}] assert card.find( tag_with_text_filter( @@ -366,7 +366,7 @@ def test_skill_wheel_script(self, user_skill, auth_soup): assert card.find( tag_with_text_filter( "script", - "renderRadialBarChart(target, charts[i].user_data, skillLevels);", + "renderRadialBarChart(target, charts[i].skill_profile_data, skillLevels);", # noqa: E501 ) ) @@ -677,9 +677,9 @@ def test_redirects_to_self_assess_when_no_skills(self, client, user): assert response.url == reverse("self_assess") @pytest.mark.django_db - def test_redirects_to_skill_profile_when_skills_exist(self, client, user_skill): + def test_redirects_to_skill_profile_when_skills_exist(self, client, skill_profile): """Test that users with skills are redirected to their skill profile.""" - client.force_login(user_skill.user) + client.force_login(skill_profile.profile.user) response = client.get(self._get_url()) assert response.status_code == HTTPStatus.FOUND @@ -751,8 +751,8 @@ def test_extract_and_combine_roles(): assert len(data) == 2 assert isinstance(data[0], dict) assert data[0]["name"] == "User 1" - assert data[0]["user_data"][0]["skill"] == "Skill 1" - assert data[1]["user_data"][1]["category"] == "Category 2" + assert data[0]["skill_profile_data"][0]["skill"] == "Skill 1" + assert data[1]["skill_profile_data"][1]["category"] == "Category 2" class TestFrameworkOverviewPageView(TemplateOkMixin): @@ -787,16 +787,16 @@ class TestViewSkillProfilePageView(TemplateOkMixin, BS4Mixin): _template_name = "main/shared-skills-profile.html" - def _example_chart_data(self, user_skill): + def _example_chart_data(self, skill_profile): return [ { "user_id": "root", - "user_data": [ + "skill_profile_data": [ { - "skill": user_skill.skill.name, - "category": user_skill.skill.competency.competency_domain.name, - "subcategory": user_skill.skill.competency.name, - "skill_level": user_skill.skill_level.level, + "skill": skill_profile.skill.name, + "category": skill_profile.skill.competency.competency_domain.name, # noqa: E501 + "subcategory": skill_profile.skill.competency.name, + "skill_level": skill_profile.skill_level.level, } ], } @@ -816,36 +816,36 @@ def _get_url(self, chart_data=None): url = f"{url}?{params}" return url - def test_provides_required_context(self, client, user_skill): + def test_provides_required_context(self, client, skill_profile): """Test that the view skill profile view provides the correct context.""" - url = self._get_url(self._example_chart_data(user_skill)) + url = self._get_url(self._example_chart_data(skill_profile)) response = client.get(url) assert response.status_code == HTTPStatus.OK assert "chart_data" in response.context assert isinstance(response.context["chart_data"], list) - assert response.context["chart_data"] == self._example_chart_data(user_skill) + assert response.context["chart_data"] == self._example_chart_data(skill_profile) assert "skill_levels" in response.context assert isinstance(response.context["skill_levels"], list) assert response.context["skill_levels"] == list( SkillLevel.objects.values("level", "name") ) - def test_skill_wheel_script(self, soup_factory, user_skill): + def test_skill_wheel_script(self, soup_factory, skill_profile): """Test that the skill profile view contains the correct script.""" - soup = soup_factory(chart_data=self._example_chart_data(user_skill)) + soup = soup_factory(chart_data=self._example_chart_data(skill_profile)) card = soup.find("div", class_="card-body") assert card.find(tag_with_text_filter("h1", "Skills profile")) assert card.find("div", id="dataviz_root") skill_level_list = list(SkillLevel.objects.values("level", "name")) - user_skill_dict = { - "skill": user_skill.skill.name, - "category": user_skill.skill.competency.competency_domain.name, - "subcategory": user_skill.skill.competency.name, - "skill_level": user_skill.skill_level.level, + skill_profile_dict = { + "skill": skill_profile.skill.name, + "category": skill_profile.skill.competency.competency_domain.name, + "subcategory": skill_profile.skill.competency.name, + "skill_level": skill_profile.skill_level.level, } - chart_data = [{"user_id": "root", "user_data": [user_skill_dict]}] + chart_data = [{"user_id": "root", "skill_profile_data": [skill_profile_dict]}] assert card.find( tag_with_text_filter("script", f"const skillLevels = {skill_level_list};") @@ -856,6 +856,6 @@ def test_skill_wheel_script(self, soup_factory, user_skill): assert card.find( tag_with_text_filter( "script", - "renderRadialBarChart(target, charts[i].user_data, skillLevels);", + "renderRadialBarChart(target, charts[i].skill_profile_data, skillLevels);", # noqa: E501 ) ) From 7b5bce468f5efb8a9b93d2d99f55a5668e8e9a3a Mon Sep 17 00:00:00 2001 From: Adrian D'Alessandro Date: Tue, 27 Oct 2026 15:09:42 +0000 Subject: [PATCH 5/6] Automatically add a profile to all newly created users --- docs/dev-guide/architecture.md | 11 ++-- docs/dev-guide/development.md | 7 +++ main/apps.py | 4 ++ main/signals.py | 22 ++++++++ tests/main/conftest.py | 4 +- tests/main/test_admin_views.py | 7 --- tests/main/test_main_views.py | 2 +- tests/main/test_registration_views.py | 3 ++ tests/main/test_user_models.py | 75 +++++++++++++++++++++++---- 9 files changed, 111 insertions(+), 24 deletions(-) create mode 100644 main/signals.py diff --git a/docs/dev-guide/architecture.md b/docs/dev-guide/architecture.md index 3fb843db..975ae641 100644 --- a/docs/dev-guide/architecture.md +++ b/docs/dev-guide/architecture.md @@ -14,7 +14,8 @@ DIRECT is a Django project with a single application, `main`. | `direct_webapp/urls.py` | Project routing, delegating application routes to `main.urls`. | | `direct_webapp/context_processors.py` | Shared template context, including the site title. | | `main/models/framework_models.py` | The competency framework's models. | -| `main/models/user_models.py` | The custom user model and user-specific skill assessments. | +| `main/models/user_models.py` | The custom user model, profiles, and profile skill assessments. | +| `main/signals.py` | Signal receivers, including automatic profile creation for new users. | | `main/models/community_models.py` | Community content, including events. | | `main/views/` | Views grouped by page, account, and data concerns. | | `main/forms.py` | Input validation, form-driven updates, and crispy-form layouts. | @@ -30,8 +31,12 @@ application does not have a mandatory service or repository layer. ## Domain orientation The framework describes skills and their levels. A user's self-assessment is -separate from that shared framework data: `UserSkill` associates a user, a skill, -and a skill level. +separate from that shared framework data: `SkillProfile` associates a profile, a +skill, and a skill level. A profile can exist without a user, but every user has +exactly one profile, created automatically by a `post_save` receiver in +`main/signals.py`. Code can therefore rely on `user.profile`. Users loaded from +fixtures do not trigger this receiver, so such fixtures must include their +profiles. The configured authentication model is `main.User`. Account views use the current request's user for personal data. Existing access behavior includes authentication diff --git a/docs/dev-guide/development.md b/docs/dev-guide/development.md index d208fc6f..eed49eab 100644 --- a/docs/dev-guide/development.md +++ b/docs/dev-guide/development.md @@ -66,6 +66,12 @@ pre-commit run --files mkdocs build --strict ``` +Ensure the documentation that is built is deleted: + +```bash +rm -r site/ +``` + At handover, report commands run and their results, including failures or checks that could not run. Distinguish a pre-existing failure from one introduced by the change; an unrun check is not a passing check. @@ -76,4 +82,5 @@ Before handover: - Update directly related documentation and examples for feature changes. - Run pre-commit checks on changed files. - Run the documentation build for documentation changes. +- Delete built documentation. - Report commands run and their results. diff --git a/main/apps.py b/main/apps.py index 56b54223..574060f6 100644 --- a/main/apps.py +++ b/main/apps.py @@ -8,3 +8,7 @@ class MainConfig(AppConfig): default_auto_field = "django.db.models.BigAutoField" name = "main" + + def ready(self) -> None: + """Connect the app's signal receivers.""" + from . import signals # noqa: F401 diff --git a/main/signals.py b/main/signals.py new file mode 100644 index 00000000..740dca45 --- /dev/null +++ b/main/signals.py @@ -0,0 +1,22 @@ +"""Signal receivers for the main app.""" + +from typing import Any + +from django.conf import settings +from django.db.models.signals import post_save +from django.dispatch import receiver + +from .models import Profile, User + + +@receiver(post_save, sender=settings.AUTH_USER_MODEL) +def create_user_profile( + sender: type[User], instance: User, created: bool, raw: bool, **kwargs: Any +) -> None: + """Create a profile for each newly created user. + + Raw saves from fixture loading are skipped, so fixtures that contain users + must also contain their profiles. + """ + if created and not raw: + Profile.objects.create(user=instance) diff --git a/tests/main/conftest.py b/tests/main/conftest.py index e815bb7e..029c1fcb 100644 --- a/tests/main/conftest.py +++ b/tests/main/conftest.py @@ -7,7 +7,6 @@ CompetencyDomain, Event, LearningResource, - Profile, Provider, Skill, SkillLevel, @@ -104,9 +103,8 @@ def skill_level() -> SkillLevel: @pytest.fixture def skill_profile(user, skill: Skill, skill_level: SkillLevel) -> SkillProfile: """Fixture for creating a SkillProfile instance.""" - profile, _ = Profile.objects.get_or_create(user=user) return SkillProfile.objects.create( - profile=profile, skill=skill, skill_level=skill_level + profile=user.profile, skill=skill, skill_level=skill_level ) diff --git a/tests/main/test_admin_views.py b/tests/main/test_admin_views.py index 25c1ef1a..c584201b 100644 --- a/tests/main/test_admin_views.py +++ b/tests/main/test_admin_views.py @@ -164,10 +164,3 @@ class TestProfileAdmin(AdminMixin): """Test suite for the SkillProfile admin views.""" _model = Profile - - def _create_object(self): - """Overwrite to also create the related user for the profile being changed.""" - user = TestUserAdmin()._create_object() - self._data["user"] = user - - return super()._create_object() diff --git a/tests/main/test_main_views.py b/tests/main/test_main_views.py index 030b39cc..49c643b3 100644 --- a/tests/main/test_main_views.py +++ b/tests/main/test_main_views.py @@ -284,7 +284,7 @@ def test_post(self, client, user, competency, skill, skill_level): response = client.post(self._get_url(), data=form_data) # Assert skill profiles are created - skill_profiles = SkillProfile.objects.filter(user=user) + skill_profiles = SkillProfile.objects.filter(profile=user.profile) assert skill_profiles.count() == 2 # Assert the skills are correctly associated diff --git a/tests/main/test_registration_views.py b/tests/main/test_registration_views.py index bf716624..adb052d3 100644 --- a/tests/main/test_registration_views.py +++ b/tests/main/test_registration_views.py @@ -7,6 +7,8 @@ from django.urls import reverse from pytest_django.asserts import assertContains, assertNotContains +from main.models import Profile + from .view_utils import LoginRequiredMixin, TemplateOkMixin @@ -201,6 +203,7 @@ def test_post(self, client, django_user_model): logged_in_user = auth.get_user(client) assert logged_in_user.is_authenticated assert logged_in_user == django_user_model.objects.get(email=user.email) + assert Profile.objects.filter(user=logged_in_user).count() == 1 def test_closed(self, client, settings): """Test the view when registration is closed.""" diff --git a/tests/main/test_user_models.py b/tests/main/test_user_models.py index 76344449..a114b966 100644 --- a/tests/main/test_user_models.py +++ b/tests/main/test_user_models.py @@ -3,17 +3,17 @@ import pytest from django.core.exceptions import ValidationError -from main.models import Skill, SkillLevel, UserSkill +from main.models import Profile, Skill, SkillLevel, SkillProfile @pytest.mark.django_db -def test_user_skill_model( - user_skill: UserSkill, user, skill: Skill, skill_level: SkillLevel +def test_skill_profile_model( + skill_profile: SkillProfile, user, skill: Skill, skill_level: SkillLevel ) -> None: - """Test the UserSkill model.""" - assert user_skill.user == user - assert user_skill.skill == skill - assert user_skill.skill_level == skill_level + """Test the SkillProfile model.""" + assert skill_profile.profile == user.profile + assert skill_profile.skill == skill + assert skill_profile.skill_level == skill_level new_skill_level = SkillLevel.objects.create( name="Intermediate", @@ -22,8 +22,63 @@ def test_user_skill_model( short_description="Intermediate", ) - new_user_skill = UserSkill(user=user, skill=skill, skill_level=new_skill_level) + new_skill_profile = SkillProfile( + profile=user.profile, skill=skill, skill_level=new_skill_level + ) with pytest.raises( - ValidationError, match=r"User skill with this User and Skill already exists." + ValidationError, + match=r"Skill profile with this Profile and Skill already exists.", ): - new_user_skill.full_clean() + new_skill_profile.full_clean() + + +@pytest.mark.django_db +def test_creating_a_user_creates_their_profile(django_user_model) -> None: + """Test that a newly created user has exactly one profile.""" + user = django_user_model.objects.create_user(username="new-user") + + assert Profile.objects.filter(user=user).count() == 1 + assert user.profile.user == user + + +@pytest.mark.django_db +def test_creating_a_superuser_creates_their_profile(django_user_model) -> None: + """Test that a newly created superuser has exactly one profile.""" + user = django_user_model.objects.create_superuser( + username="new-admin", email="admin@example.com", password="password" + ) + + assert Profile.objects.filter(user=user).count() == 1 + + +@pytest.mark.django_db +def test_saving_an_existing_user_keeps_their_profile(user) -> None: + """Test that saving an existing user does not create another profile.""" + profile = user.profile + + user.first_name = "Updated" + user.save() + + assert list(Profile.objects.filter(user=user)) == [profile] + + +@pytest.mark.django_db +def test_deleting_a_user_deletes_their_profile_and_skill_profiles( + skill_profile: SkillProfile, user +) -> None: + """Test that deleting a user deletes their profile and assessments.""" + profile = user.profile + + user.delete() + + assert not Profile.objects.filter(pk=profile.pk).exists() + assert not SkillProfile.objects.filter(pk=skill_profile.pk).exists() + + +@pytest.mark.django_db +def test_profile_can_exist_without_a_user() -> None: + """Test that an anonymous profile can be saved without a user.""" + profile = Profile.objects.create() + + profile.full_clean() + assert profile.user is None From 389e17e15cba87d47f9012eaf4a968e96e865295 Mon Sep 17 00:00:00 2001 From: Adrian D'Alessandro Date: Wed, 28 Oct 2026 12:16:13 +0000 Subject: [PATCH 6/6] Restrict deletions of user-associated Profiles in Admin --- main/admin.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/main/admin.py b/main/admin.py index 1dae2b19..30bad4a4 100644 --- a/main/admin.py +++ b/main/admin.py @@ -4,6 +4,7 @@ from django.contrib import admin from django.contrib.auth.admin import UserAdmin +from django.http import HttpRequest from import_export.admin import ImportExportModelAdmin from .io_resources import ( @@ -233,6 +234,18 @@ def user_is_active(self, obj: Profile) -> bool: return False return obj.user.is_active + def has_delete_permission( + self, request: HttpRequest, obj: Profile | None = None + ) -> bool: + """Prevent direct deletion of Profiles with associated users.""" + # Show checkboxes for bulk-deletion + if obj is None: + return True + # Allow deletion via the User admin page and when there is no related user + if "user" in request.path or obj.user is None: + return True + return False + class TeamMembershipInline(admin.TabularInline[TeamMembership, Team]): """Inline admin class for the Competency model."""