Use serializer create/update methods

XanderVertegaal · XanderVertegaal · commit 75146f9c35c3 · 2025-12-04T13:40:07.000+01:00
diff --git a/backend/problem/serializers.py b/backend/problem/serializers.py
@@ -1,6 +1,6 @@
 from rest_framework import serializers
 from problem.services import FracasData, SNLIData, SickData
-from problem.models import Problem, KnowledgeBase
+from problem.models import Problem, KnowledgeBase, Sentence
 
 
 class KnowledgeBaseSerializer(serializers.ModelSerializer):
@@ -21,6 +21,23 @@ def validate_id(self, value):
                 )
         return value
 
+    def create_for_problem(
+        self, validated_data: dict, problem: Problem
+    ) -> KnowledgeBase:
+        """Create a new KnowledgeBase item for a problem."""
+        return KnowledgeBase.objects.create(
+            **validated_data,
+            problem=problem,
+        )
+
+    def update(self, instance: KnowledgeBase, validated_data: dict) -> KnowledgeBase:
+        """Update an existing KnowledgeBase item."""
+        instance.entity1 = validated_data["entity1"]
+        instance.relationship = validated_data["relationship"]
+        instance.entity2 = validated_data["entity2"]
+        instance.save()
+        return instance
+
 
 class ProblemSerializer(serializers.ModelSerializer):
     """
@@ -71,6 +88,85 @@ def get_kbItems(self, problem):
         kb_items = problem.knowledge_bases.all()
         return KnowledgeBaseSerializer(kb_items, many=True).data
 
+    def create(self, validated_data: dict) -> Problem:
+        """
+        Create a new Problem instance from validated input data.
+        Handles creation of related Sentence and KnowledgeBase objects.
+        """
+        premise_sentences = [
+            Sentence.objects.get_or_create(text=premise)[0]
+            for premise in validated_data["premises"]
+        ]
+
+        hypothesis_sentence = Sentence.objects.get_or_create(
+            text=validated_data["hypothesis"]
+        )[0]
+
+        problem = Problem.objects.create(
+            hypothesis=hypothesis_sentence,
+            dataset=Problem.Dataset.USER,
+            # TODO: Determine entailment label based on LangPro parser output.
+            entailment_label=Problem.EntailmentLabel.UNKNOWN,
+            extra_data={},
+        )
+
+        problem.premises.set(premise_sentences)
+
+        kb_items = validated_data.get("kbItems", [])
+        if kb_items:
+            self._update_or_create_kb_items(problem, kb_items)
+
+        return problem
+
+    def update(self, instance: Problem, validated_data: dict) -> Problem:
+        """
+        Update an existing Problem instance from validated input data.
+        Handles updating of related Sentence and KnowledgeBase objects.
+        """
+        if instance.dataset != Problem.Dataset.USER:
+            raise serializers.ValidationError(
+                "Cannot update a problem that is not a user-created problem."
+            )
+
+        instance.hypothesis = Sentence.objects.get_or_create(
+            text=validated_data["hypothesis"],
+        )[0]
+        instance.save()
+
+        premise_sentences = [
+            Sentence.objects.get_or_create(text=premise)[0]
+            for premise in validated_data["premises"]
+        ]
+        instance.premises.set(premise_sentences)
+
+        self._update_or_create_kb_items(instance, validated_data.get("kbItems", []))
+
+        return instance
+
+    def _update_or_create_kb_items(
+        self, problem: Problem, kb_items: list[dict]
+    ) -> None:
+        """Create or update KnowledgeBase items for a problem."""
+        kb_ids: list[int] = []
+        kb_serializer = KnowledgeBaseSerializer()
+
+        for item in kb_items:
+            kb_id = item.get("id", None)
+
+            if kb_id is None:
+                kb = kb_serializer.create_for_problem(item, problem=problem)  # type: ignore
+            else:
+                kb_instance = KnowledgeBase.objects.get(id=kb_id, problem_id=problem.pk)
+                kb = kb_serializer.update(kb_instance, item)
+
+            kb_ids.append(kb.pk)
+
+        # Delete existing knowledge bases associated to this problem that are
+        # not included in the input.
+        KnowledgeBase.objects.filter(problem_id=problem.pk).exclude(
+            id__in=kb_ids
+        ).delete()
+
 
 class ProblemInputSerializer(serializers.Serializer):
     """
@@ -91,16 +187,3 @@ class ProblemInputSerializer(serializers.Serializer):
         many=True, allow_empty=True, help_text="List of knowledge base items"
     )
 
-    def validate_id(self, value):
-        """
-        Validate that the problem ID exists and belongs to a user problem.
-        Users are not allowed to modify non-user problems.
-        """
-        if value is not None:
-            if not Problem.objects.filter(
-                id=value, dataset=Problem.Dataset.USER
-            ).exists():
-                raise serializers.ValidationError(
-                    f"Problem with ID {value} does not exist or is not a user problem."
-                )
-        return value
diff --git a/backend/problem/views/problem.py b/backend/problem/views/problem.py
@@ -4,11 +4,13 @@
 from rest_framework.viewsets import ModelViewSet
 from rest_framework.status import HTTP_201_CREATED, HTTP_200_OK
 
