Skip to content
This repository has been archived by the owner on May 16, 2020. It is now read-only.

189 #191

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

189 #191

wants to merge 5 commits into from

Conversation

kracekumar
Copy link
Contributor

Fixed #189 . This error was due double parsing of time field, one during form rendering and another in template filter.

@jace
Copy link
Member

jace commented May 8, 2013

Time should always be stored in UTC. From this patch it appears time is not being stored in UTC.

@kracekumar
Copy link
Contributor Author

Somewhere we went wrong, I think we should create new column for datetime timezone field to store timezone and convert the datetime to UTC. While displaying convert UTC to respective timezone. Alembic migration need to address existing data. Does that sound right ?

@jace
Copy link
Member

jace commented May 8, 2013

Dates should always be stored in UTC. We used to do that earlier. Can you
look at current data in db?

Kiran

Kiran Jonnalagadda
+91-99452-35123
http://hasgeek.com/

(Sent from my phone)
On May 8, 2013 10:01 AM, "kracekumar" [email protected] wrote:

Somewhere we went wrong, I think we should create new column for datetime
timezone field to store timezone and convert the datetime to UTC. While
displaying convert UTC to respective timezone. Alembic migration need to
address existing data. Does that sound right ?


Reply to this email directly or view it on GitHubhttps://github.com//pull/191#issuecomment-17586101
.

@kracekumar
Copy link
Contributor Author

I looked into DB.

jsFoo 2012 hacknight start time is 12.00PM (http://hacknight.in/jsfoo/2012#/participants).

DB Value 2012-10-13 06:30:00, IST is + 5.30 UTC. While editing the form start datetime is 2012-10-13 6:30am.

Same for other events.

What user enters is not what user sees. Looking into the commits of coaster and Hacknight

@jace
Copy link
Member

jace commented May 8, 2013

So the form edit isn't setting timezone. You have to set form.start_datetime.timezone = app.config['TIMEZONE'] in the view (similarly for end_datetime).

@kracekumar
Copy link
Contributor Author

Yes, /edit endpoint needs a fix. /new also faces same issue. This is because https://github.com/hasgeek/baseframe/blob/master/baseframe/forms.py#L188 fails. Assigning timezone value to form doesn't work. One way to solve the issue is to have validate_start_datetime.

@kracekumar
Copy link
Contributor Author

Now datetime is displayed properly.



@app.template_filter('weekdate')
def weekdate(date):
return utc.localize(date).astimezone(app.config['tz']).strftime("%a, %b %e")
return utc.localize(date).strftime("%a, %b %e")
Copy link
Member

Choose a reason for hiding this comment

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

This is can't be right. If the date is stored in UTC, you need to convert to the correct timezone when displaying or it will display in UTC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, something is still wrong, converting to tz puts back same behaviour.

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

Successfully merging this pull request may close these issues.

Time Field Error
2 participants