From f381e44af28a713455bc5318575f43178236445e Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 28 May 2025 09:45:03 +0200 Subject: [PATCH 01/24] feat: created-patch-onboarding-endpoint-and-added-in-model --- api/custom_auth/views.py | 21 ++++++++++++++++ api/users/migrations/0041_users.py | 18 ++++++++++++++ api/users/models.py | 15 ++++++++++-- api/users/serializers.py | 39 ++++++++++++++++++++++++++++++ 4 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 api/users/migrations/0041_users.py diff --git a/api/custom_auth/views.py b/api/custom_auth/views.py index 0987c8981338..e6a2f832035d 100644 --- a/api/custom_auth/views.py +++ b/api/custom_auth/views.py @@ -29,6 +29,7 @@ from custom_auth.mfa.trench.utils import user_token_generator from custom_auth.serializers import CustomUserDelete from users.constants import DEFAULT_DELETE_ORPHAN_ORGANISATIONS_VALUE +from users.serializers import PatchOnboardingSerializer from .models import UserPasswordResetRequest @@ -134,6 +135,26 @@ def perform_destroy(self, instance): # type: ignore[no-untyped-def] ) ) + @action( + detail=True, + methods=["PATCH"], + url_path="onboarding", + permission_classes=[IsAuthenticated], + ) + def update_onboarding_task(self, request, *args, **kwargs): # type: ignore[no-untyped-def] + user = self.get_object() + if request.user != user and not request.user.is_superuser: + return Response(status=status.HTTP_403_FORBIDDEN) + + serializer = PatchOnboardingSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + + onboarding = {**(user.onboarding or {}), **serializer.data} + user.onboarding = onboarding + user.save(update_fields=["onboarding"]) + + return Response(status=status.HTTP_204_NO_CONTENT) + @action(["post"], detail=False) def reset_password(self, request, *args, **kwargs): # type: ignore[no-untyped-def] serializer = self.get_serializer(data=request.data) diff --git a/api/users/migrations/0041_users.py b/api/users/migrations/0041_users.py new file mode 100644 index 000000000000..098284563804 --- /dev/null +++ b/api/users/migrations/0041_users.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.21 on 2025-05-27 14:20 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("users", "0040_default_marketing_consent_given_true"), + ] + + operations = [ + migrations.AddField( + model_name="ffadminuser", + name="onboarding", + field=models.JSONField(blank=True, null=True), + ), + ] diff --git a/api/users/models.py b/api/users/models.py index 9cb3bf7e0db2..135caffe9f92 100644 --- a/api/users/models.py +++ b/api/users/models.py @@ -1,7 +1,7 @@ import logging import typing import uuid -from datetime import timedelta +from datetime import timedelta, datetime from django.conf import settings from django.contrib.auth.base_user import BaseUserManager @@ -56,6 +56,15 @@ logger = logging.getLogger(__name__) +class OnboardingTask(typing.TypedDict): + name: str + completed_at: datetime # or str if you store as ISO string + + +class OnboardingType(typing.TypedDict): + tasks: typing.List[OnboardingTask] + + class SignUpType(models.TextChoices): NO_INVITE = "NO_INVITE" INVITE_EMAIL = "INVITE_EMAIL" @@ -111,7 +120,9 @@ class FFAdminUser(LifecycleModel, AbstractUser): # type: ignore[django-manager- last_name = models.CharField("last name", max_length=150) google_user_id = models.CharField(max_length=50, null=True, blank=True) github_user_id = models.CharField(max_length=50, null=True, blank=True) - + onboarding: typing.Optional[OnboardingType] = models.JSONField( + blank=True, null=True + ) # Default to True, since it is covered in our Terms of Service. marketing_consent_given = models.BooleanField( default=True, diff --git a/api/users/serializers.py b/api/users/serializers.py index d2d0b834c468..7d797ea50304 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -1,3 +1,5 @@ +from datetime import datetime + from djoser.serializers import ( # type: ignore[import-untyped] UserSerializer as DjoserUserSerializer, ) @@ -148,10 +150,46 @@ class UserPermissionGroupSerializerDetail(UserPermissionGroupSerializer): users = UserPermissionGroupMembershipSerializer(many=True, read_only=True) +class OnboardingTaskSerializer(serializers.Serializer): + name = serializers.CharField() + completed_at = serializers.DateTimeField(allow_null=True) + + def to_representation(self, instance): + rep = super().to_representation(instance) + completed_at = rep.get("completed_at") + + if isinstance(completed_at, datetime): + rep["completed_at"] = completed_at.isoformat() + + return rep + + +class PatchOnboardingSerializer(serializers.Serializer): + tasks = OnboardingTaskSerializer(many=True, required=False) + + # TODO: Onboarding will have integrations and tasks - one of them at least is required + def validate(self, data): + if "tasks" not in data: + raise serializers.ValidationError( + "At least one of 'tasks' or 'integrations' must be provided." + ) + return data + + def to_representation(self, instance): + return super().to_representation(instance) + + +class OnboardingTypeSerializer(serializers.Serializer): + tasks = OnboardingTaskSerializer(many=True) + + class CustomCurrentUserSerializer(DjoserUserSerializer): # type: ignore[misc] auth_type = serializers.CharField(read_only=True) is_superuser = serializers.BooleanField(read_only=True) uuid = serializers.UUIDField(read_only=True) + onboarding = OnboardingTypeSerializer( + read_only=True, required=False, allow_null=True + ) class Meta(DjoserUserSerializer.Meta): # type: ignore[misc] fields = DjoserUserSerializer.Meta.fields + ( @@ -159,6 +197,7 @@ class Meta(DjoserUserSerializer.Meta): # type: ignore[misc] "is_superuser", "date_joined", "uuid", + "onboarding", ) From ab26ff37be42ba6357098d3b8d6e13e9c4d5df55 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 28 May 2025 10:03:01 +0200 Subject: [PATCH 02/24] feat: implemented-patch-tools-in-onboarding --- api/users/models.py | 8 +++++++- api/users/serializers.py | 18 +++++++++++++++--- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/api/users/models.py b/api/users/models.py index 135caffe9f92..b733bc513edd 100644 --- a/api/users/models.py +++ b/api/users/models.py @@ -58,11 +58,17 @@ class OnboardingTask(typing.TypedDict): name: str - completed_at: datetime # or str if you store as ISO string + completed_at: datetime + + +class OnboardingTools(typing.TypedDict): + completed: bool + integrations: typing.List[str] class OnboardingType(typing.TypedDict): tasks: typing.List[OnboardingTask] + tools: OnboardingTools class SignUpType(models.TextChoices): diff --git a/api/users/serializers.py b/api/users/serializers.py index 7d797ea50304..419cbb1a1212 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -150,6 +150,18 @@ class UserPermissionGroupSerializerDetail(UserPermissionGroupSerializer): users = UserPermissionGroupMembershipSerializer(many=True, read_only=True) +class OnboardingToolsSerializer(serializers.Serializer): + completed = serializers.BooleanField(required=False) + integrations = serializers.ListField( + child=serializers.CharField(), allow_empty=True, required=True + ) + + def to_representation(self, instance): + if len(instance.get("integrations", [])) > 0 and not instance.get("completed"): + instance["completed"] = True + return instance + + class OnboardingTaskSerializer(serializers.Serializer): name = serializers.CharField() completed_at = serializers.DateTimeField(allow_null=True) @@ -166,12 +178,12 @@ def to_representation(self, instance): class PatchOnboardingSerializer(serializers.Serializer): tasks = OnboardingTaskSerializer(many=True, required=False) + tools = OnboardingToolsSerializer(required=False) - # TODO: Onboarding will have integrations and tasks - one of them at least is required def validate(self, data): - if "tasks" not in data: + if "tasks" not in data and "tools" not in data: raise serializers.ValidationError( - "At least one of 'tasks' or 'integrations' must be provided." + "At least one of 'tasks' or 'tools' must be provided." ) return data From fe83bdc87cc6600233fd58a47841e40ad8c2d4f7 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 28 May 2025 10:07:51 +0200 Subject: [PATCH 03/24] feat: default-tools-completed-to-true-but-allows-false --- api/users/serializers.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/api/users/serializers.py b/api/users/serializers.py index 419cbb1a1212..37fb975060cd 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -151,16 +151,11 @@ class UserPermissionGroupSerializerDetail(UserPermissionGroupSerializer): class OnboardingToolsSerializer(serializers.Serializer): - completed = serializers.BooleanField(required=False) + completed = serializers.BooleanField(required=False, default=True) integrations = serializers.ListField( child=serializers.CharField(), allow_empty=True, required=True ) - def to_representation(self, instance): - if len(instance.get("integrations", [])) > 0 and not instance.get("completed"): - instance["completed"] = True - return instance - class OnboardingTaskSerializer(serializers.Serializer): name = serializers.CharField() From ad3614badb4f91418e0f65c3dc405be35810bad4 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 28 May 2025 10:11:57 +0200 Subject: [PATCH 04/24] feat: only-return-completed-in-me-tools --- api/users/serializers.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/api/users/serializers.py b/api/users/serializers.py index 37fb975060cd..46096fbda002 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -150,14 +150,14 @@ class UserPermissionGroupSerializerDetail(UserPermissionGroupSerializer): users = UserPermissionGroupMembershipSerializer(many=True, read_only=True) -class OnboardingToolsSerializer(serializers.Serializer): +class OnboardingToolsSerializer(serializers.Serializer[None]): completed = serializers.BooleanField(required=False, default=True) integrations = serializers.ListField( child=serializers.CharField(), allow_empty=True, required=True ) -class OnboardingTaskSerializer(serializers.Serializer): +class OnboardingTaskSerializer(serializers.Serializer[None]): name = serializers.CharField() completed_at = serializers.DateTimeField(allow_null=True) @@ -171,7 +171,7 @@ def to_representation(self, instance): return rep -class PatchOnboardingSerializer(serializers.Serializer): +class PatchOnboardingSerializer(serializers.Serializer[None]): tasks = OnboardingTaskSerializer(many=True, required=False) tools = OnboardingToolsSerializer(required=False) @@ -186,8 +186,18 @@ def to_representation(self, instance): return super().to_representation(instance) -class OnboardingTypeSerializer(serializers.Serializer): +class OnboardingTypeSerializer(serializers.Serializer[None]): tasks = OnboardingTaskSerializer(many=True) + tools = OnboardingToolsSerializer(required=False) + + def to_representation(self, instance): + rep = super().to_representation(instance) + + tools = rep.get("tools") + if tools and isinstance(tools, dict): + rep["tools"] = {"completed": tools.get("completed", False)} + + return rep class CustomCurrentUserSerializer(DjoserUserSerializer): # type: ignore[misc] From a7be1fc3c2537164ff90da27d92a93de341c396c Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 28 May 2025 08:52:45 +0000 Subject: [PATCH 05/24] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- api/users/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/users/models.py b/api/users/models.py index b733bc513edd..184812d3d7c2 100644 --- a/api/users/models.py +++ b/api/users/models.py @@ -1,7 +1,7 @@ import logging import typing import uuid -from datetime import timedelta, datetime +from datetime import datetime, timedelta from django.conf import settings from django.contrib.auth.base_user import BaseUserManager From b825799a6678782f0ec39d1915ac4f9a3b64eec0 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 28 May 2025 11:00:40 +0200 Subject: [PATCH 06/24] feat: finetuned-typing --- api/users/models.py | 19 +------------------ api/users/serializers.py | 14 +++++++------- 2 files changed, 8 insertions(+), 25 deletions(-) diff --git a/api/users/models.py b/api/users/models.py index b733bc513edd..d97ce7623054 100644 --- a/api/users/models.py +++ b/api/users/models.py @@ -56,21 +56,6 @@ logger = logging.getLogger(__name__) -class OnboardingTask(typing.TypedDict): - name: str - completed_at: datetime - - -class OnboardingTools(typing.TypedDict): - completed: bool - integrations: typing.List[str] - - -class OnboardingType(typing.TypedDict): - tasks: typing.List[OnboardingTask] - tools: OnboardingTools - - class SignUpType(models.TextChoices): NO_INVITE = "NO_INVITE" INVITE_EMAIL = "INVITE_EMAIL" @@ -126,9 +111,7 @@ class FFAdminUser(LifecycleModel, AbstractUser): # type: ignore[django-manager- last_name = models.CharField("last name", max_length=150) google_user_id = models.CharField(max_length=50, null=True, blank=True) github_user_id = models.CharField(max_length=50, null=True, blank=True) - onboarding: typing.Optional[OnboardingType] = models.JSONField( - blank=True, null=True - ) + onboarding = models.JSONField(blank=True, null=True) # Default to True, since it is covered in our Terms of Service. marketing_consent_given = models.BooleanField( default=True, diff --git a/api/users/serializers.py b/api/users/serializers.py index 46096fbda002..16987efba563 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -1,5 +1,5 @@ from datetime import datetime - +from typing import Any from djoser.serializers import ( # type: ignore[import-untyped] UserSerializer as DjoserUserSerializer, ) @@ -161,7 +161,7 @@ class OnboardingTaskSerializer(serializers.Serializer[None]): name = serializers.CharField() completed_at = serializers.DateTimeField(allow_null=True) - def to_representation(self, instance): + def to_representation(self, instance: Any) -> dict[str, Any]: rep = super().to_representation(instance) completed_at = rep.get("completed_at") @@ -175,22 +175,22 @@ class PatchOnboardingSerializer(serializers.Serializer[None]): tasks = OnboardingTaskSerializer(many=True, required=False) tools = OnboardingToolsSerializer(required=False) - def validate(self, data): + def validate(self, data: dict[str, Any]) -> dict[str, Any]: if "tasks" not in data and "tools" not in data: raise serializers.ValidationError( "At least one of 'tasks' or 'tools' must be provided." ) return data - def to_representation(self, instance): + def to_representation(self, instance: Any) -> dict[str, Any]: return super().to_representation(instance) -class OnboardingTypeSerializer(serializers.Serializer[None]): +class OnboardingResponseTypeSerializer(serializers.Serializer[None]): tasks = OnboardingTaskSerializer(many=True) tools = OnboardingToolsSerializer(required=False) - def to_representation(self, instance): + def to_representation(self, instance: Any) -> dict[str, Any]: rep = super().to_representation(instance) tools = rep.get("tools") @@ -204,7 +204,7 @@ class CustomCurrentUserSerializer(DjoserUserSerializer): # type: ignore[misc] auth_type = serializers.CharField(read_only=True) is_superuser = serializers.BooleanField(read_only=True) uuid = serializers.UUIDField(read_only=True) - onboarding = OnboardingTypeSerializer( + onboarding = OnboardingResponseTypeSerializer( read_only=True, required=False, allow_null=True ) From 492687f574b19d5c75f97cd29155a722a7771f30 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 28 May 2025 11:04:34 +0200 Subject: [PATCH 07/24] feat: completed-at-api-fallback-to-now --- api/users/serializers.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/users/serializers.py b/api/users/serializers.py index 16987efba563..03c2a78e0d31 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -165,7 +165,9 @@ def to_representation(self, instance: Any) -> dict[str, Any]: rep = super().to_representation(instance) completed_at = rep.get("completed_at") - if isinstance(completed_at, datetime): + if not completed_at: + rep["completed_at"] = datetime.now().isoformat() + elif isinstance(completed_at, datetime): rep["completed_at"] = completed_at.isoformat() return rep From d46e8d3ffe186ee64a905f9a5e1e5684bba92302 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 28 May 2025 09:13:07 +0000 Subject: [PATCH 08/24] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- api/users/models.py | 2 +- api/users/serializers.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/api/users/models.py b/api/users/models.py index 1d7fa8f57a21..6edbec618cc0 100644 --- a/api/users/models.py +++ b/api/users/models.py @@ -1,7 +1,7 @@ import logging import typing import uuid -from datetime import datetime, timedelta +from datetime import timedelta from django.conf import settings from django.contrib.auth.base_user import BaseUserManager diff --git a/api/users/serializers.py b/api/users/serializers.py index 03c2a78e0d31..0f0c7645e8cc 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -1,5 +1,6 @@ from datetime import datetime from typing import Any + from djoser.serializers import ( # type: ignore[import-untyped] UserSerializer as DjoserUserSerializer, ) From a460132e8b003bbc233c6d621206475fe436a07e Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 28 May 2025 11:56:45 +0200 Subject: [PATCH 09/24] feat: added-user-onboarding-serializer-tests --- .../unit/users/test_unit_users_serializers.py | 92 ++++++++++++++++++- api/users/serializers.py | 9 +- 2 files changed, 96 insertions(+), 5 deletions(-) diff --git a/api/tests/unit/users/test_unit_users_serializers.py b/api/tests/unit/users/test_unit_users_serializers.py index 94b0a34a6eca..3256a7c0824f 100644 --- a/api/tests/unit/users/test_unit_users_serializers.py +++ b/api/tests/unit/users/test_unit_users_serializers.py @@ -1,13 +1,99 @@ import pytest from rest_framework.exceptions import ValidationError +from freezegun import freeze_time +from datetime import datetime +from users.serializers import ( + UserIdsSerializer, + OnboardingTaskSerializer, + PatchOnboardingSerializer, + OnboardingResponseTypeSerializer, +) -from users.serializers import UserIdsSerializer - -def test_user_ids_serializer_raises_exception_for_invalid_user_id(db): # type: ignore[no-untyped-def] +def test_user_ids_serializer_raises_exception_for_invalid_user_id(db) -> None: # Given serializer = UserIdsSerializer(data={"user_ids": [99999]}) # Then with pytest.raises(ValidationError): serializer.is_valid(raise_exception=True) + + +@freeze_time("2025-01-01T12:00:00Z") +def test_onboarding_task_serializer_list_returns_correct_format(): + # Given + data = [ + {"name": "task-1"}, + {"name": "task-2", "completed_at": "2024-01-02T15:00:00Z"}, + {"name": "task-3", "completed_at": None}, + ] + + # When + serializer = OnboardingTaskSerializer(data=data, many=True) + assert serializer.is_valid(), serializer.errors + + # Then + results = serializer.data + assert results[0]["completed_at"] == datetime.now().isoformat() + assert results[0]["name"] == "task-1" + assert results[1]["completed_at"] == "2024-01-02T15:00:00Z" + assert results[1]["name"] == "task-2" + assert results[2]["completed_at"] == datetime.now().isoformat() + assert results[2]["name"] == "task-3" + + +@pytest.mark.parametrize("tools_completed", [True, False, None]) +def test_patch_onboarding_serializer_returns_correct_format(tools_completed) -> None: + # Given + data = { + "tasks": [ + {"name": "task-1", "completed_at": "2024-01-02T15:00:00Z"}, + ], + "tools": { + "completed": tools_completed, + "integrations": ["integration-1", "integration-2"], + }, + } + + # When + serializer = PatchOnboardingSerializer(data=data) + assert serializer.is_valid(), serializer.errors + + # Then + data = serializer.validated_data + assert data["tasks"] == [ + { + "name": "task-1", + "completed_at": datetime.fromisoformat("2024-01-02T15:00:00Z"), + }, + ] + assert data["tools"] == { + "completed": True if tools_completed is None else tools_completed, + "integrations": ["integration-1", "integration-2"], + } + + +def test_onboarding_response_type_serializer_omits_integrations() -> None: + # Given + data = { + "tasks": [{"name": "task-1", "completed_at": "2024-01-02T15:00:00Z"}], + "tools": { + "completed": True, + "integrations": ["integration-1", "integration-2"], + }, + } + + # When + serializer = OnboardingResponseTypeSerializer(data=data) + assert serializer.is_valid(), serializer.errors + + # Then + data = serializer.data + assert data["tools"] == {"completed": True} + assert "integrations" not in data["tools"] + assert data["tasks"] == [ + { + "name": "task-1", + "completed_at": "2024-01-02T15:00:00Z", + }, + ] diff --git a/api/users/serializers.py b/api/users/serializers.py index 03c2a78e0d31..799fb4557366 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -151,15 +151,20 @@ class UserPermissionGroupSerializerDetail(UserPermissionGroupSerializer): class OnboardingToolsSerializer(serializers.Serializer[None]): - completed = serializers.BooleanField(required=False, default=True) + completed = serializers.BooleanField(required=False, allow_null=True) integrations = serializers.ListField( child=serializers.CharField(), allow_empty=True, required=True ) + def validate(self, data: dict[str, Any]) -> dict[str, Any]: + if data.get("completed") is None: + data["completed"] = True + return data + class OnboardingTaskSerializer(serializers.Serializer[None]): name = serializers.CharField() - completed_at = serializers.DateTimeField(allow_null=True) + completed_at = serializers.DateTimeField(allow_null=True, required=False) def to_representation(self, instance: Any) -> dict[str, Any]: rep = super().to_representation(instance) From 4dce3b3aa08adef63b6ad5f5d7ad15c9bcf8bd9a Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 28 May 2025 14:56:09 +0200 Subject: [PATCH 10/24] feat: added-user-onboarding-views-tests --- api/custom_auth/views.py | 2 +- .../test_unit_custom_auth_views.py | 95 +++++++++++++++++++ .../unit/users/test_unit_users_serializers.py | 18 ++-- 3 files changed, 107 insertions(+), 8 deletions(-) diff --git a/api/custom_auth/views.py b/api/custom_auth/views.py index e6a2f832035d..f58e1fa066bf 100644 --- a/api/custom_auth/views.py +++ b/api/custom_auth/views.py @@ -141,7 +141,7 @@ def perform_destroy(self, instance): # type: ignore[no-untyped-def] url_path="onboarding", permission_classes=[IsAuthenticated], ) - def update_onboarding_task(self, request, *args, **kwargs): # type: ignore[no-untyped-def] + def patch_onboarding(self, request, *args, **kwargs): # type: ignore[no-untyped-def] user = self.get_object() if request.user != user and not request.user.is_superuser: return Response(status=status.HTTP_403_FORBIDDEN) diff --git a/api/tests/unit/custom_auth/test_unit_custom_auth_views.py b/api/tests/unit/custom_auth/test_unit_custom_auth_views.py index 09b6d45de632..bbd4a2ea5911 100644 --- a/api/tests/unit/custom_auth/test_unit_custom_auth_views.py +++ b/api/tests/unit/custom_auth/test_unit_custom_auth_views.py @@ -1,3 +1,6 @@ +from typing import Any + +import pytest from django.urls import reverse from rest_framework import status from rest_framework.test import APIClient @@ -20,3 +23,95 @@ def test_get_current_user(staff_user: FFAdminUser, staff_client: APIClient) -> N assert response_json["first_name"] == staff_user.first_name assert response_json["last_name"] == staff_user.last_name assert response_json["uuid"] == str(staff_user.uuid) + + +@pytest.mark.django_db() +def test_get_me_should_return_onboarding_object_without_integrations() -> None: + # Given + new_user = FFAdminUser.objects.create( + email="testuser@mail.com", + onboarding={ + "tasks": [{"name": "task-1"}], + "tools": {"completed": True, "integrations": ["integration-1"]}, + }, + ) + new_user.save() + client = APIClient() + client.force_authenticate(user=new_user) + url = reverse("api-v1:custom_auth:ffadminuser-me") + + # When + response = client.get(url) + + # Then + assert response.status_code == status.HTTP_200_OK + response_json = response.json() + assert response_json["onboarding"] is not None + assert response_json["onboarding"].get("tools", {}).get("completed") is True + assert response_json["onboarding"].get("tools", {}).get("integrations") is None + assert response_json["onboarding"].get("tasks") is not None + assert response_json["onboarding"].get("tasks", [])[0].get("name") == "task-1" + + +def test_patch_user_onboarding_returns_403_if_not_from_user( + staff_user: FFAdminUser, staff_client: APIClient +) -> None: + # Given + new_user = FFAdminUser.objects.create(email="testuser@mail.com") + new_user.save() + url = reverse("api-v1:custom_auth:ffadminuser-patch-onboarding", args=[new_user.id]) + + # When + response = staff_client.patch(url, data={"tasks": [{"name": "task-1"}]}) + + # Then + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.parametrize( + "data,expected_keys", + [ + ( + {"tasks": [{"name": "task-1", "completed_at": "2024-01-01T12:00:00Z"}]}, + {"tasks"}, + ), + ({"tools": {"completed": True, "integrations": ["integration-1"]}}, {"tools"}), + ( + { + "tasks": [{"name": "task-1", "completed_at": "2024-01-01T12:00:00Z"}], + "tools": {"completed": True, "integrations": ["integration-1"]}, + }, + {"tasks", "tools"}, + ), + ], +) +def test_patch_user_onboarding_updates_only_tasks_without_tools( + staff_user: FFAdminUser, + staff_client: APIClient, + data: dict[str, Any], + expected_keys: set[str], +) -> None: + # Given + url = reverse( + "api-v1:custom_auth:ffadminuser-patch-onboarding", args=[staff_user.id] + ) + + # When + response = staff_client.patch(url, data=data, format="json") + + # Then + staff_user.refresh_from_db() + + assert response.status_code == status.HTTP_204_NO_CONTENT + onboarding = staff_user.onboarding + assert onboarding is not None + if "tasks" in expected_keys: + assert onboarding.get("tasks", [])[0] + assert onboarding.get("tasks", [])[0].get("name") == data.get("tasks", [])[ + 0 + ].get("name") + if "tools" in expected_keys: + assert onboarding.get("tools", {}).get("completed") is True + assert onboarding.get("tools", {}).get("integrations") == data.get( + "tools", {} + ).get("integrations") diff --git a/api/tests/unit/users/test_unit_users_serializers.py b/api/tests/unit/users/test_unit_users_serializers.py index 3256a7c0824f..729ee938b480 100644 --- a/api/tests/unit/users/test_unit_users_serializers.py +++ b/api/tests/unit/users/test_unit_users_serializers.py @@ -1,16 +1,18 @@ +from datetime import datetime + import pytest -from rest_framework.exceptions import ValidationError from freezegun import freeze_time -from datetime import datetime +from rest_framework.exceptions import ValidationError + from users.serializers import ( - UserIdsSerializer, + OnboardingResponseTypeSerializer, OnboardingTaskSerializer, PatchOnboardingSerializer, - OnboardingResponseTypeSerializer, + UserIdsSerializer, ) -def test_user_ids_serializer_raises_exception_for_invalid_user_id(db) -> None: +def test_user_ids_serializer_raises_exception_for_invalid_user_id() -> None: # Given serializer = UserIdsSerializer(data={"user_ids": [99999]}) @@ -20,7 +22,7 @@ def test_user_ids_serializer_raises_exception_for_invalid_user_id(db) -> None: @freeze_time("2025-01-01T12:00:00Z") -def test_onboarding_task_serializer_list_returns_correct_format(): +def test_onboarding_task_serializer_list_returns_correct_format() -> None: # Given data = [ {"name": "task-1"}, @@ -43,7 +45,9 @@ def test_onboarding_task_serializer_list_returns_correct_format(): @pytest.mark.parametrize("tools_completed", [True, False, None]) -def test_patch_onboarding_serializer_returns_correct_format(tools_completed) -> None: +def test_patch_onboarding_serializer_returns_correct_format( + tools_completed: bool | None, +) -> None: # Given data = { "tasks": [ From f668852e0538e4e11618bca6c393f1962fc5d209 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 28 May 2025 15:15:25 +0200 Subject: [PATCH 11/24] feat: moved-endpoint-to-me-onboarding --- api/custom_auth/views.py | 11 ++++------ .../test_unit_custom_auth_views.py | 22 ++----------------- .../unit/users/test_unit_users_serializers.py | 2 +- 3 files changed, 7 insertions(+), 28 deletions(-) diff --git a/api/custom_auth/views.py b/api/custom_auth/views.py index f58e1fa066bf..93042284627d 100644 --- a/api/custom_auth/views.py +++ b/api/custom_auth/views.py @@ -136,16 +136,13 @@ def perform_destroy(self, instance): # type: ignore[no-untyped-def] ) @action( - detail=True, - methods=["PATCH"], - url_path="onboarding", + detail=False, + methods=["patch"], + url_path="me/onboarding", permission_classes=[IsAuthenticated], ) def patch_onboarding(self, request, *args, **kwargs): # type: ignore[no-untyped-def] - user = self.get_object() - if request.user != user and not request.user.is_superuser: - return Response(status=status.HTTP_403_FORBIDDEN) - + user = request.user serializer = PatchOnboardingSerializer(data=request.data) serializer.is_valid(raise_exception=True) diff --git a/api/tests/unit/custom_auth/test_unit_custom_auth_views.py b/api/tests/unit/custom_auth/test_unit_custom_auth_views.py index bbd4a2ea5911..ce03b7cc017f 100644 --- a/api/tests/unit/custom_auth/test_unit_custom_auth_views.py +++ b/api/tests/unit/custom_auth/test_unit_custom_auth_views.py @@ -25,8 +25,7 @@ def test_get_current_user(staff_user: FFAdminUser, staff_client: APIClient) -> N assert response_json["uuid"] == str(staff_user.uuid) -@pytest.mark.django_db() -def test_get_me_should_return_onboarding_object_without_integrations() -> None: +def test_get_me_should_return_onboarding_object_without_integrations(db: None) -> None: # Given new_user = FFAdminUser.objects.create( email="testuser@mail.com", @@ -53,21 +52,6 @@ def test_get_me_should_return_onboarding_object_without_integrations() -> None: assert response_json["onboarding"].get("tasks", [])[0].get("name") == "task-1" -def test_patch_user_onboarding_returns_403_if_not_from_user( - staff_user: FFAdminUser, staff_client: APIClient -) -> None: - # Given - new_user = FFAdminUser.objects.create(email="testuser@mail.com") - new_user.save() - url = reverse("api-v1:custom_auth:ffadminuser-patch-onboarding", args=[new_user.id]) - - # When - response = staff_client.patch(url, data={"tasks": [{"name": "task-1"}]}) - - # Then - assert response.status_code == status.HTTP_403_FORBIDDEN - - @pytest.mark.parametrize( "data,expected_keys", [ @@ -92,9 +76,7 @@ def test_patch_user_onboarding_updates_only_tasks_without_tools( expected_keys: set[str], ) -> None: # Given - url = reverse( - "api-v1:custom_auth:ffadminuser-patch-onboarding", args=[staff_user.id] - ) + url = reverse("api-v1:custom_auth:ffadminuser-patch-onboarding") # When response = staff_client.patch(url, data=data, format="json") diff --git a/api/tests/unit/users/test_unit_users_serializers.py b/api/tests/unit/users/test_unit_users_serializers.py index 729ee938b480..e585e56a063d 100644 --- a/api/tests/unit/users/test_unit_users_serializers.py +++ b/api/tests/unit/users/test_unit_users_serializers.py @@ -12,7 +12,7 @@ ) -def test_user_ids_serializer_raises_exception_for_invalid_user_id() -> None: +def test_user_ids_serializer_raises_exception_for_invalid_user_id(db: None) -> None: # Given serializer = UserIdsSerializer(data={"user_ids": [99999]}) From ff1496b21c43e719e58aaee4a114d1afe1fe1b94 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 28 May 2025 15:33:56 +0200 Subject: [PATCH 12/24] feat: added-test-coverage --- .../custom_auth/test_unit_custom_auth_views.py | 17 +++++++++++++++++ .../unit/users/test_unit_users_serializers.py | 16 ++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/api/tests/unit/custom_auth/test_unit_custom_auth_views.py b/api/tests/unit/custom_auth/test_unit_custom_auth_views.py index ce03b7cc017f..1c43e0bdef9e 100644 --- a/api/tests/unit/custom_auth/test_unit_custom_auth_views.py +++ b/api/tests/unit/custom_auth/test_unit_custom_auth_views.py @@ -97,3 +97,20 @@ def test_patch_user_onboarding_updates_only_tasks_without_tools( assert onboarding.get("tools", {}).get("integrations") == data.get( "tools", {} ).get("integrations") + + +def test_patch_user_onboarding_returns_error_if_tasks_and_tools_are_missing( + staff_user: FFAdminUser, + staff_client: APIClient, +) -> None: + # Given + url = reverse("api-v1:custom_auth:ffadminuser-patch-onboarding") + + # When + response = staff_client.patch(url, data={}, format="json") + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json() == { + "non_field_errors": ["At least one of 'tasks' or 'tools' must be provided."] + } diff --git a/api/tests/unit/users/test_unit_users_serializers.py b/api/tests/unit/users/test_unit_users_serializers.py index e585e56a063d..bcdd174d7fff 100644 --- a/api/tests/unit/users/test_unit_users_serializers.py +++ b/api/tests/unit/users/test_unit_users_serializers.py @@ -101,3 +101,19 @@ def test_onboarding_response_type_serializer_omits_integrations() -> None: "completed_at": "2024-01-02T15:00:00Z", }, ] + + +def test_onboarding_task_to_representation_converts_datetime_to_json_compatible_string() -> ( + None +): + serializer = OnboardingTaskSerializer() + + result = serializer.to_representation( + { + "name": "task-1", + "completed_at": datetime(2025, 1, 1, 12, 0, 0), + } + ) + + assert result["completed_at"] == "2025-01-01T12:00:00" + assert result["name"] == "task-1" From bf085686560117df99dda7c546a5e9bf80e7c889 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 28 May 2025 15:43:09 +0200 Subject: [PATCH 13/24] feat: fixed-non-utc-datetime-test --- api/tests/unit/users/test_unit_users_serializers.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/api/tests/unit/users/test_unit_users_serializers.py b/api/tests/unit/users/test_unit_users_serializers.py index bcdd174d7fff..1e74b18d7d00 100644 --- a/api/tests/unit/users/test_unit_users_serializers.py +++ b/api/tests/unit/users/test_unit_users_serializers.py @@ -106,8 +106,10 @@ def test_onboarding_response_type_serializer_omits_integrations() -> None: def test_onboarding_task_to_representation_converts_datetime_to_json_compatible_string() -> ( None ): + # Given serializer = OnboardingTaskSerializer() + # When result = serializer.to_representation( { "name": "task-1", @@ -115,5 +117,6 @@ def test_onboarding_task_to_representation_converts_datetime_to_json_compatible_ } ) + # Then assert result["completed_at"] == "2025-01-01T12:00:00" assert result["name"] == "task-1" From 9aab71250cf511fddc774978d08419fd3a0bc7eb Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 28 May 2025 15:58:50 +0200 Subject: [PATCH 14/24] feat: expose-integrations-in-me --- api/tests/unit/users/test_unit_users_serializers.py | 5 +++-- api/users/serializers.py | 9 --------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/api/tests/unit/users/test_unit_users_serializers.py b/api/tests/unit/users/test_unit_users_serializers.py index 1e74b18d7d00..9a57ff5500cc 100644 --- a/api/tests/unit/users/test_unit_users_serializers.py +++ b/api/tests/unit/users/test_unit_users_serializers.py @@ -1,4 +1,4 @@ -from datetime import datetime +from datetime import datetime, timezone import pytest from freezegun import freeze_time @@ -118,5 +118,6 @@ def test_onboarding_task_to_representation_converts_datetime_to_json_compatible_ ) # Then - assert result["completed_at"] == "2025-01-01T12:00:00" + expected = datetime(2025, 1, 1, 12, 0, 0, tzinfo=timezone.utc).isoformat() + assert result["completed_at"] == expected assert result["name"] == "task-1" diff --git a/api/users/serializers.py b/api/users/serializers.py index 64925e10d9e3..66ba8c243507 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -198,15 +198,6 @@ class OnboardingResponseTypeSerializer(serializers.Serializer[None]): tasks = OnboardingTaskSerializer(many=True) tools = OnboardingToolsSerializer(required=False) - def to_representation(self, instance: Any) -> dict[str, Any]: - rep = super().to_representation(instance) - - tools = rep.get("tools") - if tools and isinstance(tools, dict): - rep["tools"] = {"completed": tools.get("completed", False)} - - return rep - class CustomCurrentUserSerializer(DjoserUserSerializer): # type: ignore[misc] auth_type = serializers.CharField(read_only=True) From 617171b1168df74d65555f2a833fa7de486d12f2 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 28 May 2025 16:02:55 +0200 Subject: [PATCH 15/24] feat: updated-tests-with-integrations --- .../test_unit_custom_auth_views.py | 8 +++--- .../unit/users/test_unit_users_serializers.py | 26 ------------------- 2 files changed, 5 insertions(+), 29 deletions(-) diff --git a/api/tests/unit/custom_auth/test_unit_custom_auth_views.py b/api/tests/unit/custom_auth/test_unit_custom_auth_views.py index 1c43e0bdef9e..f935e838039a 100644 --- a/api/tests/unit/custom_auth/test_unit_custom_auth_views.py +++ b/api/tests/unit/custom_auth/test_unit_custom_auth_views.py @@ -25,7 +25,7 @@ def test_get_current_user(staff_user: FFAdminUser, staff_client: APIClient) -> N assert response_json["uuid"] == str(staff_user.uuid) -def test_get_me_should_return_onboarding_object_without_integrations(db: None) -> None: +def test_get_me_should_return_onboarding_object(db: None) -> None: # Given new_user = FFAdminUser.objects.create( email="testuser@mail.com", @@ -47,7 +47,9 @@ def test_get_me_should_return_onboarding_object_without_integrations(db: None) - response_json = response.json() assert response_json["onboarding"] is not None assert response_json["onboarding"].get("tools", {}).get("completed") is True - assert response_json["onboarding"].get("tools", {}).get("integrations") is None + assert response_json["onboarding"].get("tools", {}).get("integrations") == [ + "integration-1" + ] assert response_json["onboarding"].get("tasks") is not None assert response_json["onboarding"].get("tasks", [])[0].get("name") == "task-1" @@ -69,7 +71,7 @@ def test_get_me_should_return_onboarding_object_without_integrations(db: None) - ), ], ) -def test_patch_user_onboarding_updates_only_tasks_without_tools( +def test_patch_user_onboarding_updates_only_nested_objects_if_provided( staff_user: FFAdminUser, staff_client: APIClient, data: dict[str, Any], diff --git a/api/tests/unit/users/test_unit_users_serializers.py b/api/tests/unit/users/test_unit_users_serializers.py index 9a57ff5500cc..4637ed9899cd 100644 --- a/api/tests/unit/users/test_unit_users_serializers.py +++ b/api/tests/unit/users/test_unit_users_serializers.py @@ -77,32 +77,6 @@ def test_patch_onboarding_serializer_returns_correct_format( } -def test_onboarding_response_type_serializer_omits_integrations() -> None: - # Given - data = { - "tasks": [{"name": "task-1", "completed_at": "2024-01-02T15:00:00Z"}], - "tools": { - "completed": True, - "integrations": ["integration-1", "integration-2"], - }, - } - - # When - serializer = OnboardingResponseTypeSerializer(data=data) - assert serializer.is_valid(), serializer.errors - - # Then - data = serializer.data - assert data["tools"] == {"completed": True} - assert "integrations" not in data["tools"] - assert data["tasks"] == [ - { - "name": "task-1", - "completed_at": "2024-01-02T15:00:00Z", - }, - ] - - def test_onboarding_task_to_representation_converts_datetime_to_json_compatible_string() -> ( None ): From c79a9f54b44008aaa90d9c64a72c5a7c190c4aab Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 28 May 2025 14:03:20 +0000 Subject: [PATCH 16/24] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- api/tests/unit/users/test_unit_users_serializers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/api/tests/unit/users/test_unit_users_serializers.py b/api/tests/unit/users/test_unit_users_serializers.py index 4637ed9899cd..0fd300a55dc0 100644 --- a/api/tests/unit/users/test_unit_users_serializers.py +++ b/api/tests/unit/users/test_unit_users_serializers.py @@ -5,7 +5,6 @@ from rest_framework.exceptions import ValidationError from users.serializers import ( - OnboardingResponseTypeSerializer, OnboardingTaskSerializer, PatchOnboardingSerializer, UserIdsSerializer, From ebb72bc9df03fe503c3225d31f85c6a419142655 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 28 May 2025 16:18:36 +0200 Subject: [PATCH 17/24] feat: reviewed-timezone-in-test --- api/tests/unit/users/test_unit_users_serializers.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/api/tests/unit/users/test_unit_users_serializers.py b/api/tests/unit/users/test_unit_users_serializers.py index 4637ed9899cd..0fbfd198ba43 100644 --- a/api/tests/unit/users/test_unit_users_serializers.py +++ b/api/tests/unit/users/test_unit_users_serializers.py @@ -1,11 +1,10 @@ -from datetime import datetime, timezone +from datetime import datetime import pytest from freezegun import freeze_time from rest_framework.exceptions import ValidationError from users.serializers import ( - OnboardingResponseTypeSerializer, OnboardingTaskSerializer, PatchOnboardingSerializer, UserIdsSerializer, @@ -82,16 +81,15 @@ def test_onboarding_task_to_representation_converts_datetime_to_json_compatible_ ): # Given serializer = OnboardingTaskSerializer() - + date = datetime(2025, 1, 1, 12, 0, 0) # When result = serializer.to_representation( { "name": "task-1", - "completed_at": datetime(2025, 1, 1, 12, 0, 0), + "completed_at": date, } ) # Then - expected = datetime(2025, 1, 1, 12, 0, 0, tzinfo=timezone.utc).isoformat() - assert result["completed_at"] == expected + assert result["completed_at"] == date.isoformat() assert result["name"] == "task-1" From 243ae48260449790543405f9761f597e2aabea0d Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 28 May 2025 17:34:39 +0200 Subject: [PATCH 18/24] feat: fixed-test --- api/tests/unit/users/test_unit_users_serializers.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/tests/unit/users/test_unit_users_serializers.py b/api/tests/unit/users/test_unit_users_serializers.py index 0fbfd198ba43..c79c0e965999 100644 --- a/api/tests/unit/users/test_unit_users_serializers.py +++ b/api/tests/unit/users/test_unit_users_serializers.py @@ -91,5 +91,7 @@ def test_onboarding_task_to_representation_converts_datetime_to_json_compatible_ ) # Then - assert result["completed_at"] == date.isoformat() + assert isinstance(date, datetime) + assert isinstance(result["completed_at"], str) + assert result["completed_at"] == "2025-01-01T12:00:00Z" assert result["name"] == "task-1" From 93c612a328a744a35e8a17c5868d24eff7c90378 Mon Sep 17 00:00:00 2001 From: wadii Date: Thu, 29 May 2025 10:23:43 +0200 Subject: [PATCH 19/24] feat: cleaned-up-redundant-to-representation --- .../unit/users/test_unit_users_serializers.py | 38 +++++++++---------- api/users/serializers.py | 10 +---- 2 files changed, 20 insertions(+), 28 deletions(-) diff --git a/api/tests/unit/users/test_unit_users_serializers.py b/api/tests/unit/users/test_unit_users_serializers.py index c79c0e965999..f4efe774fdfe 100644 --- a/api/tests/unit/users/test_unit_users_serializers.py +++ b/api/tests/unit/users/test_unit_users_serializers.py @@ -76,22 +76,22 @@ def test_patch_onboarding_serializer_returns_correct_format( } -def test_onboarding_task_to_representation_converts_datetime_to_json_compatible_string() -> ( - None -): - # Given - serializer = OnboardingTaskSerializer() - date = datetime(2025, 1, 1, 12, 0, 0) - # When - result = serializer.to_representation( - { - "name": "task-1", - "completed_at": date, - } - ) - - # Then - assert isinstance(date, datetime) - assert isinstance(result["completed_at"], str) - assert result["completed_at"] == "2025-01-01T12:00:00Z" - assert result["name"] == "task-1" +# def test_onboarding_task_to_representation_converts_datetime_to_json_compatible_string() -> ( +# None +# ): +# # Given +# serializer = OnboardingTaskSerializer() +# date = datetime(2025, 1, 1, 12, 0, 0) +# # When +# result = serializer.to_representation( +# { +# "name": "task-1", +# "completed_at": date, +# } +# ) + +# # Then +# assert isinstance(date, datetime) +# assert isinstance(result["completed_at"], str) +# assert result["completed_at"] == "2025-01-01T12:00:00Z" +# assert result["name"] == "task-1" diff --git a/api/users/serializers.py b/api/users/serializers.py index 66ba8c243507..951200743743 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -169,13 +169,8 @@ class OnboardingTaskSerializer(serializers.Serializer[None]): def to_representation(self, instance: Any) -> dict[str, Any]: rep = super().to_representation(instance) - completed_at = rep.get("completed_at") - - if not completed_at: + if rep.get("completed_at") is None: rep["completed_at"] = datetime.now().isoformat() - elif isinstance(completed_at, datetime): - rep["completed_at"] = completed_at.isoformat() - return rep @@ -190,9 +185,6 @@ def validate(self, data: dict[str, Any]) -> dict[str, Any]: ) return data - def to_representation(self, instance: Any) -> dict[str, Any]: - return super().to_representation(instance) - class OnboardingResponseTypeSerializer(serializers.Serializer[None]): tasks = OnboardingTaskSerializer(many=True) From 88c6eaadb35fbd33dc262c55b51b7c0a025e940f Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 2 Jun 2025 15:01:39 +0200 Subject: [PATCH 20/24] feat: store-onboarding-as-text --- api/custom_auth/views.py | 9 ++++-- .../test_unit_custom_auth_views.py | 24 +++++++++------- ..._users.py => 0041_add_onboarding_field.py} | 4 +-- api/users/models.py | 2 +- api/users/serializers.py | 28 +++++++++++++------ 5 files changed, 44 insertions(+), 23 deletions(-) rename api/users/migrations/{0041_users.py => 0041_add_onboarding_field.py} (74%) diff --git a/api/custom_auth/views.py b/api/custom_auth/views.py index 93042284627d..0e08b7f938d7 100644 --- a/api/custom_auth/views.py +++ b/api/custom_auth/views.py @@ -1,3 +1,4 @@ +import json from typing import Any from django.conf import settings @@ -145,9 +146,13 @@ def patch_onboarding(self, request, *args, **kwargs): # type: ignore[no-untyped user = request.user serializer = PatchOnboardingSerializer(data=request.data) serializer.is_valid(raise_exception=True) + try: + existing_onboarding = json.loads(user.onboarding) if user.onboarding else {} + except json.JSONDecodeError: + existing_onboarding = {} - onboarding = {**(user.onboarding or {}), **serializer.data} - user.onboarding = onboarding + updated_onboarding = {**existing_onboarding, **serializer.data} + user.onboarding = json.dumps(updated_onboarding) user.save(update_fields=["onboarding"]) return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/api/tests/unit/custom_auth/test_unit_custom_auth_views.py b/api/tests/unit/custom_auth/test_unit_custom_auth_views.py index f935e838039a..33d77dbd5b69 100644 --- a/api/tests/unit/custom_auth/test_unit_custom_auth_views.py +++ b/api/tests/unit/custom_auth/test_unit_custom_auth_views.py @@ -1,3 +1,4 @@ +import json from typing import Any import pytest @@ -27,13 +28,16 @@ def test_get_current_user(staff_user: FFAdminUser, staff_client: APIClient) -> N def test_get_me_should_return_onboarding_object(db: None) -> None: # Given + onboarding = { + "tasks": [{"name": "task-1"}], + "tools": {"completed": True, "integrations": ["integration-1"]}, + } + onboarding_serialized = json.dumps(onboarding) new_user = FFAdminUser.objects.create( email="testuser@mail.com", - onboarding={ - "tasks": [{"name": "task-1"}], - "tools": {"completed": True, "integrations": ["integration-1"]}, - }, + onboarding=onboarding_serialized, ) + new_user.save() client = APIClient() client.force_authenticate(user=new_user) @@ -87,16 +91,16 @@ def test_patch_user_onboarding_updates_only_nested_objects_if_provided( staff_user.refresh_from_db() assert response.status_code == status.HTTP_204_NO_CONTENT - onboarding = staff_user.onboarding - assert onboarding is not None + onboarding_json = json.loads(staff_user.onboarding or "{}") + assert onboarding_json is not None if "tasks" in expected_keys: - assert onboarding.get("tasks", [])[0] - assert onboarding.get("tasks", [])[0].get("name") == data.get("tasks", [])[ + assert onboarding_json.get("tasks", [])[0] + assert onboarding_json.get("tasks", [])[0].get("name") == data.get("tasks", [])[ 0 ].get("name") if "tools" in expected_keys: - assert onboarding.get("tools", {}).get("completed") is True - assert onboarding.get("tools", {}).get("integrations") == data.get( + assert onboarding_json.get("tools", {}).get("completed") is True + assert onboarding_json.get("tools", {}).get("integrations") == data.get( "tools", {} ).get("integrations") diff --git a/api/users/migrations/0041_users.py b/api/users/migrations/0041_add_onboarding_field.py similarity index 74% rename from api/users/migrations/0041_users.py rename to api/users/migrations/0041_add_onboarding_field.py index 098284563804..1f1d5d3505fb 100644 --- a/api/users/migrations/0041_users.py +++ b/api/users/migrations/0041_add_onboarding_field.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.21 on 2025-05-27 14:20 +# Generated by Django 4.2.21 on 2025-06-02 12:19 from django.db import migrations, models @@ -13,6 +13,6 @@ class Migration(migrations.Migration): migrations.AddField( model_name="ffadminuser", name="onboarding", - field=models.JSONField(blank=True, null=True), + field=models.TextField(blank=True, null=True), ), ] diff --git a/api/users/models.py b/api/users/models.py index 6edbec618cc0..66bbea1b541c 100644 --- a/api/users/models.py +++ b/api/users/models.py @@ -111,7 +111,7 @@ class FFAdminUser(LifecycleModel, AbstractUser): # type: ignore[django-manager- last_name = models.CharField("last name", max_length=150) google_user_id = models.CharField(max_length=50, null=True, blank=True) github_user_id = models.CharField(max_length=50, null=True, blank=True) - onboarding = models.JSONField(blank=True, null=True) + onboarding = models.TextField(blank=True, null=True) # Default to True, since it is covered in our Terms of Service. marketing_consent_given = models.BooleanField( default=True, diff --git a/api/users/serializers.py b/api/users/serializers.py index 951200743743..6ab84d7175ec 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -1,3 +1,4 @@ +import json from datetime import datetime from typing import Any @@ -167,11 +168,8 @@ class OnboardingTaskSerializer(serializers.Serializer[None]): name = serializers.CharField() completed_at = serializers.DateTimeField(allow_null=True, required=False) - def to_representation(self, instance: Any) -> dict[str, Any]: - rep = super().to_representation(instance) - if rep.get("completed_at") is None: - rep["completed_at"] = datetime.now().isoformat() - return rep + def validate_completed_at(self, completed_at: datetime) -> datetime: + return completed_at or datetime.now() class PatchOnboardingSerializer(serializers.Serializer[None]): @@ -195,9 +193,23 @@ class CustomCurrentUserSerializer(DjoserUserSerializer): # type: ignore[misc] auth_type = serializers.CharField(read_only=True) is_superuser = serializers.BooleanField(read_only=True) uuid = serializers.UUIDField(read_only=True) - onboarding = OnboardingResponseTypeSerializer( - read_only=True, required=False, allow_null=True - ) + + def to_representation(self, instance: FFAdminUser) -> dict[str, Any]: + rep = super().to_representation(instance) + + try: + onboarding_json = ( + json.loads(instance.onboarding) if instance.onboarding else None + ) + except json.JSONDecodeError: + onboarding_json = None + + rep["onboarding"] = ( + OnboardingResponseTypeSerializer(onboarding_json).data + if onboarding_json + else None + ) + return rep # type: ignore[no-any-return] class Meta(DjoserUserSerializer.Meta): # type: ignore[misc] fields = DjoserUserSerializer.Meta.fields + ( From 3d4be90ec341de535aafd43d0134e5f04bd3b571 Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 2 Jun 2025 15:57:20 +0200 Subject: [PATCH 21/24] feat: adapted-serializer-test --- api/tests/unit/users/test_unit_users_serializers.py | 8 ++++---- api/users/serializers.py | 6 +++++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/api/tests/unit/users/test_unit_users_serializers.py b/api/tests/unit/users/test_unit_users_serializers.py index f4efe774fdfe..f088ec30a434 100644 --- a/api/tests/unit/users/test_unit_users_serializers.py +++ b/api/tests/unit/users/test_unit_users_serializers.py @@ -34,12 +34,12 @@ def test_onboarding_task_serializer_list_returns_correct_format() -> None: assert serializer.is_valid(), serializer.errors # Then - results = serializer.data - assert results[0]["completed_at"] == datetime.now().isoformat() + results = serializer.validated_data + assert results[0]["completed_at"] == datetime.now() assert results[0]["name"] == "task-1" - assert results[1]["completed_at"] == "2024-01-02T15:00:00Z" + assert results[1]["completed_at"] == datetime.fromisoformat("2024-01-02T15:00:00Z") assert results[1]["name"] == "task-2" - assert results[2]["completed_at"] == datetime.now().isoformat() + assert results[2]["completed_at"] == datetime.now() assert results[2]["name"] == "task-3" diff --git a/api/users/serializers.py b/api/users/serializers.py index 6ab84d7175ec..3c66aa81d647 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -166,7 +166,11 @@ def validate(self, data: dict[str, Any]) -> dict[str, Any]: class OnboardingTaskSerializer(serializers.Serializer[None]): name = serializers.CharField() - completed_at = serializers.DateTimeField(allow_null=True, required=False) + completed_at = serializers.DateTimeField( + allow_null=True, + required=False, + default=lambda: datetime.now(), + ) def validate_completed_at(self, completed_at: datetime) -> datetime: return completed_at or datetime.now() From 02b46bb28d8f83605a35914af620c5b899a2cbc8 Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 2 Jun 2025 16:15:54 +0200 Subject: [PATCH 22/24] feat: reworked-json-fallbacks --- api/custom_auth/views.py | 5 +---- api/users/serializers.py | 8 +++----- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/api/custom_auth/views.py b/api/custom_auth/views.py index 0e08b7f938d7..e47f97a62fef 100644 --- a/api/custom_auth/views.py +++ b/api/custom_auth/views.py @@ -146,10 +146,7 @@ def patch_onboarding(self, request, *args, **kwargs): # type: ignore[no-untyped user = request.user serializer = PatchOnboardingSerializer(data=request.data) serializer.is_valid(raise_exception=True) - try: - existing_onboarding = json.loads(user.onboarding) if user.onboarding else {} - except json.JSONDecodeError: - existing_onboarding = {} + existing_onboarding = json.loads(user.onboarding) if user.onboarding else {} updated_onboarding = {**existing_onboarding, **serializer.data} user.onboarding = json.dumps(updated_onboarding) diff --git a/api/users/serializers.py b/api/users/serializers.py index 3c66aa81d647..d6903dc17bd3 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -201,11 +201,9 @@ class CustomCurrentUserSerializer(DjoserUserSerializer): # type: ignore[misc] def to_representation(self, instance: FFAdminUser) -> dict[str, Any]: rep = super().to_representation(instance) - try: - onboarding_json = ( - json.loads(instance.onboarding) if instance.onboarding else None - ) - except json.JSONDecodeError: + if instance.onboarding is not None: + onboarding_json = json.loads(instance.onboarding) + else: onboarding_json = None rep["onboarding"] = ( From 5af677190f556e1a1270834ed74bdea2111b25d8 Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 3 Jun 2025 15:48:27 +0200 Subject: [PATCH 23/24] feat: renamed-to-onboarding-data --- api/custom_auth/views.py | 14 ++++++++----- .../test_unit_custom_auth_views.py | 4 ++-- .../unit/users/test_unit_users_serializers.py | 21 ------------------- .../migrations/0041_add_onboarding_field.py | 2 +- api/users/models.py | 2 +- api/users/serializers.py | 5 ++--- 6 files changed, 15 insertions(+), 33 deletions(-) diff --git a/api/custom_auth/views.py b/api/custom_auth/views.py index e47f97a62fef..f4a80619cb26 100644 --- a/api/custom_auth/views.py +++ b/api/custom_auth/views.py @@ -30,6 +30,7 @@ from custom_auth.mfa.trench.utils import user_token_generator from custom_auth.serializers import CustomUserDelete from users.constants import DEFAULT_DELETE_ORPHAN_ORGANISATIONS_VALUE +from users.models import FFAdminUser from users.serializers import PatchOnboardingSerializer from .models import UserPasswordResetRequest @@ -142,20 +143,23 @@ def perform_destroy(self, instance): # type: ignore[no-untyped-def] url_path="me/onboarding", permission_classes=[IsAuthenticated], ) - def patch_onboarding(self, request, *args, **kwargs): # type: ignore[no-untyped-def] + def patch_onboarding(self, request: Request, *args: Any, **kwargs: Any) -> Response: user = request.user + assert isinstance(user, FFAdminUser) serializer = PatchOnboardingSerializer(data=request.data) serializer.is_valid(raise_exception=True) - existing_onboarding = json.loads(user.onboarding) if user.onboarding else {} + existing_onboarding = ( + json.loads(user.onboarding_data) if user.onboarding_data else {} + ) updated_onboarding = {**existing_onboarding, **serializer.data} - user.onboarding = json.dumps(updated_onboarding) - user.save(update_fields=["onboarding"]) + user.onboarding_data = json.dumps(updated_onboarding) + user.save(update_fields=["onboarding_data"]) return Response(status=status.HTTP_204_NO_CONTENT) @action(["post"], detail=False) - def reset_password(self, request, *args, **kwargs): # type: ignore[no-untyped-def] + def reset_password(self, request: Request, *args: Any, **kwargs: Any) -> Response: serializer = self.get_serializer(data=request.data) serializer.is_valid(raise_exception=True) user = serializer.get_user() diff --git a/api/tests/unit/custom_auth/test_unit_custom_auth_views.py b/api/tests/unit/custom_auth/test_unit_custom_auth_views.py index 33d77dbd5b69..7f7536f32f95 100644 --- a/api/tests/unit/custom_auth/test_unit_custom_auth_views.py +++ b/api/tests/unit/custom_auth/test_unit_custom_auth_views.py @@ -35,7 +35,7 @@ def test_get_me_should_return_onboarding_object(db: None) -> None: onboarding_serialized = json.dumps(onboarding) new_user = FFAdminUser.objects.create( email="testuser@mail.com", - onboarding=onboarding_serialized, + onboarding_data=onboarding_serialized, ) new_user.save() @@ -91,7 +91,7 @@ def test_patch_user_onboarding_updates_only_nested_objects_if_provided( staff_user.refresh_from_db() assert response.status_code == status.HTTP_204_NO_CONTENT - onboarding_json = json.loads(staff_user.onboarding or "{}") + onboarding_json = json.loads(staff_user.onboarding_data or "{}") assert onboarding_json is not None if "tasks" in expected_keys: assert onboarding_json.get("tasks", [])[0] diff --git a/api/tests/unit/users/test_unit_users_serializers.py b/api/tests/unit/users/test_unit_users_serializers.py index f088ec30a434..6f6255afeff5 100644 --- a/api/tests/unit/users/test_unit_users_serializers.py +++ b/api/tests/unit/users/test_unit_users_serializers.py @@ -74,24 +74,3 @@ def test_patch_onboarding_serializer_returns_correct_format( "completed": True if tools_completed is None else tools_completed, "integrations": ["integration-1", "integration-2"], } - - -# def test_onboarding_task_to_representation_converts_datetime_to_json_compatible_string() -> ( -# None -# ): -# # Given -# serializer = OnboardingTaskSerializer() -# date = datetime(2025, 1, 1, 12, 0, 0) -# # When -# result = serializer.to_representation( -# { -# "name": "task-1", -# "completed_at": date, -# } -# ) - -# # Then -# assert isinstance(date, datetime) -# assert isinstance(result["completed_at"], str) -# assert result["completed_at"] == "2025-01-01T12:00:00Z" -# assert result["name"] == "task-1" diff --git a/api/users/migrations/0041_add_onboarding_field.py b/api/users/migrations/0041_add_onboarding_field.py index 1f1d5d3505fb..cfe0e54b24b6 100644 --- a/api/users/migrations/0041_add_onboarding_field.py +++ b/api/users/migrations/0041_add_onboarding_field.py @@ -12,7 +12,7 @@ class Migration(migrations.Migration): operations = [ migrations.AddField( model_name="ffadminuser", - name="onboarding", + name="onboarding_data", field=models.TextField(blank=True, null=True), ), ] diff --git a/api/users/models.py b/api/users/models.py index 66bbea1b541c..f09e5e4be79e 100644 --- a/api/users/models.py +++ b/api/users/models.py @@ -111,7 +111,7 @@ class FFAdminUser(LifecycleModel, AbstractUser): # type: ignore[django-manager- last_name = models.CharField("last name", max_length=150) google_user_id = models.CharField(max_length=50, null=True, blank=True) github_user_id = models.CharField(max_length=50, null=True, blank=True) - onboarding = models.TextField(blank=True, null=True) + onboarding_data = models.TextField(blank=True, null=True) # Default to True, since it is covered in our Terms of Service. marketing_consent_given = models.BooleanField( default=True, diff --git a/api/users/serializers.py b/api/users/serializers.py index d6903dc17bd3..a3c848ac29de 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -201,8 +201,8 @@ class CustomCurrentUserSerializer(DjoserUserSerializer): # type: ignore[misc] def to_representation(self, instance: FFAdminUser) -> dict[str, Any]: rep = super().to_representation(instance) - if instance.onboarding is not None: - onboarding_json = json.loads(instance.onboarding) + if instance.onboarding_data is not None: + onboarding_json = json.loads(instance.onboarding_data) else: onboarding_json = None @@ -219,7 +219,6 @@ class Meta(DjoserUserSerializer.Meta): # type: ignore[misc] "is_superuser", "date_joined", "uuid", - "onboarding", ) From 88fb4401d7c5e3a8515ff2371c4e146a88bead96 Mon Sep 17 00:00:00 2001 From: wadii Date: Wed, 4 Jun 2025 16:12:14 +0200 Subject: [PATCH 24/24] feat: improved-typing --- api/users/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/users/serializers.py b/api/users/serializers.py index a3c848ac29de..648787239548 100644 --- a/api/users/serializers.py +++ b/api/users/serializers.py @@ -172,7 +172,7 @@ class OnboardingTaskSerializer(serializers.Serializer[None]): default=lambda: datetime.now(), ) - def validate_completed_at(self, completed_at: datetime) -> datetime: + def validate_completed_at(self, completed_at: datetime | None) -> datetime: return completed_at or datetime.now()