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

Changes logged for JSON-field aren't valid JSON #476

Closed
lukaszett opened this issue Dec 15, 2022 · 3 comments · Fixed by #489
Closed

Changes logged for JSON-field aren't valid JSON #476

lukaszett opened this issue Dec 15, 2022 · 3 comments · Fixed by #489

Comments

@lukaszett
Copy link

Hello everyone,

suppose I have the following model:

from django.db import models

from auditlog.registry import auditlog

class Foo(models.Model):
    bar = models.JSONField()


auditlog.register(Foo)

Now I create a new instance and check for the last LogEntry:

Foo.objects.create(bar={"baz":1})
entry = LogEntry.objects.latest()
bar_changes = entry.changes_dict.get("bar")
new_bar = bar_changes[1]
print(new_bar)

This outputs:

{'baz': 1}

I would expect the log entries for JsonFields to be a valid string to feed into json.loads however, the double quotes around the keys get converted into single quotes which is not allowed for json. I did some digging into where this conversion happens but I didn't really come to a conclusion.

Is this the desired behaviour or is it a bug?

@hramezani
Copy link
Member

Hello @lukaszett,

Thanks for reporting this issue.

I think it's because we don't dumps the JSONField value to when we make the diff.

value = field.to_python(getattr(obj, field.name, None))

Probably it can be fixed by changing it like

value = json.dumps(field.to_python(getattr(obj, field.name, None)))

Would you like to work on it @lukaszett ?

@CleitonDeLima
Copy link
Contributor

CleitonDeLima commented Dec 29, 2022

Can i do a PR of this @hramezani?

@hramezani
Copy link
Member

@CleitonDeLima sure!
But first of all try to reproduce the problem on master. I guess the problem might be fixed by the breaking chane PR that got merged yesterday.

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

Successfully merging a pull request may close this issue.

3 participants