Skip to content

feat: delete with provider validation#592

Merged
nsprenkle merged 4 commits intomasterfrom
nsprenkle/delete-with-provider
Oct 15, 2025
Merged

feat: delete with provider validation#592
nsprenkle merged 4 commits intomasterfrom
nsprenkle/delete-with-provider

Conversation

@nsprenkle
Copy link
Contributor

Adds an optional provider param for the video-transcript DELETE endpoint for additional validation.

DELETE /val/v0/videos/video-transcripts?video_id={video_id}&language_code={language_code}&provider={provider} /HTTP/1.1

For backwards compatibility, this is optional: where omitted, the delete endpoint operates as previously defined and, whether found or not, returns a 204.

HTTP/1.1 204 No Content

Where provided (pun intended), the provider param checks that the given provider matches that of the identified transcript, failing with a 404.

HTTP/1.1 404 Not Found
Content-Type: application/json

{
    "message": "Transcript provider does not match, cannot delete the transcript."
}

@nsprenkle nsprenkle requested review from Copilot and jansenk October 14, 2025 19:36
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds optional provider validation to the video-transcript DELETE endpoint. When a provider parameter is supplied, the endpoint validates that it matches the existing transcript's provider before deletion, returning a 404 error if there's a mismatch.

  • Added optional provider query parameter to the DELETE endpoint for additional validation
  • Created new TranscriptNotFoundError exception for provider mismatch scenarios
  • Updated the delete_video_transcript API function to accept and validate the provider parameter

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
edxval/views.py Added provider parameter extraction and TranscriptNotFoundError handling in DELETE endpoint
edxval/api.py Enhanced delete_video_transcript function with optional provider validation logic
edxval/exceptions.py Introduced new TranscriptNotFoundError exception class
edxval/tests/test_views.py Added test cases for provider validation scenarios and updated existing tests

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@codecov
Copy link

codecov bot commented Oct 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.84%. Comparing base (a46f718) to head (51577e4).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #592      +/-   ##
==========================================
+ Coverage   94.81%   94.84%   +0.03%     
==========================================
  Files          33       33              
  Lines        3315     3337      +22     
  Branches      127      128       +1     
==========================================
+ Hits         3143     3165      +22     
  Misses        154      154              
  Partials       18       18              
Flag Coverage Δ
unittests 94.84% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This is a way to more safely call deletion of a transcript:
If the provider does not match the supplied provider
do not move forward with deletion of the transcript.
Have I mentioned that we need to add an auto-formatter?
It is insane to break on quality failure without an opinionated way to auto-format.
@nsprenkle nsprenkle force-pushed the nsprenkle/delete-with-provider branch from 219988c to 2a51452 Compare October 14, 2025 20:20
Copy link

@michaelroytman michaelroytman 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!

@nsprenkle nsprenkle merged commit aab27ec into master Oct 15, 2025
12 checks passed
@nsprenkle nsprenkle deleted the nsprenkle/delete-with-provider branch October 15, 2025 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants