Fix bug where annotations could be deleted without warning

If an annotation was dismissed and then relabeled with the same
label than before, the (new) order of processing the changes would
leave without any label so it would be deleted. These cases are now
filtered out before the changes are processed.

See: https://github.com/orgs/biigle/discussions/774

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.
      *

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