Skip to content
This repository has been archived by the owner on Nov 2, 2019. It is now read-only.

Converting Date/Time to EST #200

Merged
merged 38 commits into from
Apr 2, 2018
Merged

Converting Date/Time to EST #200

merged 38 commits into from
Apr 2, 2018

Conversation

ashley
Copy link
Member

@ashley ashley commented Mar 28, 2018

For Issue #170

  • Created functions in preProcess to reformat dates and times to EST.
  • Implemented this function to created and modified fields for all forms (including default controller)

Question: preProcess.prettifyDate() can be replaced with moment functions. Should I refactored that?

@ashley ashley requested a review from ashleytqy March 28, 2018 23:53
app/index.html Outdated
@@ -27,9 +27,14 @@
<script src="/js/lib/chosen-angular.js"></script>
<script src="//cdn.tinymce.com/4/tinymce.min.js"></script>
<script src="/js/lib/angular-ui-tinymce.js"></script>

<script src="https://cdnjs.cloudflare.com/ajax/libs/moment-timezone/0.5.11/moment-timezone.js"></script>
Copy link
Contributor

@ashleytqy ashleytqy Mar 29, 2018

Choose a reason for hiding this comment

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

should moment-timezone be part of user-defined libraries, since you're including it in app/js/lib/moment-timezone.min.js

Copy link
Contributor

Choose a reason for hiding this comment

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

also, would https://github.com/urish/angular-moment be more suitable?

@@ -22,6 +22,8 @@ angular
}

_.each($scope.data, element => {
element = preProcess.changeDate(element);
element.attributes.recievedOn = preProcess.convertTimeToEST(element.attributes.recievedOn);
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this necessary / not covered in preProcess.changeDate(element)?

Copy link
Member Author

Choose a reason for hiding this comment

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

changeDate() is a helper function that called convertTimeToEST() except it only calls it on the element's create and modified attributes. Since some pages have other fields that require the conversion, you can use convertTimeToEST() on them in their controller.

hour -= 12;
night = 'PM';
}
//return this.prettifyDate(time) + " " + hour + ":" + minute + night;
Copy link
Contributor

@ashleytqy ashleytqy Mar 29, 2018

Choose a reason for hiding this comment

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

remove comment!

night = 'PM';
}
//return this.prettifyDate(time) + " " + hour + ":" + minute + night;
return moment(time).tz('America/New_York').format('LLLL');;
Copy link
Contributor

Choose a reason for hiding this comment

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

can we do October 1, 2014 12:00 AM (Wednesday) instead of Wednesday, October 1, 2014 12:00 AM?

@ashleytqy
Copy link
Contributor

Question: preProcess.prettifyDate() can be replaced with moment functions. Should I refactored that?

Sure, go ahead!

@@ -22,7 +22,7 @@ angular
$scope.model = _.find($scope.data, {id: resourceId});
}
_.each($scope.data, function(element) {
element = preProcess.changeDate(element);
element = preProcess.convertTimeAtrtributes(element);
Copy link
Contributor

Choose a reason for hiding this comment

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

spelling -> Attributes

.then(function(list){
list.forEach(function(element) {
objectIdToNameHash[element.id] = element.attributes.name;
.then(function(element){
Copy link
Contributor

@ashleytqy ashleytqy Mar 30, 2018

Choose a reason for hiding this comment

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

can you keep it the way it was before for line 18 to 20?

@@ -22,6 +22,7 @@ angular
}

_.each($scope.data, element => {
element = preProcess.convertTimeAtrtributes(element, Array("recievedOn"));
Copy link
Contributor

@ashleytqy ashleytqy Mar 30, 2018

Choose a reason for hiding this comment

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

since you are using ...attributeArray, "recievedOn" doesn't need to be in an array. u could do element = preProcess.convertTimeAttributes(element, "recievedOn"), unless you want to change the signature of the function

var day = parseInt(date.substring(8, 10));
return monthNames[month - 1] + " " + day + ", "+ year;
},
convertTimeAtrtributes: function(element, ...attributeArray){
Copy link
Contributor

Choose a reason for hiding this comment

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

rmb to update spelling for all of the places u r using convertTimeAtrtributes -> convertTimeAttributes

Copy link
Contributor

@ashleytqy ashleytqy left a comment

Choose a reason for hiding this comment

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

🚢💯🚀

@ashleytqy ashleytqy merged commit 969fb61 into develop Apr 2, 2018
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.

2 participants