Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions backend/annotation/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ class AnnotationBaseSerializer(serializers.ModelSerializer):
"""

createdAt = serializers.DateTimeField(source="created_at", read_only=True)
createdBy = serializers.SerializerMethodField(method_name="get_createdBy", read_only=True)
createdBy = serializers.SerializerMethodField(
method_name="get_created_by", read_only=True
)
removedAt = serializers.DateTimeField(
source="removed_at", allow_null=True, read_only=True
)
removedBy = serializers.PrimaryKeyRelatedField(
source="removed_by", allow_null=True, read_only=True
removedBy = serializers.SerializerMethodField(
method_name="get_removed_by", allow_null=True, read_only=True
)
removable = serializers.SerializerMethodField(read_only=True)

Expand All @@ -45,10 +47,15 @@ def get_removable(self, annotation) -> bool:
"""This should be overridden in subclasses."""
raise NotImplementedError("Subclasses must implement get_removable method.")

def get_createdBy(self, annotation) -> str | None:
def get_created_by(self, annotation) -> str | None:
"""Returns the full name of the user who created the annotation."""
return annotation.created_by.get_full_name()

def get_removed_by(self, annotation) -> str | None:
"""Returns the full name of the user who removed the annotation, or None if not removed."""
if annotation.removed_by:
return annotation.removed_by.get_full_name()
return None


class KnowledgeBaseAnnotationSerializer(AnnotationBaseSerializer):
Expand Down Expand Up @@ -95,6 +102,7 @@ def validate_id(self, value):
f"KnowledgeBaseAnnotation item with ID {value} does not exist."
)


class LabelSerializer(serializers.ModelSerializer):
"""
Serializer for Label model.
Expand Down Expand Up @@ -156,6 +164,7 @@ def get_removable(self, annotation: LabelAnnotation) -> bool:

return False


class SelectedLabelSerializer(serializers.Serializer):
"""Serializer for a selected label in the save labels input."""

Expand Down
2 changes: 0 additions & 2 deletions backend/annotation/serializers_test.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import pytest
from typing import Any

from django.utils import timezone
from django.contrib.auth.models import Permission
from rest_framework.test import APIRequestFactory

Expand All @@ -11,7 +10,6 @@
LabelAnnotationSerializer,
SaveLabelsInputSerializer,
)
from problem.serializers import ProblemSerializer


@pytest.mark.django_db
Expand Down
8 changes: 2 additions & 6 deletions backend/problem/views/problem.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,8 @@ def _get_problem_response(self, request: Request, pk: int | None) -> Response:

serializer = self.get_serializer(problem)

kb_annotations = KnowledgeBaseAnnotation.objects.filter(
problem=problem, removed_at__isnull=True
)
label_annotations = LabelAnnotation.objects.filter(
problem=problem, removed_at__isnull=True
)
kb_annotations = KnowledgeBaseAnnotation.objects.filter(problem=problem)
label_annotations = LabelAnnotation.objects.filter(problem=problem)

# kbAnnotations and labelAnnotations are not included in the
# ProblemSerializer because they require additional context for
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,112 @@
<ul>
<li>Annotation history: who did what when?</li>
<li>Ability to 'gold'/'ungold' sentences</li>
@let events = filteredEvents();

@if (events === null) {
<p class="text-muted small fst-italic" i18n>Loading…</p>
} @else {
<div
class="d-flex align-items-center justify-content-between mb-2 gap-2 flex-wrap"
>
<div class="d-flex gap-2 flex-wrap">
<div
class="btn-group btn-group-sm"
role="group"
aria-label="Filter by event kind"
i18n-aria-label
>
<input
type="checkbox"
class="btn-check"
id="filter-added"
[checked]="showAdded()"
(change)="showAdded.set(!showAdded())"
/>
<label class="btn btn-outline-success" for="filter-added">
<fa-icon [icon]="faPlus" class="me-2" />
<ng-container i18n>Added</ng-container>
</label>
<input
type="checkbox"
class="btn-check"
id="filter-removed"
[checked]="showRemoved()"
(change)="showRemoved.set(!showRemoved())"
/>
<label class="btn btn-outline-danger" for="filter-removed">
<fa-icon [icon]="faMinus" class="me-2" />
<ng-container i18n>Removed</ng-container>
</label>
</div>
<div
class="btn-group btn-group-sm"
role="group"
aria-label="Filter by annotation type"
i18n-aria-label
>
<input
type="checkbox"
class="btn-check"
id="filter-labels"
[checked]="showLabels()"
(change)="showLabels.set(!showLabels())"
/>
<label class="btn btn-outline-secondary" for="filter-labels">
<fa-icon [icon]="faTag" class="me-2" />
<ng-container i18n>Labels</ng-container>
</label>
<input
type="checkbox"
class="btn-check"
id="filter-kb"
[checked]="showKB()"
(change)="showKB.set(!showKB())"
/>
<label class="btn btn-outline-secondary" for="filter-kb">
<fa-icon [icon]="faBook" class="me-2" />
<ng-container i18n>KB items</ng-container>
</label>
</div>
</div>
<span class="text-muted small" i18n>
Showing {{ events.length }}/{{ totalCount() }} annotations
</span>
</div>
@if (events.length === 0) {
<p class="text-muted small fst-italic" i18n>
No annotations match the current filters.
</p>
} @else {
<ul class="list-unstyled d-flex flex-column gap-2 mb-0">
@for (event of events; track $index) {
@let added = event.eventKind === "added";
@let isLabel = event.annotationKind === "label";
Comment on lines +79 to +81
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the @for and @let statements deserve to be at different indentation levels. Maybe keep the @for with the <ul> above? What do the conventions say?

<li>
<div class="card border-0 shadow-sm overflow-hidden d-flex flex-row">
<div
class="text-white d-flex align-items-center justify-content-center px-2 flex-shrink-0"
[class]="added ? 'bg-success' : 'bg-danger'"
>
<fa-icon [icon]="added ? faPlus : faMinus" />
</div>
<div
class="card-body py-2 px-3 d-flex align-items-center justify-content-start gap-3"
>
<fa-icon [icon]="isLabel ? faTag : faBook" />
<span class="flex-grow-1">
<strong>{{ event.actor }}</strong>
{{ event.eventKind }}
{{ isLabel ? "a label" : "a knowledge base item" }}:
<span
[class]="isLabel ? 'label-pill ms-1' : 'font-monospace'"
>
{{ event.description }}
</span>
</span>
<span class="text-muted small text-end flex-shrink-0">
{{ event.timestamp | date : "medium" }}
</span>
</div>
</div>
</li>
}
</ul>
} }
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

