Skip to content

Commit

Permalink
fix: don't override DraftReply.send_status based on application-level…
Browse files Browse the repository at this point in the history
… state

Since #750, application-level state transitions (logging in, logging
out, and switching into offline mode) call
securedrop_client.storage.mark_all_pending_draft_replies().  However,
one or more SendReplyJobs (and their underlying POST requests to
"/sources/<source_uuid/replies") may be in flight on the network at that
time, whether or not the application is connected (or even running) to
receive their responses.  Until we have better (ideally generalized)
logic around upload jobs that have not yet been confirmed by the server,
these application-level events should not make assumptions about the
results of jobs that have already been dispatched.
  • Loading branch information
cfm committed May 11, 2022
1 parent 21e6cd2 commit 692eda5
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 14 deletions.
6 changes: 0 additions & 6 deletions securedrop_client/logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,6 @@ def login(self, username: str, password: str, totp: str) -> None:
default_request_timeout for Queue API requests in ApiJobQueue in order to display errors
faster.
"""
storage.mark_all_pending_drafts_as_failed(self.session)
self.api = sdclientapi.API(
self.hostname, username, password, totp, self.proxy, default_request_timeout=60
)
Expand Down Expand Up @@ -579,7 +578,6 @@ def login_offline_mode(self) -> None:
# may have attempted online mode login, then switched to offline)
self.gui.clear_clipboard()
self.gui.show_main_window()
storage.mark_all_pending_drafts_as_failed(self.session)
self.update_sources()
self.show_last_sync()
self.show_last_sync_timer.start(TIME_BETWEEN_SHOWING_LAST_SYNC_MS)
Expand Down Expand Up @@ -775,10 +773,6 @@ def logout(self) -> None:

self.invalidate_token()

failed_replies = storage.mark_all_pending_drafts_as_failed(self.session)
for failed_reply in failed_replies:
self.reply_failed.emit(failed_reply.uuid)

self.api_sync.stop()
self.api_job_queue.stop()
self.gui.logout()
Expand Down
10 changes: 2 additions & 8 deletions tests/test_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ def test_Controller_login(homedir, config, mocker, session_maker):
"""
mock_gui = mocker.MagicMock()
mock_api = mocker.patch("securedrop_client.logic.sdclientapi.API")
fail_draft_replies = mocker.patch("securedrop_client.storage.mark_all_pending_drafts_as_failed")

co = Controller("http://localhost", mock_gui, session_maker, homedir, None)
co.call_api = mocker.MagicMock()
Expand All @@ -167,7 +166,6 @@ def test_Controller_login(homedir, config, mocker, session_maker):
co.call_api.assert_called_once_with(
mock_api().authenticate, co.on_authenticate_success, co.on_authenticate_failure
)
fail_draft_replies.assert_called_once_with(co.session)
co.show_last_sync_timer.stop.assert_called_once_with()


Expand Down Expand Up @@ -1016,7 +1014,7 @@ def test_Controller_invalidate_token(mocker, homedir, session_maker):

def test_Controller_logout_with_pending_replies(mocker, session_maker, homedir, reply_status_codes):
"""
Ensure draft reply fails on logout and that the reply_failed signal is emitted.
Ensure a pending reply is not automatically marked failed on logout.
"""
co = Controller("http://localhost", mocker.MagicMock(), session_maker, homedir, None)
co.api_job_queue = mocker.MagicMock()
Expand All @@ -1031,19 +1029,15 @@ def test_Controller_logout_with_pending_replies(mocker, session_maker, homedir,
.filter_by(name=db.ReplySendStatusCodes.PENDING.value)
.one()
)
failed_status = (
session.query(db.ReplySendStatus).filter_by(name=db.ReplySendStatusCodes.FAILED.value).one()
)
pending_draft_reply = factory.DraftReply(source=source, send_status=pending_status)
session.add(source)
session.add(pending_draft_reply)

co.logout()

for draft in session.query(db.DraftReply).all():
assert draft.send_status == failed_status
assert draft.send_status == pending_status

co.reply_failed.emit.assert_called_once_with(pending_draft_reply.uuid)
co.api_job_queue.stop.assert_called_once_with()


Expand Down

0 comments on commit 692eda5

Please sign in to comment.