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

remove auto_add=now from last-modified field #7758

Merged
merged 3 commits into from
Nov 7, 2020
Merged

remove auto_add=now from last-modified field #7758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aecbfe4
remove auto_add=now from last-modified field
hhio618 Oct 25, 2020
feccf50
uncomment test_ingest_community_events_calender.py
hhio618 Oct 25, 2020
af9f064
Merge branch 'master' into master
danlipert Nov 4, 2020
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
2 changes: 1 addition & 1 deletion app/marketing/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ class UpcomingDate(SuperModel):
"""Define the upcoming date model"""
# These fields are meant to use for update UpcomingDate based on the icalendar updates
uid = models.CharField(max_length=255, null=True, blank=True)
last_modified = models.DateTimeField(db_index=True, auto_now=True)
last_modified = models.DateTimeField(db_index=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason for this? to the best of my knowledge, removing this means this field would no longer be updated when a record is changed.

If you're doing this because of failing tests, what you want to do is a unittest.mock

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This field needs to be filled manually from an input iCalendar file. I had added a auto_now=True to it accidentally. So yes definitely this attribute need to be deleted in my opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This field is different from modified_on that is defined in SuperModel and tracks last modification time of the object.

Copy link
Contributor

Choose a reason for hiding this comment

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

since we already have modified_on then we dont need those one as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As may you already knew, this field is related to the last_modified field of an actual event modification date in an iCal file @danlipert. I used this field to check if some event needs to get updated.

If I remove this field and use modified_on then there will be no modification date of actual db record, Is that ok?

sequence = models.SmallIntegerField(default=0)

title = models.CharField(max_length=255)
Expand Down
2 changes: 0 additions & 2 deletions app/marketing/tests/management/commands/test_ingest_community_events_calender.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ def parse_ical_file(self, fname):
with open(os.path.join(os.path.dirname(os.path.realpath(__file__)), fname)) as f:
return Calendar.from_ical(f.read())

'''
# These Tests don't actually work - DL
class TestIngestCommunityEvents(TestCase):
"""Define tests for ingest community events. """
Expand Down Expand Up @@ -96,4 +95,3 @@ def test_handle_update(self):
def tearDown(self):
calendar, event = load_icalendar("test_ics_files/update.ics")
UpcomingDate.objects.filter(title=event.get('summary')).delete()
'''