Add action to (re-)generate and export conversation transcript #1624
Conversation
|
@cfm I have a few questions about a failing test that doesn't seem related (but probably is, somehow 🤨), but this PR is ready for a first review. 🙂 |
|
(Just linking to the issues that should resolve the bookworm failures: freedomofpress/securedrop-builder#412, #1623; will do initial review while those get sorted) |
|
@rocodes I added the patch to the PR description in case you want to test this without a Workstation / USB drive. ☝️ |
|
@rocodes The Bookworm failures are not a blocker for this PR. They're intentionally not required. |
|
Hi @gonzalo-bulnes, thanks for preparing this PR. I have stepped through it and have a quick question before I do a more detailed code review: unlike the For example, entering the wrong password in the password dialog does not show a user-visible error, and entering the correct password successfully exports the conversation transcript and unmounts the drive, but does not inform the user that the action succeeded. (STR: staging setup, latest .deb from sd-devices built and installed manually; cloned the client repo in sd-app, checked out this branch, and ran it via |
gonzalo-bulnes
force-pushed
the
export-conversation-transcript
branch
from
7317754
to
d56c754
Compare
|
(rebased without changes) |
|
I'm done touching this until you submit your review @rocodes 🙂 |
There was a problem hiding this comment.
Hi @gonzalo-bulnes, thanks for preparing.
- Print functionality working well
- I'll retest the export transcript functionality shortly
- I've left one comment for discussion about export_service
- I think additional functional testing might be beneficial here. I'm looking into that based on similar tests (for delete source, from the same Qaction menu, or for exporting a file)
gonzalo-bulnes
force-pushed
the
export-conversation-transcript
branch
2 times, most recently
from
e9aeb5a
to
3796ea9
Compare
- Enforces the usage of the export.Service instance as a singleton. - Removes the need to inject it as a dependency through the widget tree. References: - Pattern: logging.getLogger - The Qubes OS API responses cannot be attributed to any given command or query if the API is used concurrently. See freedomofpress/securedrop-export#112
gonzalo-bulnes
force-pushed
the
export-conversation-transcript
branch
from
3796ea9
to
6e0eff6
Compare
|
(squashed the three commits that your had already reviewed @rocodes — and kept the |
There was a problem hiding this comment.
Test plan
- Transcript can be exported successfully, confirmation message shown
- Transcript can be printed successfully
- Improper export device yields correct error msg
- Bad device passphrase reprompts for passphrase
- No/misconfigured printer yields printer error dialog
- Transcript before files are downloaded shows generic file-counter name (3-source-designator-doc.gz.gpg); transcript after files are downloaded shows decrypted file name (memo.txt)
- Tests are passing locally and on CI
Description
The shortest path to providing an option to export a conversation transcript to journalists.
Context
This is a direct continuation of #1621, the same context applies.
Implementation
Test plan
conversation.txt(I can provide guidance for a lightweight tests like I did for #1621, but since exporting doesn't depend on hard to get hardware, I think we should eventually test the feature using an actual USB drive. Edit: here is the patch.)