diff --git a/securedrop_client/api_jobs/uploads.py b/securedrop_client/api_jobs/uploads.py index 61ba02ccc4..2b90596177 100644 --- a/securedrop_client/api_jobs/uploads.py +++ b/securedrop_client/api_jobs/uploads.py @@ -75,7 +75,9 @@ 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 update_draft_replies( - session, source.id, draft_timestamp, draft_file_counter, new_file_counter) + session, source.id, draft_timestamp, draft_file_counter, new_file_counter, + commit=False + ) # Add reply to replies table and increase the source interaction count by 1 and delete # the draft reply. diff --git a/securedrop_client/storage.py b/securedrop_client/storage.py index 8f47d35a2a..6a8dcc7cbe 100644 --- a/securedrop_client/storage.py +++ b/securedrop_client/storage.py @@ -287,7 +287,7 @@ def update_replies(remote_replies: List[SDKReply], local_replies: List[Reply], if not user: logger.debug(f"user {reply.journalist_uuid} not cached") user = find_or_create_user( - reply.journalist_uuid, reply.journalist_username, session + reply.journalist_uuid, reply.journalist_username, session, commit=False ) users[reply.journalist_uuid] = user @@ -335,9 +335,7 @@ def update_replies(remote_replies: List[SDKReply], local_replies: List[Reply], session.commit() -def find_or_create_user(uuid: str, - username: str, - session: Session) -> User: +def find_or_create_user(uuid: str, username: str, session: Session, commit: bool = True) -> User: """ Returns a user object representing the referenced journalist UUID. If the user does not already exist in the data, a new instance is created. @@ -349,12 +347,14 @@ def find_or_create_user(uuid: str, new_user = User(username=username) new_user.uuid = uuid session.add(new_user) - session.commit() + if commit: + session.commit() return new_user if user.username != username: user.username = username - session.commit() + if commit: + session.commit() return user diff --git a/securedrop_client/utils.py b/securedrop_client/utils.py index 515a293032..30d4cb0405 100644 --- a/securedrop_client/utils.py +++ b/securedrop_client/utils.py @@ -84,4 +84,4 @@ def chronometer(logger: logging.Logger, description: str) -> Generator: yield finally: elapsed = time.perf_counter() - start - logger.debug(f"{description} duration: {elapsed:.4f}s") + logger.info(f"{description} duration: {elapsed:.4f}s") diff --git a/tests/test_storage.py b/tests/test_storage.py index 9e624a7dab..6cf997dc87 100644 --- a/tests/test_storage.py +++ b/tests/test_storage.py @@ -20,7 +20,8 @@ delete_single_submission_or_reply_on_disk, get_local_files, find_new_files, \ source_exists, set_message_or_reply_content, mark_as_downloaded, mark_as_decrypted, get_file, \ get_message, get_reply, update_and_get_user, update_missing_files, mark_as_not_downloaded, \ - mark_all_pending_drafts_as_failed, delete_local_source_by_uuid, update_file_size + mark_all_pending_drafts_as_failed, delete_local_source_by_uuid, update_file_size, \ + update_draft_replies from securedrop_client import db from tests import factory @@ -1180,3 +1181,16 @@ def test_update_file_size(homedir, session): update_file_size(f.uuid, data_dir, session) assert f.size == real_size + + +def test_update_draft_replies_commit(mocker, session): + """ + Tests the commit argument of storage.update_draft_replies. + """ + session.commit = mocker.MagicMock() + + update_draft_replies(session, "notreal", datetime.datetime.now(), 1, 2, commit=False) + assert session.commit.call_count == 0 + + update_draft_replies(session, "notreal", datetime.datetime.now(), 1, 2) + assert session.commit.call_count == 1