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] REST API should store revisions with django-reversion #894

Open
nemesifier opened this issue Jul 24, 2024 · 4 comments
Open

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

nemesifier opened this issue Jul 24, 2024 · 4 comments

Comments

@nemesifier
Copy link
Member

nemesifier commented Jul 24, 2024

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.

@okraits
Copy link
Member

okraits commented Aug 12, 2024

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

@nemesifier
Copy link
Member Author

nemesifier commented Aug 12, 2024

@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 nemesifier changed the title [feature] Keep track of changes with django-reversion [feature] REST API should store revisions with django-reversion Aug 12, 2024
@nemesifier nemesifier moved this to To do (general) in OpenWISP Contributor's Board Aug 21, 2024
@dee077
Copy link
Contributor

dee077 commented Jan 13, 2025

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.

@pandafy
Copy link
Member

pandafy commented Jan 24, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: To do (general)
Development

No branches or pull requests

4 participants