diff --git a/.env.example b/.env.example index 05f3652b..813d4d12 100644 --- a/.env.example +++ b/.env.example @@ -22,6 +22,7 @@ DRF_THROTTLE_TOKEN_OBTAIN=10/min DRF_THROTTLE_PROGRAM_REGISTER_NEW=10/min DRF_THROTTLE_APPLICATION_CREATE=10/min DRF_THROTTLE_SUBMISSION_CREATE=10/min +DRF_THROTTLE_EVALUATION_AMEND=30/min SENTRY_DSN= diff --git a/docs/expert-evaluation-api.md b/docs/expert-evaluation-api.md index a7b9b336..f1d2b88d 100644 --- a/docs/expert-evaluation-api.md +++ b/docs/expert-evaluation-api.md @@ -5,8 +5,9 @@ - DEV-050 — Expert Submission API - DEV-051 — Evaluation mutation API - DEV-052 — Manager evaluation read API +- DEV-073 — изменение submitted Evaluation с неизменяемой историей -Roadmap-IDs: DEV-050, DEV-051, DEV-052 +Roadmap-IDs: DEV-050, DEV-051, DEV-052, DEV-073 ## Назначение @@ -23,6 +24,8 @@ membership в программе. - `SubmissionExpertAssignment` связывает конкретную Submission и Expert. - `Evaluation` хранит форму оценки, комментарий и lifecycle. - `EvaluationScore` хранит одно числовое значение и snapshot критерия. +- `EvaluationAmendment` хранит неизменяемые снимки до и после изменения + отправленной оценки. - `project_rates.Criteria` временно используется как каталог критериев программы. @@ -40,16 +43,21 @@ Assignment: Evaluation: - `draft` — доступно атомарное обновление `comment` и полного набора `scores`; -- `submitted` — терминальное неизменяемое состояние. +- `submitted` — отправленное состояние; обычный PATCH запрещен, изменение + возможно только через DEV-073 amend с audit-записью. Финальный submit в одной транзакции переводит Evaluation в `submitted`, а -assignment в `completed`. Reopen и revision history отсутствуют. +assignment в `completed`. Amend не возвращает Evaluation в draft, не меняет +`submitted_at` и сохраняет историю. Reopen отсутствует. ## Права доступа - Expert list/detail доступны только эксперту с текущим membership программы и assignment `assigned` или `completed`. - Создание и изменение draft требуют assignment `assigned`. +- Amend submitted Evaluation доступен только владельцу при assignment + `assigned` или `completed` и текущем membership программы. +- Историю amend читают владелец, manager программы и staff/superuser. - Staff может открыть PII-safe expert detail в административном режиме. - Владелец-эксперт, manager программы и staff могут читать Evaluation detail. - Manager list/detail ограничены конкретной программой. @@ -111,12 +119,31 @@ Autosave владельца draft. Можно передать `comment`, `score Повторный submit идемпотентен: возвращает `200` и сохраняет первоначальный `submitted_at`. +### PATCH /evaluations/\/amend/ + +Изменяет `comment` и/или полный набор `scores` уже отправленной Evaluation. +Endpoint доступен только эксперту-владельцу. Evaluation остается `submitted`, +исходный `submitted_at` сохраняется, `amended_at` обновляется, а `total_score` +сбрасывается в `null` до появления формулы пересчета. + +Если передан `scores`, payload обязан содержать все текущие числовые Criteria +программы. Валидация и замена scores, обновление Evaluation и создание +`EvaluationAmendment` выполняются в одной транзакции. Полностью совпадающий +запрос возвращает `200`, но не обновляет `amended_at` и не создает историю. + +### GET /evaluations/\/amendments/ + +Возвращает упорядоченную неизменяемую историю со снимками comment, scores и +total_score до и после каждой правки. Доступ имеют эксперт-владелец, manager +программы и staff/superuser. Для остальных существование Evaluation скрывается +ответом `404`. + ## Manager API ### GET /programs/\/submission-assignments/ Существующий контракт дополнен nullable-полем `evaluation` с полями `id`, -`status`, `updated_at`, `submitted_at`, `total_score`. Старое поле +`status`, `updated_at`, `submitted_at`, `amended_at`, `total_score`. Старое поле `evaluation_status` сохранено. ### GET /programs/\/evaluations/ @@ -131,7 +158,7 @@ Autosave владельца draft. Можно передать `comment`, `score - `limit`, `offset`. Ответ содержит безопасные Submission/Expert/Assignment summaries, scores, -comment, total_score и timestamps. +comment, total_score, `submitted_at`, `amended_at` и остальные timestamps. ### GET /programs/\/evaluations/\/ @@ -147,6 +174,8 @@ Read-only detail в пределах программы manager. Mutation-мет - PATCH сначала валидирует весь новый набор и только затем удаляет старый. - Submit требует все текущие числовые Criteria программы и повторно проверяет типы и диапазоны. +- Amend с `scores` также требует полный набор текущих числовых Criteria до + начала любых изменений. - Нечисловые Criteria не включаются в форму; свободный текст хранится в `Evaluation.comment`. @@ -155,6 +184,8 @@ Read-only detail в пределах программы manager. Mutation-мет - Повторный create существующего draft возвращает его без изменения. - Повторный PATCH с теми же данными не создаёт дублей. - Повторный submit submitted Evaluation не меняет `submitted_at`. +- Повторный amend с теми же comment и scores не создает новую историю и не + меняет `amended_at`. - Уникальность `submission + expert` и `evaluation + criterion` дополнительно защищена существующими constraints. @@ -165,6 +196,8 @@ Read-only detail в пределах программы manager. Mutation-мет - PATCH блокирует assignment и Evaluation; набор scores заменяется атомарно. - Submit блокирует assignment и Evaluation в стабильном порядке, валидирует полную форму и записывает одинаковый timestamp в Evaluation и assignment. +- Amend блокирует assignment и Evaluation в том же порядке, затем атомарно заменяет scores, + обновляет Evaluation и записывает снимок `EvaluationAmendment`. - Два конкурентных submit не создают противоречивое терминальное состояние. ## Защита персональных данных @@ -182,7 +215,8 @@ Manager responses содержат только минимальные имя/ф - `evaluation_create`: `10/min`; - `evaluation_update`: `120/min`; -- `evaluation_submit`: `20/min`. +- `evaluation_submit`: `20/min`; +- `evaluation_amend`: `30/min`. Scopes применяются только к соответствующим mutation-методам и не включают глобальный DRF throttle. @@ -205,8 +239,8 @@ Scopes применяются только к соответствующим mut `partner_programs.tests.test_expert_evaluation_api` Он покрывает expert list/detail, PII regression, draft create, полную замену -scores, rollback, submit lifecycle, concurrent submit, manager read-only API, -filters, pagination и throttling. +scores, rollback, submit lifecycle, amend submitted Evaluation, audit history, +concurrent submit, manager read-only API, filters, pagination и throttling. Также сохраняются model tests и Assignment API regression. diff --git a/docs/submission-evaluation-domain.md b/docs/submission-evaluation-domain.md index f2f7660f..c15a639f 100644 --- a/docs/submission-evaluation-domain.md +++ b/docs/submission-evaluation-domain.md @@ -1,3 +1,4 @@ + # Submission Evaluation Domain RFC Статус: proposal. @@ -11,6 +12,8 @@ `POST /submission-assignments//revoke/`; - Expert Submission read API, Evaluation mutation API и manager Evaluation read API реализованы в рамках DEV-050, DEV-051 и DEV-052; +- изменение submitted Evaluation и неизменяемая история реализованы в рамках + DEV-073; - frontend, `Result`, ranking и публикация итогов еще не реализованы; - временно используется существующий `project_rates.Criteria`; - дедлайном MVP остается существующий `datetime_evaluation_ends`; @@ -190,6 +193,21 @@ backfill ProjectScore в Evaluation и обратная синхронизаци отдельный `EvaluationCriterion` в самостоятельном PR. Не следует молча добавлять эти значения в legacy Criteria. +### EvaluationAmendment + +`EvaluationAmendment` — неизменяемая audit-запись изменения submitted +Evaluation: + +- `evaluation`; +- `changed_by`; +- `previous_comment`, `comment`; +- `previous_scores`, `scores`; +- `previous_total_score`, `total_score`; +- `created_at`. + +`Evaluation.amended_at` хранит время последнего фактического изменения. +Снимки scores включают идентификатор и snapshot критерия вместе со значением. + ## 4. Сущности и связи ```text @@ -242,10 +260,11 @@ Cross-table и ManyToMany invariants проверяются транзакцио | Статус | Редактирование | Допустимый переход | |---|---|---| | `draft` | Назначенным экспертом | `submitted` | -| `submitted` | Запрещено | Терминальный в MVP | +| `submitted` | Только отдельный amend владельца с audit | Остается `submitted` | MVP намеренно не добавляет `cancelled`, `revised` или `reopened` без готового -audit contract. Возврат submitted Evaluation к редактированию запрещен. +audit contract. Возврат submitted Evaluation в draft запрещен; DEV-073 меняет +ее только отдельной атомарной операцией с неизменяемой историей. Будущее явное правило может добавить manager-only action `reopen` с обязательной причиной. До перехода прежняя финальная форма и scores должны @@ -428,6 +447,28 @@ Service под transaction и row locks: Рекомендуемый scope: `evaluation_submit`, default `20/min`. +#### `PATCH /evaluations//amend/` + +Права: только владелец-эксперт. Evaluation должна оставаться в статусе +`submitted`, Submission — в `submitted/final`, назначение — в +`assigned/completed`, а эксперт должен по-прежнему состоять в Program. + +Можно изменить `comment` и полностью заменить `scores`. Переданный набор +scores обязан содержать все числовые Criteria Program; типы, диапазоны и +принадлежность Program проверяются теми же правилами, что для draft. +Изменение выполняется атомарно, не создает новую Evaluation, не меняет +`submitted_at` и статус assignment. `amended_at` обновляется только при +фактическом изменении, а `total_score` сбрасывается до отдельного пересчета. + +Полностью совпадающий запрос является no-op и не создает запись истории. +Рекомендуемый scope: `evaluation_amend`, default `30/min`. + +#### `GET /evaluations//amendments/` + +Неизменяемые снимки до и после изменения доступны владельцу Evaluation, +manager соответствующей Program и staff/superuser. Остальные получают `404`, +чтобы endpoint не раскрывал существование оценки. + ### Назначение экспертов менеджером #### `GET /programs//submission-assignments/` @@ -479,10 +520,10 @@ Delete endpoint не используется, чтобы сохранять и ### Не входящий в MVP reopen -Если продукт подтвердит исправление финальной оценки, отдельный manager action -может иметь вид `POST /evaluations//reopen/`. Он требует reason и -неизменяемого snapshot предыдущей submitted revision. До реализации revision -model endpoint добавлять нельзя. +DEV-073 разрешает владельцу точечно изменить submitted Evaluation без возврата +в draft. Отдельный manager action `POST /evaluations//reopen/` по-прежнему +не входит в MVP: он потребует reason, отдельной модели переходов статуса и +самостоятельного продуктового решения. ## 8. DB constraints @@ -494,14 +535,16 @@ model endpoint добавлять нельзя. историю. 2. `UniqueConstraint(submission, expert)` для Evaluation. 3. `UniqueConstraint(evaluation, criterion)` для EvaluationScore. -4. Общий `value >= 0` не добавляется без продуктового правила: существующий +4. `EvaluationAmendment` хранит неизменяемые JSON-снимки comment, scores и + total_score до и после каждой фактической правки. +5. Общий `value >= 0` не добавляется без продуктового правила: существующий Criteria может допускать другой диапазон. Индивидуальные min/max являются cross-row правилом и проверяются service/model validation; для Criteria типа `int` дополнительно запрещается дробное значение. -5. `submitted_at IS NOT NULL` для submitted Evaluation и `IS NULL` для draft, +6. `submitted_at IS NOT NULL` для submitted Evaluation и `IS NULL` для draft, если синтаксис текущей версии Django/PostgreSQL позволяет выразить это без неоднозначности. -6. Assignment timestamps согласуются со статусом: revoked требует +7. Assignment timestamps согласуются со статусом: revoked требует `revoked_at`, completed требует `completed_at`. Транзакционный service дополнительно проверяет: diff --git a/partner_programs/admin.py b/partner_programs/admin.py index 1fe646b0..b85d250b 100644 --- a/partner_programs/admin.py +++ b/partner_programs/admin.py @@ -1,3 +1,5 @@ +# Roadmap: DEV-073 + import re import tablib @@ -13,6 +15,7 @@ from partner_programs.models import ( Application, Evaluation, + EvaluationAmendment, EvaluationScore, PartnerProgram, PartnerProgramField, @@ -311,6 +314,7 @@ class EvaluationAdmin(admin.ModelAdmin): "expert", "status", "submitted_at", + "amended_at", "created_at", "updated_at", ) @@ -335,6 +339,7 @@ class EvaluationAdmin(admin.ModelAdmin): readonly_fields = ( "created_at", "updated_at", + "amended_at", ) list_select_related = ( "submission", @@ -386,6 +391,52 @@ class EvaluationScoreAdmin(admin.ModelAdmin): date_hierarchy = "created_at" +@admin.register(EvaluationAmendment) +class EvaluationAmendmentAdmin(admin.ModelAdmin): + list_display = ( + "id", + "evaluation", + "changed_by", + "created_at", + ) + list_filter = ( + "evaluation__submission__program", + "created_at", + ) + search_fields = ( + "=evaluation__id", + "=evaluation__submission__id", + "changed_by__email", + ) + readonly_fields = ( + "evaluation", + "changed_by", + "previous_comment", + "comment", + "previous_scores", + "scores", + "previous_total_score", + "total_score", + "created_at", + ) + list_select_related = ( + "evaluation", + "evaluation__submission", + "changed_by", + ) + date_hierarchy = "created_at" + actions = None + + def has_add_permission(self, request): + return False + + def has_change_permission(self, request, obj=None): + return False + + def has_delete_permission(self, request, obj=None): + return False + + class PartnerProgramMaterialInline(admin.StackedInline): model = PartnerProgramMaterial extra = 1 diff --git a/partner_programs/evaluation_urls.py b/partner_programs/evaluation_urls.py index d9ad322f..82a8c43a 100644 --- a/partner_programs/evaluation_urls.py +++ b/partner_programs/evaluation_urls.py @@ -1,6 +1,10 @@ +# Roadmap: DEV-073 + from django.urls import path from partner_programs.evaluation_views import ( + EvaluationAmendmentListView, + EvaluationAmendView, EvaluationDetailView, EvaluationSubmitView, ) @@ -14,4 +18,14 @@ EvaluationSubmitView.as_view(), name="submit", ), + path( + "/amend/", + EvaluationAmendView.as_view(), + name="amend", + ), + path( + "/amendments/", + EvaluationAmendmentListView.as_view(), + name="amendment-list", + ), ] diff --git a/partner_programs/evaluation_views.py b/partner_programs/evaluation_views.py index 92ad20cb..a25bd155 100644 --- a/partner_programs/evaluation_views.py +++ b/partner_programs/evaluation_views.py @@ -1,4 +1,4 @@ -# Roadmap: DEV-050, DEV-051, DEV-052 +# Roadmap: DEV-050, DEV-051, DEV-052, DEV-073 # Контур экспертного доступа к Submission и управления Evaluation. from drf_yasg import openapi @@ -14,6 +14,8 @@ from partner_programs.pagination import PartnerProgramPagination from partner_programs.permissions import IsAdminOrManagerOfProgram from partner_programs.serializers.evaluations import ( + EvaluationAmendmentSerializer, + EvaluationAmendSerializer, EvaluationDraftCreateSerializer, EvaluationDraftUpdateSerializer, EvaluationReadSerializer, @@ -29,8 +31,10 @@ EvaluationNotFoundError, EvaluationServiceError, EvaluationValidationError, + amend_submitted_evaluation, create_or_get_draft_evaluation, expert_submission_assignments, + get_evaluation_amendments, get_expert_submission_detail, get_my_evaluation, get_visible_evaluation, @@ -334,6 +338,68 @@ def post(self, request, evaluation_id): return Response(EvaluationReadSerializer(evaluation).data) +class EvaluationAmendView(APIView): + permission_classes = [IsAuthenticated] + throttle_classes = [PatchOnlyScopedRateThrottle] + throttle_scope = "evaluation_amend" + + @swagger_auto_schema( + operation_description=( + "Изменяет comment и/или полный набор scores уже отправленной Evaluation. " + "Операция доступна только эксперту-владельцу и сохраняет submitted_at." + ), + request_body=EvaluationAmendSerializer, + responses={ + 200: EvaluationReadSerializer, + 400: "Передан неполный или некорректный набор Criteria.", + 401: "Требуется авторизация.", + 404: "Evaluation скрыта или не существует.", + 409: "Статус Evaluation, Submission или assignment не допускает изменение.", + 429: "Превышен evaluation_amend throttle.", + }, + ) + def patch(self, request, evaluation_id): + serializer = EvaluationAmendSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + try: + evaluation = amend_submitted_evaluation( + evaluation_id=evaluation_id, + user=request.user, + comment_supplied="comment" in serializer.validated_data, + comment=serializer.validated_data.get("comment", ""), + scores_supplied="scores" in serializer.validated_data, + scores=serializer.validated_data.get("scores"), + ) + except EvaluationServiceError as exc: + return _domain_error_response(exc) + return Response(EvaluationReadSerializer(evaluation).data) + + +class EvaluationAmendmentListView(APIView): + permission_classes = [IsAuthenticated] + + @swagger_auto_schema( + operation_description=( + "Возвращает неизменяемую историю правок Evaluation владельцу, " + "manager программы или staff." + ), + responses={ + 200: EvaluationAmendmentSerializer(many=True), + 401: "Требуется авторизация.", + 404: "Evaluation скрыта или не существует.", + }, + ) + def get(self, request, evaluation_id): + try: + amendments = get_evaluation_amendments( + evaluation_id=evaluation_id, + user=request.user, + ) + except EvaluationServiceError as exc: + return _domain_error_response(exc) + return Response(EvaluationAmendmentSerializer(amendments, many=True).data) + + class ProgramEvaluationListView(ProgramPermissionMixin, APIView): permission_classes = [IsAuthenticated, IsAdminOrManagerOfProgram] pagination_class = PartnerProgramPagination @@ -400,3 +466,6 @@ def get(self, request, program_id, evaluation_id): pk=evaluation_id, ) return Response(ManagerEvaluationSerializer(evaluation).data) + + amend_submitted_evaluation, + get_evaluation_amendments, diff --git a/partner_programs/migrations/0024_evaluation_amended_at_evaluationamendment.py b/partner_programs/migrations/0024_evaluation_amended_at_evaluationamendment.py new file mode 100644 index 00000000..333cedee --- /dev/null +++ b/partner_programs/migrations/0024_evaluation_amended_at_evaluationamendment.py @@ -0,0 +1,80 @@ +# Generated by Django 4.2.11 on 2026-07-28 23:30 +# Roadmap: DEV-073 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("partner_programs", "0023_submission_evaluation_models"), + ] + + operations = [ + migrations.AddField( + model_name="evaluation", + name="amended_at", + field=models.DateTimeField(blank=True, null=True), + ), + migrations.CreateModel( + name="EvaluationAmendment", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("previous_comment", models.TextField(blank=True)), + ("comment", models.TextField(blank=True)), + ("previous_scores", models.JSONField(default=list)), + ("scores", models.JSONField(default=list)), + ( + "previous_total_score", + models.DecimalField( + blank=True, decimal_places=6, max_digits=18, null=True + ), + ), + ( + "total_score", + models.DecimalField( + blank=True, decimal_places=6, max_digits=18, null=True + ), + ), + ("created_at", models.DateTimeField(auto_now_add=True)), + ( + "changed_by", + models.ForeignKey( + on_delete=django.db.models.deletion.PROTECT, + related_name="evaluation_amendments", + to=settings.AUTH_USER_MODEL, + ), + ), + ( + "evaluation", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="amendments", + to="partner_programs.evaluation", + ), + ), + ], + options={ + "verbose_name": "Изменение экспертной оценки", + "verbose_name_plural": "История изменений экспертных оценок", + "ordering": ("created_at", "id"), + "indexes": [ + models.Index( + fields=["evaluation", "created_at"], + name="eval_amend_eval_created_idx", + ) + ], + }, + ), + ] diff --git a/partner_programs/models.py b/partner_programs/models.py index 781abfcb..8b67ce37 100644 --- a/partner_programs/models.py +++ b/partner_programs/models.py @@ -1,3 +1,5 @@ +# Roadmap: DEV-073 + from django.contrib.auth import get_user_model from django.core.exceptions import ValidationError from django.db import models @@ -974,6 +976,7 @@ class Evaluation(models.Model): blank=True, ) submitted_at = models.DateTimeField(null=True, blank=True) + amended_at = models.DateTimeField(null=True, blank=True) created_at = models.DateTimeField(auto_now_add=True) updated_at = models.DateTimeField(auto_now=True) @@ -1095,6 +1098,63 @@ def __str__(self): ) +class EvaluationAmendment(models.Model): + """Неизменяемый снимок изменения уже отправленной экспертной оценки.""" + + evaluation = models.ForeignKey( + Evaluation, + on_delete=models.CASCADE, + related_name="amendments", + ) + changed_by = models.ForeignKey( + User, + on_delete=models.PROTECT, + related_name="evaluation_amendments", + ) + previous_comment = models.TextField(blank=True) + comment = models.TextField(blank=True) + previous_scores = models.JSONField(default=list) + scores = models.JSONField(default=list) + previous_total_score = models.DecimalField( + max_digits=18, + decimal_places=6, + null=True, + blank=True, + ) + total_score = models.DecimalField( + max_digits=18, + decimal_places=6, + null=True, + blank=True, + ) + created_at = models.DateTimeField(auto_now_add=True) + + def save(self, *args, **kwargs): + if self.pk: + raise ValidationError("Историю изменения оценки нельзя редактировать.") + return super().save(*args, **kwargs) + + def delete(self, *args, **kwargs): + raise ValidationError("Историю изменения оценки нельзя удалять.") + + class Meta: + verbose_name = "Изменение экспертной оценки" + verbose_name_plural = "История изменений экспертных оценок" + ordering = ("created_at", "id") + indexes = [ + models.Index( + fields=["evaluation", "created_at"], + name="eval_amend_eval_created_idx", + ), + ] + + def __str__(self): + return ( + f"EvaluationAmendment<{self.pk}> evaluation={self.evaluation_id} " + f"changed_by={self.changed_by_id}" + ) + + class PartnerProgramUserProfile(models.Model): """ PartnerProgramUserProfile model diff --git a/partner_programs/serializers/evaluations.py b/partner_programs/serializers/evaluations.py index 1458457d..aef396ef 100644 --- a/partner_programs/serializers/evaluations.py +++ b/partner_programs/serializers/evaluations.py @@ -1,10 +1,11 @@ -# Roadmap: DEV-050, DEV-051, DEV-052 +# Roadmap: DEV-050, DEV-051, DEV-052, DEV-073 # PII-safe контракты решений и экспертных оценок. from rest_framework import serializers from partner_programs.models import ( Evaluation, + EvaluationAmendment, EvaluationScore, PartnerProgram, Submission, @@ -46,6 +47,10 @@ def validate(self, attrs): return attrs +class EvaluationAmendSerializer(EvaluationDraftUpdateSerializer): + pass + + class ProgramSummarySerializer(serializers.ModelSerializer): class Meta: model = PartnerProgram @@ -118,6 +123,7 @@ class Meta: "scores", "total_score", "submitted_at", + "amended_at", "created_at", "updated_at", ) @@ -141,6 +147,7 @@ class ExpertEvaluationSummarySerializer(serializers.Serializer): status = serializers.CharField(allow_null=True, read_only=True) updated_at = serializers.DateTimeField(allow_null=True, read_only=True) submitted_at = serializers.DateTimeField(allow_null=True, read_only=True) + amended_at = serializers.DateTimeField(allow_null=True, read_only=True) class ExpertSubmissionListSerializer(serializers.ModelSerializer): @@ -183,6 +190,7 @@ def get_my_evaluation(self, assignment): "status": assignment.my_evaluation_status, "updated_at": assignment.my_evaluation_updated_at, "submitted_at": assignment.my_evaluation_submitted_at, + "amended_at": assignment.my_evaluation_amended_at, } @@ -279,6 +287,7 @@ class Meta: "comment", "total_score", "submitted_at", + "amended_at", "created_at", "updated_at", ) @@ -293,3 +302,24 @@ def get_assignment(self, evaluation): "assigned_at": evaluation.assignment_assigned_at, "completed_at": evaluation.assignment_completed_at, } + + +class EvaluationAmendmentSerializer(serializers.ModelSerializer): + evaluation_id = serializers.IntegerField(read_only=True) + changed_by_id = serializers.IntegerField(read_only=True) + + class Meta: + model = EvaluationAmendment + fields = ( + "id", + "evaluation_id", + "changed_by_id", + "previous_comment", + "comment", + "previous_scores", + "scores", + "previous_total_score", + "total_score", + "created_at", + ) + read_only_fields = fields diff --git a/partner_programs/services/evaluations.py b/partner_programs/services/evaluations.py index cae4ad12..d1ade334 100644 --- a/partner_programs/services/evaluations.py +++ b/partner_programs/services/evaluations.py @@ -1,4 +1,4 @@ -# Roadmap: DEV-050, DEV-051, DEV-052 +# Roadmap: DEV-050, DEV-051, DEV-052, DEV-073 # Контур экспертного доступа к Submission и управления Evaluation. from dataclasses import dataclass @@ -11,6 +11,7 @@ from partner_programs.models import ( Evaluation, + EvaluationAmendment, EvaluationScore, PartnerProgram, Submission, @@ -64,6 +65,11 @@ class EvaluationSubmittedError(EvaluationConflictError): default_detail = "Отправленная оценка недоступна для изменения." +class EvaluationNotSubmittedError(EvaluationConflictError): + code = "evaluation_not_submitted" + default_detail = "Изменить можно только отправленную оценку." + + @dataclass(frozen=True) class EvaluationCreationResult: evaluation: Evaluation @@ -170,6 +176,7 @@ def expert_submission_assignments(*, user): my_evaluation_status=Subquery(evaluation.values("status")[:1]), my_evaluation_updated_at=Subquery(evaluation.values("updated_at")[:1]), my_evaluation_submitted_at=Subquery(evaluation.values("submitted_at")[:1]), + my_evaluation_amended_at=Subquery(evaluation.values("amended_at")[:1]), ) .order_by("-assigned_at", "-id") ) @@ -322,6 +329,32 @@ def _replace_scores(*, evaluation, validated_scores): ) +def _validated_complete_scores(*, program, scores): + validated_scores = _validated_scores(program=program, scores=scores) + expected_ids = set(get_numeric_criteria(program).values_list("id", flat=True)) + actual_ids = {criterion.pk for criterion, _value in validated_scores} + if actual_ids != expected_ids: + raise EvaluationValidationError( + "Передайте полный набор числовых критериев программы.", + field="scores", + ) + return validated_scores + + +def _score_snapshot(evaluation): + return [ + { + "criterion_id": score.criterion_id, + "criterion_name": score.criterion_name, + "criterion_type": score.criterion_type, + "min_value": score.min_value, + "max_value": score.max_value, + "value": str(score.value), + } + for score in evaluation.scores.order_by("criterion_id", "id") + ] + + def _existing_creation_result(evaluation): if evaluation.status == Evaluation.STATUS_SUBMITTED: raise EvaluationSubmittedError() @@ -536,6 +569,119 @@ def submit_evaluation(*, evaluation_id, user): return evaluation +def amend_submitted_evaluation( + *, + evaluation_id, + user, + comment_supplied=False, + comment="", + scores_supplied=False, + scores=None, +): + identity, expert = _evaluation_identity_for_owner( + evaluation_id=evaluation_id, + user=user, + ) + with transaction.atomic(): + assignment = _assignment_queryset( + submission_id=identity.submission_id, + expert_id=expert.pk, + for_update=True, + ).first() + if assignment is None: + raise EvaluationNotFoundError() + if assignment.status not in SubmissionExpertAssignment.ACTIVE_STATUSES: + raise AssignmentUnavailableError() + + evaluation = ( + Evaluation.objects.select_for_update() + .select_related("submission", "submission__program") + .get(pk=identity.pk) + ) + _require_expert_membership(expert, evaluation.submission.program) + if evaluation.status != Evaluation.STATUS_SUBMITTED: + raise EvaluationNotSubmittedError() + _require_submission_status(evaluation.submission) + + validated_scores = None + if scores_supplied: + validated_scores = _validated_complete_scores( + program=evaluation.submission.program, + scores=scores or [], + ) + + previous_comment = evaluation.comment + previous_scores = _score_snapshot(evaluation) + next_comment = comment if comment_supplied else previous_comment + current_score_values = { + score.criterion_id: score.value + for score in evaluation.scores.only("criterion_id", "value") + } + next_score_values = ( + {criterion.pk: value for criterion, value in validated_scores} + if validated_scores is not None + else current_score_values + ) + if next_comment == previous_comment and next_score_values == current_score_values: + return evaluation + + previous_total_score = evaluation.total_score + if validated_scores is not None: + _replace_scores( + evaluation=evaluation, + validated_scores=validated_scores, + ) + + evaluation.comment = next_comment + evaluation.total_score = None + evaluation.amended_at = timezone.now() + evaluation.save( + update_fields=[ + "comment", + "total_score", + "amended_at", + "updated_at", + ] + ) + EvaluationAmendment.objects.create( + evaluation=evaluation, + changed_by=user, + previous_comment=previous_comment, + comment=evaluation.comment, + previous_scores=previous_scores, + scores=_score_snapshot(evaluation), + previous_total_score=previous_total_score, + total_score=evaluation.total_score, + ) + return evaluation + + +def get_evaluation_amendments(*, evaluation_id, user): + evaluation = ( + Evaluation.objects.select_related( + "submission", + "submission__program", + "expert", + ) + .filter(pk=evaluation_id) + .first() + ) + if evaluation is None: + raise EvaluationNotFoundError() + if user.is_staff or user.is_superuser: + pass + elif evaluation.submission.program.is_manager(user): + pass + else: + try: + expert = user.expert + except Expert.DoesNotExist as exc: + raise EvaluationNotFoundError() from exc + if evaluation.expert_id != expert.pk: + raise EvaluationNotFoundError() + return evaluation.amendments.select_related("changed_by").all() + + def get_visible_evaluation(*, evaluation_id, user): evaluation = ( Evaluation.objects.select_related( diff --git a/partner_programs/tests/test_expert_evaluation_api.py b/partner_programs/tests/test_expert_evaluation_api.py index a21bec91..3f6d8deb 100644 --- a/partner_programs/tests/test_expert_evaluation_api.py +++ b/partner_programs/tests/test_expert_evaluation_api.py @@ -1,4 +1,4 @@ -# Roadmap: DEV-050, DEV-051, DEV-052 +# Roadmap: DEV-050, DEV-051, DEV-052, DEV-073 # Проверки экспертного доступа, autosave, submit и manager read-only API. from decimal import Decimal @@ -19,6 +19,7 @@ from partner_programs.models import ( Application, Evaluation, + EvaluationAmendment, EvaluationScore, Submission, SubmissionExpertAssignment, @@ -782,6 +783,305 @@ def test_submit_is_scoped_throttled(self): self.assertEqual(second.status_code, 429) +class EvaluationAmendmentAPITests(ExpertEvaluationAPITestCase): + def setUp(self): + super().setUp() + self.submitted_at = timezone.now() + self.evaluation = self.create_evaluation( + comment="Initial submitted comment", + status=Evaluation.STATUS_SUBMITTED, + submitted_at=self.submitted_at, + total_score=Decimal("12.5"), + ) + for criterion, value in ( + (self.int_criterion, Decimal("6")), + (self.float_criterion, Decimal("3.5")), + ): + EvaluationScore.objects.create( + evaluation=self.evaluation, + criterion=criterion, + value=value, + ) + self.assignment.status = SubmissionExpertAssignment.STATUS_COMPLETED + self.assignment.completed_at = self.submitted_at + self.assignment.save() + self.amend_url = f"/evaluations/{self.evaluation.pk}/amend/" + self.history_url = f"/evaluations/{self.evaluation.pk}/amendments/" + + def test_owner_amends_comment_and_complete_scores(self): + completed_at = self.assignment.completed_at + self.authenticate() + + response = self.client.patch( + self.amend_url, + { + "comment": "Corrected comment", + "scores": self.score_payload(int_value="9", float_value="5.25"), + }, + format="json", + ) + + self.assertEqual(response.status_code, 200) + self.evaluation.refresh_from_db() + self.assignment.refresh_from_db() + self.assertEqual(self.evaluation.status, Evaluation.STATUS_SUBMITTED) + self.assertEqual(self.evaluation.submitted_at, self.submitted_at) + self.assertIsNotNone(self.evaluation.amended_at) + self.assertIsNone(self.evaluation.total_score) + self.assertIsNotNone(response.data["amended_at"]) + self.assertEqual( + {score.criterion_id: score.value for score in self.evaluation.scores.all()}, + { + self.int_criterion.pk: Decimal("9"), + self.float_criterion.pk: Decimal("5.25"), + }, + ) + self.assertEqual( + self.assignment.status, + SubmissionExpertAssignment.STATUS_COMPLETED, + ) + self.assertEqual(self.assignment.completed_at, completed_at) + self.assertEqual( + Evaluation.objects.filter( + submission=self.submission, + expert=self.expert, + ).count(), + 1, + ) + + amendment = EvaluationAmendment.objects.get(evaluation=self.evaluation) + self.assertEqual(amendment.changed_by, self.expert_user) + self.assertEqual(amendment.previous_comment, "Initial submitted comment") + self.assertEqual(amendment.comment, "Corrected comment") + self.assertEqual(amendment.previous_total_score, Decimal("12.5")) + self.assertIsNone(amendment.total_score) + self.assertEqual( + {item["criterion_id"] for item in amendment.previous_scores}, + {self.int_criterion.pk, self.float_criterion.pk}, + ) + self.assertEqual( + {item["criterion_id"]: Decimal(item["value"]) for item in amendment.scores}, + { + self.int_criterion.pk: Decimal("9"), + self.float_criterion.pk: Decimal("5.25"), + }, + ) + + list_response = self.client.get("/expert/submissions/") + self.assertEqual( + list_response.data["results"][0]["my_evaluation"]["amended_at"], + self.evaluation.amended_at, + ) + + def test_comment_only_preserves_scores_and_creates_history(self): + score_ids = tuple( + self.evaluation.scores.order_by("id").values_list("id", flat=True) + ) + self.authenticate() + + response = self.client.patch( + self.amend_url, + {"comment": "Comment only"}, + format="json", + ) + + self.assertEqual(response.status_code, 200) + self.assertEqual( + tuple(self.evaluation.scores.order_by("id").values_list("id", flat=True)), + score_ids, + ) + amendment = EvaluationAmendment.objects.get(evaluation=self.evaluation) + self.assertEqual(amendment.previous_scores, amendment.scores) + + def test_identical_request_does_not_create_history(self): + self.authenticate() + payload = { + "comment": "Changed once", + "scores": self.score_payload(int_value="8", float_value="4.5"), + } + + first = self.client.patch(self.amend_url, payload, format="json") + self.evaluation.refresh_from_db() + first_amended_at = self.evaluation.amended_at + second = self.client.patch(self.amend_url, payload, format="json") + + self.assertEqual(first.status_code, 200) + self.assertEqual(second.status_code, 200) + self.evaluation.refresh_from_db() + self.assertEqual(self.evaluation.amended_at, first_amended_at) + self.assertEqual( + EvaluationAmendment.objects.filter(evaluation=self.evaluation).count(), + 1, + ) + + def test_incomplete_scores_roll_back_comment_and_scores(self): + score_ids = tuple( + self.evaluation.scores.order_by("id").values_list("id", flat=True) + ) + self.authenticate() + + response = self.client.patch( + self.amend_url, + { + "comment": "Must rollback", + "scores": [ + {"criterion_id": self.int_criterion.pk, "value": "8"}, + ], + }, + format="json", + ) + + self.assertEqual(response.status_code, 400) + self.evaluation.refresh_from_db() + self.assertEqual(self.evaluation.comment, "Initial submitted comment") + self.assertEqual( + tuple(self.evaluation.scores.order_by("id").values_list("id", flat=True)), + score_ids, + ) + self.assertFalse( + EvaluationAmendment.objects.filter(evaluation=self.evaluation).exists() + ) + + def test_draft_evaluation_cannot_be_amended(self): + self.evaluation.status = Evaluation.STATUS_DRAFT + self.evaluation.submitted_at = None + self.evaluation.save() + self.authenticate() + + response = self.client.patch( + self.amend_url, + {"comment": "Forbidden"}, + format="json", + ) + + self.assertEqual(response.status_code, 409) + + def test_revoked_assignment_cannot_be_amended(self): + self.assignment.status = SubmissionExpertAssignment.STATUS_REVOKED + self.assignment.completed_at = None + self.assignment.revoked_by = self.manager + self.assignment.revoked_at = timezone.now() + self.assignment.revoke_reason = "Revoked" + self.assignment.save() + self.authenticate() + + response = self.client.patch( + self.amend_url, + {"comment": "Forbidden"}, + format="json", + ) + + self.assertEqual(response.status_code, 409) + + def test_other_expert_and_manager_cannot_amend(self): + for user in (self.other_expert_user, self.manager): + with self.subTest(user=user.pk): + self.authenticate(user) + response = self.client.patch( + self.amend_url, + {"comment": "Forbidden"}, + format="json", + ) + self.assertEqual(response.status_code, 404) + + def test_history_is_visible_to_owner_manager_and_staff(self): + self.authenticate() + amend_response = self.client.patch( + self.amend_url, + {"comment": "Visible history"}, + format="json", + ) + self.assertEqual(amend_response.status_code, 200) + + for user in (self.expert_user, self.manager, self.staff): + with self.subTest(user=user.pk): + self.authenticate(user) + response = self.client.get(self.history_url) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data), 1) + self.assertEqual( + response.data[0]["evaluation_id"], + self.evaluation.pk, + ) + + self.authenticate(self.manager) + manager_response = self.client.get( + f"/programs/{self.program.pk}/evaluations/{self.evaluation.pk}/" + ) + self.assertEqual(manager_response.status_code, 200) + self.assertEqual( + manager_response.data["amended_at"], + amend_response.data["amended_at"], + ) + + self.authenticate(self.other_expert_user) + hidden_response = self.client.get(self.history_url) + self.assertEqual(hidden_response.status_code, 404) + + def test_assigned_episode_can_be_amended_and_stays_assigned(self): + self.assignment.status = SubmissionExpertAssignment.STATUS_ASSIGNED + self.assignment.completed_at = None + self.assignment.save() + self.authenticate() + + response = self.client.patch( + self.amend_url, + {"comment": "Assigned correction"}, + format="json", + ) + + self.assertEqual(response.status_code, 200) + self.assignment.refresh_from_db() + self.assertEqual( + self.assignment.status, + SubmissionExpertAssignment.STATUS_ASSIGNED, + ) + + def test_invalid_submission_or_missing_membership_blocks_amendment(self): + self.authenticate() + self.submission.status = Submission.STATUS_RETURNED + self.submission.submitted_at = None + self.submission.save() + + invalid_submission = self.client.patch( + self.amend_url, + {"comment": "Forbidden"}, + format="json", + ) + self.assertEqual(invalid_submission.status_code, 409) + + self.submission.status = Submission.STATUS_SUBMITTED + self.submission.submitted_at = timezone.now() + self.submission.save() + self.expert.programs.remove(self.program) + missing_membership = self.client.patch( + self.amend_url, + {"comment": "Still forbidden"}, + format="json", + ) + self.assertEqual(missing_membership.status_code, 404) + + @override_settings(REST_FRAMEWORK=throttle_settings(evaluation_amend="1/min")) + def test_amend_is_scoped_throttled(self): + self.authenticate() + + first = self.client.patch( + self.amend_url, + {"comment": "First amendment"}, + format="json", + REMOTE_ADDR="203.0.113.93", + ) + second = self.client.patch( + self.amend_url, + {"comment": "Second amendment"}, + format="json", + REMOTE_ADDR="203.0.113.93", + ) + + self.assertEqual(first.status_code, 200) + self.assertEqual(second.status_code, 429) + + class EvaluationReadAndManagerAPITests(ExpertEvaluationAPITestCase): def setUp(self): super().setUp() diff --git a/partner_programs/throttling.py b/partner_programs/throttling.py index 6d20536b..41070023 100644 --- a/partner_programs/throttling.py +++ b/partner_programs/throttling.py @@ -1,7 +1,7 @@ from rest_framework.throttling import ScopedRateThrottle from rest_framework.settings import api_settings -# Roadmap: DEV-051 +# Roadmap: DEV-051, DEV-073 # Отдельный лимит частого autosave черновика Evaluation. diff --git a/procollab/settings.py b/procollab/settings.py index 43889f18..af74fdfc 100644 --- a/procollab/settings.py +++ b/procollab/settings.py @@ -214,6 +214,9 @@ "evaluation_submit": config( "DRF_THROTTLE_EVALUATION_SUBMIT", default="20/min", cast=str ), + "evaluation_amend": config( + "DRF_THROTTLE_EVALUATION_AMEND", default="30/min", cast=str + ), }, }