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

Fixes #11295 Sync dates compensate for timezone #5400

Merged
merged 1 commit into from
Aug 7, 2015
Merged

Fixes #11295 Sync dates compensate for timezone #5400

merged 1 commit into from
Aug 7, 2015

Conversation

cpeters
Copy link
Contributor

@cpeters cpeters commented Aug 5, 2015

To ensure the correct date is saved per the user's input
adjust the date to account for timezone difference

@ehelms
Copy link
Member

ehelms commented Aug 5, 2015

In the original version, what is actually sent to the server? Does that include timezone information that isn't being correctly handled by the server?

@cpeters
Copy link
Contributor Author

cpeters commented Aug 5, 2015

The sync date is composed in two parts; the desired date and then the desired time. When the user enters the desired date it is captured with a time of 00:00:00. This date is then passed to a constructor that offsets the date according to the timezone and therefore may actually push the date backwards.

@ehelms
Copy link
Member

ehelms commented Aug 5, 2015

Do you think we could craft a test to ensure this outputs properly in the future?

@@ -44,7 +44,8 @@ angular.module('Bastion.sync-plans').controller('NewSyncPlanController',
}

$scope.createSyncPlan = function (syncPlan) {
var syncDate = new Date(syncPlan.startDate),
var syncDate = new Date(syncPlan.startDate.getTime() +
syncPlan.startDate.getTimezoneOffset() * 60000),
Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to create a variable (or better yet a const) for the 60,000 so it is immediately clear what it represents, i.e.: const HOURS_IN_DAY = 24;

@waldenraines
Copy link
Contributor

Thanks for the PR and good catch.

Do you think we could craft a test to ensure this outputs properly in the future?

Agreed, you will have to fix the existing test(s) anyway so you should also ensure this is tested.

When committing code that alters bastion or bastion_katello it is good practice to run grunt ci before committing. That way all of the tests and code quality checks are run.

See http://www.katello.org/developers/bastion/#contributing

To ensure the correct date is saved per the user's input
adjust the date to account for timezone difference
@waldenraines
Copy link
Contributor

ACK

waldenraines pushed a commit that referenced this pull request Aug 7, 2015
Fixes #11295 Sync dates compensate for timezone
@waldenraines waldenraines merged commit efa0c53 into Katello:master Aug 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants