add unit tests for seen_by feature

securedrop_client/gui/widgets.py

@@ -1186,6 +1186,13 @@ def _on_mark_seen(self, source_uuid: str):
         if self.source_uuid != source_uuid:
             return
 
+        # Avoid marking as seen when switching to offline mode (this is an edge case since
+        # we do not emit the mark_seen signal from the SourceList if not authenticated)
+        if not self.controller.authenticated_user:
+            return
+        else:
+            journalist_id = self.controller.authenticated_user.id
+
         # immediately update styles to mark as seen
         self.seen = True
         self.update_styles()
@@ -1199,15 +1206,10 @@ def _on_mark_seen(self, source_uuid: str):
             replies = []  # type: List[str]
             source_items = self.source.collection
             for item in source_items:
-                # Avoid marking as seen when switching to offline mode (this is an edge case since
-                # we do not emit the mark_seen signal from the SourceList if not authenticated)
-                if not self.controller.authenticated_user:
-                    return
-
-                if item.seen_by(self.controller.authenticated_user.id):
-                    continue
-
                 try:
+                    if item.seen_by(journalist_id):
+                        continue
+
                     if isinstance(item, File):
                         files.append(item.uuid)
                     elif isinstance(item, Message):

securedrop_client/storage.py

@@ -257,38 +257,10 @@ def __update_submissions(
             lazy_setattr(local_submission, "is_read", submission.is_read)
             lazy_setattr(local_submission, "download_url", submission.download_url)
 
-            # Add seen record if one doesn't yet exist
-            for journalist_uuid in submission.seen_by:
-                journalist = session.query(User).filter_by(uuid=journalist_uuid).one_or_none()
-
-                # Do not add seen record if journalist is missing from the local db. If the
-                # journalist accont was deleted, wait until the server says so.
-                if not journalist:
-                    return
-
-                journalist_id = journalist.id
-                if model == File:
-                    seen_file = (
-                        session.query(SeenFile)
-                        .filter_by(file_id=local_submission.id, journalist_id=journalist_id)
-                        .one_or_none()
-                    )
-                    if not seen_file:
-                        seen_file = SeenFile(
-                            file_id=local_submission.id, journalist_id=journalist_id
-                        )
-                        session.add(seen_file)
-                elif model == Message:
-                    seen_message = (
-                        session.query(SeenMessage)
-                        .filter_by(message_id=local_submission.id, journalist_id=journalist_id)
-                        .one_or_none()
-                    )
-                    if not seen_message:
-                        seen_message = SeenMessage(
-                            message_id=local_submission.id, journalist_id=journalist_id
-                        )
-                        session.add(seen_message)
+            if model == File:
+                add_seen_file_records(local_submission.id, submission.seen_by, session)
+            elif model == Message:
+                add_seen_message_records(local_submission.id, submission.seen_by, session)
 
             # Removing the UUID from local_uuids ensures this record won't be
             # deleted at the end of this function.
@@ -306,6 +278,11 @@ def __update_submissions(
                     download_url=submission.download_url,
                 )
                 session.add(ns)
+                session.flush()
+                if model == File:
+                    add_seen_file_records(ns.id, submission.seen_by, session)
+                elif model == Message:
+                    add_seen_message_records(ns.id, submission.seen_by, session)
                 logger.debug(f"Added {model.__name__} {submission.uuid}")
 
     # The uuids remaining in local_uuids do not exist on the remote server, so
@@ -318,6 +295,72 @@ def __update_submissions(
     session.commit()
 
 
+def add_seen_file_records(file_id: int, journalist_uuids: List[str], session: Session) -> None:
+    """
+    Add a seen record for each journalist that saw the file.
+    """
+    for journalist_uuid in journalist_uuids:
+        journalist = session.query(User).filter_by(uuid=journalist_uuid).one_or_none()
+
+        # Do not add seen record if journalist is missing from the local db. If the
+        # journalist account needs to be created or deleted, wait until the server says so.
+        if not journalist:
+            return
+
+        seen_file = (
+            session.query(SeenFile)
+            .filter_by(file_id=file_id, journalist_id=journalist.id)
+            .one_or_none()
+        )
+        if not seen_file:
+            seen_file = SeenFile(file_id=file_id, journalist_id=journalist.id)
+            session.add(seen_file)
+
+
+def add_seen_message_records(msg_id: int, journalist_uuids: List[str], session: Session) -> None:
+    """
+    Add a seen record for each journalist that saw the message.
+    """
+    for journalist_uuid in journalist_uuids:
+        journalist = session.query(User).filter_by(uuid=journalist_uuid).one_or_none()
+
+        # Do not add seen record if journalist is missing from the local db. If the
+        # journalist account needs to be created or deleted, wait until the server says so.
+        if not journalist:
+            return
+
+        seen_message = (
+            session.query(SeenMessage)
+            .filter_by(message_id=msg_id, journalist_id=journalist.id)
+            .one_or_none()
+        )
+        if not seen_message:
+            seen_message = SeenMessage(message_id=msg_id, journalist_id=journalist.id)
+            session.add(seen_message)
+
+
+def add_seen_reply_records(reply_id: int, journalist_uuids: List[str], session: Session) -> None:
+    """
+    Add a seen record for each journalist that saw the reply.
+    """
+    for journalist_uuid in journalist_uuids:
+        journalist = session.query(User).filter_by(uuid=journalist_uuid).one_or_none()
+
+        # Do not add seen record if journalist is missing from the local db. If the
+        # journalist account needs to be created or deleted, wait until the server says so.
+        if not journalist:
+            return
+
+        seen_reply = (
+            session.query(SeenReply)
+            .filter_by(reply_id=reply_id, journalist_id=journalist.id)
+            .one_or_none()
+        )
+        if not seen_reply:
+            seen_reply = SeenReply(reply_id=reply_id, journalist_id=journalist.id)
+            session.add(seen_reply)
+
+
 def update_replies(
     remote_replies: List[SDKReply], local_replies: List[Reply], session: Session, data_dir: str
 ) -> None:
@@ -365,25 +408,7 @@ def update_replies(
             lazy_setattr(local_reply, "size", reply.size)
             lazy_setattr(local_reply, "filename", reply.filename)
 
-            # Add seen record if one doesn't yet exist
-            for journalist_uuid in reply.seen_by:
-                journalist = session.query(User).filter_by(uuid=journalist_uuid).one_or_none()
-
-                # Do not add seen record if journalist is missing from the local db. If the
-                # journalist accont was deleted, wait until the server says so.
-                if not journalist:
-                    return
-
-                journalist_id = journalist.id
-
-                seen_reply = (
-                    session.query(SeenReply)
-                    .filter_by(reply_id=local_reply.id, journalist_id=journalist_id)
-                    .one_or_none()
-                )
-                if not seen_reply:
-                    seen_reply = SeenReply(reply_id=local_reply.id, journalist_id=journalist_id)
-                    session.add(seen_reply)
+            add_seen_reply_records(local_reply.id, reply.seen_by, session)
 
             del local_replies_by_uuid[reply.uuid]
             logger.debug("Updated reply {}".format(reply.uuid))
@@ -402,6 +427,9 @@ def update_replies(
                 size=reply.size,
             )
             session.add(nr)
+            session.flush()
+
+            add_seen_reply_records(nr.id, reply.seen_by, session)
 
             # All replies fetched from the server have succeeded in being sent,
             # so we should delete the corresponding draft locally if it exists.

tests/api_jobs/test_seen.py

@@ -0,0 +1,22 @@
+from securedrop_client.api_jobs.seen import SeenJob
+from tests import factory
+
+
+def test_seen(homedir, mocker, session, source):
+    """
+    Check if we call add_star method if a source is not stared.
+    """
+    file = factory.File(id=1, source=source["source"])
+    message = factory.Message(id=2, source=source["source"])
+    reply = factory.Reply(source=factory.Source())
+    session.add(file)
+    session.add(message)
+    session.add(reply)
+
+    api_client = mocker.MagicMock()
+
+    job = SeenJob([file], [message], [reply])
+
+    job.call_api(api_client, session)
+
+    api_client.seen.assert_called_once_with([file], [message], [reply])

tests/factory.py

@@ -9,6 +9,7 @@
 
 from sdclientapi import Reply as SDKReply
 from sdclientapi import Source as SDKSource
+from sdclientapi import Submission as SDKSubmission
 
 from securedrop_client import db
 from securedrop_client.api_jobs.base import ApiJob
@@ -168,7 +169,6 @@ def call_api(self, api_client, session):
 
 
 def RemoteSource(**attrs):
-
     with open(os.path.join(os.path.dirname(__file__), "files", "test-key.gpg.pub.asc")) as f:
         pub_key = f.read()
 
@@ -196,20 +196,70 @@ def RemoteSource(**attrs):
 
 
 def RemoteReply(**attrs):
-
     source_url = "/api/v1/sources/{}".format(str(uuid.uuid4()))
     defaults = dict(
         filename="1-reply.filename",
         journalist_uuid=str(uuid.uuid4()),
         journalist_username="test",
+        journalist_first_name="",
+        journalist_last_name="",
         file_counter=1,
         is_deleted_by_source=False,
         reply_url="test",
         size=1234,
         uuid=str(uuid.uuid4()),
         source_url=source_url,
+        seen_by=[],
     )
 
     defaults.update(attrs)
 
     return SDKReply(**defaults)
+
+
+def RemoteFile(**attrs):
+    source_url = "/api/v1/sources/{}".format(str(uuid.uuid4()))
+    defaults = dict(
+        source_uuid="user-uuid-1",
+        download_url="test",
+        submission_url="test",
+        filename="1-submission.filename",
+        is_read=False,
+        file_counter=1,
+        is_deleted_by_source=False,
+        reply_url="test",
+        size=1234,
+        is_decrypted=True,
+        is_downloaded=True,
+        uuid=str(uuid.uuid4()),
+        source_url=source_url,
+        seen_by=[],
+    )
+
+    defaults.update(attrs)
+
+    return SDKSubmission(**defaults)
+
+
+def RemoteMessage(**attrs):
+    source_url = "/api/v1/sources/{}".format(str(uuid.uuid4()))
+    defaults = dict(
+        source_uuid="user-uuid-1",
+        download_url="test",
+        submission_url="test",
+        filename="1-submission.filename",
+        is_read=False,
+        file_counter=1,
+        is_deleted_by_source=False,
+        reply_url="test",
+        size=1234,
+        is_decrypted=True,
+        is_downloaded=True,
+        uuid=str(uuid.uuid4()),
+        source_url=source_url,
+        seen_by=[],
+    )
+
+    defaults.update(attrs)
+
+    return SDKSubmission(**defaults)