Skip to content

feat: add specialty validation for doctor signup #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
M0stafa077 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add specialty validation for doctor signup #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions users/migrations/0007_alter_user_date_of_birth_alter_user_phone_number.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Generated by Django 5.2.13 on 2026-04-15 12:12

import users.models
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('users', '0006_user_optional_phone_and_dob'),
]

operations = [
migrations.AlterField(
model_name='user',
name='date_of_birth',
field=models.DateField(blank=True, null=True, validators=[users.models.validate_date_of_birth]),
),
migrations.AlterField(
model_name='user',
name='phone_number',
field=models.CharField(blank=True, max_length=11, null=True, validators=[users.models.validate_phone_number]),
),
]
14 changes: 7 additions & 7 deletions users/serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from __future__ import annotations

import logging
from dataclasses import dataclass
from datetime import date

from django.contrib.auth import authenticate, get_user_model, password_validation
Expand All @@ -19,12 +18,6 @@

logger = logging.getLogger(__name__)

@dataclass(frozen=True)
class TokenPair:
access: str
refresh: str


class TokenRefreshRequestSerializer(serializers.Serializer):
refresh = serializers.CharField(help_text="Valid refresh token.")

Expand Down Expand Up @@ -60,6 +53,7 @@ class Meta:
"phone_number",
"date_of_birth",
"role",
"specialty",

@coderabbitai coderabbitai Bot Apr 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Strip specialty for non-doctor roles to prevent stale data.

The new validate only enforces presence for doctors, but specialty is accepted from any caller and passed through create() into User.objects.create_user(**validated_data). A patient/receptionist can supply a specialty value and it will be persisted on the user. Consider dropping it for non-doctor roles:

♻️ Proposed change 
     def validate(self, attrs: dict) -> dict:
         role: str = attrs.get("role", "")
-        if role == "doctor" and not str(attrs.get("specialty") or "").strip():
-            raise serializers.ValidationError({"specialty": "specialty is required for doctor signup."})
+        if role == "doctor":
+            if not str(attrs.get("specialty") or "").strip():
+                raise serializers.ValidationError({"specialty": "specialty is required for doctor signup."})
+        else:
+            attrs.pop("specialty", None)
         return attrs

Also applies to: 66-70

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@users/serializers.py` at line 56, The serializer currently allows a
non-doctor to submit a specialty and it gets persisted; update the serializer's
validate and/or create flow to remove the "specialty" key from validated_data
unless the role equals the doctor role (e.g., Role.DOCTOR or "doctor"), so only
doctors keep specialty; specifically, inside the validate(self, attrs) or
create(self, validated_data) methods remove validated_data.pop("specialty",
None) when attrs.get("role") (or validated_data.get("role")) is not the doctor
role before calling User.objects.create_user(**validated_data), and apply the
same logic to any duplicate handling around lines referenced (66-70) to ensure
non-doctor roles cannot persist stale specialty values.

]

def validate_date_of_birth(self, value: date) -> date:
Expand All @@ -69,6 +63,12 @@ def validate_date_of_birth(self, value: date) -> date:
raise serializers.ValidationError("dateOfBirth must be in the past.")
return value

def validate(self, attrs: dict) -> dict:
role: str = attrs.get("role", "")
if role == "doctor" and not str(attrs.get("specialty") or "").strip():
raise serializers.ValidationError({"specialty": "specialty is required for doctor signup."})
return attrs

def validate_password(self, value: str) -> str:
try:
password_validation.validate_password(value)
Expand Down
5 changes: 2 additions & 3 deletions users/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
from drf_spectacular.utils import extend_schema

from users.google_oauth_service import GoogleOAuthService, GoogleOneTimeCodePayload
from users.models import User
from users.permissions import IsApproved
from users.password_otp import verify_otp
from users.serializers import (
Expand All @@ -34,11 +33,11 @@
SignupSerializer,
UserMeSerializer,
)

User = get_user_model()
from django.contrib.auth.models import Group
from users.welcome_email import send_welcome_email

User = get_user_model()


class SignupView(APIView):
permission_classes = [AllowAny]
Expand Down