diff --git a/partner_programs/management_views.py b/partner_programs/management_views.py index d55b2566..4386b983 100644 --- a/partner_programs/management_views.py +++ b/partner_programs/management_views.py @@ -56,6 +56,18 @@ def put(self, request, pk): return Response(save_form(pk, request.user, request.data)) +class ManagementCriteriaView(ManagementBase): + def get(self, request, pk): + from partner_programs.services.management_criteria import criteria_data + + return Response(criteria_data(managed_program(pk, request.user))) + + def put(self, request, pk): + from partner_programs.services.management_criteria import save_criteria + + return Response(save_criteria(pk, request.user, request.data)) + + class ManagementParticipantsView(ManagementBase): def get(self, request, pk): program = managed_program(pk, request.user) diff --git a/partner_programs/migrations/0021_evaluation_criteria_lock.py b/partner_programs/migrations/0021_evaluation_criteria_lock.py new file mode 100644 index 00000000..1bd89af6 --- /dev/null +++ b/partner_programs/migrations/0021_evaluation_criteria_lock.py @@ -0,0 +1,33 @@ +from django.db import migrations, models +from django.utils import timezone + + +def freeze_existing(apps, schema_editor): + scores = apps.get_model("project_rates", "ProjectScore") + programs = apps.get_model("partner_programs", "PartnerProgram") + ids = scores.objects.using(schema_editor.connection.alias).values_list( + "criteria__partner_program_id", flat=True + ) + programs.objects.using(schema_editor.connection.alias).filter(pk__in=ids).update( + evaluation_criteria_locked_at=timezone.now() + ) + + +class Migration(migrations.Migration): + dependencies = [ + ("partner_programs", "0020_partnerprogram_image_framing"), + ("project_rates", "0003_projectexpertassignment"), + ] + operations = [ + migrations.AddField( + model_name="partnerprogram", + name="evaluation_criteria_locked_at", + field=models.DateTimeField(null=True, editable=False), + ), + migrations.AddField( + model_name="partnerprogram", + name="evaluation_criteria_version", + field=models.PositiveIntegerField(default=0, editable=False), + ), + migrations.RunPython(freeze_existing, migrations.RunPython.noop), + ] diff --git a/partner_programs/models.py b/partner_programs/models.py index fd3ce000..6dca390f 100644 --- a/partner_programs/models.py +++ b/partner_programs/models.py @@ -116,6 +116,8 @@ class PartnerProgram(models.Model): "Если включено, проекты для оценки доступны только назначенным экспертам" ), ) + evaluation_criteria_locked_at = models.DateTimeField(null=True, editable=False) + evaluation_criteria_version = models.PositiveIntegerField(default=0, editable=False) data_schema = models.JSONField( verbose_name="Схема данных в формате JSON", help_text="Ключи - имена полей, значения - тип поля ввода", diff --git a/partner_programs/services/management_criteria.py b/partner_programs/services/management_criteria.py new file mode 100644 index 00000000..387ecfa7 --- /dev/null +++ b/partner_programs/services/management_criteria.py @@ -0,0 +1,116 @@ +"""Atomic editing of the shared program criteria, before the first score.""" + +import math + +from django.db import transaction +from django.utils import timezone +from rest_framework import serializers + +from partner_programs.models import PartnerProgram +from partner_programs.serializers.management import StrictFieldsMixin +from partner_programs.services.management import managed_program +from project_rates.criteria_policy import CriteriaConflict, definitions, locked, revision +from project_rates.models import Criteria + +SUPPORTED_TYPES = ("int", "bool", "str") + + +class CriterionWrite(StrictFieldsMixin, serializers.Serializer): + id = serializers.IntegerField(required=False, allow_null=True, min_value=1) + name = serializers.CharField(max_length=50) + description = serializers.CharField( + required=False, allow_blank=True, allow_null=True, default="", max_length=4000 + ) + type = serializers.ChoiceField(choices=(*SUPPORTED_TYPES, "float")) + min_value = serializers.FloatField(allow_null=True) + max_value = serializers.FloatField(allow_null=True) + + +class CriteriaWrite(StrictFieldsMixin, serializers.Serializer): + expected_updated_at = serializers.DateTimeField() + expected_revision = serializers.CharField(max_length=64) + criteria = CriterionWrite(many=True) + + def validate_criteria(self, rows): + ids = [r["id"] for r in rows if r.get("id") is not None] + if len(rows) > 100 or len(ids) != len(set(ids)): + raise serializers.ValidationError( + "Не больше 100 критериев, без повторяющихся ID." + ) + return rows + + +def criteria_data(program): + return { + "updated_at": program.datetime_updated, + "revision": revision(program), + "locked": locked(program), + # Historical scores have no timestamps; do not present the backfill date as first score. + "supported_types": SUPPORTED_TYPES, + "criteria": definitions(program), + } + + +@transaction.atomic +def save_criteria(program_id, user, payload): + program = managed_program(program_id, user, lock=True) + if locked(program): + raise CriteriaConflict( + "criteria_locked", + "В программе уже сохранена первая оценка. Критерии больше нельзя менять.", + ) + serializer = CriteriaWrite(data=payload) + serializer.is_valid(raise_exception=True) + data = serializer.validated_data + if data["expected_updated_at"] != program.datetime_updated or data[ + "expected_revision" + ] != revision(program): + raise CriteriaConflict( + "stale_criteria", "Настройки изменены. Обновите действующие критерии." + ) + existing = {c.pk: c for c in program.criterias.all()} + keep = [] + for row in data["criteria"]: + pk = row.pop("id", None) + old = existing.get(pk) + if pk is not None and old is None: + raise serializers.ValidationError( + {"criteria": "Критерий не принадлежит этой программе."} + ) + unchanged = old and all(getattr(old, key) == value for key, value in row.items()) + if not unchanged: + if row["type"] not in SUPPORTED_TYPES: + raise serializers.ValidationError( + {"criteria": "Дробный критерий можно только сохранить без изменений."} + ) + low, high = row["min_value"], row["max_value"] + if row["type"] == "int": + if ( + any( + v is None or not math.isfinite(v) or not v.is_integer() + for v in (low, high) + ) + or low > high + ): + raise serializers.ValidationError( + { + "criteria": "Укажите целые минимум и максимум; максимум не меньше минимума." + } + ) + elif low is not None or high is not None: + raise serializers.ValidationError( + {"criteria": "Шкала доступна только для баллов."} + ) + if old: + Criteria.objects.filter(pk=pk).update(**row) + keep.append(pk) + else: + keep.append(Criteria.objects.create(partner_program=program, **row).pk) + program.criterias.exclude(pk__in=keep).delete() + program.evaluation_criteria_version += 1 + program.datetime_updated = timezone.now() + PartnerProgram.objects.filter(pk=program.pk).update( + evaluation_criteria_version=program.evaluation_criteria_version, + datetime_updated=program.datetime_updated, + ) + return criteria_data(program) diff --git a/partner_programs/tests/test_management_criteria_api.py b/partner_programs/tests/test_management_criteria_api.py new file mode 100644 index 00000000..fda5a179 --- /dev/null +++ b/partner_programs/tests/test_management_criteria_api.py @@ -0,0 +1,264 @@ +from concurrent.futures import ThreadPoolExecutor +from threading import Barrier +from unittest.mock import patch + +from django.core.exceptions import ValidationError +from django.db import connections +from django.test import TestCase, TransactionTestCase, skipUnlessDBFeature +from rest_framework.test import APIClient + +from project_rates.models import ProjectScore +from project_rates.tests.helpers import ( + create_rate_criteria, + create_rate_expert, + create_rate_program, + create_rate_project, + create_rate_user, + link_project_to_program, +) + + +class ManagementCriteriaTests(TestCase): + def setUp(self): + self.program = create_rate_program() + self.manager = create_rate_user() + self.program.managers.add(self.manager) + self.expert = create_rate_expert(program=self.program) + self.project = create_rate_project() + link_project_to_program(self.program, self.project) + self.client = APIClient() + self.client.force_authenticate(self.manager) + self.url = f"/programs/{self.program.pk}/management/criteria/" + + def payload(self): + snapshot = self.client.get(self.url).data + return dict( + expected_updated_at=snapshot["updated_at"], + expected_revision=snapshot["revision"], + criteria=snapshot["criteria"], + ) + + def put(self, data): + return self.client.put(self.url, data, format="json") + + def criterion(self, **overrides): + return dict( + name="Качество решения", + description="Обоснованность", + type="int", + min_value=0, + max_value=10, + **overrides, + ) + + def test_roundtrip_all_types_and_default_comment_not_resurrected(self): + data = self.payload() + data["criteria"] = [ + self.criterion(), + {**self.criterion(), "type": "bool", "min_value": None, "max_value": None}, + ] + response = self.put(data) + self.assertEqual(response.status_code, 200, response.data) + self.assertFalse(response.data["locked"]) + self.assertEqual(len(response.data["criteria"]), 2) + self.assertNotEqual(response.data["revision"], data["expected_revision"]) + self.program.refresh_from_db() + self.program.save() + self.assertEqual(self.program.criterias.count(), 2) + self.assertFalse(ProjectScore.objects.exists()) + + def test_auth_scope_read_and_write(self): + for user, code in [(None, 401), (create_rate_user(), 403)]: + self.client.force_authenticate(user) + self.assertEqual(self.client.get(self.url).status_code, code) + self.assertEqual(self.put({}).status_code, code) + + def test_validation_and_foreign_ids_are_atomic(self): + foreign = create_rate_criteria(create_rate_program()) + for invalid in [ + {**self.criterion(), "name": " "}, + {**self.criterion(), "max_value": -1}, + {**self.criterion(), "min_value": 0.5}, + {**self.criterion(), "max_value": None}, + {**self.criterion(), "type": "float"}, + {**self.criterion(), "id": foreign.pk}, + ]: + data = self.payload() + data["criteria"] = [self.criterion(), invalid] + self.assertEqual(self.put(data).status_code, 400) + self.assertEqual(self.program.criterias.count(), 1) + + def test_unchanged_legacy_float_and_unbounded_integer_survive(self): + create_rate_criteria(self.program, type="float", min_value=0.1) + create_rate_criteria(self.program) + self.assertEqual(self.put(self.payload()).status_code, 200) + + def test_stale_configuration_rejected(self): + data = self.payload() + self.assertEqual(self.put(data).status_code, 200) + self.assertEqual(self.put(data).status_code, 409) + + def test_admin_guards_and_historical_score_backfill(self): + from importlib import import_module + from types import SimpleNamespace + from django.apps import apps + from django.contrib.admin.sites import AdminSite + from django.db import connection + from django.test import RequestFactory + from project_rates.admin import CriteriaAdmin + from project_rates.models import Criteria + + criterion = self.program.criterias.first() + ProjectScore.objects.bulk_create( + [ + ProjectScore( + criteria=criterion, + user=self.expert, + project=self.project, + value="Legacy", + ) + ] + ) + migration = import_module( + "partner_programs.migrations.0021_evaluation_criteria_lock" + ) + # The data backfill uses only the connection alias, not schema DDL. + # Opening SQLite's schema editor inside TestCase.atomic is unsupported. + migration.freeze_existing(apps, SimpleNamespace(connection=connection)) + self.program.refresh_from_db() + self.assertIsNotNone(self.program.evaluation_criteria_locked_at) + self.manager.is_staff = True + self.manager.is_superuser = True + request = RequestFactory().get("/") + request.user = self.manager + admin = CriteriaAdmin(Criteria, AdminSite()) + self.assertFalse(admin.has_change_permission(request, criterion)) + self.assertFalse(admin.has_delete_permission(request, criterion)) + + @patch("project_rates.services.send_email.delay") + def test_zero_and_no_freeze_whole_set_and_reject_all_mutations(self, email): + data = self.payload() + data["criteria"] = [ + self.criterion(), + {**self.criterion(), "type": "bool", "min_value": None, "max_value": None}, + ] + snapshot = self.put(data).data + self.client.force_authenticate(self.expert) + response = self.client.post( + f"/rate-project/rate/{self.project.pk}", + { + "expected_revision": snapshot["revision"], + "scores": [ + { + "criterion_id": c["id"], + "value": "0" if c["type"] == "int" else "False", + } + for c in snapshot["criteria"] + ], + }, + format="json", + ) + self.assertEqual(response.status_code, 201, response.data) + self.client.force_authenticate(self.manager) + self.assertTrue(self.client.get(self.url).data["locked"]) + self.assertEqual(self.put(self.payload()).data["code"], "criteria_locked") + criterion = self.program.criterias.first() + criterion.name = "Changed" + with self.assertRaises(ValidationError): + criterion.save() + with self.assertRaises(ValidationError): + self.program.criterias.all().delete() + with self.assertRaises(ValidationError): + create_rate_criteria(self.program) + ProjectScore.objects.all().delete() + self.assertTrue(self.client.get(self.url).data["locked"]) + + @patch("project_rates.services.send_email.delay") + def test_stale_expert_form_and_missing_revision_do_not_save_or_lock(self, email): + old = self.payload() + snapshot = self.put(old).data + self.client.force_authenticate(self.expert) + url = f"/rate-project/rate/{self.project.pk}" + scores = [{"criterion_id": snapshot["criteria"][0]["id"], "value": "OK"}] + # A no-op configuration save still requires a current form revision, not an old cached client. + self.assertEqual(self.client.post(url, scores, format="json").status_code, 409) + self.assertEqual( + self.client.post( + url, {"expected_revision": "old", "scores": scores}, format="json" + ).status_code, + 409, + ) + self.assertFalse(ProjectScore.objects.exists()) + self.program.refresh_from_db() + self.assertIsNone(self.program.evaluation_criteria_locked_at) + + @patch("project_rates.services.send_email.delay") + def test_integer_validation_rejects_fractions_nan_and_out_of_range(self, email): + criterion = create_rate_criteria(self.program, min_value=-2, max_value=10) + self.client.force_authenticate(self.expert) + for value in ["1.5", "NaN", "Infinity", "-3", "11"]: + response = self.client.post( + f"/rate-project/rate/{self.project.pk}", + [{"criterion_id": criterion.pk, "value": value}], + format="json", + ) + self.assertEqual(response.status_code, 400, (value, response.data)) + self.assertFalse(ProjectScore.objects.exists()) + + +class CriteriaRaceTests(TransactionTestCase): + @skipUnlessDBFeature("has_select_for_update") + @patch("project_rates.services.send_email.delay") + def test_first_score_and_manager_save_are_serialized(self, email): + from partner_programs.services.management_criteria import ( + criteria_data, + save_criteria, + ) + from project_rates.services import submit_project_scores + from project_rates.criteria_policy import CriteriaConflict + + program = create_rate_program() + manager = create_rate_user() + program.managers.add(manager) + expert = create_rate_expert(program=program) + project = create_rate_project() + link_project_to_program(program, project) + criterion = create_rate_criteria(program, min_value=0, max_value=10) + snapshot = criteria_data(program) + payload = { + "expected_updated_at": snapshot["updated_at"], + "expected_revision": snapshot["revision"], + "criteria": snapshot["criteria"], + } + payload["criteria"][-1]["max_value"] = 5 + barrier = Barrier(2) + + def run(action): + connections.close_all() + try: + barrier.wait(timeout=10) + if action == "manager": + save_criteria(program.pk, manager, payload) + else: + submit_project_scores( + user=expert, + project_id=project.pk, + data={ + "expected_revision": snapshot["revision"], + "scores": [{"criterion_id": criterion.pk, "value": "8"}], + }, + ) + return "ok" + except CriteriaConflict: + return "conflict" + finally: + connections.close_all() + + with ThreadPoolExecutor(max_workers=2) as pool: + results = list(pool.map(run, ["manager", "expert"])) + self.assertCountEqual(results, ["ok", "conflict"]) + criterion.refresh_from_db() + if ProjectScore.objects.exists(): + self.assertEqual(criterion.max_value, 10) + else: + self.assertEqual(criterion.max_value, 5) diff --git a/partner_programs/urls.py b/partner_programs/urls.py index 8fade1ff..2d1f4ec7 100644 --- a/partner_programs/urls.py +++ b/partner_programs/urls.py @@ -33,6 +33,7 @@ from partner_programs.management_views import ( ManagementView, ManagementProjectFormView, + ManagementCriteriaView, ManagementParticipantsView, ManagementProjectsView, ManagementTeamView, @@ -45,6 +46,11 @@ app_name = "partner_programs" urlpatterns = [ + path( + "/management/criteria/", + ManagementCriteriaView.as_view(), + name="management-criteria", + ), path( "/management/project-form/", ManagementProjectFormView.as_view(), diff --git a/project_rates/admin.py b/project_rates/admin.py index ba22e4b3..683acbc5 100644 --- a/project_rates/admin.py +++ b/project_rates/admin.py @@ -11,11 +11,28 @@ from users.models import Expert from .models import Criteria, ProjectExpertAssignment, ProjectScore +from .criteria_policy import locked + + +class CriteriaAdminForm(forms.ModelForm): + class Meta: + model = Criteria + fields = "__all__" + + def clean(self): + data = super().clean() + program = data.get("partner_program") + if program and locked(program): + raise forms.ValidationError( + "После первой оценки критерии программы зафиксированы." + ) + return data # Register your models here. @admin.register(Criteria) class CriteriaAdmin(admin.ModelAdmin): + form = CriteriaAdminForm list_display = ("id", "name", "type", "get_program_name") list_display_links = ( "id", @@ -25,6 +42,22 @@ class CriteriaAdmin(admin.ModelAdmin): def get_program_name(self, obj): return obj.partner_program.name + def has_change_permission(self, request, obj=None): + return super().has_change_permission(request, obj) and ( + obj is None or not locked(obj.partner_program) + ) + + def has_delete_permission(self, request, obj=None): + return super().has_delete_permission(request, obj) and ( + obj is None or not locked(obj.partner_program) + ) + + def get_actions(self, request): + actions = super().get_actions(request) + # Single-object deletion has a program-specific permission check. + actions.pop("delete_selected", None) + return actions + @admin.register(ProjectScore) class ProjectScoreAdmin(admin.ModelAdmin): diff --git a/project_rates/criteria_policy.py b/project_rates/criteria_policy.py new file mode 100644 index 00000000..8731b7b5 --- /dev/null +++ b/project_rates/criteria_policy.py @@ -0,0 +1,47 @@ +"""Shared revision and permanent freeze policy for organizer/admin/expert writes.""" + +import hashlib +import json + +from django.utils import timezone +from rest_framework.exceptions import APIException + + +class CriteriaConflict(APIException): + status_code = 409 + + def __init__(self, code, detail): + super().__init__({"code": code, "detail": detail}) + + +def definitions(program): + return list( + program.criterias.order_by("pk").values( + "id", "name", "description", "type", "min_value", "max_value" + ) + ) + + +def revision(program): + return hashlib.sha256( + json.dumps(definitions(program), ensure_ascii=False, sort_keys=True).encode() + ).hexdigest() + + +def locked(program): + from project_rates.models import ProjectScore + + return ( + bool(program.evaluation_criteria_locked_at) + or ProjectScore.objects.filter(criteria__partner_program=program).exists() + ) + + +def mark_locked(program): + from partner_programs.models import PartnerProgram + + if not program.evaluation_criteria_locked_at: + program.evaluation_criteria_locked_at = timezone.now() + PartnerProgram.objects.filter(pk=program.pk).update( + evaluation_criteria_locked_at=program.evaluation_criteria_locked_at + ) diff --git a/project_rates/models.py b/project_rates/models.py index e086cd28..74dea968 100644 --- a/project_rates/models.py +++ b/project_rates/models.py @@ -1,6 +1,6 @@ from django.contrib.auth import get_user_model from django.core.exceptions import ValidationError -from django.db import models +from django.db import models, transaction from .constants import VERBOSE_TYPES from partner_programs.models import PartnerProgram, PartnerProgramProject @@ -10,6 +10,22 @@ User = get_user_model() +class CriteriaQuerySet(models.QuerySet): + def delete(self): + from .criteria_policy import locked + + with transaction.atomic(): + ids = self.values_list("partner_program_id", flat=True).distinct() + programs = ( + PartnerProgram.objects.select_for_update() + .filter(pk__in=ids) + .order_by("pk") + ) + if any(locked(p) for p in programs): + raise ValidationError("После первой оценки критерии нельзя удалять.") + return super().delete() + + class Criteria(models.Model): """ Criteria model @@ -46,6 +62,39 @@ class Criteria(models.Model): related_name="criterias", ) + objects = CriteriaQuerySet.as_manager() + + def save(self, *args, **kwargs): + from .criteria_policy import locked + + with transaction.atomic(): + old = type(self).objects.filter(pk=self.pk).first() if self.pk else None + ids = {self.partner_program_id} + if old: + ids.add(old.partner_program_id) + programs = list( + PartnerProgram.objects.select_for_update() + .filter(pk__in=ids) + .order_by("pk") + ) + changed = old is None or any( + getattr(old, field) != getattr(self, field) + for field in ( + "name", + "description", + "type", + "min_value", + "max_value", + "partner_program_id", + ) + ) + if changed and any(locked(p) for p in programs): + raise ValidationError("После первой оценки критерии нельзя менять.") + return super().save(*args, **kwargs) + + def delete(self, *args, **kwargs): + return type(self).objects.filter(pk=self.pk).delete() + def __str__(self): return f"Criteria<{self.id}> - {self.name} {self.partner_program.name}" @@ -81,14 +130,22 @@ def __str__(self): return f"ProjectScore<{self.id}> - {self.criteria.name}" def save(self, *args, **kwargs): - data_to_validate = { - "criteria_type": self.criteria.type, - "value": self.value, - "criteria_min_value": self.criteria.min_value, - "criteria_max_value": self.criteria.max_value, - } - ProjectScoreValidator.validate(**data_to_validate) - super().save(*args, **kwargs) + from .criteria_policy import mark_locked + + program_id = Criteria.objects.values_list("partner_program_id", flat=True).get( + pk=self.criteria_id + ) + with transaction.atomic(): + program = PartnerProgram.objects.select_for_update().get(pk=program_id) + self.criteria = Criteria.objects.get(pk=self.criteria_id) + ProjectScoreValidator.validate( + criteria_type=self.criteria.type, + value=self.value, + criteria_min_value=self.criteria.min_value, + criteria_max_value=self.criteria.max_value, + ) + super().save(*args, **kwargs) + mark_locked(program) class Meta: verbose_name = "Оценка проекта" @@ -140,15 +197,21 @@ def has_scores(self) -> bool: def clean(self): errors = {} - if self.expert_id and self.partner_program_id and not self.expert.programs.filter( - id=self.partner_program_id - ).exists(): + if ( + self.expert_id + and self.partner_program_id + and not self.expert.programs.filter(id=self.partner_program_id).exists() + ): errors["expert"] = "Эксперт не состоит в указанной программе." - if self.project_id and self.partner_program_id and not PartnerProgramProject.objects.filter( - partner_program_id=self.partner_program_id, - project_id=self.project_id, - ).exists(): + if ( + self.project_id + and self.partner_program_id + and not PartnerProgramProject.objects.filter( + partner_program_id=self.partner_program_id, + project_id=self.project_id, + ).exists() + ): errors["project"] = "Проект не привязан к указанной программе." if self.partner_program_id and self.project_id: diff --git a/project_rates/serializers.py b/project_rates/serializers.py index b1b32294..a061c8f3 100644 --- a/project_rates/serializers.py +++ b/project_rates/serializers.py @@ -60,6 +60,7 @@ def to_representation(self, instance): class ProjectListForRateSerializer(serializers.ModelSerializer): + criteria_revision = serializers.SerializerMethodField() views_count = serializers.SerializerMethodField() criterias = serializers.SerializerMethodField() scored = serializers.SerializerMethodField() @@ -84,8 +85,12 @@ class Meta: "rated_count", "max_rates", "criterias", + "criteria_revision", ] + def get_criteria_revision(self, obj): + return self.context.get("criteria_revision") + def get_views_count(self, obj) -> int: return get_views_count(obj) diff --git a/project_rates/services.py b/project_rates/services.py index 11c71819..20e1ca6c 100644 --- a/project_rates/services.py +++ b/project_rates/services.py @@ -9,6 +9,7 @@ from projects.models import Project from project_rates.models import Criteria, ProjectExpertAssignment, ProjectScore from project_rates.serializers import ProjectScoreCreateSerializer +from project_rates.criteria_policy import CriteriaConflict, mark_locked, revision from users.models import Expert from vacancy.mapping import ProjectRatedParams, MessageTypeEnum from vacancy.tasks import send_email @@ -73,8 +74,7 @@ def get_projects_for_rate_queryset( ) return ( - projects_qs - .annotate( + projects_qs.annotate( rated_count=Count( "scores__user", filter=Q(scores__criteria__partner_program=program), @@ -86,7 +86,43 @@ def get_projects_for_rate_queryset( ) +@transaction.atomic def submit_project_scores(*, user, project_id: int, data) -> None: + expected_revision = None + if isinstance(data, dict): + expected_revision = data.get("expected_revision") + data = data.get("scores") + if ( + not isinstance(data, list) + or not data + or any(not isinstance(row, dict) for row in data) + ): + raise ValidationError("Передайте непустой список оценок.") + # Determine scope before locking; re-read all definitions and validate under the + # same program lock used by organizer saves. Never validate against a stale scale. + ids = [row.get("criterion_id") for row in data] + if any(not isinstance(pk, int) or isinstance(pk, bool) for pk in ids) or len( + set(ids) + ) != len(ids): + raise ValidationError("Некорректные ID критериев.") + programs = set( + Criteria.objects.filter(pk__in=ids).values_list("partner_program_id", flat=True) + ) + if len(programs) != 1: + if expected_revision: + raise CriteriaConflict( + "criteria_changed", "Критерии изменились. Обновите форму оценки." + ) + raise ValueError("All criteria must belong to the same program") + program = PartnerProgram.objects.select_for_update().get(pk=programs.pop()) + Expert.objects.get(user__id=user.id, programs=program) + if ( + expected_revision is not None or program.evaluation_criteria_version + ) and expected_revision != revision(program): + raise CriteriaConflict( + "criteria_changed", + "Критерии изменились. Обновите форму оценки; введённые ответы сохранены в форме.", + ) rating_data, criteria_ids, program = _prepare_project_score_data( user=user, project_id=project_id, @@ -119,20 +155,24 @@ def submit_project_scores(*, user, project_id: int, data) -> None: unique_fields=["criteria", "user", "project"], ) + mark_locked(program) + project = Project.objects.select_related("leader").get(id=project_id) _send_project_rated_email(project=project, program=program) -def _prepare_project_score_data(*, user, project_id: int, data) -> tuple[list, list, PartnerProgram]: +def _prepare_project_score_data( + *, user, project_id: int, data +) -> tuple[list, list, PartnerProgram]: rating_data = [dict(criterion) for criterion in data] criteria_ids = [criterion["criterion_id"] for criterion in rating_data] criteria_qs = Criteria.objects.filter(id__in=criteria_ids).select_related( "partner_program" ) - partner_program_ids = ( - criteria_qs.values_list("partner_program_id", flat=True).distinct() - ) + partner_program_ids = criteria_qs.values_list( + "partner_program_id", flat=True + ).distinct() if not criteria_qs.exists(): raise ValueError("Criteria not found") if partner_program_ids.count() != 1: @@ -152,11 +192,14 @@ def _prepare_project_score_data(*, user, project_id: int, data) -> tuple[list, l ).exists(): raise ValueError("Project is not linked to the program") - if program.is_distributed_evaluation and not ProjectExpertAssignment.objects.filter( - partner_program=program, - project_id=project_id, - expert__user_id=user.id, - ).exists(): + if ( + program.is_distributed_evaluation + and not ProjectExpertAssignment.objects.filter( + partner_program=program, + project_id=project_id, + expert__user_id=user.id, + ).exists() + ): raise ValueError("you are not assigned to rate this project") return rating_data, criteria_ids, program diff --git a/project_rates/signals.py b/project_rates/signals.py index af6743e3..226b9f3b 100644 --- a/project_rates/signals.py +++ b/project_rates/signals.py @@ -7,6 +7,8 @@ @receiver(post_save, sender=PartnerProgram) def create_comment(sender, instance, created, **kwargs): + if not created: + return Criteria.objects.get_or_create( name="Комментарий", description="Доп. поле для впечатлений о проекте", diff --git a/project_rates/validators.py b/project_rates/validators.py index c8c9d871..11470423 100644 --- a/project_rates/validators.py +++ b/project_rates/validators.py @@ -1,3 +1,5 @@ +import math + from project_rates.constants import ValidatableTypesNames, NumericTypes @@ -19,7 +21,11 @@ def validate(cls, **kwargs): def _validate_data_type(criteria_type: str, value: str): if criteria_type in NumericTypes: try: - float(value) + number = float(value) + if not math.isfinite(number) or ( + criteria_type == "int" and not number.is_integer() + ): + raise ValueError("Требуется конечное число подходящего типа.") except ValueError: raise ValueError("Введённое значение не соответствует формату!") except TypeError: diff --git a/project_rates/views.py b/project_rates/views.py index cf818a34..25ecd5e8 100644 --- a/project_rates/views.py +++ b/project_rates/views.py @@ -1,5 +1,6 @@ from rest_framework import generics, status from rest_framework.response import Response +from rest_framework.exceptions import APIException from django_filters import rest_framework as filters @@ -47,6 +48,8 @@ def create(self, request, *args, **kwargs) -> Response: ) except ValueError as e: return Response({"error": str(e)}, status=status.HTTP_400_BAD_REQUEST) + except APIException as e: + return Response(e.detail, status=e.status_code) except Exception as e: return Response({"error": str(e)}, status=status.HTTP_400_BAD_REQUEST) @@ -78,6 +81,9 @@ def get_queryset(self): ) def get_serializer_context(self): + from project_rates.criteria_policy import revision + context = super().get_serializer_context() context["program_max_rates"] = self._get_program().max_project_rates + context["criteria_revision"] = revision(self._get_program()) return context