Show analytics table data Date column in user's local time zone #2273
Conversation
dashboard/client/charts/charts.js
Outdated
| let localTime = moment.utc(e.fields.request_at[0]).toISOString(); | ||
| localTime = moment.utc(localTime).toDate(); | ||
|
|
||
| try { time = localTime; } catch (err) { time = ''; } |
There was a problem hiding this comment.
Don't use try/catch for conditional logic. Use if/else, or something similar.
Try/catch are for error handling.
There was a problem hiding this comment.
The complete table is populated using try .. catch logic. So i did not alter the code. Let me know if i need to change it to if--else logic.
dashboard/client/charts/charts.js
Outdated
| let localTime = moment.utc(e.fields.request_at[0]).toISOString(); | ||
| localTime = moment.utc(localTime).toDate(); | ||
|
|
||
| try { time = localTime; } catch (err) { time = ''; } |
There was a problem hiding this comment.
Add comments describing what this code is intended to do. Why is it written?
There was a problem hiding this comment.
Sorry i should have added comments.
| const UsernameRegEx = /^(?!\d)(?!.*-.*-)(?!.*-$)(?!-)[a-zA-Z0-9-]{3,15}$/; | ||
| // Username must be 3-15 alphanumeric string combinations with hyphens and underscore allowed | ||
| // Username cannot begin with a hypen , underscore | ||
| const UsernameRegEx = /^(?!-)(?!_)[a-zA-Z0-9-_]{3,15}$/; |
There was a problem hiding this comment.
This belongs in a different pull request, and should not be part of this review.
|
This looks alright. We can ignore the |
|
Please resolve the conflicts, and I will merge. |
|
Changes look good. @snittoor please resolve the conflicts. |
Displays analytics table data Date column in user's local time zone.
#2183