From 02c9c17c30b847283d2e6a56c17ad6241f2742cd Mon Sep 17 00:00:00 2001 From: Amirhossein Khalili Date: Mon, 27 Apr 2026 22:58:27 +0330 Subject: [PATCH] fix(time-entries): preserve deleted tags in timesheet edits --- apps/time_entries/api/serializers.py | 243 ++++++++++++++------ apps/time_entries/api/views.py | 22 +- apps/time_entries/tests/test_serializers.py | 30 +++ apps/time_entries/tests/test_services.py | 33 ++- apps/time_entries/tests/test_views.py | 89 +++++++ 5 files changed, 331 insertions(+), 86 deletions(-) diff --git a/apps/time_entries/api/serializers.py b/apps/time_entries/api/serializers.py index 3eacd7e..b5923de 100644 --- a/apps/time_entries/api/serializers.py +++ b/apps/time_entries/api/serializers.py @@ -1,80 +1,175 @@ -from rest_framework import serializers - -from core.serializers.base import BaseModelSerializer -from apps.time_entries.models import TimeEntry -from apps.projects.models import Project -from apps.tags.models import Tag - - -class TimeEntrySerializer(BaseModelSerializer): - """ - Output serializer for TimeEntry. - """ - start_time = serializers.DateTimeField(format="%Y-%m-%d %H:%M:%S") - end_time = serializers.DateTimeField(format="%Y-%m-%d %H:%M:%S", allow_null=True) - - class Meta: - model = TimeEntry - fields = BaseModelSerializer.Meta.fields + ( - "workspace", - "user", - "project", - "description", - "start_time", - "end_time", - "duration", - "tags", - "is_billable", - "hourly_rate", - "currency", - ) - read_only_fields = fields - - -class TimeEntryCreateSerializer(serializers.Serializer): +from rest_framework import serializers + +from core.serializers.base import BaseModelSerializer +from apps.time_entries.models import TimeEntry +from apps.projects.models import Project +from apps.tags.models import Tag + + +class TimeEntryProjectDetailsSerializer(serializers.Serializer): + id = serializers.UUIDField(read_only=True) + name = serializers.CharField(read_only=True) + is_deleted = serializers.BooleanField(read_only=True) + client_name = serializers.CharField(read_only=True, allow_null=True) + + +class TimeEntryTagDetailsSerializer(serializers.Serializer): + id = serializers.UUIDField(read_only=True) + name = serializers.CharField(read_only=True) + color = serializers.CharField(read_only=True) + is_deleted = serializers.BooleanField(read_only=True) + + +class TimeEntrySerializer(BaseModelSerializer): + """ + Output serializer for TimeEntry. + """ + project = serializers.UUIDField(source="project_id", allow_null=True, read_only=True) + tags = serializers.SerializerMethodField() + project_details = serializers.SerializerMethodField() + tag_details = serializers.SerializerMethodField() + start_time = serializers.DateTimeField(format="%Y-%m-%d %H:%M:%S") + end_time = serializers.DateTimeField(format="%Y-%m-%d %H:%M:%S", allow_null=True) + + def get_tags(self, obj): + return [str(tag.id) for tag in Tag.all_objects.filter(time_entries=obj).order_by("-updated_at", "-created_at")] + + def get_project_details(self, obj): + if not obj.project_id: + return None + + project = Project.all_objects.select_related("client").filter(id=obj.project_id).first() + if not project: + return None + + return TimeEntryProjectDetailsSerializer( + { + "id": project.id, + "name": project.name, + "is_deleted": project.is_deleted, + "client_name": project.client.name if project.client else None, + } + ).data + + def get_tag_details(self, obj): + tags = Tag.all_objects.filter(time_entries=obj).order_by("-updated_at", "-created_at") + return TimeEntryTagDetailsSerializer( + [ + { + "id": tag.id, + "name": tag.name, + "color": tag.color, + "is_deleted": tag.is_deleted, + } + for tag in tags + ], + many=True, + ).data + + class Meta: + model = TimeEntry + fields = BaseModelSerializer.Meta.fields + ( + "workspace", + "user", + "project", + "project_details", + "description", + "start_time", + "end_time", + "duration", + "tags", + "tag_details", + "is_billable", + "hourly_rate", + "currency", + ) + read_only_fields = fields + + +class TimeEntryCreateSerializer(serializers.Serializer): """ Validates input data for creating/starting a time entry. """ - workspace_id = serializers.UUIDField() - project_id = serializers.PrimaryKeyRelatedField( - queryset=Project.objects.filter(is_deleted=False), - required=False, - allow_null=True, - source='project' - ) - start_time = serializers.DateTimeField() - end_time = serializers.DateTimeField(required=False, allow_null=True) - description = serializers.CharField(required=False, allow_blank=True, default="") - tags = serializers.PrimaryKeyRelatedField( - queryset=Tag.objects.filter(is_deleted=False), - many=True, - required=False - ) - is_billable = serializers.BooleanField(default=False) - - -class TimeEntryUpdateSerializer(serializers.Serializer): - """ - Validates input data for updating an existing time entry. - """ - project_id = serializers.PrimaryKeyRelatedField( - queryset=Project.objects.filter(is_deleted=False), - required=False, - allow_null=True, - source='project' - ) - start_time = serializers.DateTimeField(required=False) - end_time = serializers.DateTimeField(required=False, allow_null=True) - description = serializers.CharField(required=False, allow_blank=True) - tags = serializers.PrimaryKeyRelatedField( - queryset=Tag.objects.filter(is_deleted=False), - many=True, - required=False - ) - is_billable = serializers.BooleanField(required=False) - - -class TimeEntryStopSerializer(serializers.Serializer): + workspace_id = serializers.UUIDField() + project_id = serializers.UUIDField(required=False, allow_null=True) + start_time = serializers.DateTimeField() + end_time = serializers.DateTimeField(required=False, allow_null=True) + description = serializers.CharField(required=False, allow_blank=True, default="") + tags = serializers.ListField(child=serializers.UUIDField(), required=False) + is_billable = serializers.BooleanField(default=False) + + def validate(self, attrs): + project_id = attrs.pop("project_id", serializers.empty) + if project_id is not serializers.empty: + if project_id is None: + attrs["project"] = None + else: + project = Project.objects.filter(id=project_id).first() + if not project: + raise serializers.ValidationError({"project_id": "Selected project is unavailable."}) + attrs["project"] = project + + tag_ids = attrs.pop("tags", serializers.empty) + if tag_ids is not serializers.empty: + active_tags = list(Tag.objects.filter(id__in=tag_ids)) + active_tag_ids = {str(tag.id) for tag in active_tags} + missing_ids = [str(tag_id) for tag_id in tag_ids if str(tag_id) not in active_tag_ids] + if missing_ids: + raise serializers.ValidationError({"tags": "One or more selected tags are unavailable."}) + attrs["tags"] = active_tags + + return attrs + + +class TimeEntryUpdateSerializer(serializers.Serializer): + """ + Validates input data for updating an existing time entry. + """ + project_id = serializers.UUIDField(required=False, allow_null=True) + start_time = serializers.DateTimeField(required=False) + end_time = serializers.DateTimeField(required=False, allow_null=True) + description = serializers.CharField(required=False, allow_blank=True) + tags = serializers.ListField(child=serializers.UUIDField(), required=False) + is_billable = serializers.BooleanField(required=False) + + def validate(self, attrs): + entry = self.instance + + project_id = attrs.pop("project_id", serializers.empty) + if project_id is not serializers.empty: + current_project = Project.all_objects.filter(id=entry.project_id).first() if entry and entry.project_id else None + if project_id is None: + attrs["project"] = None + elif current_project and str(current_project.id) == str(project_id): + attrs["project"] = current_project + else: + project = Project.objects.filter(id=project_id).first() + if not project: + raise serializers.ValidationError({"project_id": "Selected project is unavailable."}) + attrs["project"] = project + + tag_ids = attrs.pop("tags", serializers.empty) + if tag_ids is not serializers.empty: + active_tags = list(Tag.objects.filter(id__in=tag_ids)) + active_tag_ids = {str(tag.id) for tag in active_tags} + current_tag_ids = { + str(tag_id) + for tag_id in Tag.all_objects.filter(time_entries=entry).values_list("id", flat=True) + } if entry else set() + requested_tag_ids = [str(tag_id) for tag_id in tag_ids] + missing_ids = [tag_id for tag_id in requested_tag_ids if tag_id not in active_tag_ids] + + if missing_ids: + if not set(missing_ids).issubset(current_tag_ids): + raise serializers.ValidationError({"tags": "One or more selected tags are unavailable."}) + attrs["tags"] = list(Tag.all_objects.filter(id__in=tag_ids)) + else: + attrs["tags"] = active_tags + + return attrs + + +class TimeEntryStopSerializer(serializers.Serializer): """ Optional specific serializer for stopping a timer manually. """ diff --git a/apps/time_entries/api/views.py b/apps/time_entries/api/views.py index 9ccadba..eb3ffcf 100644 --- a/apps/time_entries/api/views.py +++ b/apps/time_entries/api/views.py @@ -138,9 +138,9 @@ class TimeEntryViewSet(ModelViewSet): return TimeEntryStopSerializer return TimeEntrySerializer - def create(self, request, *args, **kwargs): - serializer = self.get_serializer(data=request.data) - serializer.is_valid(raise_exception=True) + def create(self, request, *args, **kwargs): + serializer = self.get_serializer(data=request.data) + serializer.is_valid(raise_exception=True) entry = create_time_entry( user=request.user, @@ -148,8 +148,8 @@ class TimeEntryViewSet(ModelViewSet): **serializer.validated_data ) - output_serializer = TimeEntrySerializer(entry) - return Response(output_serializer.data, status=status.HTTP_201_CREATED) + output_serializer = TimeEntrySerializer(entry, context=self.get_serializer_context()) + return Response(output_serializer.data, status=status.HTTP_201_CREATED) def update(self, request, *args, **kwargs): partial = kwargs.pop("partial", False) @@ -160,16 +160,16 @@ class TimeEntryViewSet(ModelViewSet): status=status.HTTP_403_FORBIDDEN, ) - serializer = self.get_serializer(data=request.data, partial=partial) - serializer.is_valid(raise_exception=True) + serializer = self.get_serializer(entry, data=request.data, partial=partial) + serializer.is_valid(raise_exception=True) updated_entry = update_time_entry( entry=entry, **serializer.validated_data ) - output_serializer = TimeEntrySerializer(updated_entry) - return Response(output_serializer.data, status=status.HTTP_200_OK) + output_serializer = TimeEntrySerializer(updated_entry, context=self.get_serializer_context()) + return Response(output_serializer.data, status=status.HTTP_200_OK) @action(detail=True, methods=["post"]) def stop(self, request, pk=None): @@ -189,8 +189,8 @@ class TimeEntryViewSet(ModelViewSet): end_time = serializer.validated_data.get("end_time") stopped_entry = stop_time_entry(entry, end_time=end_time) - output_serializer = TimeEntrySerializer(stopped_entry) - return Response(output_serializer.data, status=status.HTTP_200_OK) + output_serializer = TimeEntrySerializer(stopped_entry, context=self.get_serializer_context()) + return Response(output_serializer.data, status=status.HTTP_200_OK) def destroy(self, request, *args, **kwargs): """ diff --git a/apps/time_entries/tests/test_serializers.py b/apps/time_entries/tests/test_serializers.py index bd9b8c2..39f4a6d 100644 --- a/apps/time_entries/tests/test_serializers.py +++ b/apps/time_entries/tests/test_serializers.py @@ -4,6 +4,8 @@ from django.utils import timezone from apps.time_entries.api.serializers import TimeEntrySerializer from apps.time_entries.models import TimeEntry +from apps.projects.models import Project +from apps.tags.models import Tag from apps.users.models import User from apps.workspaces.models import Workspace @@ -27,3 +29,31 @@ def test_time_entry_serializer_keeps_seconds(db): assert data["start_time"] == start_time.strftime("%Y-%m-%d %H:%M:%S") assert data["end_time"] == end_time.strftime("%Y-%m-%d %H:%M:%S") + + +def test_time_entry_serializer_includes_deleted_project_and_tags(db): + user = User.objects.create_user(mobile="09124444444", password="secret123") + workspace = Workspace.objects.create(name="Core", owner=user) + project = Project.objects.create(workspace=workspace, name="Legacy Project") + tag = Tag.objects.create(workspace=workspace, name="Legacy Tag", color="#334155") + project.delete() + tag.delete() + + entry = TimeEntry.objects.create( + workspace=workspace, + user=user, + project=Project.all_objects.get(id=project.id), + description="Historical work", + start_time=timezone.now(), + end_time=timezone.now(), + ) + entry.tags.set([Tag.all_objects.get(id=tag.id)]) + + data = TimeEntrySerializer(entry).data + + assert data["project"] == str(project.id) + assert data["project_details"]["name"] == "Legacy Project" + assert data["project_details"]["is_deleted"] is True + assert data["tags"] == [str(tag.id)] + assert data["tag_details"][0]["name"] == "Legacy Tag" + assert data["tag_details"][0]["is_deleted"] is True diff --git a/apps/time_entries/tests/test_services.py b/apps/time_entries/tests/test_services.py index ddb060f..89d1ad0 100644 --- a/apps/time_entries/tests/test_services.py +++ b/apps/time_entries/tests/test_services.py @@ -4,7 +4,9 @@ import pytest from django.utils import timezone from rest_framework.exceptions import ValidationError -from apps.time_entries.services.time_entries import create_time_entry, stop_time_entry +from apps.projects.models import Project +from apps.tags.models import Tag +from apps.time_entries.services.time_entries import create_time_entry, stop_time_entry, update_time_entry from apps.users.models import User from apps.workspaces.models import Workspace @@ -45,3 +47,32 @@ def test_stop_time_entry_sets_end_time_and_duration(workspace_owner): assert stopped_entry.end_time is not None assert stopped_entry.duration is not None + + +def test_update_time_entry_preserves_deleted_project_and_tags(workspace_owner): + user, workspace = workspace_owner + project = Project.objects.create(workspace=workspace, name="Deleted project") + tag = Tag.objects.create(workspace=workspace, name="Deleted tag", color="#0f172a") + entry = create_time_entry( + user=user, + workspace_id=workspace.id, + start_time=timezone.now() - timedelta(hours=1), + end_time=timezone.now(), + project=project, + tags=[tag], + description="Before delete", + ) + + project.delete() + tag.delete() + + updated_entry = update_time_entry( + entry, + project=Project.all_objects.get(id=project.id), + tags=[Tag.all_objects.get(id=tag.id)], + description="After delete", + ) + + assert updated_entry.description == "After delete" + assert updated_entry.project_id == project.id + assert list(Tag.all_objects.filter(time_entries=updated_entry).values_list("id", flat=True)) == [tag.id] diff --git a/apps/time_entries/tests/test_views.py b/apps/time_entries/tests/test_views.py index afbb5f4..8daf746 100644 --- a/apps/time_entries/tests/test_views.py +++ b/apps/time_entries/tests/test_views.py @@ -3,6 +3,7 @@ from datetime import datetime from django.utils import timezone from rest_framework.test import APIClient +from apps.tags.models import Tag from apps.time_entries.models import TimeEntry from apps.users.models import User from apps.workspaces.models import Workspace @@ -49,3 +50,91 @@ def test_time_entry_list_returns_grouped_payload_for_ended_entries(db): assert len(response.data["groups"]) == 1 assert len(response.data["groups"][0]["days"]) == 1 assert response.data["groups"][0]["days"][0]["entries"][0]["id"] == str(first_entry.id) + + +def test_time_entry_update_preserves_current_deleted_tags(db): + user = User.objects.create_user(mobile="09127777777", password="secret123") + workspace = Workspace.objects.create(name="Core", owner=user) + tag = Tag.objects.create(workspace=workspace, name="Legacy Tag", color="#475569") + entry = TimeEntry.objects.create( + workspace=workspace, + user=user, + description="Old", + start_time=make_aware(2026, 4, 24, 9, 0, 0), + end_time=make_aware(2026, 4, 24, 10, 30, 0), + ) + entry.tags.set([tag]) + tag.delete() + + client = APIClient() + client.force_authenticate(user=user) + + response = client.patch( + f"/api/time-entries/{entry.id}/", + { + "description": "Still editable", + "tags": [str(tag.id)], + }, + format="json", + ) + + assert response.status_code == 200 + assert response.data["description"] == "Still editable" + assert response.data["tag_details"][0]["is_deleted"] is True + + +def test_time_entry_update_rejects_new_deleted_tag_attachment(db): + user = User.objects.create_user(mobile="09128888888", password="secret123") + workspace = Workspace.objects.create(name="Core", owner=user) + deleted_tag = Tag.objects.create(workspace=workspace, name="Deleted tag", color="#475569") + deleted_tag.delete() + entry = TimeEntry.objects.create( + workspace=workspace, + user=user, + description="Entry", + start_time=make_aware(2026, 4, 24, 9, 0, 0), + end_time=make_aware(2026, 4, 24, 10, 30, 0), + ) + + client = APIClient() + client.force_authenticate(user=user) + + response = client.patch( + f"/api/time-entries/{entry.id}/", + { + "tags": [str(deleted_tag.id)], + }, + format="json", + ) + + assert response.status_code == 400 + assert "unavailable" in response.data["error"].lower() + + +def test_time_entry_update_can_remove_current_deleted_tag(db): + user = User.objects.create_user(mobile="09129999999", password="secret123") + workspace = Workspace.objects.create(name="Core", owner=user) + deleted_tag = Tag.objects.create(workspace=workspace, name="Deleted tag", color="#475569") + entry = TimeEntry.objects.create( + workspace=workspace, + user=user, + description="Entry", + start_time=make_aware(2026, 4, 24, 9, 0, 0), + end_time=make_aware(2026, 4, 24, 10, 30, 0), + ) + entry.tags.set([deleted_tag]) + deleted_tag.delete() + + client = APIClient() + client.force_authenticate(user=user) + + response = client.patch( + f"/api/time-entries/{entry.id}/", + { + "tags": [], + }, + format="json", + ) + + assert response.status_code == 200 + assert response.data["tags"] == []