[feature] Add support to reversion to the REST API #894
Checklist
- [x] I have read the OpenWISP Contributing Guidelines.
- [x] I have manually tested the changes proposed in this pull request.
- [x] I have written new test cases for new code and/or updated existing tests for changes to existing code.
- [ ] I have updated the documentation.
Reference to Existing Issue
Closes #894.
Description of Changes
Introduces three new REST API endpoints for managing revisions:
-
List all revisions with optional filtering by model:
GET /controller/reversion/?model=<model_name> -
Inspect a specific revision using its ID:
GET /controller/reversion/<revision_id>/ -
Restore a revision based on its ID:
POST /controller/reversion/<revision_id>/restore/
Let me know if any changes are needed in the response data whether anything should be included, excluded, or modified. Also, please share any suggestions for adding more filters or adjusting the response format for the restore endpoint
coverage: 98.828% (+0.01%) from 98.816% when pulling 3afb7599e573410df0afb69db89ec2e6243ef3d2 on dee077:feature/894-rest-api-revisions into 85eee35ea52c64018c64baa9d6e61507ffdd4831 on openwisp:master.
I’ve added REST API revision support using django-reversion. Here’s a summary
What’s Added
AutoRevisionMixinuses RevisionMixin suggested by @pandafy which wraps API requests in reversion.create_revision().- Automatically sets the user and a comment for each revision.
New Endpoints:
controller/<str:model_slug>/revision/– List revisionscontroller/<str:model_slug>/revision/<str:pk>/– Revision detailcontroller/<str:model_slug>/revision/<str:pk>/restore/– Restore a revision These endpoints are placed above similar paths likecontroller/device/<str:pk>/to avoid conflicts, as pk will never be revision.
How It Works:
- Each change creates an entry in the
reversion_revisiontable. - The corresponding object snapshot is stored in
reversion_versionwith the samerevision_id.
Issues Noticed
- When using AutoRevisionMixin in the DeviceDetailView, the
devicegroup_change_handlerbehaves unexpectedly, so the template doesn’t update on group change. - Fails the test: test_device_api_change_group.
- Tried the mixin in other modules (connection, geo, pki), observed complexity increase (some entries show +10 additional SQL queries).
Question
Given the unexpected behavior in signals and added complexity: Is this approach okay to proceed with, or alternative design for API revision support is needed?
Updates
- Fix single quotes
- Added the
AutoRevisionMixinacross the module. This caused 2 big issues - The test was failing due to
AutoRevisionMixinwrapping the view in atransaction.atomic()block, which delayed thepost_save signal(registered viatransaction.on_commit) until the outermost transaction committed. Since the outer transaction wasn’t triggered when PUT request generating the response,that returned outdated data while the database was updated afterward. Resolved by setting revision_atomic = False in the mixin to avoid unnecessary nesting. - Some
GeoTestsApiwere inconsistently counting queries due to Django Reversion caching theContentTypelookup (used for versioning). On the first run, the lookup incurred extra queries, on subsequent runs, caching reduced them, leading to assertion failures. Fixed by addingContentType.objects.clear_cache()in thesetUp methodof the test case to ensure consistent query counts.
Updates
- Again added test with increased queries.
- The queryset uses Version objects because there is no reverse relationship from Revision to Version, but each Version is linked to a Revision.
- Rename
RevisionSerializertoVersionSerializer - In
VersionSerializerdirectly defined the fields instead of using SerializerMethodField - Added a filter by revision_id in RevisionListView to allow retrieving versions linked to the same revision, which helps group related changes made at the same time.
- Added test for revision_id filter
- Small qa fix for double quotes.
Updates
- Removed RevisionFilter class as no longer using model filter by params
- Added additional check for registered model
With this approach, only registered models will store revisions, while unregistered models can still include the mixin without any side effects. This allows in the future to safely add the mixin to new POST, PUT, or PATCH requests without needing to worry about whether the model is registered with reversion.