Feature/annotation log#109
Conversation
jgonggrijp
left a comment
jgonggrijp
left a comment
There was a problem hiding this comment.
I have reviewed the code and the screenshot (which looks nice!), but did not try the changes locally. Based on the code, I wonder whether it is possible to see both additions and removals but only for labels, or only removals but both for labels and KB items. Other than that, the functionality looks very nice.
| @for (event of events; track $index) { | ||
| @let added = event.eventKind === "added"; | ||
| @let isLabel = event.annotationKind === "label"; |
There was a problem hiding this comment.
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?
| type AnnotationEventKind = "added" | "removed"; | ||
| type AnnotationKind = "label" | "knowledgeBase"; | ||
|
|
||
| export interface AnnotationEvent { | ||
| eventKind: AnnotationEventKind; | ||
| annotationKind: AnnotationKind; | ||
| timestamp: string; | ||
| actor: string; | ||
| description: string; | ||
| } | ||
|
|
||
| const relationshipSymbols: Record<KnowledgeBaseRelationship, string> = { | ||
| [KnowledgeBaseRelationship.EQUAL]: "=", | ||
| [KnowledgeBaseRelationship.NOT_EQUAL]: "≠", | ||
| [KnowledgeBaseRelationship.SUBSET]: "⊂", | ||
| [KnowledgeBaseRelationship.SUPERSET]: "⊃", | ||
| }; | ||
|
|
||
| function annotationToEvent(annotation: LabelAnnotation | KnowledgeBaseAnnotation): AnnotationEvent[] { | ||
| const isLabelAnnotation = "label" in annotation; | ||
| const description = isLabelAnnotation ? annotation.label.text : `${annotation.entity1} ${relationshipSymbols[annotation.relationship]} ${annotation.entity2}`; | ||
| const annotationKind: AnnotationKind = isLabelAnnotation ? "label" : "knowledgeBase"; | ||
| const events: AnnotationEvent[] = [{ | ||
| eventKind: "added", | ||
| annotationKind, | ||
| timestamp: annotation.createdAt, | ||
| actor: annotation.createdBy, | ||
| description, | ||
| }]; | ||
| if (annotation.removedAt && annotation.removedBy) { | ||
| events.push({ | ||
| eventKind: "removed", | ||
| annotationKind, | ||
| timestamp: annotation.removedAt, | ||
| actor: annotation.removedBy, | ||
| description, | ||
| }); | ||
| } | ||
| return events; | ||
| } |
There was a problem hiding this comment.
This is model-level logic. Maybe it deserves living in a dedicated module?
| (e.eventKind === "added" ? this.showAdded() : this.showRemoved()) && | ||
| (e.annotationKind === "label" ? this.showLabels() : this.showKB()) |
There was a problem hiding this comment.
Does this mean that you can either see all events, or filter by event kind and annotation kind at the same time, but not filter only by event kind or only by annotation kind?
2 participants
Closes #69
This PR adds a simple list of annotations to each problem, so users can see when annotations were added/removed and by whom.
The annotations are sorted by date from most to least recent. As a bonus, I've added some frontend filtering. We can always add backend filtering, sorting, pagination and other fancy list goodies if/when they are requested.