From b55c0aca9d5c631d3726a320292646c55596e75d Mon Sep 17 00:00:00 2001 From: Payam Date: Thu, 30 Apr 2026 16:03:41 +0400 Subject: [PATCH] feat: #398 Add display name and photo for organization users --- ...ser_display_name_organizationuser_photo.py | 24 ++++++ django_email_learning/models.py | 11 ++- .../platform/api/serializers.py | 34 +++++++- django_email_learning/platform/api/views.py | 12 ++- django_email_learning/platform/views.py | 11 +++ .../components/AddInstructorsSection.jsx | 25 +++++- .../components/CreateInstructorForm.jsx | 65 +++++++++++++-- .../organization/components/UserForm.jsx | 71 +++++++++++++++- tests/conftest.py | 5 +- .../api/test_views/test_course_view.py | 10 ++- .../test_views/test_organization_user_api.py | 80 +++++++++++++++++++ tests/test_models/test_course_instructor.py | 9 ++- 12 files changed, 332 insertions(+), 25 deletions(-) create mode 100644 django_email_learning/migrations/0022_organizationuser_display_name_organizationuser_photo.py diff --git a/django_email_learning/migrations/0022_organizationuser_display_name_organizationuser_photo.py b/django_email_learning/migrations/0022_organizationuser_display_name_organizationuser_photo.py new file mode 100644 index 0000000..970f685 --- /dev/null +++ b/django_email_learning/migrations/0022_organizationuser_display_name_organizationuser_photo.py @@ -0,0 +1,24 @@ +# Generated by Django 6.0.4 on 2026-04-30 10:47 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("django_email_learning", "0021_courseinstructor"), + ] + + operations = [ + migrations.AddField( + model_name="organizationuser", + name="display_name", + field=models.CharField(blank=True, max_length=200, null=True), + ), + migrations.AddField( + model_name="organizationuser", + name="photo", + field=models.ImageField( + blank=True, null=True, upload_to="org_user_photos/" + ), + ), + ] diff --git a/django_email_learning/models.py b/django_email_learning/models.py index 3d8633d..19da716 100644 --- a/django_email_learning/models.py +++ b/django_email_learning/models.py @@ -129,17 +129,24 @@ class OrganizationUser(models.Model): ], db_index=True, ) + display_name = models.CharField(max_length=200, null=True, blank=True) + photo = models.ImageField(upload_to="org_user_photos/", null=True, blank=True) def __str__(self) -> str: return f"{self.user.username} - {self.organization.name}" def can_act_as_instructor(self) -> bool: - # TODO: When we add display name for org user we can also accept admin role as instructor - # if they have display name set, for now only users with instructor role can be course instructors if self.role == "instructor": return True + if self.role == "admin" and self.display_name: + return True return False + def save(self, *args, **kwargs) -> None: # type: ignore[no-untyped-def] + if self.role == "instructor" and not self.display_name: + raise ValidationError("Instructor role requires a display name.") + super().save(*args, **kwargs) + class Meta: unique_together = [["user", "organization"]] diff --git a/django_email_learning/platform/api/serializers.py b/django_email_learning/platform/api/serializers.py index d29480c..3341337 100644 --- a/django_email_learning/platform/api/serializers.py +++ b/django_email_learning/platform/api/serializers.py @@ -498,10 +498,30 @@ class UserRole(enum.StrEnum): class AddOrganizationUserRequest(BaseModel): user_id: int = Field(gt=0, examples=[1]) role: UserRole = Field(min_length=1, examples=[UserRole.ADMIN]) + display_name: Optional[str] = Field(None, examples=["John Doe"]) + photo: Optional[str] = Field(None, examples=["/path/to/photo.png"]) + @model_validator(mode="before") + def validate_instructor_display_name(cls, values: dict) -> dict: + role = values.get("role") + display_name = values.get("display_name") + if role == UserRole.INSTRUCTOR and not display_name: + raise ValueError("Instructor role requires a display name.") + return values -class UpdateOrganizationUserRoleRequest(BaseModel): + +class UpdateOrganizationUserRequest(BaseModel): role: UserRole = Field(min_length=1, examples=[UserRole.ADMIN]) + display_name: Optional[str] = Field(None, examples=["John Doe"]) + photo: Optional[str] = Field(None, examples=["/path/to/photo.png"]) + + @model_validator(mode="before") + def validate_instructor_display_name(cls, values: dict) -> dict: + role = values.get("role") + display_name = values.get("display_name") + if role == UserRole.INSTRUCTOR and not display_name: + raise ValueError("Instructor role requires a display name.") + return values class OrganizationUserResponse(BaseModel): @@ -511,9 +531,14 @@ class OrganizationUserResponse(BaseModel): email: str role: UserRole can_act_as_instructor: bool + display_name: Optional[str] = None + photo: Optional[str] = None + photo_url: Optional[str] = None @staticmethod - def from_django_model(org_user: OrganizationUser) -> "OrganizationUserResponse": + def from_django_model( + org_user: OrganizationUser, request: Any + ) -> "OrganizationUserResponse": return OrganizationUserResponse( id=org_user.id, user_id=org_user.user.id, @@ -521,6 +546,11 @@ def from_django_model(org_user: OrganizationUser) -> "OrganizationUserResponse": email=org_user.user.email, role=UserRole(org_user.role), can_act_as_instructor=org_user.can_act_as_instructor(), + display_name=org_user.display_name, + photo=org_user.photo.name if org_user.photo else None, + photo_url=request.build_absolute_uri(org_user.photo.url) + if org_user.photo + else None, ) diff --git a/django_email_learning/platform/api/views.py b/django_email_learning/platform/api/views.py index 047b4ae..d364855 100644 --- a/django_email_learning/platform/api/views.py +++ b/django_email_learning/platform/api/views.py @@ -626,11 +626,13 @@ def post(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-unt user_id=serializer.user_id, organization=organization, role=serializer.role, + display_name=serializer.display_name, + photo=serializer.photo, ) org_user.save() return JsonResponse( serializers.OrganizationUserResponse.from_django_model( - org_user + org_user, request ).model_dump(), status=201, ) @@ -649,7 +651,7 @@ def get(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-unty for org_user in organization_users: response_list.append( serializers.OrganizationUserResponse.from_django_model( - org_user + org_user, request ).model_dump() ) return JsonResponse({"organization_users": response_list}, status=200) @@ -674,17 +676,19 @@ def delete(self, request, *args, **kwargs): # type: ignore[no-untyped-def] def post(self, request, *args, **kwargs) -> JsonResponse: # type: ignore[no-untyped-def] try: payload = json.loads(request.body) - serializer = serializers.UpdateOrganizationUserRoleRequest.model_validate( + serializer = serializers.UpdateOrganizationUserRequest.model_validate( payload ) org_user = OrganizationUser.objects.get( organization_id=kwargs["organization_id"], user_id=kwargs["user_id"] ) org_user.role = serializer.role + org_user.display_name = serializer.display_name + org_user.photo = serializer.photo org_user.save() return JsonResponse( serializers.OrganizationUserResponse.from_django_model( - org_user + org_user, request ).model_dump(), status=200, ) diff --git a/django_email_learning/platform/views.py b/django_email_learning/platform/views.py index fc92101..85b7235 100644 --- a/django_email_learning/platform/views.py +++ b/django_email_learning/platform/views.py @@ -248,6 +248,11 @@ def get_locale_messages(self) -> Dict[str, str]: "select_instructors": _("Select Instructors"), "new_instructor": _("New Instructor"), "instructor_email": _("Instructor Email"), + "instructor_display_name": _("Display Name"), + "instructor_display_name_required": _( + "Display name is required for instructors." + ), + "instructor_photo": _("Instructor Photo"), "add_instructor": _("Add Instructor"), "instructor_add_failed": _("Failed to add instructor. Please try again."), } @@ -522,6 +527,9 @@ def get_locale_messages(self) -> Dict[str, str]: "change_user_role": _("Change User Role"), "user": _("User"), "role": _("Role"), + "display_name": _("Display Name"), + "display_name_required": _("Display name is required for instructors."), + "photo": _("Photo"), "admin": _("Admin"), "editor": _("Editor"), "instructor": _("Instructor"), @@ -545,6 +553,9 @@ def get_locale_messages(self) -> Dict[str, str]: ), "cancel": _("Cancel"), "delete": _("Delete"), + "upload_button_label": _("Upload Image"), + "remove_image": _("Remove Image"), + "uploaded_image_alt": _("User Photo"), } diff --git a/frontend/platform/courses/components/AddInstructorsSection.jsx b/frontend/platform/courses/components/AddInstructorsSection.jsx index f54c3f8..c976f90 100644 --- a/frontend/platform/courses/components/AddInstructorsSection.jsx +++ b/frontend/platform/courses/components/AddInstructorsSection.jsx @@ -3,6 +3,7 @@ import { Accordion, AccordionDetails, AccordionSummary, + Avatar, Box, Chip, FormControl, @@ -81,8 +82,13 @@ function AddInstructorsSection({ onChangeCallback, activeOrganizationId, initial return instructor ? ( + : {(instructor.display_name || instructor.email)[0].toUpperCase()} + } onDelete={(e) => { e.stopPropagation(); const updatedIds = selectedIds.filter((i) => i !== id); @@ -98,7 +104,22 @@ function AddInstructorsSection({ onChangeCallback, activeOrganizationId, initial > {orgInstructors.map((instructor) => ( - {instructor.email} + + {instructor.photo + ? + : {(instructor.display_name || instructor.email)[0].toUpperCase()} + } + + + {instructor.display_name || instructor.email} + + {instructor.display_name && ( + + {instructor.email} + + )} + + ))} diff --git a/frontend/platform/courses/components/CreateInstructorForm.jsx b/frontend/platform/courses/components/CreateInstructorForm.jsx index b2805fb..d3a3d30 100644 --- a/frontend/platform/courses/components/CreateInstructorForm.jsx +++ b/frontend/platform/courses/components/CreateInstructorForm.jsx @@ -1,6 +1,7 @@ import { useState } from 'react'; -import { Alert, Box, Button } from '@mui/material'; +import { Alert, Box, Button, Typography } from '@mui/material'; import RequiredTextField from '../../../src/components/RequiredTextField'; +import ImageUpload from '../../../src/components/ImageUpload.jsx'; import { useAppContext } from '../../../src/render.jsx'; import { getCookie } from '../../../src/utils'; @@ -8,6 +9,10 @@ import { getCookie } from '../../../src/utils'; const CreateInstructorForm = ({ onSuccess, activeOrganizationId }) => { const [email, setEmail] = useState(''); const [emailHelperText, setEmailHelperText] = useState(''); + const [displayName, setDisplayName] = useState(''); + const [displayNameHelperText, setDisplayNameHelperText] = useState(''); + const [photoPath, setPhotoPath] = useState(null); + const [photoUrl, setPhotoUrl] = useState(null); const [errorMessage, setErrorMessage] = useState(''); const { localeMessages, apiBaseUrl } = useAppContext(); @@ -15,15 +20,27 @@ const CreateInstructorForm = ({ onSuccess, activeOrganizationId }) => { const handleSubmit = () => { const trimmedEmail = email.trim(); + const trimmedDisplayName = displayName.trim(); + let valid = true; + if (!trimmedEmail) { setEmailHelperText(localeMessages['email_required_helper_text']); - return; - } - if (!isValidEmail(trimmedEmail)) { + valid = false; + } else if (!isValidEmail(trimmedEmail)) { setEmailHelperText(localeMessages['invalid_email_helper_text']); - return; + valid = false; + } else { + setEmailHelperText(''); + } + + if (!trimmedDisplayName) { + setDisplayNameHelperText(localeMessages['instructor_display_name_required']); + valid = false; + } else { + setDisplayNameHelperText(''); } - setEmailHelperText(''); + + if (!valid) return; setErrorMessage(''); fetch(`${apiBaseUrl}/users/get-or-create-by-email/`, { @@ -47,7 +64,12 @@ const CreateInstructorForm = ({ onSuccess, activeOrganizationId }) => { 'Content-Type': 'application/json', 'X-CSRFToken': getCookie('csrftoken'), }, - body: JSON.stringify({ user_id: userData.id, role: 'instructor' }), + body: JSON.stringify({ + user_id: userData.id, + role: 'instructor', + display_name: trimmedDisplayName, + photo: photoPath, + }), }) ) .then((response) => { @@ -57,6 +79,9 @@ const CreateInstructorForm = ({ onSuccess, activeOrganizationId }) => { .then((orgUserData) => { if (onSuccess) onSuccess(orgUserData); setEmail(''); + setDisplayName(''); + setPhotoPath(null); + setPhotoUrl(null); }) .catch((error) => { console.error('Error adding instructor:', error); @@ -76,9 +101,33 @@ const CreateInstructorForm = ({ onSuccess, activeOrganizationId }) => { onChange={(e) => setEmail(e.target.value)} type="email" /> - + ); }; diff --git a/frontend/platform/organization/components/UserForm.jsx b/frontend/platform/organization/components/UserForm.jsx index fe661d6..ffcd5f8 100644 --- a/frontend/platform/organization/components/UserForm.jsx +++ b/frontend/platform/organization/components/UserForm.jsx @@ -7,6 +7,7 @@ import Select from '@mui/material/Select'; import MenuItem from '@mui/material/MenuItem'; import Typography from '@mui/material/Typography'; import Button from '@mui/material/Button'; +import ImageUpload from '../../../src/components/ImageUpload.jsx'; import { getCookie } from '../../../src/utils.js'; import { useAppContext } from '../../../src/render.jsx'; @@ -15,6 +16,10 @@ const UserForm = ({ onClose, organizationId, refreshUsers, user = null }) => { const { localeMessages, apiBaseUrl } = useAppContext(); const [email, setEmail] = useState(user ? user.email : ''); const [role, setRole] = useState(user ? user.role : 'viewer'); + const [displayName, setDisplayName] = useState(user ? (user.display_name || '') : ''); + const [displayNameError, setDisplayNameError] = useState(''); + const [photoUrl, setPhotoUrl] = useState(user? user.photo_url : null); + const [photoPath, setPhotoPath] = useState(user ? (user.photo || null) : null); const [error, setError] = useState(''); const roleDescriptionByRole = { @@ -26,6 +31,18 @@ const UserForm = ({ onClose, organizationId, refreshUsers, user = null }) => { const selectedRoleDescription = roleDescriptionByRole[role] || ''; + const validateForm = () => { + setError(''); + + if (role === 'instructor' && !displayName.trim()) { + setDisplayNameError(localeMessages['display_name_required']); + return false; + } + + setDisplayNameError(''); + return true; + }; + const createUser = (id) => { fetch(`${apiBaseUrl}/organizations/${organizationId}/users/`, { method: 'POST', @@ -33,7 +50,12 @@ const UserForm = ({ onClose, organizationId, refreshUsers, user = null }) => { 'Content-Type': 'application/json', 'X-CSRFToken': getCookie('csrftoken'), }, - body: JSON.stringify({ 'user_id': id, 'role': role }), + body: JSON.stringify({ + 'user_id': id, + 'role': role, + 'display_name': displayName.trim() || null, + 'photo': photoPath, + }), }) .then(response => { if (!response.ok) { @@ -53,6 +75,9 @@ const UserForm = ({ onClose, organizationId, refreshUsers, user = null }) => { const handleSubmit = (event) => { event.preventDefault(); + if (!validateForm()) { + return; + } if (user) { updateUser(); } else { @@ -88,13 +113,19 @@ const UserForm = ({ onClose, organizationId, refreshUsers, user = null }) => { const updateUser = () => { console.log('Updating user:', user); + const payload = { + 'role': role , + display_name: displayName.trim() || null, + photo: photoPath, + }; + fetch(`${apiBaseUrl}/organizations/${organizationId}/users/${user.user_id}/`, { method: 'POST', headers: { 'Content-Type': 'application/json', 'X-CSRFToken': getCookie('csrftoken'), }, - body: JSON.stringify({ 'role': role }), + body: JSON.stringify(payload), }) .then(response => { if (!response.ok) { @@ -122,13 +153,31 @@ const UserForm = ({ onClose, organizationId, refreshUsers, user = null }) => { required disabled={!!user} /> + { + setDisplayName(e.target.value); + if (displayNameError) { + setDisplayNameError(''); + } + }} + required={role === 'instructor'} + error={Boolean(displayNameError)} + helperText={displayNameError} + /> {localeMessages["role"]}