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

prevent tampering with timestamps #17

Open
set5think opened this issue Jun 19, 2014 · 3 comments
Open

prevent tampering with timestamps #17

set5think opened this issue Jun 19, 2014 · 3 comments

Comments

@set5think
Copy link

This has caused me a great deal of pain in the last week (was thinking it was a limitation on the Zuora side):

https://github.com/sportngin/active_zuora/blob/master/lib/active_zuora/fields/date_time_field.rb#L11-14

It should just use %Y-%m-%dT%H:%M:%S-08:00. This will add the 00:00:00 Timestamp even if one doesn't exist, but at least it won't overwrite timestamps if they're provided.

Also, usage startdatetime and enddatetime are good examples of user-set attributes that are not just dates, but a full timestamp.

I can submit a patch, but there are no tests for that stuff, so I will spend some time over the weekend adding a test for that, hopefullly.

@anfleene
Copy link
Contributor

Sorry this caused you trouble. I agree We shouldn't be removing that on fields that are legitimately datetimes. I think we ran into some issues with timezones that converted dates to the wrong day which led to this, but regardless we shouldn't be stripping it off of fields that are actually datetimes.

Thanks for the help,
Andy

@set5think
Copy link
Author

@anfleene Yea, I can empathize with the timezone issue; time is never a fun issue, especially when a service (e.g. Zuora in this case) makes you bound to a specific timezone.

I think the onus on determining which fields are actually datetime is on the app that's assigning the value to the field. If it's a date, pass it a date; if it's a datetime, pass it a datetime. Active-Zuora should just guarantee the final format it should be in. I don't think Zuora has any other type other than datetime to represent dates and/or times. So, it seems clear that the app is the only reasonable place to decide this.

In terms of timezone, dealing with time transformation is a business-level decision in this case; so I'm partial to the idea of the app that's responsible for making the right decision. I definitely have concerns with Active-Zuora explicitly making it PST, but I think it's the lesser of two evils. Though, it may make sense to make that configurable, or something like allow_transformation as a configuration option.

Anyway, step one is to resolve the timestamp overwrite, then timezone stuff is the next step, and a different beast. :).

@claritee
Copy link

I've committed something in my fork a while ago to handle the timezone issue (was giving us a few issues when generating invoices - duplicate invoices being generated)

claritee@ecce717

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

3 participants