app, test: run draft re-ordering after sync in addition to reply send

if a user sends multiple replies A, B, C, the order should always be
A, B, C, even if:

* A fails
* B sends successfully
* C is pending

we ensure that once a reply sends successfully - or we find that
a reply _did_ send but we have it marked as failed as we never got
the response (h/t @creviera for testing this case) - that we ensure
that C appears after B. this is done by updating the file_counter.

create_dev_data.py

@@ -5,8 +5,7 @@
 import sys
 
 from securedrop_client.config import Config
-from securedrop_client.db import Base, make_session_maker, ReplySendStatus
-from securedrop_client.api_jobs.uploads import ReplySendStatusCodes
+from securedrop_client.db import Base, make_session_maker, ReplySendStatus, ReplySendStatusCodes
 
 sdc_home = sys.argv[1]
 session = make_session_maker(sdc_home)()

securedrop_client/api_jobs/uploads.py

@@ -1,24 +1,17 @@
-from enum import Enum
 import logging
 import sdclientapi
 
 from sdclientapi import API, RequestTimeoutError
-from sqlalchemy import and_
 from sqlalchemy.orm.session import Session
 
 from securedrop_client.api_jobs.base import ApiJob
 from securedrop_client.crypto import GpgHelper
-from securedrop_client.db import DraftReply, Reply, ReplySendStatus, Source
+from securedrop_client.db import DraftReply, Reply, ReplySendStatus, ReplySendStatusCodes, Source
+from securedrop_client.storage import update_draft_replies
 
 logger = logging.getLogger(__name__)
 
 
-class ReplySendStatusCodes(Enum):
-    """In progress (sending) replies can currently have the following statuses"""
-    PENDING = 'PENDING'
-    FAILED = 'FAILED'
-
-
 class SendReplyJob(ApiJob):
     def __init__(self, source_uuid: str, reply_uuid: str, message: str, gpg: GpgHelper) -> None:
         super().__init__()
@@ -63,15 +56,8 @@ def call_api(self, api_client: API, session: Session) -> str:
             draft_file_counter = draft_reply_db_object.file_counter
             draft_timestamp = draft_reply_db_object.timestamp
 
-            # If there were replies also in draft state sent after this one,
-            # re-position them after this successfully sent reply.
-            for draft_reply in session.query(DraftReply) \
-                                      .filter(and_(DraftReply.source_id == source.id,
-                                                   DraftReply.timestamp > draft_timestamp,
-                                                   DraftReply.file_counter == draft_file_counter)) \
-                                      .all():
-                draft_reply.file_counter = new_file_counter
-                session.add(draft_reply)
+            update_draft_replies(session, source.id, draft_timestamp,
+                                 draft_file_counter, new_file_counter)
 
             # Delete draft, add reply to replies table.
             session.add(reply_db_object)

securedrop_client/db.py

@@ -1,4 +1,5 @@
 import datetime
+from enum import Enum
 import os
 
 from typing import Any, List, Union  # noqa: F401
@@ -268,6 +269,12 @@ def __repr__(self) -> str:
         return '<Reply status {}>'.format(self.name)
 
 
+class ReplySendStatusCodes(Enum):
+    """In progress (sending) replies can currently have the following statuses"""
+    PENDING = 'PENDING'
+    FAILED = 'FAILED'
+
+
 class User(Base):
 
     __tablename__ = 'users'

securedrop_client/logic.py

@@ -35,7 +35,7 @@
 from securedrop_client.api_jobs.downloads import FileDownloadJob, MessageDownloadJob, \
     ReplyDownloadJob, DownloadChecksumMismatchException
 from securedrop_client.api_jobs.uploads import SendReplyJob, SendReplyJobError, \
-    SendReplyJobTimeoutError, ReplySendStatusCodes
+    SendReplyJobTimeoutError
 from securedrop_client.api_jobs.updatestar import UpdateStarJob, UpdateStarJobException
 from securedrop_client.crypto import GpgHelper, CryptoError
 from securedrop_client.export import Export
