Hardened the logic so obs that are not part of an alert cannot be mar…

…ked as unseen

dashboard/models.py

@@ -58,9 +58,7 @@ class User(AbstractUser):
     notification_delay_days = models.IntegerField(default=365)
 
     def obs_match_alerts(self, obs: "Observation") -> bool:
-        """Return True if the observation matches at least one of the user's alerts
-        # TODO: test this
-        """
+        """Return True if the observation matches at least one of the user's alerts"""
         for alert in self.alert_set.all():
             if obs in alert.observations():
                 return True
@@ -350,9 +348,12 @@ def already_seen_by(self, user: WebsiteUser) -> bool | None:
     def mark_as_unseen_by(self, user: WebsiteUser) -> bool:
         """Mark the observation as "unseen" for a given user.
 
-        :return: True is successful (most probable causes of failure: user has not seen this observation yet / user is
-        anonymous)"""
-        if user.is_authenticated:
+        :return: True is successful. Most common causes of failure:
+            - the observation doesn't match one of the user's alerts
+            - user has not seen this observation yet
+            - user is anonymous
+        """
+        if user.is_authenticated and user.obs_match_alerts(self):
             _, created = ObservationUnseen.objects.get_or_create(
                 observation=self, user=user
             )

dashboard/templates/dashboard/observation_details.html

@@ -19,7 +19,7 @@ <h2>Observation of {{ observation.species.display_name_html|safe }} on {{ observ
         </div>
         <div class="row mb-3">
             <div class="col">
-                {% if user.is_authenticated and already_seen_by_user %}{# TODO: check: isn't it always the case for a logged in user (he's seeing it now, so it's "already seen") #}
+                {% if can_be_marked_unseen %}
                     <form class="d-inline-block" method="post"
                           action="{% url 'dashboard:actions:mark-observation-as-unseen' %}">
                         {% csrf_token %}

dashboard/tests/models/test_observation.py

@@ -57,10 +57,10 @@ def setUp(self):
         )
 
         # comment_author has also an alert that match second_obs
-        alert = Alert.objects.create(
+        self.alert = Alert.objects.create(
             user=self.comment_author,
         )
-        alert.datasets.add(self.dataset)
+        self.alert.datasets.add(self.dataset)
 
         self.first_comment = ObservationComment.objects.create(
             author=self.comment_author,
@@ -158,35 +158,58 @@ def test_mark_as_seen_by_case_3(self):
         ovs_after = ObservationUnseen.objects.filter(observation=self.obs).values()
         self.assertQuerysetEqual(ovs_after, ovs_before)
 
-    def test_mark_as_unseen_by_case_1(self):
-        """Normal case: the user has indeed previously seen the occurrence"""
-        # Before, we can find it
+    def test_mark_as_unseen_by_happy_path(self):
+        """Normal case: the user has previously seen the occurrence and it matches one
+        of its users alerts"""
+
+        # Situation before check
         with self.assertRaises(ObservationUnseen.DoesNotExist):
             ObservationUnseen.objects.get(
                 observation=self.obs, user=self.comment_author
             )
+        self.assertTrue(self.comment_author.obs_match_alerts(self.obs))
 
         r = self.obs.mark_as_unseen_by(user=self.comment_author)
         self.assertTrue(r)
         # After, we can find it
         ObservationUnseen.objects.get(observation=self.obs, user=self.comment_author)
 
-    def test_mark_as_unseen_by_case_2(self):
+    def test_mark_as_unseen_by_failure_1(self):
         """Anonymous user: nothing happens and the method return False"""
         all_ous_before = ObservationUnseen.objects.all().values()
         r = self.obs.mark_as_unseen_by(user=AnonymousUser())
         self.assertFalse(r)
         all_ous_after = ObservationUnseen.objects.all().values()
         self.assertQuerysetEqual(all_ous_after, all_ous_before)
 
-    def test_mark_as_unseen_by_case_3(self):
+    def test_mark_as_unseen_by_failure_2(self):
         """User has not seen the observation before: method returns False, nothing happens to the db"""
         all_ous_before = ObservationUnseen.objects.all().values()
         r = self.second_obs.mark_as_unseen_by(user=self.comment_author)
         self.assertFalse(r)
         all_ous_after = ObservationUnseen.objects.all().values()
         self.assertQuerysetEqual(all_ous_after, all_ous_before)
 
+    def test_mark_as_unseen_by_failure_3(self):
+        """The user has seen the observation, but it doesn't match any of its alerts"""
+        # We do the same thing that the happy path test, but we remove the alert first
+        # Situation before check
+        with self.assertRaises(ObservationUnseen.DoesNotExist):
+            ObservationUnseen.objects.get(
+                observation=self.obs, user=self.comment_author
+            )
+        self.assertTrue(self.comment_author.obs_match_alerts(self.obs))
+
+        self.alert.delete()
+
+        r = self.obs.mark_as_unseen_by(user=self.comment_author)
+        self.assertFalse(r)
+        # There's still no unseen entry
+        with self.assertRaises(ObservationUnseen.DoesNotExist):
+            ObservationUnseen.objects.get(
+                observation=self.obs, user=self.comment_author
+            )
+
     def test_replace_observation(self):
         """High-level test: after creating a new observation with the same stable_id, make sure we can migrate the
         linked entities then and then delete the initial observation"""

dashboard/tests/models/test_user.py

@@ -37,6 +37,11 @@ def setUpTestData(cls):
 
         di = DataImport.objects.create(start=timezone.now())
 
