diff --git a/src/Jobs/ApplyLargoSession.php b/src/Jobs/ApplyLargoSession.php index 3da07a6..93d1524 100644 --- a/src/Jobs/ApplyLargoSession.php +++ b/src/Jobs/ApplyLargoSession.php @@ -146,6 +146,11 @@ protected function handleImageAnnotations() { [$dismissed, $changed] = $this->ignoreDeletedLabels($this->dismissedImageAnnotations, $this->changedImageAnnotations); + // This is essential, otherwise annotations could be deleted without warning + // below (because changes are applied first, then labels are dismissed, then + // dangling annotations are deleted)! + [$dismissed, $changed] = $this->ignoreNullChanges($dismissed, $changed); + // Change labels first, then dismiss to keep the opportunity to copy feature // vectors. If labels are deleted first, the feature vectors will be immediately // deleted, too, and nothing can be copied any more. @@ -161,6 +166,11 @@ protected function handleVideoAnnotations() { [$dismissed, $changed] = $this->ignoreDeletedLabels($this->dismissedVideoAnnotations, $this->changedVideoAnnotations); + // This is essential, otherwise annotations could be deleted without warning + // below (because changes are applied first, then labels are dismissed, then + // dangling annotations are deleted)! + [$dismissed, $changed] = $this->ignoreNullChanges($dismissed, $changed); + // Change labels first, then dismiss to keep the opportunity to copy feature // vectors. If labels are deleted first, the feature vectors will be immediately // deleted, too, and nothing can be copied any more. @@ -207,6 +217,35 @@ protected function ignoreDeletedLabels($dismissed, $changed) return [$dismissed, $changed]; } + /** + * Removes changes to annotations where the same label was dismissed than should be + * attached again later. + */ + protected function ignoreNullChanges(array $dismissed, array $changed): array + { + foreach ($dismissed as $labelId => $annotationIds) { + if (array_key_exists($labelId, $changed)) { + $toIgnore = []; + foreach ($annotationIds as $id) { + if (array_search($id, $changed[$labelId]) !== false) { + $toIgnore[] = $id; + } + } + + $dismissed[$labelId] = array_filter($annotationIds, + fn ($x) => !in_array($x, $toIgnore)); + $changed[$labelId] = array_filter($changed[$labelId], + fn ($x) => !in_array($x, $toIgnore)); + } + } + + // Remove elements that may now be emepty. + $dismissed = array_filter($dismissed); + $changed = array_filter($changed); + + return [$dismissed, $changed]; + } + /** * Detach annotation labels that were dismissed in a Largo session. * diff --git a/tests/Jobs/ApplyLargoSessionTest.php b/tests/Jobs/ApplyLargoSessionTest.php index 1041e5f..a05697f 100644 --- a/tests/Jobs/ApplyLargoSessionTest.php +++ b/tests/Jobs/ApplyLargoSessionTest.php @@ -672,6 +672,58 @@ public function testChangeVideoAnnotationCopyFeatureVector() $this->assertEquals($l1->id, $vectors[0]->label_id); $this->assertEquals($vector1->vector, $vectors[0]->vector); } + + public function testDismissButChangeBackToSameLabelImage() + { + $user = UserTest::create(); + $image = ImageTest::create(); + $a1 = ImageAnnotationTest::create(['image_id' => $image->id]); + $al1 = ImageAnnotationLabelTest::create([ + 'annotation_id' => $a1->id, + 'user_id' => $user->id, + ]); + + $a2 = ImageAnnotationTest::create(['image_id' => $image->id]); + $al2 = ImageAnnotationLabelTest::create([ + 'annotation_id' => $a2->id, + 'user_id' => $user->id, + 'label_id' => $al1->label_id, + ]); + + $dismissed = [$al1->label_id => [$a1->id, $a2->id]]; + $changed = [$al1->label_id => [$a1->id, $a2->id]]; + $job = new ApplyLargoSession('job_id', $user, $dismissed, $changed, [], [], false); + $job->handle(); + + $this->assertNotNull($a1->fresh()); + $this->assertNotNull($a2->fresh()); + } + + public function testDismissButChangeBackToSameLabelVideo() + { + $user = UserTest::create(); + $video = VideoTest::create(); + $a1 = VideoAnnotationTest::create(['video_id' => $video->id]); + $al1 = VideoAnnotationLabelTest::create([ + 'annotation_id' => $a1->id, + 'user_id' => $user->id, + ]); + + $a2 = VideoAnnotationTest::create(['video_id' => $video->id]); + $al2 = VideoAnnotationLabelTest::create([ + 'annotation_id' => $a2->id, + 'user_id' => $user->id, + 'label_id' => $al1->label_id, + ]); + + $dismissed = [$al1->label_id => [$a1->id, $a2->id]]; + $changed = [$al1->label_id => [$a1->id, $a2->id]]; + $job = new ApplyLargoSession('job_id', $user, [], [], $dismissed, $changed, false); + $job->handle(); + + $this->assertNotNull($a1->fresh()); + $this->assertNotNull($a2->fresh()); + } } class ApplyLargoSessionStub extends ApplyLargoSession