-
Notifications
You must be signed in to change notification settings - Fork 429
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: admin action to delete all segments for project #2646
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Uffizzi Preview |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2646 +/- ##
==========================================
- Coverage 95.44% 95.43% -0.01%
==========================================
Files 976 976
Lines 27319 27294 -25
==========================================
- Hits 26074 26049 -25
Misses 1245 1245
☔ View full report in Codecov by Sentry. |
api/projects/admin.py
Outdated
@@ -9,6 +11,10 @@ | |||
from projects.tags.models import Tag | |||
from segments.models import Segment | |||
|
|||
if typing.TYPE_CHECKING: |
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.
Why are we only importing them if TYPE_CHECKING?
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.
Because we only need them for typing.
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.
Is that the convention? Because both of them are going to be available at runtime anyway, and that if
is a bit ugly? What do you think?
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.
Yeah, fair enough - I'm not too sure what the convention is. Seems like it should only really be used to avoid circular dependencies. I'll remove.
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.
one small comment
Changes
Adds an admin action to delete all segments for a project.
This is mostly a support tool for a particular customer that breached the limit of segments and we need to clear them down. Since django handles cascading manually, it is challenging / risky to delete them manually in the database.
How did you test this code?
Added unit test and tested manually.