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

Make log entries read-only in the admin #449

Merged
merged 1 commit into from
Nov 21, 2022

Conversation

aleh-rymasheuski
Copy link
Contributor

This is a follow-up fix after a real-world staff user deleted a few auditlog entries through the admin by mistake.

@hramezani
Copy link
Member

Thanks @alieh-rymasheuski for this. two things:

  • I think it's a breaking change. I think we have to make it configurable or mention it as a breaking change.(note: next release will be a 3.0 which is a major release, so we can consider this one as well.
  • Test are failing please adopt them

@aleh-rymasheuski
Copy link
Contributor Author

I think it's a breaking change

I think it's a bugfix. This is audit log, it shouldn't be possible to alter it with a simple button in the admin.

@hramezani
Copy link
Member

@alieh-rymasheuski Do you have a plant to fix the failing test?

@aleh-rymasheuski
Copy link
Contributor Author

@hramezani, done. :)

@hramezani hramezani merged commit 1ba3bd9 into jazzband:master Nov 21, 2022
@madomdy
Copy link

madomdy commented Jan 3, 2023

I'm afraid with that, we can't delete any record from django admin. 😬😄

@aleh-rymasheuski
Copy link
Contributor Author

@madomdy, do you mean that this change causes django admin to disallow deleting auditlogged objects? I am confident that some objects still can be deleted - in fact, I removed one this morning. 😄

Could you please elaborate on your issue? I'm happy to help and fix the bug that I may have introduced.

image

@madomdy
Copy link

madomdy commented Jan 4, 2023

@alieh-rymasheuski happily! Here is the thing:
image (11)

And this happens on whatever object of whatever model connected to the audit log. I am just a bit confused how come others do not encounter it.

@aleh-rymasheuski
Copy link
Contributor Author

@madomdy, I registered an issue to continue this discussion there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants