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

Membership expiration #1513

Merged
merged 9 commits into from
Apr 20, 2016
Merged

Membership expiration #1513

merged 9 commits into from
Apr 20, 2016

Conversation

nornes
Copy link

@nornes nornes commented Apr 13, 2016

Fix #1400
Sets the expiration date for membership-applications where estimated expiry is in the past to upcoming September.

new_expiry_date = datetime.date(
started_year, 9, 16) + datetime.timedelta(days=365*length_of_fos)
# Expiry dates in the past sets the expiry date to next september
if new_expiry_date < datetime.date.today():
Copy link
Member

Choose a reason for hiding this comment

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

Use django.utils.timezone instead of datetime.datetime. :-) That takes care of all timezone stuff (and DST/Summer time).

Copy link
Author

Choose a reason for hiding this comment

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

I can see how that is better on a general basis, but is it really necessary here when it's only dealing with dates? Even if it gets set off by one day due to time-differences that wouldn't be that bad, would it?

Copy link
Member

Choose a reason for hiding this comment

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

https://docs.djangoproject.com/en/1.9/topics/i18n/timezones/#naive-and-aware-datetime-objects

This matters mostly for datetime objects, so you're right in the fact that it's less important for date. Anywho, it's better practice to do today = timzone.now().date.today() and work with today rather than doing datetime.date.today() every time you need the variable (the same goes for datetime.date.today() for that matter). :-)

So yes, I was a bit too quick to respond in this case.

@nornes nornes changed the title Membership expiration (WIP) Membership expiration Apr 18, 2016
@nornes nornes changed the title (WIP) Membership expiration Membership expiration Apr 20, 2016
@sklirg sklirg merged commit bba4b49 into develop Apr 20, 2016
@sklirg sklirg deleted the membership-expiration branch April 20, 2016 18:44
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.

2 participants