-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ensure we delete individual submissions on disk #923
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Source is deleted by the local user. The entire source directory
~/.securedrop_client/data/intrusive_smattering
should be deleted. - Source is deleted by another user (you can simulate this by deleting an entire source in the JI). The entire source directory
~/.securedrop_client/data/intrusive_smattering
should be deleted. - An individual submission is deleted by another user (you can do this by deleting an individual submission in the JI). The file and its enclosing folder should be deleted:
~/.securedrop_client/data/intrusive_smattering/1-intrusive_smattering-doc
.
In the third test, the file and its directory were indeed deleted, but the file was still shown in the conversation view, with export and print buttons. Selecting another source and returning cleared the conversation view, but the filename was still visible in the source's snippet.
20da467
to
12b0b6c
Compare
Changes look good, can be merged after a rebase. |
12b0b6c
to
2b7fdad
Compare
thanks for review! rebased |
2b7fdad
to
a6bed3a
Compare
Oh, bad news. I think some of the changes on master have broken things.
This worked, but the new "pending deletion" state wrote "Deleting..." over the empty conversation view text. It eventually corrected of course, but it was ugly in the meantime.
This crashed the client with:
This went fine, except for the previously noted bugs (#891, #922). |
Ahh ok, thanks for flagging. OK I'll need to take another look at this on Monday to see what happened and fix. |
a6bed3a
to
1a0481d
Compare
I believe this one is #929 (recent bug on master) Otherwise for the traceback you found, I was not able to reproduce, however I added a guard to ensure that we gracefully handle if the uuid of the source widget is missing from the Rebased/retested/pushed on latest master (8ab6101) |
422d6f2
to
a610a3e
Compare
Rebased/retested on latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Description
Fixes #892.
Test Plan
Given the importance of the deletion functionality, I recommend testing the following cases:
Precondition for each case: Via the source interface, create a source with at least one file. Log into the client and download the file.
~/.securedrop_client/data/intrusive_smattering
should be deleted.~/.securedrop_client/data/intrusive_smattering
should be deleted.~/.securedrop_client/data/intrusive_smattering/1-intrusive_smattering-doc
.Checklist
If these changes modify code paths involving cryptography, the opening of files in VMs or network (via the RPC service) traffic, Qubes testing in the staging environment is required. For fine tuning of the graphical user interface, testing in any environment in Qubes is required. Please check as applicable:
If these changes add or remove files other than client code, packaging logic (e.g., the AppArmor profile) may need to be updated. Please check as applicable: