feat: consider annotation=-1 as a vote (#908)

So that this mechanism does not have to be implemented client-side.
The voting mechanism has also been updated: before, all classes
(0, 1, -1) could be considered as a valid value. Now, -1 is exclusively
used for users to ignore insights, so it's not taken into account when
computing vote counts (only 0 and 1 are considered).
Another bug was fixed: when vote counts for 0 and 1 are both >= 2,
we save the insight as invalid (-1) instead of saving it as negative
(0).

robotoff/app/core.py

@@ -287,10 +287,31 @@ def save_annotation(
     auth: Optional[OFFAuthentication] = None,
     trusted_annotator: bool = False,
 ) -> AnnotationResult:
-    """Saves annotation either by using a single response as ground truth or by using several responses.
-
-    trusted_annotator: defines whether the given annotation comes from an authoritative source (e.g.
-    a trusted user), ot whether the annotation should be subject to the voting system.
+    """Saves annotation either by using a single response as ground truth or
+    by using several responses.
+
+    If annotation == -1 (ignore), we consider the annotation as a vote, so
+    that we don't return the insight to the user again.
+    If there are >= 2 votes on one of the 2 other possible values (0/1),
+    including the vote in process, we set the annotation value of the largest
+    voting group. The only exception is when both groups >= 2 votes, in which
+    case we mark the insight as invalid (annotation=-1).
+    Note that `annotation=-1` has two different meanings here: if it's a vote,
+    we consider it as a "ignore", and if it's the insight annotation value we
+    consider the insight as invalid, so that it's not available for annotation
+    again.
+
+    :param insight_id: The ID of the insight
+    :param annotation: The annotation, either -1, 0, or 1
+    :param device_id: Unique identifier of the device, see
+      `device_id_from_request`
+    :param update: If True, perform the update on Product Opener if annotation=1,
+      otherwise only save the annotation (default: True)
+    :param data: Optional additional data, required for some insight types
+    :param auth: User authentication data
+    :param trusted_annotator: Defines whether the given annotation comes from
+    an authoritative source (e.g. a trusted user), ot whether the annotation
+    should be subject to the voting system.
     """
     try:
         insight: Union[ProductInsight, None] = ProductInsight.get_by_id(insight_id)
@@ -303,7 +324,9 @@ def save_annotation(
     if insight.annotation is not None:
         return ALREADY_ANNOTATED_RESULT
 
-    if not trusted_annotator:
+    # We use AnnotationVote mechanism to save annotation = -1 (ignore) for
+    # authenticated users, so that it's not returned again to the user
+    if not trusted_annotator or annotation == -1:
         verified = False
 
         AnnotationVote.create(
@@ -320,7 +343,11 @@ def save_annotation(
                         AnnotationVote.value,
                         peewee.fn.COUNT(AnnotationVote.value).alias("num_votes"),
                     )
-                    .where(AnnotationVote.insight_id == insight_id)
+                    .where(
+                        AnnotationVote.insight_id == insight_id,
+                        # We don't consider any ignore (annotation = -1) vote
+                        AnnotationVote.value != -1,
+                    )
                     .group_by(AnnotationVote.value)
                     .order_by(peewee.SQL("num_votes").desc())
                 )
@@ -332,24 +359,25 @@ def save_annotation(
                 tx.rollback()
                 raise e
 
-        # If the top annotation has more than 2 votes, consider applying it to the insight.
-        if existing_votes[0].num_votes > 2:
-            annotation = existing_votes[0].value
-            verified = True
-
-        # But first check for the following cases:
-        #  1) The 1st place annotation has >2 votes, and the 2nd place annotation has >= 2 votes.
-        #  2) 1st place and 2nd place have 2 votes each.
-        #
-        # In both cases, we consider this an ambiguous result and mark it with 'I don't know'.
-        if (
-            existing_votes[0].num_votes >= 2
-            and len(existing_votes) > 1
-            and existing_votes[1].num_votes >= 2
-        ):
-            # This code credits the last person to contribute a vote with a potentially not their annotation.
-            annotation = 0
-            verified = True
+        if existing_votes:
+            # If the top annotation has more than 2 votes, consider applying it to the insight.
+            if existing_votes[0].num_votes > 2:
+                annotation = existing_votes[0].value
+                verified = True
+
+            # But first check for the following cases:
+            #  1) The 1st place annotation has > 2 votes, and the 2nd place annotation has >= 2 votes.
+            #  2) 1st place and 2nd place have 2 votes each.
+            #
+            # In both cases, we consider this an ambiguous result and mark it with 'I don't know'.
+            if (
+                existing_votes[0].num_votes >= 2
+                and len(existing_votes) > 1
+                and existing_votes[1].num_votes >= 2
+            ):
+                # This code credits the last person to contribute a vote with a potentially not their annotation.
+                annotation = -1
+                verified = True
 
         if not verified:
             return SAVED_ANNOTATION_VOTE_RESULT

tests/integration/test_api.py

@@ -175,7 +175,7 @@ def test_annotate_insight_authenticated(client):
         "/api/v1/insights/annotate",
         params={
             "insight_id": insight_id,
-            "annotation": -1,
+            "annotation": 0,
         },
         headers={"Authorization": "Basic " + base64.b64encode(b"a:b").decode("ascii")},
     )
@@ -197,19 +197,50 @@ def test_annotate_insight_authenticated(client):
         .dicts()
         .iterator()
     )
-    assert insight.items() > {"username": "a", "annotation": -1, "n_votes": 0}.items()
+    assert insight.items() > {"username": "a", "annotation": 0, "n_votes": 0}.items()
     assert "completed_at" in insight
 
     # check if "annotated_result" is saved
     assert insight["annotated_result"] == 1
 
 
-def test_annotate_insight_not_enough_votes(client):
+def test_annotate_insight_authenticated_ignore(client):
     result = client.simulate_post(
         "/api/v1/insights/annotate",
         params={
             "insight_id": insight_id,
             "annotation": -1,
+        },
+        headers={"Authorization": "Basic " + base64.b64encode(b"a:b").decode("ascii")},
+    )
+
+    assert result.status_code == 200
+    assert result.json == {
+        "status_code": 9,
+        "status": "vote_saved",
+        "description": "the annotation vote was saved",
+    }
+
+    votes = list(AnnotationVote.select())
+    assert len(votes) == 1
+
+    insight = next(
+        ProductInsight.select()
+        .where(ProductInsight.id == insight_id)
+        .dicts()
+        .iterator()
+    )
+    assert (
+        insight.items() > {"username": None, "annotation": None, "n_votes": 0}.items()
+    )
+
+
+def test_annotate_insight_not_enough_votes(client):
+    result = client.simulate_post(
+        "/api/v1/insights/annotate",
+        params={
+            "insight_id": insight_id,
+            "annotation": 1,
             "device_id": "voter1",
         },
     )
@@ -225,7 +256,7 @@ def test_annotate_insight_not_enough_votes(client):
     votes = list(AnnotationVote.select().dicts())
     assert len(votes) == 1
 
-    assert votes[0]["value"] == -1
+    assert votes[0]["value"] == 1
     assert votes[0]["username"] is None
     assert votes[0]["device_id"] == "voter1"
 
@@ -254,9 +285,14 @@ def test_annotate_insight_majority_annotation(client):
     )
     AnnotationVoteFactory(
         insight_id=insight_id,
-        value=-1,
+        value=0,
         device_id="no-voter1",
     )
+    AnnotationVoteFactory(
+        insight_id=insight_id,
+        value=-1,
+        device_id="ignore-voter1",
+    )
 
     result = client.simulate_post(
         "/api/v1/insights/annotate",
@@ -276,7 +312,7 @@ def test_annotate_insight_majority_annotation(client):
     }
 
     votes = list(AnnotationVote.select())
-    assert len(votes) == 4
+    assert len(votes) == 5
 
     insight = next(
         ProductInsight.select()
@@ -285,7 +321,7 @@ def test_annotate_insight_majority_annotation(client):
         .iterator()
     )
     # The insight should be annoted with '1', with a None username since this was resolved with an
-    # anonymous vote.
+    # anonymous vote. `n_votes = 4, as -1 votes are not considered
     assert insight.items() > {"annotation": 1, "username": None, "n_votes": 4}.items()
 
 
@@ -304,7 +340,7 @@ def test_annotate_insight_opposite_votes(client):
     )
     AnnotationVoteFactory(
         insight_id=insight_id,
-        value=-1,
+        value=0,
         device_id="no-voter1",
     )
 
@@ -313,7 +349,7 @@ def test_annotate_insight_opposite_votes(client):
         params={
             "insight_id": insight_id,
             "device_id": "no-voter2",
-            "annotation": -1,
+            "annotation": 0,
             "update": False,  # disable actually updating the product in PO.
         },
     )
@@ -334,9 +370,9 @@ def test_annotate_insight_opposite_votes(client):
         .dicts()
         .iterator()
     )
-    # The insight should be annoted with '0', with a None username since this was resolved with an
+    # The insight should be annoted with '-1', with a None username since this was resolved with an
     # anonymous vote.
-    assert insight.items() > {"annotation": 0, "username": None, "n_votes": 4}.items()
+    assert insight.items() > {"annotation": -1, "username": None, "n_votes": 4}.items()
 
 
 # This test checks for handling of cases where we have 3 votes for one annotation,
@@ -355,12 +391,12 @@ def test_annotate_insight_majority_vote_overridden(client):
     )
     AnnotationVoteFactory(
         insight_id=insight_id,
-        value=-1,
+        value=0,
         device_id="no-voter1",
     )
     AnnotationVoteFactory(
         insight_id=insight_id,
-        value=-1,
+        value=0,
         device_id="no-voter2",
     )
 
@@ -369,7 +405,7 @@ def test_annotate_insight_majority_vote_overridden(client):
         params={
             "insight_id": insight_id,
             "device_id": "no-voter3",
-            "annotation": -1,
+            "annotation": 0,
             "update": False,  # disable actually updating the product in PO.
         },
     )
@@ -392,7 +428,7 @@ def test_annotate_insight_majority_vote_overridden(client):
     )
     # The insight should be annoted with '0', with a None username since this was resolved with an
     # anonymous vote.
-    assert insight.items() > {"annotation": 0, "username": None, "n_votes": 5}.items()
+    assert insight.items() > {"annotation": -1, "username": None, "n_votes": 5}.items()
 
 
 def test_annotate_insight_anonymous_then_authenticated(client, mocker):
@@ -559,7 +595,7 @@ def test_annotation_event(client, monkeypatch, httpserver):
             "/api/v1/insights/annotate",
             params={
                 "insight_id": insight_id,
-                "annotation": -1,
+                "annotation": 0,
                 "device_id": "test-device",
             },
             headers={