-
Notifications
You must be signed in to change notification settings - Fork 23
Feature/delete all script versions #617
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| # Testing Guide: Delete All Versions Feature | ||
|
|
||
| This guide covers both manual testing and automated testing for the new "Delete All Versions" functionality. | ||
|
|
||
| ## Manual Testing | ||
|
|
||
| ### Prerequisites | ||
| 1. Start your Django development server: | ||
| ```bash | ||
| python manage.py runserver | ||
| ``` | ||
|
|
||
| 2. Ensure you have: | ||
| - A user account with at least one script that has multiple versions | ||
| - Access to the Django admin or ability to create test data | ||
|
|
||
| ### Test Scenario 1: Web UI - Delete All Versions (Multiple Versions) | ||
|
|
||
| 1. **Navigate to a script page** that has 2+ versions: | ||
| - Go to: `http://localhost:8000/script/<script_id>` | ||
| - Or find a script you own with multiple versions | ||
|
|
||
| 2. **Verify the dropdown appears**: | ||
| - Look for the "Delete Version" button | ||
| - If the script has multiple versions, you should see a dropdown arrow next to it | ||
| - Click the dropdown to see "Delete All Versions" option | ||
|
|
||
| 3. **Test the deletion**: | ||
| - Click "Delete All Versions" from the dropdown | ||
| - A modal should appear asking for confirmation | ||
| - The modal should show: "Are you sure you want to delete all X versions of 'Script Name'?" | ||
| - Click "Delete All Versions" in the modal | ||
| - You should be redirected to the home page (`/`) | ||
| - Verify the script no longer exists in the database | ||
|
|
||
| ### Test Scenario 2: Web UI - Single Version Script | ||
|
|
||
| 1. **Navigate to a script page** that has only 1 version: | ||
| - The "Delete Version" button should NOT have a dropdown | ||
| - Only the single version deletion option should be available | ||
|
|
||
| ### Test Scenario 3: Web UI - Permission Check | ||
|
|
||
| 1. **Try to delete a script you don't own**: | ||
| - Navigate to a script owned by another user | ||
| - You should NOT see the delete button at all (if `can_delete` is False) | ||
| - Or if you somehow access the URL directly, you should get a 403 Forbidden error | ||
|
|
||
| ### Test Scenario 4: API Endpoint | ||
|
|
||
| 1. **Get authentication credentials**: | ||
| ```bash | ||
| # You'll need Basic Auth credentials for a user who owns a script | ||
| ``` | ||
|
|
||
| 2. **Test the API endpoint**: | ||
| ```bash | ||
| # Replace <script_version_pk> with an actual ScriptVersion pk | ||
| # Replace <username> and <password> with your credentials | ||
| curl -X DELETE \ | ||
| -u <username>:<password> \ | ||
| http://localhost:8000/api/scripts/<script_version_pk>/delete-all-versions/ | ||
| ``` | ||
|
|
||
| 3. **Expected responses**: | ||
| - **Success (204)**: Script and all versions deleted | ||
| - **404**: Script version not found | ||
| - **403**: Permission denied (not the owner) | ||
| - **401**: Authentication required | ||
|
|
||
| ### Test Scenario 5: Verify Cascade Deletion | ||
|
|
||
| 1. **Before deletion**, check the database: | ||
| ```python | ||
| # In Django shell: python manage.py shell | ||
| from scripts.models import Script, ScriptVersion | ||
| script = Script.objects.get(pk=<script_id>) | ||
| version_count = script.versions.count() | ||
| print(f"Script has {version_count} versions") | ||
| ``` | ||
|
|
||
| 2. **Delete all versions** using either method | ||
|
|
||
| 3. **After deletion**, verify: | ||
| ```python | ||
| # Script should be gone | ||
| Script.objects.filter(pk=<script_id>).exists() # Should be False | ||
|
|
||
| # All versions should be gone | ||
| ScriptVersion.objects.filter(script_id=<script_id>).exists() # Should be False | ||
| ``` | ||
|
|
||
| ## Automated Testing | ||
|
|
||
| ### Running Tests | ||
|
|
||
| ```bash | ||
| # Run all tests | ||
| pytest tests/ | ||
|
|
||
| # Run with coverage | ||
| pytest tests/ --cov scripts | ||
|
|
||
| # Run specific test file | ||
| pytest tests/test_delete_all_versions.py -v | ||
| ``` | ||
|
|
||
| ### Example Test File | ||
|
|
||
| See `tests/test_delete_all_versions.py` for a complete test suite. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,30 +1,67 @@ | ||
| {% load bootstrap4 %} | ||
| {% load botc_script_tags %} | ||
|
|
||
| <!-- Modal --> | ||
| <!-- Modal for deleting single version --> | ||
| <div class="modal fade" id="deletionModal" tabindex="-1" role="dialog" aria-labelledby="deletionModalLabel" aria-hidden="true"> | ||
| <div class="modal-dialog" role="document"> | ||
| <div class="modal-content"> | ||
| <div class="modal-header"> | ||
| <h5 class="modal-title" id="deletionModalLabel">Delete</h5> | ||
| <h5 class="modal-title" id="deletionModalLabel">Delete Version</h5> | ||
| <button type="button" class="close" data-dismiss="modal" aria-label="Close"> | ||
| <span aria-hidden="true">×</span> | ||
| </button> | ||
| </div> | ||
| <div class="modal-body"> | ||
| Are you sure you want to delete this script? | ||
| Are you sure you want to delete version {{ script_version.version }} of "{{ script.name }}"? | ||
| </div> | ||
| <div class="modal-footer"> | ||
| <button type="button" class="btn btn-primary" data-dismiss="modal">Cancel</button> | ||
| <form action="{% url 'delete_script' script.pk script_version.version %}" method="post"> | ||
| <form action="{% url 'delete_script' script.pk script_version.version %}" method="post" class="d-inline"> | ||
| {% csrf_token %} | ||
| <button type="submit" class="btn btn-danger">Delete</button> | ||
| <button type="submit" class="btn btn-danger">Delete Version</button> | ||
| </form> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| <button class="btn btn-danger" data-toggle="modal" data-target="#deletionModal"> | ||
| Delete | ||
| </button> | ||
| <!-- Modal for deleting all versions --> | ||
| {% if script.versions.all|length > 1 %} | ||
| <div class="modal fade" id="deleteAllVersionsModal" tabindex="-1" role="dialog" aria-labelledby="deleteAllVersionsModalLabel" aria-hidden="true"> | ||
| <div class="modal-dialog" role="document"> | ||
| <div class="modal-content"> | ||
| <div class="modal-header"> | ||
| <h5 class="modal-title" id="deleteAllVersionsModalLabel">Delete All Versions</h5> | ||
| <button type="button" class="close" data-dismiss="modal" aria-label="Close"> | ||
| <span aria-hidden="true">×</span> | ||
| </button> | ||
| </div> | ||
| <div class="modal-body"> | ||
| <p>Are you sure you want to delete <strong>all {{ script.versions.all|length }} versions</strong> of "{{ script.name }}"?</p> | ||
| <p class="text-danger"><strong>This action cannot be undone.</strong></p> | ||
| </div> | ||
| <div class="modal-footer"> | ||
| <button type="button" class="btn btn-primary" data-dismiss="modal">Cancel</button> | ||
| <form action="{% url 'delete_all_script_versions' script.pk %}" method="post" class="d-inline"> | ||
| {% csrf_token %} | ||
| <button type="submit" class="btn btn-danger">Delete All Versions</button> | ||
| </form> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| {% endif %} | ||
|
|
||
| <div class="btn-group"> | ||
| <button class="btn btn-danger" data-toggle="modal" data-target="#deletionModal"> | ||
| Delete Version | ||
| </button> | ||
| {% if script.versions.all|length > 1 %} | ||
| <button type="button" class="btn btn-danger dropdown-toggle dropdown-toggle-split" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false"> | ||
| <span class="sr-only">Toggle Dropdown</span> | ||
| </button> | ||
| <div class="dropdown-menu"> | ||
| <a class="dropdown-item text-danger" href="#" data-toggle="modal" data-target="#deleteAllVersionsModal">Delete All Versions</a> | ||
| </div> | ||
| {% endif %} | ||
| </div> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -69,6 +69,11 @@ | |
| name="favourite", | ||
| ), | ||
| path("script/<int:pk>", views.ScriptView.as_view(), name="script"), | ||
| path( | ||
| "script/<int:pk>/delete_all", | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick I'd prefer this was just |
||
| views.ScriptDeleteAllVersionsView.as_view(), | ||
| name="delete_all_script_versions", | ||
| ), | ||
| path( | ||
| "script/<int:pk>/<str:version>/similar", | ||
| views.get_similar_scripts, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -582,6 +582,26 @@ def determine_success_url(self, script): | |
| return f"/script/{script.pk}" | ||
|
|
||
|
|
||
| class ScriptDeleteAllVersionsView(LoginRequiredMixin, generic.View): | ||
| """ | ||
| Deletes all versions of a script and the script itself. | ||
| """ | ||
|
|
||
| def post(self, request, *args, **kwargs): | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue POST methods should generally only be used for creates or updates, not for deletes. That's what DELETE is for. |
||
| try: | ||
| script = models.Script.objects.get(pk=kwargs.get("pk")) | ||
| except models.Script.DoesNotExist: | ||
| raise Http404("Cannot delete a script that does not exist.") | ||
|
|
||
| if script.owner != request.user: | ||
| return HttpResponseForbidden() | ||
|
|
||
| # Delete the script, which will cascade delete all versions due to CASCADE relationship | ||
| script.delete() | ||
|
|
||
| return HttpResponseRedirect("/") | ||
|
|
||
|
|
||
| class StatisticsView(generic.ListView, FilterView): | ||
| model = models.ScriptVersion | ||
| template_name = "statistics.html" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| #!/usr/bin/env python | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue Broadly I don't think this script is doing anything, so I think I'd just prefer to get rid of it. |
||
| """ | ||
| Quick manual testing script for delete all versions functionality. | ||
| Run this with: python manage.py shell < test_manual.py | ||
| Or copy-paste into Django shell. | ||
| """ | ||
|
|
||
| from scripts.models import Script, ScriptVersion | ||
| from django.contrib.auth.models import User | ||
| from versionfield import Version | ||
|
|
||
| # Create or get a test user | ||
| user, created = User.objects.get_or_create(username="testuser", defaults={"email": "test@example.com"}) | ||
| if created: | ||
| user.set_password("testpass123") | ||
| user.save() | ||
| print(f"Created user: {user.username}") | ||
|
|
||
| # Create a test script with multiple versions | ||
| script, created = Script.objects.get_or_create(name="Test Script for Deletion", defaults={"owner": user}) | ||
| if created: | ||
| print(f"Created script: {script.name} (ID: {script.pk})") | ||
| else: | ||
| print(f"Using existing script: {script.name} (ID: {script.pk})") | ||
|
|
||
| # Check current versions | ||
| current_versions = script.versions.count() | ||
| print(f"Current versions: {current_versions}") | ||
|
|
||
| # Create multiple versions if needed | ||
| if current_versions < 2: | ||
| for i, version_str in enumerate(["1.0", "2.0", "3.0"], 1): | ||
| ScriptVersion.objects.get_or_create( | ||
| script=script, | ||
| version=Version(version_str), | ||
| defaults={ | ||
| "content": [{"id": "washerwoman"}] * i, | ||
| "latest": (i == 3), # Only last one is latest | ||
| "num_townsfolk": i, | ||
| "num_outsiders": 0, | ||
| "num_minions": 0, | ||
| "num_demons": 0, | ||
| "num_fabled": 0, | ||
| "num_loric": 0, | ||
| "num_travellers": 0, | ||
| }, | ||
| ) | ||
| print(f"Created test versions. Total versions: {script.versions.count()}") | ||
|
|
||
| # Display script info | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue The stuff below here doesn't feel valuable. |
||
| print("\n" + "=" * 50) | ||
| print("SCRIPT INFORMATION") | ||
| print("=" * 50) | ||
| print(f"Script ID: {script.pk}") | ||
| print(f"Script Name: {script.name}") | ||
| print(f"Owner: {script.owner.username if script.owner else 'None'}") | ||
| print(f"Number of versions: {script.versions.count()}") | ||
| print("\nVersions:") | ||
| for v in script.versions.all().order_by("version"): | ||
| print(f" - Version {v.version} (PK: {v.pk}, Latest: {v.latest})") | ||
|
|
||
| print("\n" + "=" * 50) | ||
| print("TESTING INSTRUCTIONS") | ||
| print("=" * 50) | ||
| print("1. Web UI Test:") | ||
| print(f" - Go to: http://localhost:8000/script/{script.pk}") | ||
| print(" - Look for 'Delete Version' button with dropdown") | ||
| print(" - Click dropdown and select 'Delete All Versions'") | ||
| print(" - Confirm deletion") | ||
| print(" - Should redirect to home page") | ||
| print("\n2. API Test:") | ||
| print(f" curl -X DELETE -u {user.username}:testpass123 \\") | ||
| print(f" http://localhost:8000/api/scripts/{script.versions.first().pk}/delete-all-versions/") | ||
| print("\n3. Verify deletion:") | ||
| print(f" Script.objects.filter(pk={script.pk}).exists() # Should be False") | ||
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.
chore Can you just remove this, how to manually test doesn't need documentation