Original file line number Diff line number Diff line change
@@ -1,22 +1,139 @@
import { ComponentFixture, TestBed } from "@angular/core/testing";
import { of } from "rxjs";

import { ProblemService } from "@/services/problem.service";
import { Dataset, EntailmentLabel, KnowledgeBaseAnnotation, KnowledgeBaseRelationship, LabelAnnotation, Problem } from "@/types";
import { AnnotationCommentsComponent } from "./annotation-comments.component";

const baseAnnotationFields = {
id: 1,
session: 1,
notes: "",
removable: false,
removedAt: null,
removedBy: null,
};

describe("AnnotationCommentsComponent", () => {
let component: AnnotationCommentsComponent;
let fixture: ComponentFixture<AnnotationCommentsComponent>;

beforeEach(async () => {
await TestBed.configureTestingModule({
imports: [AnnotationCommentsComponent],
providers: [
{ provide: ProblemService, useValue: {} },
],
}).compileComponents();
});

// Needed to override the problem$ observable in the providers.
function recreateComponent(kbAnnotations: KnowledgeBaseAnnotation[] = [], labelAnnotations: LabelAnnotation[] = []) {
const mockProblem: Problem = {
id: 1,
base: null,
premises: [],
hypothesis: null,
entailmentLabel: EntailmentLabel.UNKNOWN,
dataset: Dataset.USER,
extraData: null,
kbAnnotations,
labelAnnotations,
};

TestBed.overrideProvider(ProblemService, { useValue: { problem$: of(mockProblem) } });
fixture = TestBed.createComponent(AnnotationCommentsComponent);
component = fixture.componentInstance;
fixture.detectChanges();
});
}



it("should create", () => {
recreateComponent();
expect(component).toBeTruthy();
});

it("emits an 'added' event for each annotation", () => {
recreateComponent(
[],
[
{
...baseAnnotationFields,
createdAt: "2024-01-01T10:00:00Z",
createdBy: "Alice",
label: { id: 1, text: "negation", description: "" },
attachedByCurrentUser: false,
},
]
);

const events = component.filteredEvents()!;
expect(events.length).toBe(1);
expect(events[0]).toEqual(
jasmine.objectContaining({
eventKind: "added",
annotationKind: "label",
actor: "Alice",
description: "negation",
})
);
});

it("emits a 'removed' event when removedAt is set", () => {
recreateComponent(
[
{
...baseAnnotationFields,
createdAt: "2024-01-01T10:00:00Z",
createdBy: "Bob",
removedAt: "2024-01-02T10:00:00Z",
removedBy: "3",
entity1: "cat",
relationship: KnowledgeBaseRelationship.EQUAL,
entity2: "feline",
},
],
[]
);

const events = component.filteredEvents()!;
expect(events.length).toBe(2);
const removed = events.find((e) => e.eventKind === "removed");
expect(removed).toEqual(
jasmine.objectContaining({
eventKind: "removed",
annotationKind: "knowledgeBase",
description: "cat = feline",
})
);
});

it("sorts events by timestamp descending", () => {
recreateComponent(
[],
[
{
...baseAnnotationFields,
id: 1,
createdAt: "2024-01-01T10:00:00Z",
createdBy: "Alice",
label: { id: 1, text: "first", description: "" },
attachedByCurrentUser: false,
},
{
...baseAnnotationFields,
id: 2,
createdAt: "2024-01-03T10:00:00Z",
createdBy: "Bob",
label: { id: 2, text: "latest", description: "" },
attachedByCurrentUser: false,
},
]
);

const events = component.filteredEvents()!;
expect(events[0].description).toBe("latest");
expect(events[1].description).toBe("first");
});
});
Loading
Loading