Skip to content
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

Adds /sources/<source_uuid>/conversation endpoint supporting DELETE #5963

Merged
merged 2 commits into from Jun 2, 2021

Conversation

zenmonkeykstop
Copy link
Contributor

@zenmonkeykstop zenmonkeykstop commented May 27, 2021

Status

Work in progress

Description of Changes

Adds an API endpoint /sources/<source_uuid>/conversation, which, when called via the DELETE method, deletes all submissions and replies while preserving the source account. All other methods just return a 405.

Testing

Dev container:

  • run make dev on this branch
  • log into the JI via http://localhost:8081 in a browser
  • note the journalist designation of the second source
  • copy the test script in https://gist.github.com/zenmonkeykstop/2bd2ae05e486729743497d0914a6d28e to your local workstation, install its dependencies with pip install pyotp requests, and run it with python3 wipe-convo.py
    • The response for the GET request includes the text method not allowed, and the application log in make dev output shows a 405 status code for the request
    • After the GET request, the second listed source and its associated data is still present
  • Hit Enter when prompted by the script to run the DELETE request
    • The response for the DELETE request includes the text source data deleted, and the application log in make dev output shows a 200 status code for the request
    • After the get request, the source is still present but associated conversation data is deleted
  • Sources other than the second listed source are unaffected, with their conversations intact.

Deployment

  • This will be deployed with next server code update, no postint tasks or other fixup is required.

Checklist

If you made changes to the server application code:

  • Linting (make lint) and tests (make test) pass in the development container

If you made non-trivial code changes:

  • I have written a test plan and validated it for this PR

Choose one of the following:

  • I have opened a PR in the docs repo for these changes, or will do so later
  • I would appreciate help with the documentation
  • These changes do not require documentation

@zenmonkeykstop zenmonkeykstop requested a review from a team as a code owner May 27, 2021 03:32
@zenmonkeykstop zenmonkeykstop marked this pull request as draft May 27, 2021 03:33
@codecov-commenter
Copy link

codecov-commenter commented May 27, 2021

Codecov Report

Merging #5963 (86884d3) into develop (bac4677) will decrease coverage by 0.02%.
The diff coverage is 75.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #5963      +/-   ##
===========================================
- Coverage    85.34%   85.32%   -0.03%     
===========================================
  Files           53       53              
  Lines         3875     3883       +8     
  Branches       480      481       +1     
===========================================
+ Hits          3307     3313       +6     
- Misses         456      457       +1     
- Partials       112      113       +1     
Impacted Files Coverage Δ
securedrop/journalist_app/api.py 94.22% <75.00%> (-0.58%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bac4677...86884d3. Read the comment docs.

@eloquence eloquence added this to In Development in SecureDrop Team Board May 27, 2021
@eloquence eloquence added this to the 2.0.0 milestone Jun 1, 2021
@kushaldas kushaldas self-assigned this Jun 2, 2021
Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change is good, with proper test cases. We will also need a corresponding PR for the docs repo in the API section.

I will do a formal review after it is marked as "ready for review"

@zenmonkeykstop
Copy link
Contributor Author

thanks @kushaldas, test plan etc in progress.

@zenmonkeykstop zenmonkeykstop marked this pull request as ready for review June 2, 2021 15:16
@eloquence eloquence moved this from In Development to Ready for Review in SecureDrop Team Board Jun 2, 2021
Copy link
Contributor

@rmol rmol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Test plan passes, works with WIP client safe deletion changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants