diff --git a/backend/annotation/admin.py b/backend/annotation/admin.py index 8c38f3f..cd872d8 100644 --- a/backend/annotation/admin.py +++ b/backend/annotation/admin.py @@ -1,3 +1,10 @@ from django.contrib import admin +from annotation.models import Label + + # Register your models here. +@admin.register(Label) +class LabelAdmin(admin.ModelAdmin): + list_display = ("text", "description") + search_fields = ("text",) diff --git a/backend/annotation/fixtures/initial.json b/backend/annotation/fixtures/initial.json new file mode 100644 index 0000000..30f866e --- /dev/null +++ b/backend/annotation/fixtures/initial.json @@ -0,0 +1,50 @@ +[ + { + "model": "annotation.label", + "pk": 1, + "fields": { + "text": "Important", + "description": "This problem is particularly important." + } + }, + { + "model": "annotation.label", + "pk": 2, + "fields": { + "text": "Review needed", + "description": "This problem needs to be reviewed by a master annotator." + } + }, + { + "model": "annotation.label", + "pk": 3, + "fields": { + "text": "Ambiguous", + "description": "The problem statement is ambiguous and needs clarification." + } + }, + { + "model": "annotation.label", + "pk": 4, + "fields": { + "text": "Incomplete knowledge", + "description": "Some knowledge base items are missing." + } + }, + { + "model": "annotation.label", + "pk": 5, + "fields": { + "text": "Spelling", + "description": "The premises or hypothesis are misspelled, which may lead to wrong analyses." + } + }, + { + "model": "annotation.label", + "pk": 6, + "fields": { + "text": "Wrong entailment label", + "description": "The entailment label is incorrect." + } + } +] diff --git a/backend/annotation/migrations/0002_label_labeling.py b/backend/annotation/migrations/0002_label_labeling.py new file mode 100644 index 0000000..78dcb38 --- /dev/null +++ b/backend/annotation/migrations/0002_label_labeling.py @@ -0,0 +1,123 @@ +# Generated by Django 4.2.27 on 2025-12-11 15:10 + +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), + ("problem", "0007_alter_problem_options"), + ("annotation", "0001_initial"), + ] + + operations = [ + migrations.CreateModel( + name="Label", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("text", models.CharField(max_length=255)), + ( + "description", + models.TextField( + help_text="A detailed description of the label, indicating when it should be used." + ), + ), + ], + options={ + "ordering": ["text"], + }, + ), + migrations.CreateModel( + name="Labeling", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("attached_at", models.DateTimeField(auto_now_add=True)), + ( + "removed_at", + models.DateTimeField( + blank=True, + help_text="When this label was removed from the problem.", + null=True, + ), + ), + ( + "notes", + models.TextField( + blank=True, + help_text="Optional notes explaining why this label was added or removed.", + ), + ), + ( + "attached_by", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="labelings_attached", + to=settings.AUTH_USER_MODEL, + ), + ), + ( + "label", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="labelings", + to="annotation.label", + ), + ), + ( + "problem", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="labelings", + to="problem.problem", + ), + ), + ( + "removed_by", + models.ForeignKey( + blank=True, + help_text="User who removed this label.", + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="labelings_removed", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + "ordering": ["-attached_at"], + "permissions": [ + ("delete_own_labeling", "Can remove own labeling from problems"), + ("delete_any_labeling", "Can remove any labeling from problems"), + ], + "indexes": [ + models.Index( + fields=["problem", "removed_at"], + name="annotation__problem_e7ac7b_idx", + ), + models.Index( + fields=["label", "removed_at"], + name="annotation__label_i_a229d2_idx", + ), + ], + }, + ), + ] diff --git a/backend/annotation/models.py b/backend/annotation/models.py index e0786a2..0286386 100644 --- a/backend/annotation/models.py +++ b/backend/annotation/models.py @@ -81,3 +81,83 @@ class KnowledgeBaseAnnotation(models.Model): ) created_at = models.DateTimeField(auto_now_add=True) + + +class Label(models.Model): + text = models.CharField(max_length=255) + description = models.TextField( + help_text="A detailed description of the label, indicating when it should be used." + ) + + class Meta: + ordering = ["text"] + + def __str__(self): + return self.text + + +class Labeling(models.Model): + """ + The attachment of a label to a problem. + + Each time a label is attached to a problem, a new Labeling record is created. + When removed, the record is marked as removed (not deleted), so the history of labelings is preserved. + """ + + problem = models.ForeignKey( + Problem, + on_delete=models.CASCADE, + related_name="labelings", + ) + + label = models.ForeignKey( + Label, + on_delete=models.CASCADE, + related_name="labelings", + ) + + attached_at = models.DateTimeField(auto_now_add=True) + attached_by = models.ForeignKey( + settings.AUTH_USER_MODEL, + on_delete=models.CASCADE, + related_name="labelings_attached", + ) + + removed_at = models.DateTimeField( + null=True, + blank=True, + help_text="When this label was removed from the problem.", + ) + removed_by = models.ForeignKey( + settings.AUTH_USER_MODEL, + on_delete=models.CASCADE, + related_name="labelings_removed", + null=True, + blank=True, + help_text="User who removed this label.", + ) + + notes = models.TextField( + blank=True, + help_text="Optional notes explaining why this label was added or removed.", + ) + + class Meta: + ordering = ["-attached_at"] + indexes = [ + models.Index(fields=["problem", "removed_at"]), + models.Index(fields=["label", "removed_at"]), + ] + permissions = [ + ("delete_own_labeling", "Can remove own labeling from problems"), + ("delete_any_labeling", "Can remove any labeling from problems"), + ] + + + def is_active(self) -> bool: + """Check if this labeling is currently active (not removed).""" + return self.removed_at is None + + def __str__(self): + status = "active" if self.is_active() else f"removed at {self.removed_at}" + return f"Label '{self.label.text}' on Problem {self.problem.pk} ({status})" diff --git a/backend/annotation/serializers.py b/backend/annotation/serializers.py new file mode 100644 index 0000000..02f8f93 --- /dev/null +++ b/backend/annotation/serializers.py @@ -0,0 +1,121 @@ +from rest_framework import serializers + +from django.contrib.auth.models import AnonymousUser + +from annotation.models import Label, Labeling +from problem.models import Problem +from user.models import User + + +class LabelSerializer(serializers.ModelSerializer): + """ + Serializer for Label model. + """ + + class Meta: + model = Label + fields = ["id", "text", "description"] + + +class ActiveLabelSerializer(serializers.Serializer): + """ + Serializer for active labels attached to a problem. + Includes attachedInfo and removable status based on current user. + """ + + id = serializers.IntegerField(source="label.id") + text = serializers.CharField(source="label.text") + description = serializers.CharField(source="label.description") + attachedInfo = serializers.SerializerMethodField() + removable = serializers.SerializerMethodField() + + def get_attachedInfo(self, labeling: Labeling) -> dict: + """Get attachment information for the label.""" + request = self.context.get("request") + user: User | AnonymousUser | None = request.user if request else None + + if user and user.is_anonymous is False: + attached_by_current_user = labeling.attached_by.pk == user.pk + else: + attached_by_current_user = False + + return { + "userName": labeling.attached_by.username, + "date": labeling.attached_at.isoformat(), + "attachedByCurrentUser": attached_by_current_user, + } + + def get_removable(self, labeling: Labeling) -> bool: + """Determine if the label is removable by the current user.""" + request = self.context.get("request") + user: User | AnonymousUser | None = request.user if request else None + + if user is None or user.is_anonymous: + return False + + if user.is_superuser or user.has_perm("annotation.delete_any_labeling"): + return True + + if user.has_perm("annotation.delete_own_labeling"): + return labeling.attached_by.pk == user.pk + + return False + + +class LabelingSerializer(serializers.ModelSerializer): + """ + Serializer for Labeling model, including the full label details. + """ + + label = LabelSerializer(read_only=True) + attachedAt = serializers.DateTimeField(source="attached_at") + attachedBy = serializers.PrimaryKeyRelatedField( + source="attached_by", read_only=True + ) + removedAt = serializers.DateTimeField(source="removed_at", allow_null=True) + removedBy = serializers.PrimaryKeyRelatedField( + source="removed_by", allow_null=True, read_only=True + ) + + class Meta: + model = Labeling + fields = [ + "id", + "label", + "attached_at", + "attached_by", + "removed_at", + "removed_by", + "notes", + ] + + +class SelectedLabelSerializer(serializers.Serializer): + """Serializer for a selected label in the save labels input.""" + + id = serializers.IntegerField() + + def validate_id(self, value): + """Validate that the label exists.""" + if not Label.objects.filter(id=value).exists(): + raise serializers.ValidationError(f"Label with ID {value} does not exist.") + return value + + +class SaveLabelsInputSerializer(serializers.Serializer): + """ + Serializer for validating save labels input data. + Used when saving/updating labels for a problem. + """ + + problemId = serializers.IntegerField() + selectedLabels = SelectedLabelSerializer(many=True, allow_empty=True) + remarks = serializers.CharField(required=False, allow_blank=True, default="") + + def validate_problemId(self, value): + """Validate that the problem exists.""" + if not Problem.objects.filter(id=value).exists(): + raise serializers.ValidationError( + f"Problem with ID {value} does not exist." + ) + return value diff --git a/backend/annotation/views.py b/backend/annotation/views.py index 91ea44a..aefa18e 100644 --- a/backend/annotation/views.py +++ b/backend/annotation/views.py @@ -1,3 +1,138 @@ -from django.shortcuts import render - -# Create your views here. +from django.db import transaction +from django.utils import timezone +from rest_framework.request import Request +from rest_framework.response import Response +from rest_framework.viewsets import ModelViewSet +from rest_framework.exceptions import PermissionDenied +from rest_framework.permissions import ( + IsAuthenticated, + AllowAny, + SAFE_METHODS, +) + +from annotation.models import Label, Labeling +from annotation.serializers import ( + LabelSerializer, + SaveLabelsInputSerializer, +) +from problem.models import Problem +from user.models import User +from langpro_annotator.logger import logger + + +class SaveLabelingsPermission(IsAuthenticated): + """Permission class for saving labelings.""" + + def has_permission(self, request, view): + if not super().has_permission(request, view): + return False + + if request.user.is_superuser: + return True + + return request.user.has_perm("annotation.add_labeling") + + +class LabelView(ModelViewSet): + """ + ViewSet for Label model. + + GET: All users can list and retrieve labels. + POST: Only selected users can save labelings (attach/remove labels from problems). + """ + + queryset = Label.objects.all().order_by("text") + serializer_class = LabelSerializer + http_method_names = ["get", "post", "head", "options"] + + def get_permissions(self): + if self.request.method in SAFE_METHODS: + return [AllowAny()] + return [SaveLabelingsPermission()] + + def create(self, request: Request) -> Response: + """ + Save labelings for a problem (attach/remove labels). + + Expects a payload with: + - problemId: ID of the problem + - selectedLabels: List of labels to be attached (with at least 'id' field) + - remarks: Optional notes + """ + serializer = SaveLabelsInputSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + validated_data: dict = serializer.validated_data # type: ignore + + problem_id = validated_data["problemId"] + selected_labels = validated_data["selectedLabels"] + remarks = validated_data.get("remarks", "") + + problem = Problem.objects.get(id=problem_id) + user: User = request.user # type: ignore + + selected_label_ids = {label["id"] for label in selected_labels} + + self._update_labelings(problem, user, selected_label_ids, remarks) + + return Response({"ok": True}) + + def _update_labelings( + self, + problem: Problem, + user: User, + selected_label_ids: set[int], + remarks: str, + ) -> None: + """Update labelings for a problem based on selected labels.""" + + with transaction.atomic(): + active_labelings = Labeling.objects.filter( + problem=problem, removed_at__isnull=True + ).select_related("label", "attached_by") + + current_label_ids = {labeling.label.pk for labeling in active_labelings} + labels_to_remove = current_label_ids - selected_label_ids + labels_to_add = selected_label_ids - current_label_ids + + for labeling in active_labelings: + if labeling.label.pk in labels_to_remove: + self._remove_labeling( + labeling=labeling, + user=user, + remarks=remarks, + ) + + for label_id in labels_to_add: + self._create_labeling( + label_id=label_id, + problem=problem, + user=user, + remarks=remarks, + ) + + def _create_labeling( + self, label_id: int, problem: Problem, user: User, remarks: str + ) -> None: + """Create a new labeling.""" + + Labeling.objects.create( + problem=problem, + label_id=label_id, + attached_by=user, + notes=remarks, + ) + + def _remove_labeling(self, labeling: Labeling, user: User, remarks: str) -> None: + """Mark a labeling as removed.""" + + if not user.can_remove_labeling(labeling): + logger.warning( + f"User {user.username} attempted to remove label {labeling.label.pk} " + f"attached by {labeling.attached_by.username}" + ) + raise PermissionDenied("You can only remove labels you attached yourself.") + labeling.removed_at = timezone.now() + labeling.removed_by = user + if remarks: + labeling.notes = remarks + labeling.save() diff --git a/backend/annotation/views_test.py b/backend/annotation/views_test.py new file mode 100644 index 0000000..4daeea8 --- /dev/null +++ b/backend/annotation/views_test.py @@ -0,0 +1,290 @@ +import pytest +from rest_framework import status + +from annotation.models import Label, Labeling + + +@pytest.fixture +def sample_label(db): + """Creates a sample label for testing.""" + return Label.objects.create( + text="Test Label", + description="A test label for testing purposes.", + ) + + +@pytest.fixture +def another_label(db): + """Creates another sample label for testing.""" + return Label.objects.create( + text="Another Label", + description="Another test label.", + ) + + +@pytest.fixture +def labeling_by_annotator(db, sample_problem, sample_label, annotator): + """Creates a labeling attached by an annotator.""" + return Labeling.objects.create( + problem=sample_problem, + label=sample_label, + attached_by=annotator, + ) + + +@pytest.fixture +def labeling_by_master(db, sample_problem, another_label, master_annotator): + """Creates a labeling attached by a master annotator.""" + return Labeling.objects.create( + problem=sample_problem, + label=another_label, + attached_by=master_annotator, + ) + + +class TestLabelViewGetPermissions: + """Tests for GET permissions on the Label endpoint.""" + + def test_unauthenticated_user_can_list_labels(self, api_client, sample_label): + """Unauthenticated users should be able to list labels.""" + response = api_client.get("/api/label/") + assert response.status_code == status.HTTP_200_OK + + def test_visitor_can_list_labels(self, api_client, visitor, sample_label): + """Visitors should be able to list labels.""" + api_client.force_authenticate(user=visitor) + response = api_client.get("/api/label/") + assert response.status_code == status.HTTP_200_OK + + def test_annotator_can_list_labels(self, api_client, annotator, sample_label): + """Annotators should be able to list labels.""" + api_client.force_authenticate(user=annotator) + response = api_client.get("/api/label/") + assert response.status_code == status.HTTP_200_OK + + def test_master_annotator_can_list_labels( + self, api_client, master_annotator, sample_label + ): + """Master annotators should be able to list labels.""" + api_client.force_authenticate(user=master_annotator) + response = api_client.get("/api/label/") + assert response.status_code == status.HTTP_200_OK + + def test_unauthenticated_user_can_retrieve_label(self, api_client, sample_label): + """Unauthenticated users should be able to retrieve a single label.""" + response = api_client.get(f"/api/label/{sample_label.id}/") + assert response.status_code == status.HTTP_200_OK + + def test_visitor_can_retrieve_label(self, api_client, visitor, sample_label): + """Visitors should be able to retrieve a single label.""" + api_client.force_authenticate(user=visitor) + response = api_client.get(f"/api/label/{sample_label.id}/") + assert response.status_code == status.HTTP_200_OK + + +class TestSaveLabelingsPermissions: + """Tests for POST permissions (saving labelings) on the Label endpoint.""" + + def test_unauthenticated_user_cannot_save_labelings( + self, api_client, sample_problem, sample_label + ): + """Unauthenticated users should not be able to save labelings.""" + data = { + "problemId": sample_problem.id, + "selectedLabels": [{"id": sample_label.id}], + "remarks": "", + } + response = api_client.post("/api/label/", data, format="json") + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + def test_visitor_cannot_save_labelings( + self, api_client, visitor, sample_problem, sample_label + ): + """Visitors should not be able to save labelings.""" + api_client.force_authenticate(user=visitor) + data = { + "problemId": sample_problem.id, + "selectedLabels": [{"id": sample_label.id}], + "remarks": "", + } + response = api_client.post("/api/label/", data, format="json") + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_annotator_can_save_labelings( + self, api_client, annotator, sample_problem, sample_label + ): + """Annotators should be able to save labelings.""" + api_client.force_authenticate(user=annotator) + data = { + "problemId": sample_problem.id, + "selectedLabels": [{"id": sample_label.id}], + "remarks": "Test remark", + } + response = api_client.post("/api/label/", data, format="json") + assert response.status_code == status.HTTP_200_OK + assert response.data["ok"] is True + + # Verify labeling was created + labeling = Labeling.objects.get(problem=sample_problem, label=sample_label) + assert labeling.attached_by == annotator + assert labeling.notes == "Test remark" + + def test_master_annotator_can_save_labelings( + self, api_client, master_annotator, sample_problem, sample_label + ): + """Master annotators should be able to save labelings.""" + api_client.force_authenticate(user=master_annotator) + data = { + "problemId": sample_problem.id, + "selectedLabels": [{"id": sample_label.id}], + "remarks": "", + } + response = api_client.post("/api/label/", data, format="json") + assert response.status_code == status.HTTP_200_OK + assert response.data["ok"] is True + + +class TestRemoveLabelingPermissions: + """Tests for labeling removal permissions.""" + + def test_annotator_can_remove_own_labeling( + self, api_client, annotator, sample_problem, labeling_by_annotator + ): + """Annotators should be able to remove labels they attached.""" + api_client.force_authenticate(user=annotator) + data = { + "problemId": sample_problem.id, + "selectedLabels": [], # Empty = remove the existing label + "remarks": "Removing my own label", + } + response = api_client.post("/api/label/", data, format="json") + assert response.status_code == status.HTTP_200_OK + + # Verify labeling was marked as removed + labeling_by_annotator.refresh_from_db() + assert labeling_by_annotator.removed_at is not None + assert labeling_by_annotator.removed_by == annotator + + def test_annotator_cannot_remove_others_labeling( + self, api_client, annotator, sample_problem, labeling_by_master + ): + """Annotators should not be able to remove labels attached by others.""" + api_client.force_authenticate(user=annotator) + data = { + "problemId": sample_problem.id, + "selectedLabels": [], # Empty = try to remove the existing label + "remarks": "", + } + response = api_client.post("/api/label/", data, format="json") + assert response.status_code == status.HTTP_403_FORBIDDEN + assert ( + "You can only remove labels you attached yourself" in response.data["detail"] + ) + + # Verify labeling was NOT removed + labeling_by_master.refresh_from_db() + assert labeling_by_master.removed_at is None + + def test_master_annotator_can_remove_own_labeling( + self, api_client, master_annotator, sample_problem, labeling_by_master + ): + """Master annotators should be able to remove labels they attached.""" + api_client.force_authenticate(user=master_annotator) + data = { + "problemId": sample_problem.id, + "selectedLabels": [], + "remarks": "", + } + response = api_client.post("/api/label/", data, format="json") + assert response.status_code == status.HTTP_200_OK + + labeling_by_master.refresh_from_db() + assert labeling_by_master.removed_at is not None + + def test_master_annotator_can_remove_others_labeling( + self, api_client, master_annotator, sample_problem, labeling_by_annotator + ): + """Master annotators should be able to remove labels attached by others.""" + api_client.force_authenticate(user=master_annotator) + data = { + "problemId": sample_problem.id, + "selectedLabels": [], + "remarks": "Removing annotator's label", + } + response = api_client.post("/api/label/", data, format="json") + assert response.status_code == status.HTTP_200_OK + + labeling_by_annotator.refresh_from_db() + assert labeling_by_annotator.removed_at is not None + assert labeling_by_annotator.removed_by == master_annotator + + +class TestLabelingAddAndRemove: + """Tests for adding / removing labelings.""" + + def test_adding_label_creates_labeling( + self, api_client, annotator, sample_problem, sample_label + ): + """Adding a label should create a new Labeling record.""" + api_client.force_authenticate(user=annotator) + data = { + "problemId": sample_problem.id, + "selectedLabels": [{"id": sample_label.id}], + "remarks": "", + } + response = api_client.post("/api/label/", data, format="json") + assert response.status_code == status.HTTP_200_OK + + assert Labeling.objects.filter( + problem=sample_problem, label=sample_label, removed_at__isnull=True + ).exists() + + def test_adding_multiple_labels( + self, api_client, annotator, sample_problem, sample_label, another_label + ): + """Should be able to add multiple labels at once.""" + api_client.force_authenticate(user=annotator) + data = { + "problemId": sample_problem.id, + "selectedLabels": [{"id": sample_label.id}, {"id": another_label.id}], + "remarks": "", + } + response = api_client.post("/api/label/", data, format="json") + assert response.status_code == status.HTTP_200_OK + + active_labelings = Labeling.objects.filter( + problem=sample_problem, removed_at__isnull=True + ) + assert active_labelings.count() == 2 + + def test_keeping_existing_labels_unchanged( + self, + api_client, + annotator, + sample_problem, + labeling_by_annotator, + another_label, + ): + """Labels already attached should remain if still in selectedLabels.""" + original_labeling_id = labeling_by_annotator.id + api_client.force_authenticate(user=annotator) + data = { + "problemId": sample_problem.id, + "selectedLabels": [ + {"id": labeling_by_annotator.label.id}, + {"id": another_label.id}, + ], + "remarks": "", + } + response = api_client.post("/api/label/", data, format="json") + assert response.status_code == status.HTTP_200_OK + + # Original labeling should still be active + labeling_by_annotator.refresh_from_db() + assert labeling_by_annotator.id == original_labeling_id + assert labeling_by_annotator.removed_at is None + + # New labeling should be created + assert Labeling.objects.filter( + problem=sample_problem, label=another_label, removed_at__isnull=True + ).exists() diff --git a/backend/conftest.py b/backend/conftest.py index 581ce99..94f7c5b 100644 --- a/backend/conftest.py +++ b/backend/conftest.py @@ -2,8 +2,12 @@ import pytest from typing import Generator from django.test import Client as APIClient +from django.contrib.auth.models import Group, Permission +from rest_framework.test import APIClient as DRFAPIClient -from user.models import User +from user.models import User, GroupName +from user.permissions import ANNOTATOR_PERMISSIONS, MASTER_ANNOTATOR_PERMISSIONS +from problem.models import Problem, Sentence @pytest.fixture() @@ -37,3 +41,80 @@ def user_client(client, user) -> Generator[APIClient, None, None]: client.force_login(user) yield client client.logout() + + +@pytest.fixture +def api_client(): + """Returns a DRF APIClient instance.""" + return DRFAPIClient() + + +@pytest.fixture +def visitor(db): + """Creates a visitor user (no special permissions).""" + return User.objects.create_user( + username="visitor", + email="visitor@test.com", + password="testpassword", + ) + + +@pytest.fixture +def annotator(db): + """Creates an annotator user with annotator permissions.""" + user = User.objects.create_user( + username="annotator", + email="annotator@test.com", + password="testpassword", + ) + group, _ = Group.objects.get_or_create(name=GroupName.ANNOTATORS) + + for app_label, codename in ANNOTATOR_PERMISSIONS: + try: + perm = Permission.objects.get( + content_type__app_label=app_label, + codename=codename, + ) + group.permissions.add(perm) + except Permission.DoesNotExist: + pass + user.groups.add(group) + return user + + +@pytest.fixture +def master_annotator(db): + """Creates a master annotator user with master annotator permissions.""" + user = User.objects.create_user( + username="master_annotator", + email="master@test.com", + password="testpassword", + ) + group, _ = Group.objects.get_or_create(name=GroupName.MASTER_ANNOTATORS) + + for app_label, codename in MASTER_ANNOTATOR_PERMISSIONS: + try: + perm = Permission.objects.get( + content_type__app_label=app_label, + codename=codename, + ) + group.permissions.add(perm) + except Permission.DoesNotExist: + pass + user.groups.add(group) + return user + + +@pytest.fixture +def sample_problem(db): + """Creates a sample problem for testing.""" + hypothesis = Sentence.objects.create(text="This is a hypothesis.") + premise = Sentence.objects.create(text="This is a premise.") + problem = Problem.objects.create( + dataset=Problem.Dataset.USER, + hypothesis=hypothesis, + entailment_label=Problem.EntailmentLabel.NEUTRAL, + extra_data={}, + ) + problem.premises.add(premise) + return problem diff --git a/backend/langpro_annotator/urls.py b/backend/langpro_annotator/urls.py index f1fd866..97e77ca 100644 --- a/backend/langpro_annotator/urls.py +++ b/backend/langpro_annotator/urls.py @@ -21,6 +21,7 @@ from rest_framework import routers +from annotation.views import LabelView from problem.views.problem import ProblemView from .index import index @@ -29,6 +30,7 @@ api_router = routers.DefaultRouter() # register viewsets with this router api_router.register(r"problem", ProblemView, basename="problem") +api_router.register(r"label", LabelView, basename="labels") if settings.PROXY_FRONTEND: diff --git a/backend/problem/models.py b/backend/problem/models.py index de54e51..22b9a7b 100644 --- a/backend/problem/models.py +++ b/backend/problem/models.py @@ -1,7 +1,6 @@ from django.db import models from django.db.models import QuerySet -from problem.services import FracasData, SNLIData, SickData from langpro_annotator.logger import logger @@ -99,11 +98,3 @@ class Relationship(models.TextChoices): on_delete=models.CASCADE, related_name="knowledge_bases", ) - - def serialize(self) -> dict: - return { - "id": self.pk, - "entity1": self.entity1, - "entity2": self.entity2, - "relationship": self.relationship, - } diff --git a/backend/problem/serializers.py b/backend/problem/serializers.py index 4d89f59..3f1ce4c 100644 --- a/backend/problem/serializers.py +++ b/backend/problem/serializers.py @@ -1,4 +1,9 @@ from rest_framework import serializers +from django.contrib.auth.models import AnonymousUser + +from annotation.serializers import ActiveLabelSerializer +from user.models import User +from annotation.models import Label, Labeling from problem.services import FracasData, SNLIData, SickData from problem.models import Problem, KnowledgeBase, Sentence @@ -51,6 +56,7 @@ class ProblemSerializer(serializers.ModelSerializer): entailmentLabel = serializers.CharField(source="entailment_label") extraData = serializers.SerializerMethodField() kbItems = serializers.SerializerMethodField() + labels = serializers.SerializerMethodField() class Meta: model = Problem @@ -63,6 +69,7 @@ class Meta: "extraData", "kbItems", "base", + "labels", ] def get_premises(self, problem): @@ -90,6 +97,13 @@ def get_kbItems(self, problem): kb_items = problem.knowledge_bases.all() return KnowledgeBaseSerializer(kb_items, many=True).data + def get_labels(self, problem): + """Get active labels with attachment info and removability.""" + active_labelings = problem.labelings.filter(removed_at__isnull=True) + return ActiveLabelSerializer( + active_labelings, many=True, context=self.context + ).data + def create(self, validated_data: dict) -> Problem: """ Create a new Problem instance from validated input data. @@ -145,7 +159,7 @@ def update(self, instance: Problem, validated_data: dict) -> Problem: raise serializers.ValidationError( f"Base problem with ID {validated_base_id} does not exist." ) - instance.base = base_problem # type: ignore + instance.base = base_problem # type: ignore instance.save() diff --git a/backend/problem/views/problem.py b/backend/problem/views/problem.py index 8ad685d..3b37b3d 100644 --- a/backend/problem/views/problem.py +++ b/backend/problem/views/problem.py @@ -26,7 +26,10 @@ def has_permission(self, request, view): class ProblemView(ModelViewSet): - queryset = Problem.objects.all() + queryset = Problem.objects.prefetch_related( + "labelings__label", + "labelings__attached_by", + ) serializer_class = ProblemSerializer def get_permissions(self): diff --git a/backend/problem/views/problem_test.py b/backend/problem/views/problem_test.py index 1f53727..09ab45c 100644 --- a/backend/problem/views/problem_test.py +++ b/backend/problem/views/problem_test.py @@ -1,82 +1,6 @@ import pytest -from django.contrib.auth.models import Group, Permission from rest_framework import status -from problem.models import Problem, Sentence -from user.models import User, GroupName -from user.permissions import ANNOTATOR_PERMISSIONS, MASTER_ANNOTATOR_PERMISSIONS - - -@pytest.fixture -def visitor(db): - """Creates a visitor user (no special permissions).""" - return User.objects.create_user( - username="visitor", - email="visitor@test.com", - password="testpassword", - ) - - -@pytest.fixture -def annotator(db): - """Creates an annotator user with annotator permissions.""" - user = User.objects.create_user( - username="annotator", - email="annotator@test.com", - password="testpassword", - ) - group, _ = Group.objects.get_or_create(name=GroupName.ANNOTATORS) - - for app_label, codename in ANNOTATOR_PERMISSIONS: - try: - perm = Permission.objects.get( - content_type__app_label=app_label, - codename=codename, - ) - group.permissions.add(perm) - except Permission.DoesNotExist: - pass - user.groups.add(group) - return user - - -@pytest.fixture -def master_annotator(db): - """Creates a master annotator user with master annotator permissions.""" - user = User.objects.create_user( - username="master_annotator", - email="master@test.com", - password="testpassword", - ) - group, _ = Group.objects.get_or_create(name=GroupName.MASTER_ANNOTATORS) - - for app_label, codename in MASTER_ANNOTATOR_PERMISSIONS: - try: - perm = Permission.objects.get( - content_type__app_label=app_label, - codename=codename, - ) - group.permissions.add(perm) - except Permission.DoesNotExist: - pass - user.groups.add(group) - return user - - -@pytest.fixture -def sample_problem(db): - """Creates a sample problem for testing.""" - hypothesis = Sentence.objects.create(text="This is a hypothesis.") - premise = Sentence.objects.create(text="This is a premise.") - problem = Problem.objects.create( - dataset=Problem.Dataset.USER, - hypothesis=hypothesis, - entailment_label=Problem.EntailmentLabel.NEUTRAL, - extra_data={}, - ) - problem.premises.add(premise) - return problem - @pytest.fixture def problem_input_data(): diff --git a/backend/user/models.py b/backend/user/models.py index 6add7f9..d9a4627 100644 --- a/backend/user/models.py +++ b/backend/user/models.py @@ -1,6 +1,8 @@ from enum import StrEnum import django.contrib.auth.models as django_auth_models +from annotation.models import Labeling + class GroupName(StrEnum): MASTER_ANNOTATORS = "Master Annotators" @@ -55,3 +57,15 @@ def can_create_problem(self) -> bool: Determines whether the user can create new problems. """ return self.has_perm("problem.add_problem") + + def can_remove_labeling(self, labeling: Labeling) -> bool: + """ + Determines whether the user can remove a specific labeling. + """ + if self.is_superuser or self.has_perm("annotation.delete_any_labeling"): + return True + + if self.has_perm("annotation.delete_own_labeling"): + return labeling.attached_by.pk == self.pk + + return False diff --git a/backend/user/permissions.py b/backend/user/permissions.py index 96f29c6..0b6e93c 100644 --- a/backend/user/permissions.py +++ b/backend/user/permissions.py @@ -5,6 +5,8 @@ ("problem", "change_knowledgebase"), ("problem", "delete_knowledgebase"), ("problem", "view_knowledgebase"), + ("annotation", "add_labeling"), + ("annotation", "delete_own_labeling"), ] MASTER_ANNOTATOR_PERMISSIONS = ANNOTATOR_PERMISSIONS + [ @@ -16,4 +18,8 @@ ("problem", "change_problem"), ("problem", "delete_problem"), ("problem", "view_problem"), + ("annotation", "add_label"), + ("annotation", "change_label"), + ("annotation", "delete_label"), + ("annotation", "delete_any_labeling"), ] diff --git a/frontend/src/app/annotate/annotation-input/annotation-input.component.spec.ts b/frontend/src/app/annotate/annotation-input/annotation-input.component.spec.ts index 38ffd2c..7ca60aa 100644 --- a/frontend/src/app/annotate/annotation-input/annotation-input.component.spec.ts +++ b/frontend/src/app/annotate/annotation-input/annotation-input.component.spec.ts @@ -67,6 +67,7 @@ describe("AnnotationInputComponent", () => { relationship: KnowledgeBaseRelationship.EQUAL } ], + labels: [], dataset: Dataset.USER, extraData: null }; @@ -110,6 +111,7 @@ describe("AnnotationInputComponent", () => { hypothesis: "Empty test hypothesis", entailmentLabel: EntailmentLabel.NEUTRAL, kbItems: [], + labels: [], dataset: Dataset.USER, extraData: null }; @@ -134,6 +136,7 @@ describe("AnnotationInputComponent", () => { hypothesis: "Test hypothesis", entailmentLabel: EntailmentLabel.CONTRADICTION, kbItems: [], + labels: [], dataset: Dataset.USER, extraData: null }; @@ -156,6 +159,7 @@ describe("AnnotationInputComponent", () => { hypothesis: "", entailmentLabel: EntailmentLabel.UNKNOWN, kbItems: [], + labels: [], dataset: Dataset.USER, extraData: null }; @@ -176,6 +180,7 @@ describe("AnnotationInputComponent", () => { hypothesis: "", entailmentLabel: EntailmentLabel.UNKNOWN, kbItems: [], + labels: [], dataset: Dataset.USER, extraData: null }; diff --git a/frontend/src/app/annotate/annotation-input/problem-details/entailment-label-badge/entailment-label-badge.component.test.ts b/frontend/src/app/annotate/annotation-input/problem-details/entailment-label-badge/entailment-label-badge.component.spec.ts similarity index 91% rename from frontend/src/app/annotate/annotation-input/problem-details/entailment-label-badge/entailment-label-badge.component.test.ts rename to frontend/src/app/annotate/annotation-input/problem-details/entailment-label-badge/entailment-label-badge.component.spec.ts index f2a3028..08fd1ca 100644 --- a/frontend/src/app/annotate/annotation-input/problem-details/entailment-label-badge/entailment-label-badge.component.test.ts +++ b/frontend/src/app/annotate/annotation-input/problem-details/entailment-label-badge/entailment-label-badge.component.spec.ts @@ -15,7 +15,7 @@ describe("EntailmentLabelBadgeComponent", () => { fixture = TestBed.createComponent(EntailmentLabelBadgeComponent); component = fixture.componentInstance; const componentRef = fixture.componentRef; - componentRef.setInput("judgement", EntailmentLabel.ENTAILMENT); + componentRef.setInput("entailmentLabel", EntailmentLabel.ENTAILMENT); fixture.detectChanges(); }); diff --git a/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.html b/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.html index 8c46b72..f0dd55d 100644 --- a/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.html +++ b/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.html @@ -4,15 +4,15 @@ - + - + - + - + } @if (details.comment) { - } + + + +
ID:ID: {{ details.problemId }}
Dataset:Dataset: {{ datasetLabels[details.dataset] }}
Entailment label:Entailment label: @if (sectionString()) {
Section:Section: {{ sectionString() }}
+ Comment:
Labels: + +
diff --git a/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.scss b/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.scss index 9f33078..dca1f97 100644 --- a/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.scss +++ b/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.scss @@ -1,5 +1,6 @@ -.table { - td:first-child { - width: 8em; - } -} +.table { + td:first-child { + width: 8em; + color: var(--bs-secondary-color); + } +} diff --git a/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.spec.ts b/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.spec.ts index 380d352..f00a200 100644 --- a/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.spec.ts +++ b/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.spec.ts @@ -17,6 +17,7 @@ const createMockProblem = ( premises: ["premise"], hypothesis: "hypothesis", kbItems: [], + labels: [], extraData, }); @@ -33,10 +34,11 @@ describe("ProblemDetailsComponent", () => { fixture = TestBed.createComponent(ProblemDetailsComponent); component = fixture.componentInstance; const componentRef = fixture.componentRef; - componentRef.setInput("problem", { - id: "1", - dataset: Dataset.SICK, - }); + componentRef.setInput("problem", createMockProblem( + 1, + Dataset.SICK, + EntailmentLabel.ENTAILMENT + )); fixture.detectChanges(); }); @@ -64,6 +66,7 @@ describe("ProblemDetailsComponent", () => { section: null, subsection: null, comment: null, + labels: [], }); }); @@ -97,6 +100,7 @@ describe("ProblemDetailsComponent", () => { section: "Quantifiers", subsection: "Some", comment: "A test note", + labels: [], }); }); diff --git a/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.ts b/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.ts index 961d2ce..9f1110a 100644 --- a/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.ts +++ b/frontend/src/app/annotate/annotation-input/problem-details/problem-details.component.ts @@ -1,10 +1,11 @@ +import { Dataset, EntailmentLabel, Problem, ProblemLabel } from "../../../types"; import { Component, computed, inject, input } from "@angular/core"; -import { Dataset, EntailmentLabel, Problem } from "../../../types"; import { EntailmentLabelBadgeComponent } from "./entailment-label-badge/entailment-label-badge.component"; import { faArrowUpRightFromSquare, faQuestionCircle } from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeModule } from "@fortawesome/angular-fontawesome"; import { NgbTooltipModule } from "@ng-bootstrap/ng-bootstrap"; import { datasetLabels } from "@/shared/displayTextMappings"; +import { ProblemLabelsComponent } from "./problem-labels/problem-labels.component"; import { CommonModule } from "@angular/common"; import { RouterModule } from "@angular/router"; import { ProblemService } from "@/services/problem.service"; @@ -17,6 +18,7 @@ export interface ProblemDetails { section: string | null; subsection: string | null; comment: string | null; + labels: ProblemLabel[]; } @Component({ @@ -24,6 +26,7 @@ export interface ProblemDetails { standalone: true, imports: [ EntailmentLabelBadgeComponent, + ProblemLabelsComponent, FontAwesomeModule, NgbTooltipModule, CommonModule, @@ -70,12 +73,13 @@ export class ProblemDetailsComponent { private extractDetails(problem: Problem): ProblemDetails | null { const shared: Pick< ProblemDetails, - "problemId" | "dataset" | "entailmentLabel" | "baseProblemId" + "problemId" | "dataset" | "entailmentLabel" | "labels" | "baseProblemId" > = { problemId: problem.id?.toString() ?? $localize`new`, baseProblemId: problem.base?.toString() ?? null, dataset: problem.dataset, entailmentLabel: problem.entailmentLabel, + labels: problem.labels }; switch (problem.dataset) { diff --git a/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/manage-labels-modal/manage-labels-modal.component.html b/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/manage-labels-modal/manage-labels-modal.component.html new file mode 100644 index 0000000..5858d28 --- /dev/null +++ b/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/manage-labels-modal/manage-labels-modal.component.html @@ -0,0 +1,125 @@ +
+
Manage Labels
+ +
+
+
Attached labels
+ + + + + + + + + + @for (label of selectedLabels$ | async; track label.id) { + + + + + + } @empty { + + + + } + +
LabelDescriptionAction
+
+ {{ label.text }} +
+ 		+ {{ label.description }} + @if (getAttachedByText(label)) { +
+ {{ getAttachedByText(label) }} +
+ } + 		+ @if (label.removable) { + Click to remove + } +
+ No labels are currently attached. +
+ +
Available labels
+ + + + + + + + + + @if (loadingLabels$ | async) { + + + + } @else { + @for (label of availableLabels$ | async; track label.id) { + + + + + + } @empty { + + + + } + } + +
LabelDescriptionAction
+ +
+
+ {{ label.text }} +
+ 		{{ label.description }} + Click to add +
+ All labels have been attached. +
+ +
+
+ + +
+
+
+
+ + +
diff --git a/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/manage-labels-modal/manage-labels-modal.component.scss b/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/manage-labels-modal/manage-labels-modal.component.scss new file mode 100644 index 0000000..c8905d5 --- /dev/null +++ b/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/manage-labels-modal/manage-labels-modal.component.scss @@ -0,0 +1,17 @@ +.modal-body { + h6 { + margin-top: 0.5rem; + margin-bottom: 0.75rem; + font-weight: 600; + } + + .clickable-table { + .clickable-row { + cursor: pointer; + + &:hover { + background-color: rgba(0, 0, 0, 0.05); + } + } + } +} diff --git a/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/manage-labels-modal/manage-labels-modal.component.spec.ts b/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/manage-labels-modal/manage-labels-modal.component.spec.ts new file mode 100644 index 0000000..f7e68e2 --- /dev/null +++ b/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/manage-labels-modal/manage-labels-modal.component.spec.ts @@ -0,0 +1,219 @@ +import { ComponentFixture, TestBed, fakeAsync, tick } from "@angular/core/testing"; +import { ManageLabelsModalComponent } from "./manage-labels-modal.component"; +import { NgbActiveModal } from "@ng-bootstrap/ng-bootstrap"; +import { ProblemLabel, Label } from "@/types"; +import { ProblemService } from "@/services/problem.service"; +import { AuthService } from "@/services/auth.service"; +import { of } from "rxjs"; + +describe("ManageLabelsModalComponent", () => { + let component: ManageLabelsModalComponent; + let fixture: ComponentFixture; + let mockActiveModal: jasmine.SpyObj; + let mockProblemService: jasmine.SpyObj; + let mockAuthService: jasmine.SpyObj; + + // Test labels + const testAvailableLabel: Label = { + id: 998, + text: "Test Available Label", + description: "A label that can be added" + }; + + const testAttachedLabel: ProblemLabel = { + id: 999, + text: "Test Attached Label", + description: "A label that is already attached", + attachedInfo: { + userName: "Test User", + date: "2023-01-01", + attachedByCurrentUser: false + }, + removable: true + }; + + beforeEach(async () => { + mockActiveModal = jasmine.createSpyObj("NgbActiveModal", [ + "close", + "dismiss", + ]); + + mockProblemService = jasmine.createSpyObj("ProblemService", [], { + allLabels$: of([testAvailableLabel]) + }); + + mockAuthService = jasmine.createSpyObj("AuthService", [], { + currentUser$: of({ username: "Current User" }) + }); + + await TestBed.configureTestingModule({ + imports: [ManageLabelsModalComponent], + providers: [ + { provide: NgbActiveModal, useValue: mockActiveModal }, + { provide: ProblemService, useValue: mockProblemService }, + { provide: AuthService, useValue: mockAuthService } + ], + }).compileComponents(); + + fixture = TestBed.createComponent(ManageLabelsModalComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + it("should create", () => { + expect(component).toBeTruthy(); + }); + + + it("should initialize with empty selected labels", () => { + expect(component.form.controls.selectedLabels.value).toEqual([]); + }); + + + describe("addLabel", () => { + it("should add a label to selected list", fakeAsync(() => { + const initialLength = component.form.controls.selectedLabels.value.length; + + component.addLabel(testAvailableLabel.id); + tick(); + + expect(component.form.controls.selectedLabels.value.length).toBe(initialLength + 1); + expect(component.form.controls.selectedLabels.value.some(l => l.id === testAvailableLabel.id)).toBe(true); + })); + + it("should set attachedInfo when adding a label", fakeAsync(() => { + component.addLabel(testAvailableLabel.id); + tick(); + + const addedLabel = component.form.controls.selectedLabels.value.find(l => l.id === testAvailableLabel.id); + + expect(addedLabel?.attachedInfo).toBeDefined(); + expect(addedLabel?.attachedInfo?.userName).toBe('Current User'); + expect(addedLabel?.attachedInfo?.attachedByCurrentUser).toBe(true); + })); + + it("should set removable to true when adding a label", fakeAsync(() => { + component.addLabel(testAvailableLabel.id); + tick(); + + const addedLabel = component.form.controls.selectedLabels.value.find(l => l.id === testAvailableLabel.id); + + expect(addedLabel?.removable).toBe(true); + })); + + it("should not add a label if id is not found", fakeAsync(() => { + const initialLength = component.form.controls.selectedLabels.value.length; + component.addLabel(99999); + tick(); + expect(component.form.controls.selectedLabels.value.length).toBe(initialLength); + })); + }); + + describe("removeLabel", () => { + it("should remove a label from selected list", fakeAsync(() => { + component.addLabel(testAvailableLabel.id); + tick(); + const lengthAfterAdd = component.form.controls.selectedLabels.value.length; + + component.removeLabel(testAvailableLabel.id); + tick(); + + expect(component.form.controls.selectedLabels.value.length).toBe(lengthAfterAdd - 1); + expect(component.form.controls.selectedLabels.value.some(l => l.id === testAvailableLabel.id)).toBe(false); + })); + + it("should not fail if trying to remove a non-existent label", fakeAsync(() => { + const initialLength = component.form.controls.selectedLabels.value.length; + component.removeLabel(99999); + tick(); + expect(component.form.controls.selectedLabels.value.length).toBe(initialLength); + })); + }); + + describe("getAttachedByText", () => { + it("should return empty string for a label without attachedInfo", () => { + const label: ProblemLabel = { + ...testAvailableLabel, + attachedInfo: null, + removable: false + }; + expect(component.getAttachedByText(label)).toBe(""); + }); + + it("should return formatted text for a label attached by current user", () => { + const label: ProblemLabel = { + ...testAvailableLabel, + attachedInfo: { + userName: "John Doe", + date: "2023-01-01", + attachedByCurrentUser: true, + }, + removable: true + }; + const result = component.getAttachedByText(label); + expect(result).toContain("you"); + expect(result).toContain("January"); + }); + + it("should return formatted text for a label attached by other user", () => { + const label: ProblemLabel = { + ...testAttachedLabel, + attachedInfo: { + userName: "John Doe", + date: "2023-01-01", + attachedByCurrentUser: false, + }, + }; + const result = component.getAttachedByText(label); + expect(result).not.toContain("you"); + expect(result).toContain("John Doe"); + expect(result).toContain("January"); + }); + }); + + describe("availableLabels observable", () => { + it("should filter out selected labels from available labels", (done) => { + component.availableLabels$.subscribe(available => { + const selectedIds = component.form.controls.selectedLabels.value.map((l: ProblemLabel) => l.id); + + available.forEach(label => { + expect(selectedIds.includes(label.id)).toBe(false); + }); + done(); + }); + }); + + it("should update when a label is added", fakeAsync(() => { + let availableCount = 0; + component.availableLabels$.subscribe(labels => { + availableCount = labels.length; + }); + tick(); + + const initialCount = availableCount; + + component.addLabel(testAvailableLabel.id); + tick(); + + expect(availableCount).toBe(initialCount - 1); + })); + + it("should update when a label is removed", fakeAsync(() => { + component.addLabel(testAvailableLabel.id); + tick(); + + let availableCount = 0; + component.availableLabels$.subscribe(labels => { + availableCount = labels.length; + }); + tick(); + + const countAfterAdd = availableCount; + + component.removeLabel(testAvailableLabel.id); + tick(); + + expect(availableCount).toBe(countAfterAdd + 1); + })); + }); +}); diff --git a/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/manage-labels-modal/manage-labels-modal.component.ts b/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/manage-labels-modal/manage-labels-modal.component.ts new file mode 100644 index 0000000..7c940f9 --- /dev/null +++ b/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/manage-labels-modal/manage-labels-modal.component.ts @@ -0,0 +1,136 @@ +import { Component, DestroyRef, inject, OnInit, output } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; +import { FormControl, FormGroup, ReactiveFormsModule } from '@angular/forms'; +import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; +import { Label, ProblemLabel } from '@/types'; +import { ProblemService } from '@/services/problem.service'; +import { toSignal } from '@angular/core/rxjs-interop'; +import { AuthService } from '@/services/auth.service'; +import { map, combineLatest, Subject, withLatestFrom, startWith, tap, share, shareReplay, defer } from 'rxjs'; +import { AsyncPipe } from '@angular/common'; +import { formatDate } from '@/util'; + +type SelectedLabelsForm = FormGroup<{ + problemId: FormControl; + selectedLabels: FormControl; + remarks: FormControl; +}>; + +export type ManageLabelsModalResult = ReturnType; + +@Component({ + selector: 'la-manage-labels-modal', + imports: [ReactiveFormsModule, AsyncPipe], + templateUrl: './manage-labels-modal.component.html', + styleUrl: './manage-labels-modal.component.scss' +}) +export class ManageLabelsModalComponent implements OnInit { + private problemService = inject(ProblemService); + private authService = inject(AuthService); + private destroyRef = inject(DestroyRef); + + public form: SelectedLabelsForm = new FormGroup({ + problemId: new FormControl(-1, { nonNullable: true }), + selectedLabels: new FormControl([], { + nonNullable: true, + }), + remarks: new FormControl('', { nonNullable: true }), + }); + + private addLabelSubject = new Subject(); + private removeLabelSubject = new Subject(); + + public allLabels$ = this.problemService.allLabels$; + + // Defer ensures that the form has the correct values before we subscribe to valueChanges. + public selectedLabels$ = defer(() => this.form.controls.selectedLabels.valueChanges.pipe( + startWith(this.form.controls.selectedLabels.value), + )); + + public availableLabels$ = combineLatest([ + this.allLabels$, + this.selectedLabels$ + ]).pipe( + map(([allLabels, selectedLabels]) => { + if (!allLabels) { + return []; + } + const selectedIds = selectedLabels.map(label => label.id); + return allLabels.filter(label => !selectedIds.includes(label.id)); + }) + ); + + public loadingLabels$ = this.availableLabels$.pipe( + map(() => false), + startWith(true) + ); + + constructor(public activeModal: NgbActiveModal) { } + + ngOnInit(): void { + // Handle add label operations + this.addLabelSubject.pipe( + withLatestFrom(this.allLabels$), + takeUntilDestroyed(this.destroyRef) + ).subscribe(([labelId, allLabels]) => { + const currentSelected = this.form.controls.selectedLabels.value; + const label = allLabels?.find(l => l.id === labelId); + + if (!label) { + return; + } + + const newLabel: ProblemLabel = { + id: label.id, + text: label.text, + description: label.description, + attachedInfo: { + userName: this.currentUserName() ?? $localize`Unknown user`, + date: new Date().toISOString(), + attachedByCurrentUser: true + }, + removable: true + }; + this.form.controls.selectedLabels.setValue([...currentSelected, newLabel]); + }); + + this.removeLabelSubject.pipe( + takeUntilDestroyed(this.destroyRef) + ).subscribe((labelId) => { + const currentSelected = this.form.controls.selectedLabels.value; + const label = currentSelected.find(l => l.id === labelId); + + if (!label) { + return; + } + + label.attachedInfo = null; + label.removable = true; + this.form.controls.selectedLabels.setValue(currentSelected.filter(l => l.id !== labelId)); + }); + } + + public removeLabel(labelId: number): void { + this.removeLabelSubject.next(labelId); + } + + public addLabel(labelId: number): void { + this.addLabelSubject.next(labelId); + } + + public closeModal(): void { + this.activeModal.close(this.form.getRawValue()); + } + + public getAttachedByText(label: ProblemLabel): string { + if (!label.attachedInfo) { + return ''; + } + const attachedUser = label.attachedInfo.attachedByCurrentUser ? $localize`you` : label.attachedInfo.userName; + return $localize`Attached by ${attachedUser} on ${formatDate(label.attachedInfo.date)}`; + } + + private currentUserName = toSignal( + this.authService.currentUser$.pipe(map((user) => user?.username)) + ); +} diff --git a/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/problem-labels.component.html b/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/problem-labels.component.html new file mode 100644 index 0000000..1ccfc1b --- /dev/null +++ b/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/problem-labels.component.html @@ -0,0 +1,24 @@ +
+ @for (label of sortedLabels(); track label.text) { +
+ + + {{ label.text }} +
+ } @empty { + No labels attached to this problem yet + } + +
diff --git a/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/problem-labels.component.scss b/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/problem-labels.component.scss new file mode 100644 index 0000000..a69f3a9 --- /dev/null +++ b/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/problem-labels.component.scss @@ -0,0 +1,8 @@ +.label-icon { + cursor: help; + font-size: 0.75rem; +} + +.manage-button { + font-size: 0.75rem; +} diff --git a/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/problem-labels.component.spec.ts b/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/problem-labels.component.spec.ts new file mode 100644 index 0000000..51056f3 --- /dev/null +++ b/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/problem-labels.component.spec.ts @@ -0,0 +1,27 @@ +import { ComponentFixture, TestBed } from '@angular/core/testing'; + +import { ProblemLabelsComponent } from './problem-labels.component'; +import { provideHttpClient } from '@angular/common/http'; + +describe('ProblemLabelsComponent', () => { + let component: ProblemLabelsComponent; + let fixture: ComponentFixture; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [ProblemLabelsComponent], + providers: [provideHttpClient()] + }) + .compileComponents(); + + fixture = TestBed.createComponent(ProblemLabelsComponent); + component = fixture.componentInstance; + const componentRef = fixture.componentRef; + componentRef.setInput("attachedLabels", []); + fixture.detectChanges(); + }); + + it('should create', () => { + expect(component).toBeTruthy(); + }); +}); diff --git a/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/problem-labels.component.ts b/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/problem-labels.component.ts new file mode 100644 index 0000000..cbb7fc8 --- /dev/null +++ b/frontend/src/app/annotate/annotation-input/problem-details/problem-labels/problem-labels.component.ts @@ -0,0 +1,91 @@ +import { Component, computed, DestroyRef, inject, input, OnInit } from '@angular/core'; +import { faSearch } from '@fortawesome/free-solid-svg-icons'; +import { FontAwesomeModule } from '@fortawesome/angular-fontawesome'; +import { NgbTooltipModule, NgbModal } from '@ng-bootstrap/ng-bootstrap'; +import { ManageLabelsModalComponent, ManageLabelsModalResult } from './manage-labels-modal/manage-labels-modal.component'; +import { ProblemLabel, SaveLabelsResponse } from '@/types'; +import { catchError, Subject, switchMap } from 'rxjs'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; +import { ToastService } from '@/services/toast.service'; +import { HttpClient } from '@angular/common/http'; +import { ProblemService } from '@/services/problem.service'; +import { getLabelTooltipText, sortLabels } from '@/util'; + +@Component({ + selector: 'la-problem-labels', + imports: [FontAwesomeModule, NgbTooltipModule], + templateUrl: './problem-labels.component.html', + styleUrl: './problem-labels.component.scss' +}) +export class ProblemLabelsComponent implements OnInit { + private destroyRef = inject(DestroyRef); + private toastService = inject(ToastService); + private http = inject(HttpClient); + private problemService = inject(ProblemService); + + public problemId = input.required(); + public attachedLabels = input.required(); + + public sortedLabels = computed(() => { + return sortLabels(this.attachedLabels()); + }); + + public faSearch = faSearch; + + private saveLabels$ = new Subject(); + + constructor(private modalService: NgbModal) { } + + ngOnInit(): void { + this.saveLabels$.pipe( + switchMap(value => this.http.post("/api/label/", value)), + takeUntilDestroyed(this.destroyRef), + catchError((error) => { + this.toastService.show( + { + header: $localize`Failed to save labels.`, + body: error, + type: 'danger' + }); + return []; + }) + ).subscribe((result) => { + if (result.ok) { + this.toastService.show( + { + header: $localize`Success`, + type: 'success', + body: $localize`Labels saved successfully.` + }); + this.problemService.refetchProblem$.next(); + return; + } + this.toastService.show({ + header: $localize`Failed to save labels.`, + body: result.error || '', + type: 'danger' + }); + }); + } + + public getTooltipText = getLabelTooltipText; + + public openManageLabelsModal(): void { + const modalRef = this.modalService.open(ManageLabelsModalComponent, { + centered: true, + size: 'lg' + }); + + // Initialize form with currently attached labels. + modalRef.componentInstance.form.patchValue({ + problemId: this.problemId(), + selectedLabels: this.attachedLabels() + }); + + modalRef.result?.then((result: ManageLabelsModalResult) => { + this.saveLabels$.next(result); + }).catch(() => { + // Modal was dismissed (cancelled), do nothing. + }); + } +} diff --git a/frontend/src/app/services/problem.service.spec.ts b/frontend/src/app/services/problem.service.spec.ts index 4dceabe..0a4ab12 100644 --- a/frontend/src/app/services/problem.service.spec.ts +++ b/frontend/src/app/services/problem.service.spec.ts @@ -55,6 +55,7 @@ describe("ProblemService", () => { hypothesis: "b", entailmentLabel: EntailmentLabel.ENTAILMENT, kbItems: [], + labels: [], extraData: { pairId: 1, relatednessScore: 4.5 diff --git a/frontend/src/app/services/problem.service.ts b/frontend/src/app/services/problem.service.ts index de1d0d7..0201c81 100644 --- a/frontend/src/app/services/problem.service.ts +++ b/frontend/src/app/services/problem.service.ts @@ -1,10 +1,11 @@ import { ParseInput } from "@/annotate/annotation-input/annotation-input.component"; import extractBaseParam from "@/shared/extractBaseParam"; -import { ProblemResponse, SaveProblemResponse, Dataset, EntailmentLabel, Problem } from "@/types"; +import { ProblemResponse, SaveProblemResponse, Dataset, EntailmentLabel, Problem, Label } from "@/types"; import { HttpClient, HttpParams } from "@angular/common/http"; import { Injectable, inject } from "@angular/core"; import { ParamMap } from "@angular/router"; -import { Subject, Observable, switchMap, of, shareReplay, exhaustMap, catchError, map, BehaviorSubject, filter } from "rxjs"; +import { Subject, Observable, switchMap, of, shareReplay, exhaustMap, catchError, map, BehaviorSubject, filter, merge } from "rxjs"; +import { ToastService } from "./toast.service"; interface AllParams { params: ParamMap; @@ -23,13 +24,19 @@ export enum AppMode { }) export class ProblemService { private http = inject(HttpClient); + private toastService = inject(ToastService); public allParams$ = new BehaviorSubject(null); + public refetchProblem$ = new Subject(); + // Submit a new problem to be saved to the database. public submit$ = new Subject(); - public problemResponse$: Observable = this.allParams$.pipe( + public problemResponse$: Observable = merge( + this.allParams$, + this.refetchProblem$.pipe(map(() => this.allParams$.value)) + ).pipe( filter(allParams => allParams !== null), switchMap(({ params, queryParams }) => { const problemId = params.get("problemId"); @@ -49,6 +56,18 @@ export class ProblemService { shareReplay(1) ); + public allLabels$ = this.http.get('/api/label/').pipe( + catchError(() => { + this.toastService.show({ + header: $localize`Error fetching labels`, + body: $localize`Could not load labels from server.`, + type: 'danger', + }); + return of([]); + }), + shareReplay(1) + ); + public appMode$ = this.allParams$.pipe( filter(allParams => allParams !== null), map(({ params, edit }) => { @@ -71,7 +90,11 @@ export class ProblemService { this.http.post(`/api/problem/`, problem); return action.pipe( catchError((error) => { - console.error('Error saving problem:', error); + this.toastService.show({ + header: $localize`Error saving problem`, + body: error, + type: 'danger', + }); return of({ id: null, error: 'Failed to save problem' }); }) ); @@ -112,7 +135,8 @@ export class ProblemService { kbItems: existingProblem?.kbItems.map(kbItem => ({ ...kbItem, id: null, - })) ?? [] + })) ?? [], + labels: existingProblem?.labels ?? [], } }; @@ -124,7 +148,11 @@ export class ProblemService { return this.http.get(`/api/problem/${problemId ?? "first"}/`, { params: httpParams }).pipe( catchError((error) => { const message = `Error fetching ${problemId ? `problem ${problemId}` : "first problem"}`; - console.error(message, error); + this.toastService.show({ + header: message, + body: error.message || 'The problem could not be fetched from the server.', + type: 'danger', + }); return of(null); }) ); @@ -150,6 +178,4 @@ export class ProblemService { return new HttpParams({ fromObject: paramRecord }); } - - } diff --git a/frontend/src/app/types.ts b/frontend/src/app/types.ts index 2f5ef07..6030e43 100644 --- a/frontend/src/app/types.ts +++ b/frontend/src/app/types.ts @@ -37,6 +37,26 @@ interface KnowledgeBaseItem { entity2: string; } +export interface Label { + id: number; + text: string; + description: string; +} + +interface AttachmentInfo { + userName: string; + date: string; + attachedByCurrentUser: boolean; +} + +export interface ProblemLabel { + id: number; + text: string; + description: string; + attachedInfo: AttachmentInfo | null; + removable: boolean; +} + interface ProblemBase { id: number | null; base: number | null; @@ -44,6 +64,7 @@ interface ProblemBase { hypothesis: string | null; entailmentLabel: EntailmentLabel; kbItems: KnowledgeBaseItem[]; + labels: ProblemLabel[]; } interface SickProblem extends ProblemBase { @@ -86,6 +107,10 @@ export interface SaveProblemResponse extends BaseResponse { id: number | null; } +export interface SaveLabelsResponse extends BaseResponse { + ok: boolean; +} + export enum Dataset { SICK = "sick", FRACAS = "fracas", diff --git a/frontend/src/app/util.ts b/frontend/src/app/util.ts index 7982cc8..d2cade7 100644 --- a/frontend/src/app/util.ts +++ b/frontend/src/app/util.ts @@ -1,6 +1,40 @@ +import { ProblemLabel } from "./types"; +/** + * Make sure that the user's own labels are always last. + */ +function sortLabels(labels: ProblemLabel[]): ProblemLabel[] { + const userLabels = labels.filter(label => label.attachedInfo?.attachedByCurrentUser); + const otherLabels = labels.filter(label => !label.attachedInfo?.attachedByCurrentUser); + return [...otherLabels, ...userLabels]; +} + +/** + * Get the tooltip text for a problem label. + */ +function getLabelTooltipText(label: ProblemLabel): string { + let tooltip = label.description; + if (label.attachedInfo) { + const dateStr = formatDate(label.attachedInfo.date); + const attachedUser = label.attachedInfo.attachedByCurrentUser ? $localize`you` : label.attachedInfo.userName; + tooltip += `\n\nAttached by ${attachedUser} on ${dateStr}`; + } + return tooltip; +} + +/** + * Calculate the sum of a list of numbers. + */ function sum(list: number[]) { return list.reduce((sum: number, h: number) => sum + h, 0); } -export {sum} +/** + * Format a date string into a human-readable format. + */ +function formatDate(date: string): string { + const dateStr = new Date(date); + return Intl.DateTimeFormat(undefined, { year: 'numeric', month: 'long', day: 'numeric' }).format(dateStr); +} + +export { sum, formatDate, sortLabels, getLabelTooltipText }; diff --git a/frontend/src/styles.scss b/frontend/src/styles.scss index 75021b5..6839c0b 100644 --- a/frontend/src/styles.scss +++ b/frontend/src/styles.scss @@ -15,3 +15,15 @@ .always-block { display: block !important; } + +.label-pill { + display: inline-flex; + align-items: center; + gap: 0.25rem; + padding: 0.25rem 0.6rem; + background-color: var(--bs-primary); + color: white; + border-radius: 1rem; + font-size: 0.75rem; + white-space: nowrap; +}