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.

Sign up for GitHub

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

Jump to bottom

Reorder condition in get_field_value function #652

Closed
wants to merge 6 commits into from
Closed

Reorder condition in get_field_value function #652

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
96d3906
Reorder condition in get_field_value function: as many people have no…
GreatBahram Jun 10, 2024
988cc3f
add a test case to make sure rel_class is checked first
GreatBahram Jun 12, 2024
59ad0cb
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 12, 2024
8cda3fa
update the CHANGELOG.md file
GreatBahram Jun 12, 2024
33151f0
use unittest assert
GreatBahram Jun 12, 2024
fbb1bc7
Update CHANGELOG.md
GreatBahram Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

- Fixed a problem when setting `Value(None)` in `JSONField` ([#646](https://github.com/jazzband/django-auditlog/pull/646))
- Fixed a problem when setting `django.db.models.functions.Now()` in `DateTimeField` ([#635](https://github.com/jazzband/django-auditlog/pull/635))
- Changed the order of `rel_class` and `one_to_one` and `many_to_one` in `get_field_value` function. ([#XXX](https://github.com/jazzband/django-auditlog/pull/652))
GreatBahram marked this conversation as resolved.
Show resolved Hide resolved

## 3.0.0 (2024-04-12)

Expand Down
2 changes: 1 addition & 1 deletion auditlog/diff.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def get_field_value(obj, field):
value = json.dumps(value, sort_keys=True, cls=field.encoder)
except TypeError:
pass
elif (field.one_to_one or field.many_to_one) and hasattr(field, "rel_class"):
elif hasattr(field, "rel_class") and (field.one_to_one or field.many_to_one):
value = smart_str(
getattr(obj, field.get_attname(), None), strings_only=True
)
Expand Down
15 changes: 14 additions & 1 deletion auditlog_tests/tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
from auditlog.admin import LogEntryAdmin
from auditlog.cid import get_cid
from auditlog.context import disable_auditlog, set_actor
from auditlog.diff import model_instance_diff
from auditlog.diff import get_field_value, model_instance_diff
from auditlog.middleware import AuditlogMiddleware
from auditlog.models import DEFAULT_OBJECT_REPR, LogEntry
from auditlog.registry import AuditlogModelRegistry, AuditLogRegistrationError, auditlog
Expand Down Expand Up @@ -2164,6 +2164,19 @@ def test_log_entry_created_if_obj_strings_are_same_for_two_objs(self):
self.assertEqual(int(log_create.changes_dict["related"][1]), one_simple.id)
self.assertEqual(int(log_update.changes_dict["related"][1]), two_simple.id)

def test_rel_class_checked_first(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to create a model as you described in the PR description and test with a real model instead of mocking

mock_field = mock.Mock()

type(mock_field).rel_class = mock.PropertyMock(return_value=None)
type(mock_field).one_to_one = mock.PropertyMock(return_value=True)
type(mock_field).many_to_one = mock.PropertyMock(return_value=True)

mock_obj = mock.Mock()

get_field_value(mock_obj, mock_field)

self.assertEqual("rel_class", mock_field.method_calls[0])


class TestModelSerialization(TestCase):
def setUp(self):
Expand Down
Loading