refactor(appreciation): collapse to 3-state DRAFT → ACTIVATED → SENT
Drops the dead AI_ANALYZED status (a prior data migration already converted existing rows; the choice was never removed from the model) and the unreachable ACKNOWLEDGED status (is_recipient was hardcoded False so the acknowledge button never rendered). Changes: - AppreciationStatus: DRAFT, ACTIVATED, SENT only. SENT is terminal. - VALID_APPRECIATION_TRANSITIONS: DRAFT→ACTIVATED→SENT, SENT terminal. - Removed the model.acknowledge() method. - DRF viewset acknowledge action: returns 410 Gone (deprecated). - UI appreciation_acknowledge view: redirects with deprecation message. - Removed URL routes: appreciation_acknowledge, appreciation_send_dept_reminder. - appreciation_send_dept_reminder status check no longer references ACKNOWLEDGED. - appreciation_list stats + status filter + badge: dropped ai_analyzed/acknowledged. - appreciation_detail status badge + sent banner: dropped acknowledged references. - Migration 0010_drop_acknowledged_status: AlterField on status choices. AI analysis still runs on activation and is stored in ai_analysis JSON; it just no longer has its own status. Badges and leaderboard fire at SENT. 6 tests lock in the new lifecycle and removed URLs.
This commit is contained in:
parent
b1776494f9
commit
295ff298a8
@ -0,0 +1,18 @@
|
||||
# Generated by Django 6.0.1 on 2026-07-20 19:17
|
||||
|
||||
from django.db import migrations, models
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('appreciation', '0009_add_national_id'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.AlterField(
|
||||
model_name='appreciation',
|
||||
name='status',
|
||||
field=models.CharField(choices=[('draft', 'Draft'), ('activated', 'Activated'), ('sent', 'Sent')], db_index=True, default='draft', max_length=20),
|
||||
),
|
||||
]
|
||||
@ -17,21 +17,24 @@ from apps.core.models import SoftDeleteModel, TimeStampedModel, UUIDModel
|
||||
|
||||
|
||||
class AppreciationStatus(models.TextChoices):
|
||||
"""Appreciation status choices"""
|
||||
"""Appreciation status choices.
|
||||
|
||||
Collapsed to a 3-state lifecycle: an appreciation is drafted, PX activates
|
||||
it, and sending it to the department/manager is terminal. AI analysis still
|
||||
runs on activation but is stored in ``ai_analysis`` JSON rather than
|
||||
surfaced as a separate status. There is no acknowledge step — the recipient
|
||||
is notified on send, and recognition (badges, leaderboard) fires at SENT.
|
||||
"""
|
||||
|
||||
DRAFT = "draft", "Draft"
|
||||
ACTIVATED = "activated", "Activated"
|
||||
AI_ANALYZED = "ai_analyzed", "AI Analyzed"
|
||||
SENT = "sent", "Sent"
|
||||
ACKNOWLEDGED = "acknowledged", "Acknowledged"
|
||||
|
||||
|
||||
VALID_APPRECIATION_TRANSITIONS = {
|
||||
AppreciationStatus.DRAFT: {AppreciationStatus.ACTIVATED},
|
||||
AppreciationStatus.ACTIVATED: {AppreciationStatus.AI_ANALYZED, AppreciationStatus.SENT},
|
||||
AppreciationStatus.AI_ANALYZED: {AppreciationStatus.SENT},
|
||||
AppreciationStatus.SENT: {AppreciationStatus.ACKNOWLEDGED},
|
||||
AppreciationStatus.ACKNOWLEDGED: set(),
|
||||
AppreciationStatus.ACTIVATED: {AppreciationStatus.SENT},
|
||||
AppreciationStatus.SENT: set(), # terminal
|
||||
}
|
||||
|
||||
|
||||
@ -350,17 +353,6 @@ class Appreciation(UUIDModel, TimeStampedModel, SoftDeleteModel):
|
||||
|
||||
self.send_notification()
|
||||
|
||||
def acknowledge(self):
|
||||
"""Mark appreciation as acknowledged"""
|
||||
from django.utils import timezone
|
||||
|
||||
if self.status != AppreciationStatus.SENT:
|
||||
raise ValueError(f"Cannot acknowledge appreciation in '{self.status}' status. Must be in 'sent'.")
|
||||
|
||||
self.status = AppreciationStatus.ACKNOWLEDGED
|
||||
self.acknowledged_at = timezone.now()
|
||||
self.save(update_fields=["status", "acknowledged_at"])
|
||||
|
||||
def send_notification(self):
|
||||
"""Send notification to recipient — handled by post_save signal."""
|
||||
pass
|
||||
|
||||
45
apps/appreciation/test_workflow_realignment.py
Normal file
45
apps/appreciation/test_workflow_realignment.py
Normal file
@ -0,0 +1,45 @@
|
||||
"""Tests for the appreciation 3-state lifecycle realignment."""
|
||||
import pytest
|
||||
from django.test import TestCase
|
||||
from django.urls import reverse, NoReverseMatch
|
||||
|
||||
from apps.appreciation.models import AppreciationStatus, VALID_APPRECIATION_TRANSITIONS
|
||||
|
||||
|
||||
class TestAppreciationThreeStateLifecycle(TestCase):
|
||||
"""Appreciation has exactly 3 statuses: DRAFT, ACTIVATED, SENT."""
|
||||
|
||||
def test_only_three_statuses_exist(self):
|
||||
choices = [value for value, _ in AppreciationStatus.choices]
|
||||
assert set(choices) == {"draft", "activated", "sent"}, \
|
||||
f"Expected exactly 3 statuses, got: {choices}"
|
||||
|
||||
def test_no_ai_analyzed_or_acknowledged(self):
|
||||
assert not hasattr(AppreciationStatus, "AI_ANALYZED"), \
|
||||
"AI_ANALYZED should be removed"
|
||||
assert not hasattr(AppreciationStatus, "ACKNOWLEDGED"), \
|
||||
"ACKNOWLEDGED should be removed"
|
||||
|
||||
def test_sent_is_terminal(self):
|
||||
"""SENT has no outgoing transitions."""
|
||||
assert VALID_APPRECIATION_TRANSITIONS.get(AppreciationStatus.SENT) == set(), \
|
||||
"SENT should be terminal"
|
||||
|
||||
def test_activated_can_go_to_sent(self):
|
||||
"""ACTIVATED transitions directly to SENT (no AI_ANALYZED intermediate)."""
|
||||
assert AppreciationStatus.SENT in \
|
||||
VALID_APPRECIATION_TRANSITIONS[AppreciationStatus.ACTIVATED]
|
||||
|
||||
|
||||
class TestAppreciationRemovedUrls(TestCase):
|
||||
"""The acknowledge and send-dept-reminder URL routes are gone."""
|
||||
|
||||
def test_acknowledge_url_does_not_resolve(self):
|
||||
with pytest.raises(NoReverseMatch):
|
||||
reverse("appreciation:appreciation_acknowledge",
|
||||
kwargs={"pk": "00000000-0000-0000-0000-000000000000"})
|
||||
|
||||
def test_send_dept_reminder_url_does_not_resolve(self):
|
||||
with pytest.raises(NoReverseMatch):
|
||||
reverse("appreciation:appreciation_send_dept_reminder",
|
||||
kwargs={"pk": "00000000-0000-0000-0000-000000000000"})
|
||||
@ -100,7 +100,7 @@ def appreciation_list(request):
|
||||
"activated": base_qs.filter(
|
||||
status=AppreciationStatus.ACTIVATED
|
||||
).count(),
|
||||
"sent": base_qs.filter(status__in=[AppreciationStatus.SENT, AppreciationStatus.ACKNOWLEDGED]).count(),
|
||||
"sent": base_qs.filter(status=AppreciationStatus.SENT).count(),
|
||||
}
|
||||
|
||||
paginator = Paginator(queryset, 25)
|
||||
@ -598,27 +598,18 @@ def appreciation_send_to(request, pk):
|
||||
@login_required
|
||||
@require_http_methods(["POST"])
|
||||
def appreciation_acknowledge(request, pk):
|
||||
"""Acknowledge appreciation"""
|
||||
appreciation = get_object_or_404(Appreciation, pk=pk)
|
||||
"""DEPRECATED: ACKNOWLEDGED status was removed; SENT is terminal.
|
||||
|
||||
# Check if user is recipient
|
||||
user_content_type = ContentType.objects.get_for_model(request.user)
|
||||
if not (
|
||||
appreciation.recipient_content_type == user_content_type and
|
||||
appreciation.recipient_object_id == request.user.id
|
||||
):
|
||||
messages.error(request, "You can only acknowledge appreciations sent to you.")
|
||||
return redirect('appreciation:appreciation_detail', pk=pk)
|
||||
|
||||
if appreciation.status != AppreciationStatus.SENT:
|
||||
messages.error(request, "This appreciation cannot be acknowledged in its current status.")
|
||||
return redirect('appreciation:appreciation_detail', pk=pk)
|
||||
|
||||
# Acknowledge
|
||||
appreciation.acknowledge()
|
||||
|
||||
messages.success(request, "Appreciation acknowledged successfully.")
|
||||
return redirect('appreciation:appreciation_detail', pk=pk)
|
||||
The URL route was removed too. This view function is kept only so that
|
||||
any stale import or bookmarked URL that somehow resolves does not crash
|
||||
the worker; it redirects with a clear message. Safe to delete once no
|
||||
references remain.
|
||||
"""
|
||||
messages.error(
|
||||
request,
|
||||
_("Appreciation acknowledge is no longer supported — sending is the final step."),
|
||||
)
|
||||
return redirect("appreciation:appreciation_detail", pk=pk)
|
||||
|
||||
|
||||
@login_required
|
||||
@ -635,7 +626,7 @@ def appreciation_send_dept_reminder(request, pk):
|
||||
messages.error(request, _("You don't have permission to send reminders."))
|
||||
return redirect("appreciation:appreciation_detail", pk=pk)
|
||||
|
||||
if appreciation.status not in (AppreciationStatus.SENT, AppreciationStatus.ACKNOWLEDGED):
|
||||
if appreciation.status != AppreciationStatus.SENT:
|
||||
messages.warning(request, _("Appreciation must be sent before sending reminders."))
|
||||
return redirect("appreciation:appreciation_detail", pk=pk)
|
||||
|
||||
|
||||
@ -37,8 +37,6 @@ urlpatterns = [
|
||||
path('detail/<uuid:pk>/select-recipient/', ui_views.appreciation_select_recipient, name='appreciation_select_recipient'),
|
||||
path('detail/<uuid:pk>/send/', ui_views.appreciation_send, name='appreciation_send'),
|
||||
path('detail/<uuid:pk>/send-to/', ui_views.appreciation_send_to, name='appreciation_send_to'),
|
||||
path('detail/<uuid:pk>/send-reminder/', ui_views.appreciation_send_dept_reminder, name='appreciation_send_dept_reminder'),
|
||||
path('acknowledge/<uuid:pk>/', ui_views.appreciation_acknowledge, name='appreciation_acknowledge'),
|
||||
path('leaderboard/', ui_views.leaderboard_view, name='leaderboard_view'),
|
||||
path('badges/', ui_views.my_badges_view, name='my_badges_view'),
|
||||
|
||||
|
||||
@ -211,26 +211,16 @@ class AppreciationViewSet(viewsets.ModelViewSet):
|
||||
|
||||
@action(detail=True, methods=["post"])
|
||||
def acknowledge(self, request, pk=None):
|
||||
"""Acknowledge an appreciation"""
|
||||
appreciation = self.get_object()
|
||||
"""DEPRECATED: ACKNOWLEDGED status was removed; SENT is terminal.
|
||||
|
||||
# Check if user is the recipient
|
||||
user_content_type = ContentType.objects.get_for_model(request.user)
|
||||
if not (
|
||||
appreciation.recipient_content_type == user_content_type
|
||||
and appreciation.recipient_object_id == request.user.id
|
||||
):
|
||||
Kept as a 410 Gone endpoint so old API clients get a clear error
|
||||
instead of a 404. Will be removed in a future release.
|
||||
"""
|
||||
return Response(
|
||||
{"error": "You can only acknowledge appreciations sent to you"}, status=status.HTTP_403_FORBIDDEN
|
||||
{"error": "Appreciation acknowledge is no longer supported — SENT is terminal."},
|
||||
status=status.HTTP_410_GONE,
|
||||
)
|
||||
|
||||
# Acknowledge
|
||||
appreciation.acknowledge()
|
||||
|
||||
# Serialize and return
|
||||
serializer = AppreciationSerializer(appreciation)
|
||||
return Response(serializer.data)
|
||||
|
||||
@action(detail=False, methods=["get"])
|
||||
def my_appreciations(self, request):
|
||||
"""Get appreciations for the current user"""
|
||||
|
||||
@ -20,9 +20,7 @@
|
||||
<span class="px-2.5 py-1 rounded-full text-[10px] font-bold uppercase
|
||||
{% if appreciation.status == 'draft' %}bg-amber-100 text-amber-700
|
||||
{% elif appreciation.status == 'activated' %}bg-blue-100 text-blue-700
|
||||
{% elif appreciation.status == 'ai_analyzed' %}bg-purple-100 text-purple-700
|
||||
{% elif appreciation.status == 'sent' %}bg-green-100 text-green-700
|
||||
{% elif appreciation.status == 'acknowledged' %}bg-emerald-100 text-emerald-700
|
||||
{% else %}bg-slate-100 text-slate-600{% endif %}">
|
||||
{{ appreciation.get_status_display }}
|
||||
</span>
|
||||
@ -130,8 +128,8 @@
|
||||
<div class="bg-green-50 rounded-2xl border border-green-200 p-4 mb-6 flex items-center gap-3">
|
||||
<div class="w-9 h-9 rounded-full bg-green-100 flex items-center justify-center shrink-0"><i data-lucide="check-circle-2" class="w-4 h-4 text-green-600"></i></div>
|
||||
<div>
|
||||
<p class="text-sm font-bold text-navy">{% trans "Appreciation acknowledged" %}</p>
|
||||
<p class="text-xs text-slate">{% trans "Sent on" %} {{ appreciation.sent_at|date:"Y-m-d H:i" }} · {% trans "Acknowledged" %} {{ appreciation.acknowledged_at|date:"Y-m-d H:i" }}</p>
|
||||
<p class="text-sm font-bold text-navy">{% trans "Appreciation sent" %}</p>
|
||||
<p class="text-xs text-slate">{% trans "Sent on" %} {{ appreciation.sent_at|date:"Y-m-d H:i" }}</p>
|
||||
</div>
|
||||
</div>
|
||||
{% endif %}
|
||||
|
||||
@ -171,9 +171,7 @@
|
||||
<option value="">{% trans "All Status" %}</option>
|
||||
<option value="draft" {% if status_filter == 'draft' %}selected{% endif %}>{% trans "Draft" %}</option>
|
||||
<option value="activated" {% if status_filter == 'activated' %}selected{% endif %}>{% trans "Activated" %}</option>
|
||||
<option value="ai_analyzed" {% if status_filter == 'ai_analyzed' %}selected{% endif %}>{% trans "AI Analyzed" %}</option>
|
||||
<option value="sent" {% if status_filter == 'sent' %}selected{% endif %}>{% trans "Sent" %}</option>
|
||||
<option value="acknowledged" {% if status_filter == 'acknowledged' %}selected{% endif %}>{% trans "Acknowledged" %}</option>
|
||||
</select>
|
||||
</div>
|
||||
<button type="submit" class="px-5 py-2.5 bg-navy text-white rounded-xl text-sm font-bold hover:bg-blue transition flex items-center gap-2">
|
||||
@ -243,9 +241,7 @@
|
||||
<span class="px-2.5 py-1 rounded-full text-[10px] font-bold uppercase
|
||||
{% if apr.status == 'draft' %}bg-amber-100 text-amber-700
|
||||
{% elif apr.status == 'activated' %}bg-blue-100 text-blue-700
|
||||
{% elif apr.status == 'ai_analyzed' %}bg-purple-100 text-purple-700
|
||||
{% elif apr.status == 'sent' %}bg-green-100 text-green-700
|
||||
{% elif apr.status == 'acknowledged' %}bg-emerald-100 text-emerald-700
|
||||
{% else %}bg-slate-100 text-slate-700{% endif %}">
|
||||
{{ apr.get_status_display }}
|
||||
</span>
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user