Feature/delete all script versions#617
Conversation
- Add ScriptDeleteAllVersionsView to delete all versions of a script - Add API endpoint delete_all_versions to ScriptViewSet - Update delete_script.html template with dropdown to delete all versions - Add comprehensive test suite with 9 passing tests - Add testing documentation The feature allows script owners to delete all versions of their script at once, providing a clean interface for complete script removal. The dropdown only appears when a script has multiple versions.
| @@ -0,0 +1,111 @@ | |||
| # Testing Guide: Delete All Versions Feature | |||
There was a problem hiding this comment.
chore Can you just remove this, how to manually test doesn't need documentation
| Deletes all versions of a script and the script itself. | ||
| """ | ||
|
|
||
| def post(self, request, *args, **kwargs): |
There was a problem hiding this comment.
issue POST methods should generally only be used for creates or updates, not for deletes. That's what DELETE is for.
| ), | ||
| path("script/<int:pk>", views.ScriptView.as_view(), name="script"), | ||
| path( | ||
| "script/<int:pk>/delete_all", |
There was a problem hiding this comment.
nitpick I'd prefer this was just delete rather than delete_all as script objects have all the scripts on them.
| ) | ||
| print(f"Created test versions. Total versions: {script.versions.count()}") | ||
|
|
||
| # Display script info |
There was a problem hiding this comment.
issue The stuff below here doesn't feel valuable.
| @@ -0,0 +1,75 @@ | |||
| #!/usr/bin/env python | |||
There was a problem hiding this comment.
issue Broadly I don't think this script is doing anything, so I think I'd just prefer to get rid of it.
| num_travellers=0, | ||
| ) | ||
|
|
||
| return script, version_2 |
There was a problem hiding this comment.
issue I'm not a fan of this fixture interface. I'd much rather this return simply a script. It's already possible to get all the versions from a script so there's no real need to return an arbitrary version within the set of scripts.
|
|
||
| # Should return 404 | ||
| assert response.status_code == 404 | ||
|
|
There was a problem hiding this comment.
issue Thanks for adding scripts, but it would be good to add tests for deleting one version as well.
|
Sorry it took me so long to get to this. Thanks for the contribution, there's a few things I'd like to see changed before merging but overall this seems to be doing what it's trying to do so with some changes could be merged. |
|
oh god i totally forgot about this ill get back to this, im sorry (i thought they send emails for updates) |
2 participants
Add functionality to delete all versions of a script