Skip to content
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

[feature] Add support to reversion to the REST API #994

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dee077
Copy link
Contributor

@dee077 dee077 commented Mar 20, 2025

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • 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

@dee077 dee077 force-pushed the feature/894-rest-api-revisions branch from 8690016 to 3fdfcaf Compare March 21, 2025 08:34
@coveralls
Copy link

coveralls commented Mar 21, 2025

Coverage Status

coverage: 98.892% (+0.008%) from 98.884%
when pulling 84a68d6 on dee077:feature/894-rest-api-revisions
into 4881326 on openwisp:master.

Implemented three endpoints:
1. List all revisions with optional filtering by model.
2. Inspect a specific revision by its ID.
3. Restore a revision using its ID.

 Fixes openwisp#894
@dee077 dee077 force-pushed the feature/894-rest-api-revisions branch from 3fdfcaf to 98ca619 Compare March 21, 2025 09:03
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

Good work! The code is clean and well-structured.

However, this PR seems to be missing its main goal: ensuring that every versioned model admin has a corresponding versioned REST API that logs revisions—tracking who changed what—each time a modification is made.

Please check my comments below for further details.

model = filters.CharFilter(field_name="content_type__model", lookup_expr="iexact")

def _set_valid_filterform_labels(self):
self.filters["model"].label = _("Model")
Copy link
Member

Choose a reason for hiding this comment

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

We are using single quotes for strings, eg: 'model'. Please maintain consistency!

name='reversion_detail',
),
path(
'controller/reversion/<str:pk>/restore/',
Copy link
Member

Choose a reason for hiding this comment

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

The URLs seem to be heavily focused on the django-reversion implementation rather than aligning with OpenWISP's approach.

This was not the original intention of the issue. The goal is to introduce additional URLs for each resource (e.g., Device, Template, etc.) that supports reversion, allowing users to list revisions, review revisions, and restore them.

For example, for Template:

  • GET /controller/template/revisions/
  • GET /controller/template/revisions/{pk}/
  • POST /controller/template/revisions/{pk}/restore/

@dee077 dee077 changed the title [feature] Rivision for Rest Api changes [feature] Revisions for Rest Api changes Mar 25, 2025
@nemesifier nemesifier changed the title [feature] Revisions for Rest Api changes [feature] Add support to reversion to the REST API Mar 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature] REST API should store revisions with django-reversion
3 participants