djhi / meteor-autoform-materialize

DEPRECATED - Meteor AutoForm Materialize templates
https://atmospherejs.com/mozfet/autoform-materialize
MIT License
47 stars 29 forks source link

pickadate is not persisting the date object in the desired timezone? #102

Closed 0o-de-lally closed 7 years ago

0o-de-lally commented 8 years ago

I would expect that the date persisted to db would also be converted to timezoneID specified. Or is it just for displaying the timezone?

If one specifies a schema with:

  pickadateTimezone: {
    type: Date,
    optional: true,
    autoform: {
      type:"pickadate",
      timezoneId: "America/Los_Angeles"
    }
  },

I would expect the date in the DB to be offset to Los Angeles time, instead I get the browser time.

see reproduction here: https://github.com/keyscores/autoform_materialize_date_timzeone_issue

0o-de-lally commented 8 years ago

@serkandurusoy I think you were the last person to touch the pickadate input type. Any thoughts on this?

serkandurusoy commented 8 years ago

Hm I used that ages ago - in software years :) - and I had a single timezone use case.

But I would expect the persisted date to be whatever the server timezone is (preferably Etc/UTC) and the displayed date to be whatever it corresponds to for the chosen time zone.

0o-de-lally commented 8 years ago

This is where this package diverges from autoform. In the original implementation the UTC time is used by default, https://github.com/aldeed/meteor-autoform#typedate . In this package it's always saving with the browser timezone. Is this on purpose?

serkandurusoy commented 8 years ago

According to the source

https://github.com/djhi/meteor-autoform-materialize/blob/master/inputTypes/pickadate/pickadate.js#L15

and

https://github.com/aldeed/meteor-autoform/blob/16f100ce96a13823b293414f5c615ea67693a39e/inputTypes/value-converters.js#L105

it looks like it works as I described.

Edit: As compared to your expactation from https://github.com/aldeed/meteor-autoform/blob/devel/inputTypes/datetime/datetime.js which does the conversion both ways

0o-de-lally commented 8 years ago

Yes I see now that it is just converting the date for display. However Autoform has date types that use UTC by default. That's a bit confusing. If others would find it useful I would propose that this should at least save it by UTC by default (since servertime is harder to control). And/or optionally it could the timezone always for saving and displaying. Thoughts?

serkandurusoy commented 8 years ago

It would be a breaking change for existing apps at this point, so I'd suggest you to base a new component on this one.

In the meantime, http://yellerapp.com/posts/2015-01-12-the-worst-server-setup-you-can-make.html

Servers absolutely definitely surely inevitably undeniably must be set to Etc/UTC!

If that's not possible, then the app's enviornment should be set as such. All PaaS providers (modulus, heroku, galaxy etc) allow for that, and barebone servers (aws, digital ocean etc) definitely do.

In fact, that' the first thing I do when I set up a server.

Even in my development environment, I provide the necesssary environment variables (which is TZ) to the app server and database server instances regardless of my computer's user timezone.

0o-de-lally commented 8 years ago

So do you know if galaxy is by default on UTC. If it is UTC, then the component is actually using the browser timezone.

serkandurusoy commented 8 years ago

I don't know (I think not) its default but you can set your app instances' timezone using your settings.json. You should set time environment variable TZ to whatever timezone you'd like it to use, but I'd suggest Etc\UTC

0o-de-lally commented 8 years ago

I confirmed that the component is using the browser timezone, rather than server. It returns a pickadate.obj, which is a client Date() object. Is this a feature or a bug that should be changed? It may be breaking for some people but I suspect few people are using local timezones intentionally.

valueOut: function() { var picker = this.pickadate('picker'); var item = picker && picker.get('select'); return item && item.obj; },

serkandurusoy commented 8 years ago

Well, this component has been out and in use for a year now and I am sure there are a lot of people who either consciously or unconsciously rely on how it handles dates. So changing it would probably come at a big surprize and even be damaging for some people.

I believe the way to fix this is not to change this component but either one of:

djhi commented 7 years ago

Future work on this package has been transferred here. Thanks @mozfet