-
Notifications
You must be signed in to change notification settings - Fork 7
Adds delete revision option for admin
#56
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
Adds delete revision option for admin
#56
Conversation
lonvia
left a comment
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.
A general thought: are there anywhere places in the code where we assume that revision numbers for each model are continuous?
| if not revision: | ||
| models = Model.objects.filter(model_id=model_id) | ||
| else: | ||
| models = Model.objects.filter(model_id=model_id, revision=revision) |
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.
I see a few corner cases here:
-
if the latest revision is deleted, then the previous one needs to become latest.
-
If the last available revision is deleted, the model is gone. If it wasn't the last revision it is still there.
Are these covered through automatic updates? Can we maybe have tests for that?
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.
No I missed these, I will overwrite the model's delete method to handle latest and write tests to ensure.
Most probably not. I have not seen any. |
instead of the whole model folder
Closes #54
This PR introduces
Delete Revisionfeature when the admin is on page/model/<model_id>/<revision>; and continues showingDelete Modeloption when the admin is on page/model/<model_id>instead.