openwisp-controller icon indicating copy to clipboard operation
openwisp-controller copied to clipboard

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

Open nemesifier opened this issue 1 year ago • 6 comments

Is your feature request related to a problem? Please describe.

It seems to me that changing objects via the REST API, like devices or templates , which in the admin are monitored via django-reversion, doesn't store revisions.

Therefore if a device is changed via the API, there's no way to rollback changes, nor know who changed the object.

Describe the solution you'd like

Ideally, for each model which supports reversion in the admin, we would need to add equivalent features for the REST API:

  • A way to save a revision (and the related user who created the revision) whenever a change is made via PUT/PATCH
  • An endpoint that lists the revisions
  • An endpoint which allows to inspect a revision
  • A way to restore a revision

Describe alternatives you've considered There may be third party open source tools we can use, otherwise we can add our own logic in openwisp-utils and start rolling this feature out in the main modules.

nemesifier avatar Jul 24 '24 21:07 nemesifier

A revision should possibly be created if an object is saved by anything but the admin.

https://django-reversion.readthedocs.io/en/latest/api.html#creating-revisions

Additionally it might be need to use follow() when registering a model:

https://django-reversion.readthedocs.io/en/latest/api.html#registration-api

okraits avatar Aug 12 '24 07:08 okraits

@okraits reading the issue again, I think I forgot to specify that I meant to convey that this problem happens when modifying objects via the REST API.

nemesifier avatar Aug 12 '24 14:08 nemesifier

Hi @nemesifier @pandafy To implement revision tracking for API endpoints and models, there are several ways to implement the logic. Below are four approaches I came up with, and their respective pros and cons. Using Package: django-reversion


1. Create another mixin

  • Create a class with logic for the reversion and use it as a base class for all the view classes requiring reversion (will override these methods perform_create, perform_update, and perform_destroy).
  • Class may need to be added as base class repeatedly across multiple views.

2. Using Middleware

  • Keeps revision tracking logic separate from views and models.
  • Adds additional processing for each request, which can affect performance (not sure).

3. Using Dynamic Patching (Monkey Patching)

  • Modify perform_create, perform_update, and perform_destroy methods dynamically at runtime may be performed asynchronously.
  • Debugging can be more challenging and may introduce unexpected behavior if DRF updates its internal method implementations in future versions.

4. Centralized Logic in Models

  • Create a base class for all the models required reversion which will track all changes through overriding the create and update methods.
  • Needed extra logic for the context of the change like user, endpoint, request type etc.

Which Approach to Choose?

or there is a better solution for this scenario?

Please let me know your thoughts.

dee077 avatar Jan 13 '25 20:01 dee077

Thanks for proposing multiple solutions @dee077. I checked the documentation of django-reversion and found reversion.views.RevisionMixin. Maybe, we can use this on the API views.

pandafy avatar Jan 24 '25 09:01 pandafy

Hi @dee077, @okraits, and @pandafy,

We can integrate reversion.views.RevisionMixin into our API views to automatically wrap every request in a revision block... this approach is designed for class-based views and handles revision creation properly... ,including associating the request user with the revision metadata.

Notes :

  1. By inheriting from RevisionMixin, every non-safe request (e.g., PUT, PATCH, POST) will automatically be wrapped in a revision block, ensuring changes are tracked without extra boilerplate.
  2. The request user is automatically added to the revision metadata, solving the "who changed it" problem.
  3. We can use reversion.set_comment() in specific methods (e.g., perform_create, perform_update) to add meaningful revision comments.
    • The revision_manage_manually attribute can be set to False (default) to ensure revisions are always saved, or toggled if we need manual control in specific cases.
    • The revision_using attribute allows us to specify a database for revision data if we need to separate it from the default.
    • We can override revision_request_creates_revision(request) to fine-tune which requests trigger revisions (e.g., exclude certain methods if needed).
  4. This approach keeps the logic within the view layer, avoiding the need for middleware or model-level changes, and uses.. django-reversion’s built-in functionality.

What do you think about this approach? Does it align with the project’s needs, or is there something else we should consider? Looking forward to your feedback!

shwetd19 avatar Feb 23 '25 05:02 shwetd19

I am working on this will make a draft PR soon.

dee077 avatar Feb 24 '25 08:02 dee077