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

feat(video-api): add DELETE route for videos #412

Merged
merged 17 commits into from
Feb 26, 2024
Merged

Conversation

connordoman
Copy link
Contributor

@connordoman connordoman commented Jan 28, 2024

Welcome to PrivacyPal! 👋

Fixes: #411
Depends on #393
Depends on #403
Depends on #406
Depends on #388

Description of the change:

This change introduces an API route for deleting videos from S3 and the database.

Motivation for the change:

Clients and professionals need to be able to delete uploaded media as a matter of privacy.

Usage

DELETE /api/video/{awsRef}?appt={apptId}

e.g. DELETE /api/video/1-privacy-pal-test-20240128T170000-processed.mp4?apptId=123

@connordoman connordoman added feat New feature or request area/back-end Back-end work labels Jan 28, 2024
@connordoman connordoman added this to the Term 2 Week 4 ❗️ milestone Jan 28, 2024
@connordoman connordoman self-assigned this Jan 28, 2024
@github-actions github-actions bot added the dependent Depending on other work label Jan 28, 2024
nganphan123
nganphan123 previously approved these changes Jan 28, 2024
Copy link
Contributor

@nganphan123 nganphan123 left a comment

Choose a reason for hiding this comment

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

lgtm!

Copy link
Contributor

@MyStackOverflows MyStackOverflows left a comment

Choose a reason for hiding this comment

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

looks good to me!

Signed-off-by: Connor Doman <domanc@student.ubc.ca>
nganphan123
nganphan123 previously approved these changes Jan 31, 2024
Copy link
Contributor

@MyStackOverflows MyStackOverflows left a comment

Choose a reason for hiding this comment

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

this is just waiting on #393 right?

@tthvo
Copy link
Contributor

tthvo commented Feb 7, 2024

Hey @connordoman, would I bother you to bring the changes in #393 needed for this in a separate PR for review?

@tthvo
Copy link
Contributor

tthvo commented Feb 8, 2024

Rebase pls :))

@github-actions github-actions bot removed the dependent Depending on other work label Feb 12, 2024
Copy link
Contributor

@MyStackOverflows MyStackOverflows left a comment

Choose a reason for hiding this comment

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

looks great, awesome work Connor!

@tthvo
Copy link
Contributor

tthvo commented Feb 22, 2024

Rebase and resolve conflicts pls.

app/web/src/lib/s3.ts Outdated Show resolved Hide resolved
app/web/src/lib/s3.ts Outdated Show resolved Hide resolved
app/web/src/app/actions.ts Outdated Show resolved Hide resolved
app/web/src/lib/s3.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@tthvo tthvo left a comment

Choose a reason for hiding this comment

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

Just a little more and rebase pls.


// delete the video from s3
try {
deleteArtifact(videoRef, getOutputBucket());
Copy link
Contributor

Choose a reason for hiding this comment

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

Following #560, the videoref in db can be under review or accepted. This means, in the case of videos being under review, deleting files in output bucket will leave files in input bucket stale/dangling.

Copy link
Contributor

Choose a reason for hiding this comment

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

One way to go around this is to fetch the video tags from S3 first, check if it has privacypal-status tag. If so, also try to delete the video from input bucket.

connordoman and others added 3 commits February 25, 2024 19:35
Co-authored-by: Thuan Vo <thuan.votann@gmail.com>
Signed-off-by: Connor Doman <connor@connordoman.dev>
Copy link
Contributor

@tthvo tthvo left a comment

Choose a reason for hiding this comment

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

Looks good! Doing a final test now...

Copy link
Contributor

@tthvo tthvo left a comment

Choose a reason for hiding this comment

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

Awesome!

@tthvo tthvo merged commit b10b53b into develop Feb 26, 2024
12 checks passed
@tthvo tthvo deleted the gh-349-video-delete-api branch February 26, 2024 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/back-end Back-end work feat New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants