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

Remove the local and gmt columns from the Date field #693

Closed
brendo opened this issue Jun 26, 2011 · 18 comments
Closed

Remove the local and gmt columns from the Date field #693

brendo opened this issue Jun 26, 2011 · 18 comments
Milestone

Comments

@brendo
Copy link
Member

brendo commented Jun 26, 2011

Look at removing the local and gmt columns. These columns at the moment seem to actually hinder the field due to the INT datalimits. This places a limit on the dates that can be used by the field, with the lower limit being December 1901.

What's interesting is that the full date is stored successfully in a VARCHAR as a complete datetime format, ie. 0800-06-15T00:00:00+10:00 and now due to the updates to the DateTimeObj class, it can handle parsing these string dates rather than the integer dates.

At the moment it looks the only benefit is the sorting can be done easily with INT.

It would be nice to move to DATETIME fields, but we then lose the timezone of the date saved, which I think is quite important (if anyone has any insight into this, please speak up!). Swapping to DATETIME will allow sorting to be natural and well, it just seems right that it uses the correct column type.

Probably not enough time for 2.2.2, but this should definitely be looked at thoroughly for 2.3.

brendo added a commit that referenced this issue Jun 26, 2011
…ering. This allows any date to now be used instead of the previous limit of 1901 - 2038. RE: #693
@brendo
Copy link
Member Author

brendo commented Jun 26, 2011

Ok with those two commits the local and gmt columns are not used for anything, essentially the can be deleted and the field will continue to work. Are they used elsewhere by anything else?

They seem utterly useless to me, in my case both local and gmt have the same value, which doesn't seem correct considering I'm +10:00. They actually seem to have more harm then good by imposing the range limits (due to the limits of INT).

@brendo
Copy link
Member Author

brendo commented Jun 27, 2011

Well, in my testing, local and gmt will never have the same value as the current logic is flawed. The processRawFieldData function generates a local ('2011-06-01T00:00:00+10:00') and GMT timestamp ('2011-05-31T14:00:00+00:00') which then gets pushed through strtotime. The thing is, the value will always be the same as the seconds from 1970 will be constant, it's just the actual timezone offset that will display the information differently.

Removing the local and gmt columns in an upcoming commit for 2.2.2

@brendo
Copy link
Member Author

brendo commented Jun 27, 2011

Reverted for now, would like to do some more serious testing around dates in multiple locales. General consensus from some research I have done is that the database should be storing UTC values, and then the frontend apply the timezone. In Symphony, the region configuration setting is used to saved dates, so they are stored with a timezone offset.

I know a large project I worked on last year had some issues with timezones when storing transmission times for TV shows and then formatting the tv guide for different locales. I'll dig into the solutions used there, but I have a feeling it revolved around storing UTC dates as well.

CC: @nilshoerrmann

@nilshoerrmann
Copy link
Contributor

Brendan, which parts need special testing?

@brendo
Copy link
Member Author

brendo commented Jun 28, 2011

Just generally everything needs testing, I've completed all the tests outlined in here but I'm just after some real world, testing not done by me ;)

The /cc was to see if you had any opinions/thoughts on storing Timezones. From what I can tell, Date/Time stores dates as UTC (datetime column) and the uses DateTime->format() to get the result. Does this use the region setting from the config at all?

@nilshoerrmann
Copy link
Contributor

I'm just after some real world, testing not done by me

So how do you call your world then? Brendonia? :)

The /cc was to see if you had any opinions/thoughts on storing Timezones.

I've actually never used the timezone feature because all dates handled by Symphony were meant to be in the site's main timezone (so even if I were in New York, logged into my site to enter a date, this date was meant to be Berlin time). I can think of different scenarios where the opposite is needed and timezone need to be respected, but Symphony was never capable to handle this because it won't display the timezone in the interface.

@brendo
Copy link
Member Author

brendo commented Jun 28, 2011

So how do you call your world then? Brendonia? :)

At the time of testing it was a beer in one hand, the footy on the TV, metal from the stereo and a laptop on my lap, so a distracting world ;)

I've actually never used the timezone feature because all dates handled by Symphony were meant to be in the site's main timezone (so even if I were in New York, logged into my site to enter a date, this date was meant to be Berlin time)

Yep that's correct, that's currently how Symphony functions. I think our issue was that we had one Symphony install and the content was being distributed into multiple regions and had to be localised for these regions. It made for some confusing results when trying to sort all German dates by a range to display in Brisbane. ie. Show all entries between 3am and 6am in Brisbane first had to make the German date UTC and then apply the timezone for the filtering.

I think you're correct though, it's fairly edge case and probably is not of immediate concern for 2.2.2. It could potentially be confusing if the timezone in the configuration is changed halfway through a site's lifetime.

For 2.3 I'd like to explore the Date/Time approach and store dates as UTC and apply the timezone at the UI level. I'm interested to see how this works with the situation described above.

@nilshoerrmann
Copy link
Contributor

I've got a few questions:

  • Regarding the timezone: wouldn't it be an idea to add a timezone setting to the field? This could be a single or multiple select which is used to convert the stored date to the needed timezone(s) for data source output.
  • Would it be an idea to reintroduce the calendar overlay known from Symphony 1.7 (with updated styles of course)? Or do we leave this to extensions like Date and Time or – uhm – Calendar Overlay?
  • Would it be an idea to add a setting to only display the time or to only show the date (Y-M-D) in the backend? Or do we need a special time field for this? The date field always displays both, date and time ...

@brendo
Copy link
Member Author

brendo commented Jun 30, 2011

Regarding the timezone: wouldn't it be an idea to add a timezone setting to the field? This could be a single or multiple select which is used to convert the stored date to the needed timezone(s) for data source output.

I think that's introducing too much unneeded complexity to be honest. Timezones is a fickle beast, and something that fortunately nobody has seemed to have any issues with, so I'd prefer to leave it alone and just default to the site's region (which is what happens now).

Would it be an idea to reintroduce the calendar overlay known from Symphony 1.7 (with updated styles of course)? Or do we leave this to extensions like Date and Time or – uhm – Calendar Overlay?

I think so, I'd be curious to @allen's input on why it was removed in the first place though. I have a feeling it might of been one of those 'keep it simple, if people want a overlay they'll create it' and well, that came true

Would it be an idea to add a setting to only display the time or to only show the date (Y-M-D) in the backend? Or do we need a special time field for this? The date field always displays both, date and time...

Perhaps not as free text, but maybe an option to hide/show the time which just chooses to use the site's __SYM_DATE_FORMAT__ settings or the combined __SYM_DATETIME_FORMAT__. Same deal as before, I'd like to keep things simple and minimum.

A Time field is interesting, but I'm not sure how this would work? I can't think of a use case where you would just say '8:00am' and not have it tied to a date. It'd be the same as Text Input field wouldn't it?

@allen
Copy link
Contributor

allen commented Jun 30, 2011

I think so, I'd be curious to @allen's input on why it was removed in the first place though. I have a feeling it might of been one of those 'keep it simple, if people want a overlay they'll create it' and well, that came true

Scott was the one that made the original calendar. Since the controls only had the ability to flip between next and previous months, it was hard to use when you had to put in say someone's birthday. For example for my birthday, I'd have to flip up to 360 times to get to 1981. Scott decided to update the calendar but didn't have enough time to do so since he had bigger fish to fry, like the rest of the Symphony's interface.

Like what Brendan said, we opted to keep it simple but the intention was to introduce Scott's new calendar overlay as an extension at a later date. Then the switch to jQuery happened--Symphony used to run on Scott's own JS framework--which prolonged the calendar. Once jQuery was introduced, it made it possible for others to introduce their own calendar systems and the idea of doing our own became less important.

@nilshoerrmann
Copy link
Contributor