+from django.shortcuts import get_object_or_404
+
 from problem.problem_details import (
     get_filters,
     get_related_problem_ids,
 )
-from problem.models import KnowledgeBase, Problem, Sentence
+from problem.models import Problem
 from problem.serializers import ProblemInputSerializer, ProblemSerializer
 
 
@@ -95,101 +97,30 @@ def partial_update(self, request: Request, pk: int) -> Response:
         """
         Updates an existing user-created Problem with the provided input data.
         """
-        return self._handle_update_create_problem(request, pk)
+        return self._handle_update_create_problem(request, problem_id=pk)
 
     def _handle_update_create_problem(
         self, request: Request, problem_id: int | None
     ) -> Response:
         input_data = request.data
 
-        serializer = ProblemInputSerializer(data=input_data)
-        serializer.is_valid(raise_exception=True)
-        validated_input: dict = serializer.validated_data  # type: ignore
-        validated_input["id"] = problem_id
+        input_serializer = ProblemInputSerializer(data=input_data)
+        input_serializer.is_valid(raise_exception=True)
+        validated_input: dict = input_serializer.validated_data  # type: ignore
+
+        problem_serializer = ProblemSerializer()
 
         if problem_id is None:
-            problem = create_problem_from_input(validated_input)
+            problem = problem_serializer.create(validated_input)
             status = HTTP_201_CREATED
         else:
-            problem = update_problem_from_input(validated_input)
+            problem_instance = get_object_or_404(
+                Problem, id=problem_id, dataset=Problem.Dataset.USER
+            )
+            problem: Problem = problem_serializer.update(
+                problem_instance, validated_input
+            )
             status = HTTP_200_OK
 
         return Response({"id": problem.pk}, status=status)
 
-
-def create_problem_from_input(parse_input: dict) -> Problem:
-    """
-    Save a new Problem instance from the given parse input data.
-    """
-
-    premise_sentences = [
-        Sentence.objects.get_or_create(text=premise)[0]
-        for premise in parse_input["premises"]
-    ]
-
-    hypothesis_sentence = Sentence.objects.get_or_create(
-        text=parse_input["hypothesis"]
-    )[0]
-
-    problem = Problem.objects.create(
-        hypothesis=hypothesis_sentence,
-        dataset=Problem.Dataset.USER,
-        # TODO: Determine entailment label based on LangPro parser output.
-        entailment_label=Problem.EntailmentLabel.UNKNOWN,
-        extra_data={},
-    )
-
-    problem.premises.set(premise_sentences)
-
-    update_or_create_kb_items(problem=problem, kb_items=parse_input["kbItems"])
-
-    return problem
-
-
-def update_or_create_kb_items(problem: Problem, kb_items: list[dict]) -> None:
-    kb_ids: list[str] = []
-    for item in kb_items:
-        id = getattr(item, "id", None)
-        entity1 = item["entity1"]
-        relationship = item["relationship"]
-        entity2 = item["entity2"]
-
-        if id is None:
-            kb = KnowledgeBase.objects.create(
-                entity1=entity1,
-                relationship=relationship,
-                entity2=entity2,
-                problem=problem,
-            )
-            kb_ids.append(kb.pk)
-        else:
-            kb = KnowledgeBase.objects.get(id=id, problem_id=problem.pk)
-            kb.entity1 = entity1
-            kb.relationship = relationship
-            kb.entity2 = entity2
-            kb.save()
-            kb_ids.append(kb.pk)
-
-    # Delete existing knowledge bases associated to this problem that are
-    # not included in the input.
-    KnowledgeBase.objects.filter(problem_id=problem.pk).exclude(id__in=kb_ids).delete()
-
-
-def update_problem_from_input(parse_input: dict) -> Problem:
-    problem = Problem.objects.get(id=parse_input["id"], dataset=Problem.Dataset.USER)
-
-    problem.hypothesis = Sentence.objects.get_or_create(
-        text=parse_input["hypothesis"],
-    )[0]
-    problem.save()
-
-    premises: list[Sentence] = []
-    for input_premise in parse_input["premises"]:
-        premise = Sentence.objects.get_or_create(text=input_premise)[0]
-        premises.append(premise)
-
-    problem.premises.set(premises)
-
-    update_or_create_kb_items(problem, parse_input["kbItems"])
-
-    return problem
diff --git a/frontend/src/app/services/problem.service.ts b/frontend/src/app/services/problem.service.ts
@@ -63,8 +63,9 @@ export class ProblemService {
 
     public saveProblem$ = this.submit$.pipe(
         exhaustMap((problem) => {
-            const url = `/api/problem/${problem.id ?? ""}`;
-            return this.http.post<SaveProblemResponse>(url, problem).pipe(
+            const action = problem.id ? this.http.patch<SaveProblemResponse>(`/api/problem/${problem.id}/`, problem) :
+                this.http.post<SaveProblemResponse>(`/api/problem/`, problem);
+            return action.pipe(
                 catchError((error) => {
                     console.error('Error saving problem:', error);
                     return of({ id: null, error: 'Failed to save problem' });