+        cls.source_dataset = Dataset.objects.create(
+            name="Test dataset",
+            gbif_dataset_key="4fa7b334-ce0d-4e88-aaae-2e0c138d049e",
+        )
+
         cls.observation = Observation.objects.create(
             gbif_id=1,
             occurrence_id="1",
@@ -46,10 +51,7 @@ def setUpTestData(cls):
             date=SEPTEMBER_13_2021,
             data_import=di,
             initial_data_import=di,
-            source_dataset=Dataset.objects.create(
-                name="Test dataset",
-                gbif_dataset_key="4fa7b334-ce0d-4e88-aaae-2e0c138d049e",
-            ),
+            source_dataset=cls.source_dataset,
             location=Point(5.09513, 50.48941, srid=4326),  # Andenne
         )
 
@@ -65,6 +67,39 @@ def setUpTestData(cls):
             text="I love this observation!",
         )
 
+    def test_obs_match_alerts_no_alert(self):
+        """The user has no alerts, so this should be false"""
+        self.assertFalse(self.jason.obs_match_alerts(self.observation))
+
+    def test_obs_match_alert_true(self):
+        """The user has an alert that matches the observation"""
+        alert = Alert.objects.create(
+            user=self.jason, email_notifications_frequency=Alert.DAILY_EMAILS
+        )
+        alert.species.add(self.observation.species)
+
+        self.assertTrue(self.jason.obs_match_alerts(self.observation))
+
+    def test_obs_match_alert_multiple(self):
+        """The user has multiple alerts which match the observation"""
+        # One alert for the species
+        alert = Alert.objects.create(
+            user=self.jason,
+            email_notifications_frequency=Alert.DAILY_EMAILS,
+            name="Test alert #1",
+        )
+        alert.species.add(self.observation.species)
+
+        # Another alert for the dataset
+        another_alert = Alert.objects.create(
+            user=self.jason,
+            email_notifications_frequency=Alert.DAILY_EMAILS,
+            name="Test alert #2",
+        )
+        another_alert.datasets.add(self.source_dataset)
+
+        self.assertTrue(self.jason.obs_match_alerts(self.observation))
+
     def test_has_alerts_with_unseen_observations_false_no_alerts(self):
         """The user has no alerts, so this should be false"""
         self.assertFalse(self.jason.has_alerts_with_unseen_observations)

dashboard/tests/views/test_pages.py

@@ -337,7 +337,7 @@ def setUpTestData(cls):
         )
 
         User = get_user_model()
-        comment_author = User.objects.create_user(
+        cls.comment_author = User.objects.create_user(
             username="frusciante",
             password="12345",
             first_name="John",
@@ -350,7 +350,7 @@ def setUpTestData(cls):
         with mock.patch("django.utils.timezone.now", mock.Mock(return_value=mocked)):
             ObservationComment.objects.create(
                 observation=cls.second_obs,
-                author=comment_author,
+                author=cls.comment_author,
                 text="This is my comment",
             )
 
@@ -695,8 +695,28 @@ def test_observation_details_observation_view_anonymous(self):
         self.assertNotContains(response, "You have first seen this observation on")
         self.assertNotContains(response, "Mark this observation as unseen")
 
-    def test_observation_details_observation_view_authenticated(self):
-        """Visiting the observation_details page while logged in: there's a button to mark as unseen"""
+    def test_observation_details_observation_view_authenticated_not_in_alerts(self):
+        """Visiting the observation_details page while logged in: there's no button to mark as unseen because the user has no matchin alert for the observation"""
+        self.client.login(username="frusciante", password="12345")
+        obs_stable_id = self.first_obs.stable_id
+
+        page_url = reverse(
+            "dashboard:pages:observation-details",
+            kwargs={"stable_id": obs_stable_id},
+        )
+
+        response = self.client.get(page_url)
+        self.assertNotContains(response, "Mark this observation as unseen")
+
+    def test_observation_details_observation_view_authenticated_in_alerts(self):
+        """Same as before, but this time the user has an alert that matches the observation, so the link is displayed"""
+        alert = Alert.objects.create(
+            name="Test alert",
+            user=self.comment_author,
+            email_notifications_frequency="N",
+        )
+        alert.species.add(self.first_obs.species)
+
         self.client.login(username="frusciante", password="12345")
         obs_stable_id = self.first_obs.stable_id
 

dashboard/views/pages.py

@@ -67,32 +67,38 @@ def news_page(request: HttpRequest) -> HttpResponse:
 
 def observation_details_page(request: HttpRequest, stable_id: str) -> HttpResponse:
     observation = get_object_or_404(Observation, stable_id=stable_id)
-    observation.mark_as_seen_by(request.user)
+
+    user = request.user
+
+    observation.mark_as_seen_by(user)
     origin_url = extract_str_request(request, "origin")
 
     if request.method == "POST":
-        if (
-            not request.user.is_authenticated
-        ):  # Only authenticated users can post comments
+        if not user.is_authenticated:  # Only authenticated users can post comments
             return HttpResponseForbidden()
 
         form = NewObservationCommentForm(request.POST)
         if form.is_valid():
             comment = form.save(commit=False)
-            comment.author = request.user
+            comment.author = user
             comment.observation = observation
             comment.save()
             form = NewObservationCommentForm()  # Show a new empty one to the user
     else:
         form = NewObservationCommentForm()
 
+    can_be_marked_unseen = False
+    if user.is_authenticated:
+        if observation.already_seen_by(user):
+            can_be_marked_unseen = user.obs_match_alerts(observation)
+
     return render(
         request,
         "dashboard/observation_details.html",
         {
             "observation": observation,
             "new_comment_form": form,
-            "already_seen_by_user": observation.already_seen_by(request.user),
+            "can_be_marked_unseen": can_be_marked_unseen,
             "origin_url": origin_url,
         },
     )