@@ -758,7 +758,7 @@ def send_reply(self, source_uuid: str, reply_uuid: str, message: str) -> None:
         # reply send status.
         source = self.session.query(db.Source).filter_by(uuid=source_uuid).one()
         reply_status = self.session.query(db.ReplySendStatus).filter_by(
-            name=ReplySendStatusCodes.PENDING.value).one()
+            name=db.ReplySendStatusCodes.PENDING.value).one()
         draft_reply = db.DraftReply(
             uuid=reply_uuid,
             timestamp=datetime.utcnow(),

securedrop_client/storage.py

@@ -19,18 +19,19 @@
 You should have received a copy of the GNU Affero General Public License
 along with this program.  If not, see <http://www.gnu.org/licenses/>.
 """
+from datetime import datetime
 import logging
 import glob
 import os
 from dateutil.parser import parse
 from typing import List, Tuple, Type, Union
 
-from sqlalchemy import or_
+from sqlalchemy import and_, or_
 from sqlalchemy.orm.exc import NoResultFound
 from sqlalchemy.orm.session import Session
 
-from securedrop_client.db import DraftReply, Source, Message, File, Reply, ReplySendStatus, User
-from securedrop_client.api_jobs.uploads import ReplySendStatusCodes
+from securedrop_client.db import (DraftReply, Source, Message, File, Reply, ReplySendStatus,
+                                  ReplySendStatusCodes, User)
 from sdclientapi import API
 from sdclientapi import Source as SDKSource
 from sdclientapi import Submission as SDKSubmission
@@ -272,21 +273,28 @@ def update_replies(remote_replies: List[SDKReply], local_replies: List[Reply],
                 reply.journalist_username,
                 session)
 
+            nr = Reply(uuid=reply.uuid,
+                       journalist_id=user.id,
+                       source_id=source.id,
+                       filename=reply.filename,
+                       size=reply.size)
+            session.add(nr)
+
             # All replies fetched from the server have succeeded in being sent,
             # so we should delete the corresponding draft locally if it exists.
             try:
                 draft_reply_db_object = session.query(DraftReply).filter_by(
                     uuid=reply.uuid).one()
+
+                update_draft_replies(session, draft_reply_db_object.source.id,
+                                     draft_reply_db_object.timestamp,
+                                     draft_reply_db_object.file_counter,
+                                     nr.file_counter)
                 session.delete(draft_reply_db_object)
+
             except NoResultFound:
                 pass  # No draft locally stored corresponding to this reply.
 
-            nr = Reply(uuid=reply.uuid,
-                       journalist_id=user.id,
-                       source_id=source.id,
-                       filename=reply.filename,
-                       size=reply.size)
-            session.add(nr)
             logger.debug('Added new reply {}'.format(reply.uuid))
 
     # The uuids remaining in local_uuids do not exist on the remote server, so
@@ -357,6 +365,23 @@ def update_missing_files(data_dir: str, session: Session) -> None:
             mark_as_not_downloaded(file.uuid, session)
 
 
+def update_draft_replies(session: Session, source_id: int, timestamp: datetime,
+                         old_file_counter: int, new_file_counter: int) -> None:
+    """
+    When we confirm a sent reply, if there are drafts that were sent after it,
+    we need to reposition them to ensure that they appear _after_ the confirmed
+    replies.
+    """
+    for draft_reply in session.query(DraftReply) \
+                              .filter(and_(DraftReply.source_id == source_id,
+                                           DraftReply.timestamp > timestamp,
+                                           DraftReply.file_counter == old_file_counter)) \
+                              .all():
+        draft_reply.file_counter = new_file_counter
+        session.add(draft_reply)
+    session.commit()
+
+
 def find_new_files(session: Session) -> List[File]:
     return session.query(File).filter_by(is_downloaded=False).all()
 

tests/conftest.py

@@ -6,9 +6,9 @@
 from configparser import ConfigParser
 from datetime import datetime
 from securedrop_client.config import Config
-from securedrop_client.api_jobs.uploads import ReplySendStatusCodes
 from securedrop_client.app import configure_locale_and_language
-from securedrop_client.db import Base, make_session_maker, Source, ReplySendStatus
+from securedrop_client.db import (Base, make_session_maker, Source, ReplySendStatus,
+                                  ReplySendStatusCodes)
 from uuid import uuid4
 
 

tests/test_storage.py

@@ -11,7 +11,6 @@
 from sqlalchemy.orm.exc import NoResultFound
 
 import securedrop_client.db
-from securedrop_client.api_jobs.uploads import ReplySendStatusCodes
 from securedrop_client.storage import get_local_sources, get_local_messages, get_local_replies, \
     get_remote_data, update_local_storage, update_sources, update_files, update_messages, \
     update_replies, find_or_create_user, find_new_messages, find_new_replies, \
@@ -59,7 +58,7 @@ def make_remote_reply(source_uuid, journalist_uuid='testymctestface'):
     """
     source_url = '/api/v1/sources/{}'.format(source_uuid)
     return Reply(filename='1-reply.filename', journalist_uuid=journalist_uuid,
-                 journalist_username='test',
+                 journalist_username='test', file_counter=1,
                  is_deleted_by_source=False, reply_url='test', size=1234,
                  source_url=source_url, uuid=str(uuid.uuid4()))
 
@@ -713,21 +712,29 @@ def test_update_replies_cleanup_drafts(homedir, mocker, session):
     draft_reply = db.DraftReply(uuid=remote_reply_create.uuid, source=source, journalist=user,
                                 file_counter=3, timestamp=datetime.datetime(2000, 6, 6, 6, 0))
     session.add(draft_reply)
+    # Another draft reply will exist that should be moved to _after_ the new reply
+    # once we confirm the previous reply. This ensures consistent ordering of interleaved
+    # drafts (pending and failed) with replies, messages, and files from the user's perspective.
+    draft_reply_new = db.DraftReply(uuid='foo', source=source, journalist=user,
+                                    file_counter=3, timestamp=datetime.datetime(2001, 6, 6, 6, 0))
+    session.add(draft_reply_new)
     session.commit()
 
     # We have no replies locally stored.
     local_replies = []
 
     update_replies(remote_replies, local_replies, session, data_dir)
 
-    # Check the expected local source object has been created with values from
-    # the API.
+    # Check the expected local source object has been created.
     new_local_replies = session.query(db.Reply).all()
     assert len(new_local_replies) == 1
 
-    # Check that the draft is now gone.
+    # Check that the only draft is the one sent with uuid 'foo' and its file_counter now
+    # matches the file_counter of the updated reply. This ensures consistent ordering.
     new_draft_replies = session.query(db.DraftReply).all()
-    assert len(new_draft_replies) == 0
+    assert len(new_draft_replies) == 1
+    assert new_draft_replies[0].file_counter == new_local_replies[0].file_counter
+    assert new_draft_replies[0].uuid == draft_reply_new.uuid
 
 
 def test_find_or_create_user_existing_uuid(mocker):
@@ -1085,9 +1092,9 @@ def test_pending_replies_are_marked_as_failed_on_logout_login(mocker, session,
                                                               reply_status_codes):
     source = factory.Source()
     pending_status = session.query(db.ReplySendStatus).filter_by(
-        name=ReplySendStatusCodes.PENDING.value).one()
+        name=db.ReplySendStatusCodes.PENDING.value).one()
     failed_status = session.query(db.ReplySendStatus).filter_by(
-        name=ReplySendStatusCodes.FAILED.value).one()
+        name=db.ReplySendStatusCodes.FAILED.value).one()
     pending_draft_reply = factory.DraftReply(source=source,
                                              send_status=pending_status)
     session.add(source)