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

Fix comment's created at wrong timezone #460

Merged
merged 5 commits into from
Jan 10, 2017

Conversation

beagleknight
Copy link
Contributor

🎩 What? Why?

Comment component was rendering the createdAt with an incorrect timezone.

📌 Related Issues

📋 Subtasks

None

📷 Screenshots (optional)

None

👻 GIF

@mention-bot
Copy link

@beagleknight, thanks for your PR! By analyzing the history of the files in this pull request, we identified @josepjaume to be a potential reviewer.

@@ -12,7 +12,7 @@ const generateCommentsData = (num = 1) => {
commentsData.push({
id: random.uuid(),
body: random.words(),
createdAt: date.past().toString(),
createdAt: Math.floor(date.past().getTime() / 1000),
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd use ISO8901 format instead. Should work seamlessly with moment.parse.

@beagleknight beagleknight force-pushed the fix/comment-created-at-timezone branch from 89f2589 to 5e3c9ca Compare January 9, 2017 15:26
@beagleknight beagleknight self-assigned this Jan 9, 2017
@codecov-io
Copy link

codecov-io commented Jan 9, 2017

Current coverage is 96.93% (diff: 100%)

Merging #460 into master will increase coverage by <.01%

@@             master       #460   diff @@
==========================================
  Files           234        234          
  Lines          3527       3528     +1   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           3419       3420     +1   
  Misses          108        108          
  Partials          0          0          

Powered by Codecov. Last update 6f374ec...2103feb

@beagleknight beagleknight force-pushed the fix/comment-created-at-timezone branch from aedb269 to e16b9be Compare January 10, 2017 08:52
@@ -11,7 +11,9 @@ module Comments
field :body, !types.String, "The comment message"

field :createdAt, !types.String, "The creation date of the comment" do
property :created_at
resolve lambda { |obj, _args, _ctx|
obj.created_at.to_time.iso8601
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need to time? Wouldn't it be better to use seconds from epoch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not really sure... I found that as an example but I think I can use the iso8601 method directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok

@beagleknight beagleknight merged commit 11c1237 into master Jan 10, 2017
@beagleknight beagleknight deleted the fix/comment-created-at-timezone branch January 10, 2017 10:30
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

Successfully merging this pull request may close these issues.

7 participants