Perhaps not as free text, but maybe an option to hide/show the time which just chooses to use the site's __SYM_DATE_FORMAT__ settings or the combined __SYM_DATETIME_FORMAT__. Same deal as before, I'd like to keep things simple and minimum.

I thought of three options:

  • __SYM_DATETIME_FORMAT__
  • __SYM_DATE_FORMAT__
  • __SYM_TIME_FORMAT__

Like what Brendan said, we opted to keep it simple but the intention was to introduce Scott's new calendar overlay as an extension at a later date.

Right, I perfectly understand that. Maybe I have to rephrase my question: We have two Symphony-like calendar interfaces – the one used by Rowan's Calendar Overlay the other provided by my Date and Time field. Do we want to provide a unified interface that can be used by these extensions? (It doesn't have to be used by the date field, but it could be provided with the core – CSS and JavaScript already exist.)

A Time field is interesting, but I'm not sure how this would work? I can't think of a use case where you would just say '8:00am' and not have it tied to a date. It'd be the same as Text Input field wouldn't it?

We often have the need to provide a section with variable business hours on our clients websites. This results in two kind of inputs:

  • Monday, 8:00 to Monday, 16:00
  • 8:00 to 16:00

We are using plain text inputs inside a Subsection Manager field. The problem is that we don't have any time filtering unless we use a date field – but a date field doesn't make sense for dates repeating weekly or daily because seeing an explicit date in the backend when entering your data is counter intuitive.


While I like the idea of simplicity, I never understood why the core fields are that limited.

@allen
Copy link
Contributor

allen commented Jun 30, 2011

Do we want to provide a unified interface that can be used by these extensions?

I'm always in favour of consistency, especially when it comes to user interface concerns. I think it will ultimately be up to whether or not there is a way to unify the UI of the two existing extensions or if the UI of each of these are actually solving different problems in their own way.

@rowan-lewis do you have any thoughts on this?

@brendo
Copy link
Member Author

brendo commented Jun 30, 2011

That could work Nils! The field setting would then modify the interface and the filtering.

I like the idea of a unified interface, how did we get on with using the Date.js library? Although it is nice, it's old and from memory has some horrible bugs. Does DateTime still use it?

@nilshoerrmann
Copy link
Contributor

Date.js was quite buggy when it came to localised dates. As of version 2.0, Date and Time doesn't use it anymore: it's now completely relying on PHP's DateTime functions using AJAX to validate the input.

@michael-e
Copy link
Member

The biggest problem to me is date filtering. There have been several changes, so with the current integration code some filtering syntax from 2.1.2 is not working anymore. (e.g. "later than" used to take the date AND the time into account).

I have no idea about the current syntax. How can I filter using the current time?

We really need some specs for this.

@brendo
Copy link
Member Author

brendo commented Jul 20, 2011

Does this help @michael-e? https://github.com/symphonycms/symphony-2/wiki/DateField

I agree with the 'problem' though. Fields often have to duplicate logic rather then inherit it, we should look at ways to remove this annoyance.

@michael-e
Copy link
Member

That wiki page helps a lot! Thanks, @brendo!

@brendo
Copy link
Member Author

brendo commented Mar 2, 2012

Right, time to do something about this issue:

  1. Add a new DATETIME column to any Date field tables, aptly named date. Create an index.
  2. Fill this column with the date derived from the gmt column
  3. Drop the gmt and local columns
  4. Change filtering to use date column (done)
  5. Make field save to auto populate this column (done)

The result is that we'll have the value column which stores the complete date time (including Timezone) and we'll have the GMT date stored as a native DATETIME. Filtering will then use the date (converting the filter input to GMT first) which should result in quicker query times (as we aren't invoking DATE_FORMAT anymore) and just because it makes sense.

4 & 5 are done, it's the migration that needs writing.

@brendo brendo closed this as completed in e73c5d6 Mar 3, 2012
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

No branches or pull requests

4 participants