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

Most recent with excluded fields #477

Merged
merged 8 commits into from
Dec 24, 2018
Merged

Most recent with excluded fields #477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
028ba91
Added changes in manager to compensate for excluded fields while feet…
Alig1493 Nov 7, 2018
829185d
Requirement changes.
Alig1493 Nov 11, 2018
7564885
hot fix
Alig1493 Nov 11, 2018
fd1a30b
Formatted test_models file
Alig1493 Dec 9, 2018
e28710a
Refactored exclude fields to another method.
Alig1493 Dec 12, 2018
b7df921
Requested changes for excluded fields
Alig1493 Dec 19, 2018
db8714f
Getting history excluded fields attribute if it exists
Alig1493 Dec 20, 2018
aa63579
changed nested ifs
Alig1493 Dec 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions simple_history/manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ def get_queryset(self):
key_name = self.instance._meta.pk.name
return self.get_super_queryset().filter(**{key_name: self.instance.pk})

def get_excluded_fields(self):
return getattr(self.model, "_history_excluded_fields", [])

def most_recent(self):
"""
Returns the most recent copy of the instance available in the history.
Expand All @@ -45,7 +48,11 @@ def most_recent(self):
)
)
tmp = []
excluded_fields = self.get_excluded_fields()

for field in self.instance._meta.fields:
if field.name in excluded_fields:
continue
if isinstance(field, models.ForeignKey):
tmp.append(field.name + "_id")
else:
Expand Down
6 changes: 6 additions & 0 deletions simple_history/tests/tests/test_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,12 @@ def test_model_with_excluded_fields(self):
self.assertIn("question", all_fields_names)
self.assertNotIn("pub_date", all_fields_names)

most_recent = p.history.most_recent()
self.assertIn("question", all_fields_names)
self.assertNotIn("pub_date", all_fields_names)
self.assertEqual(most_recent.__class__, PollWithExcludeFields)
self.assertIn("pub_date", history._history_excluded_fields)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we also get a test case that ensures that the excluded fields are indeed excluded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kseever the test here is already checking whether the field is present in the fields of the history model so I think it's already being covered. I have added another statement checking if the field is present in the excluded fields of the history model. It might be that I may have failed to understand your requirement so please feel free to elaborate. =)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, I just noticed that the assertion I was looking for already exists on line 403 - confirming that pub_date isn't in the list of returned fields. Nevermind me!

def test_user_model_override(self):
user1 = User.objects.create_user("user1", "1@example.com")
user2 = User.objects.create_user("user2", "1@example.com")
Expand Down