Bug 1739225: Timestamp component always shows dates in the future as relative dates #2341

dtaylor113 · 2019-08-13T16:28:49Z

Updated Timestamp component to only show dates 10.5 minutes in past as relative dates (ex: "2 minutes ago"). Future dates will never be displayed as relative dates, always as full dates.

Before fix, dates in the future always displayed as relative:

After fix, future dates displayed as actual date (minus year if current year):

https://bugzilla.redhat.com/show_bug.cgi

openshift-ci-robot · 2019-08-13T16:28:53Z

@dtaylor113: This pull request references a valid Bugzilla bug. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Bug 1739225: Timestamp component always shows dates in the future as relative dates

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

frontend/public/components/utils/timestamp.tsx

spadgett · 2019-08-13T16:35:58Z

Hey @dtaylor113, this doesn't really fix Bug 1739225. This is more an implementation detail for Chargeback.

The bug is that dates far off in the future should not be relative dates for any Timestamp, even if noRelative is not specified. But we should still add noRelative specifically for the Chargeback case.

dtaylor113 · 2019-08-13T16:38:30Z

The bug is that dates far off in the future should not be relative dates for any Timestamp, even if noRelative is not specified. But we should still add noRelative specifically for the Chargeback case.

Ok, if we define what 'far off in the future' is, I can implement that fix. >9 months in the future?

spadgett · 2019-08-13T16:41:18Z

Ok, if we define what 'far off in the future' is, I can implement that fix. >9 months in the future?

No, it should be the existing value we check which is 10.5 minutes

https://github.com/openshift/console/blob/master/frontend/public/components/utils/timestamp.tsx#L20-L22

We need to check +/- 10.5 minutes instead of assuming all dates more recent than 10.5 minutes are relative.

dtaylor113 · 2019-08-13T17:22:39Z

We need to check +/- 10.5 minutes instead of assuming all dates more recent than 10.5 minutes are relative.

Ok, added a Math.abs(..) around the time comparison value, so it will be compared +/- 10.5 minutes.

openshift-ci-robot · 2019-08-14T20:12:41Z

@dtaylor113: This pull request references a valid Bugzilla bug.

In response to this:

Bug 1739225: Timestamp component always shows dates in the future as relative dates

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

spadgett · 2019-08-15T19:47:59Z

frontend/public/components/utils/timestamp.tsx

  if (omitSuffix) {
    return dateTime.fromNow(mdate, undefined, {omitSuffix: true});
  }
-  if (timeAgo < 630000) { // 10.5 minutes
+  if (timeFromNow < 630000) { // 10.5 minutes


I wouldn't assign timeFromNow to a variable to make sure we don't accidentally use it as the date we show. (With Math.abs it could be the wrong value.) We should add a comment explaining as well.

Suggested change

if (timeFromNow < 630000) { // 10.5 minutes

// Show a relative time if within 10.5 minutes from the current time (in the past or future).

if (Math.abs(now.getTime() - mdate.getTime()) < 630000) { // 10.5 minutes

changed to timeDifference

spadgett · 2019-08-15T19:55:53Z

Thinking on this more, I'd recommend showing any future date as a full date. This avoids weirdness due to client clock skew where we might show something like "Build started 2 minutes from now"

chancez · 2019-08-15T20:00:02Z

Thinking on this more, I'd recommend showing any future date as a full date. This avoids weirdness due to client clock skew where we might show something like "Build started 2 minutes from now"

Holy crap I totally forgot about this. That is something annoying I've seen before if I recall. However, you could also fix it by taking into account the user's timezone.

spadgett · 2019-08-15T20:02:20Z

We account for timezones... The problem occurs when the user's clock is a few minutes off from the API server's (or the node's) :/

chancez · 2019-08-15T20:06:18Z

Ah. Either way, seems like a solid idea to fix the issue.

…relative dates

dtaylor113 · 2019-08-15T20:44:14Z

Thinking on this more, I'd recommend showing any future date as a full date.

All future dates will be in normal Timestamp format, no relative displays for future dates:

spadgett

/lgtm

spadgett · 2019-08-15T20:44:56Z

/approve

openshift-ci-robot · 2019-08-15T20:45:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dtaylor113, spadgett

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/OWNERS~~ [spadgett]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2019-08-15T20:46:57Z

@dtaylor113: This pull request references a valid Bugzilla bug.

In response to this:

Bug 1739225: Timestamp component always shows dates in the future as relative dates

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2019-08-15T20:47:54Z

@dtaylor113: This pull request references a valid Bugzilla bug.

In response to this:

Bug 1739225: Timestamp component always shows dates in the future as relative dates

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

joshuawilson · 2019-08-15T21:52:17Z

Why doesn't it have the year?

spadgett · 2019-08-15T22:25:48Z

Timestamp currently only shows the year if not the current year. We could look at changing that

openshift-bot · 2019-08-16T05:29:16Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-08-16T07:00:24Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-08-16T08:05:14Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2019-08-16T08:49:36Z

@dtaylor113: All pull requests linked via external trackers have merged. The Bugzilla bug has been moved to the MODIFIED state.

In response to this:

Bug 1739225: Timestamp component always shows dates in the future as relative dates

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Aug 13, 2019

openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. component/core Related to console core functionality labels Aug 13, 2019

openshift-ci-robot requested review from jeff-phillips-18 and TheRealJon August 13, 2019 16:29

dtaylor113 commented Aug 13, 2019

View reviewed changes

frontend/public/components/utils/timestamp.tsx Outdated Show resolved Hide resolved

dtaylor113 requested review from spadgett and chancez and removed request for jeff-phillips-18 and TheRealJon August 13, 2019 16:34

dtaylor113 force-pushed the timestamp-norelative branch from 0c90204 to f9e0fb2 Compare August 13, 2019 17:20

dtaylor113 force-pushed the timestamp-norelative branch from f9e0fb2 to 1b17699 Compare August 13, 2019 17:41

dtaylor113 mentioned this pull request Aug 14, 2019

Chargeback: updated to v2 table api which returns display columns and units #2250

Merged

1 task

dtaylor113 force-pushed the timestamp-norelative branch from 1b17699 to 2d3af1a Compare August 14, 2019 19:46

openshift-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 14, 2019

spadgett reviewed Aug 15, 2019

View reviewed changes

Bug 1739225: Timestamp component always shows dates in the future as …

fc13764

…relative dates

dtaylor113 force-pushed the timestamp-norelative branch from 2d3af1a to fc13764 Compare August 15, 2019 20:42

spadgett approved these changes Aug 15, 2019

View reviewed changes

openshift-ci-robot assigned spadgett Aug 15, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 15, 2019

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 15, 2019

spadgett added this to the v4.2 milestone Aug 15, 2019

openshift-merge-robot merged commit d72187a into openshift:master Aug 16, 2019

dtaylor113 deleted the timestamp-norelative branch August 22, 2019 13:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1739225: Timestamp component always shows dates in the future as relative dates #2341

Bug 1739225: Timestamp component always shows dates in the future as relative dates #2341

dtaylor113 commented Aug 13, 2019 •

edited

Loading

openshift-ci-robot commented Aug 13, 2019

spadgett commented Aug 13, 2019

dtaylor113 commented Aug 13, 2019

spadgett commented Aug 13, 2019 •

edited

Loading

dtaylor113 commented Aug 13, 2019 •

edited

Loading

openshift-ci-robot commented Aug 14, 2019

spadgett Aug 15, 2019

dtaylor113 Aug 15, 2019

spadgett commented Aug 15, 2019

chancez commented Aug 15, 2019

spadgett commented Aug 15, 2019 •

edited

Loading

chancez commented Aug 15, 2019

dtaylor113 commented Aug 15, 2019

spadgett left a comment

spadgett commented Aug 15, 2019

openshift-ci-robot commented Aug 15, 2019

openshift-ci-robot commented Aug 15, 2019

openshift-ci-robot commented Aug 15, 2019

joshuawilson commented Aug 15, 2019

spadgett commented Aug 15, 2019

openshift-bot commented Aug 16, 2019

openshift-bot commented Aug 16, 2019

openshift-bot commented Aug 16, 2019

openshift-ci-robot commented Aug 16, 2019

	if (timeFromNow < 630000) { // 10.5 minutes
	// Show a relative time if within 10.5 minutes from the current time (in the past or future).
	if (Math.abs(now.getTime() - mdate.getTime()) < 630000) { // 10.5 minutes

Bug 1739225: Timestamp component always shows dates in the future as relative dates #2341

Bug 1739225: Timestamp component always shows dates in the future as relative dates #2341

Conversation

dtaylor113 commented Aug 13, 2019 • edited Loading

openshift-ci-robot commented Aug 13, 2019

spadgett commented Aug 13, 2019

dtaylor113 commented Aug 13, 2019

spadgett commented Aug 13, 2019 • edited Loading

dtaylor113 commented Aug 13, 2019 • edited Loading

openshift-ci-robot commented Aug 14, 2019

spadgett Aug 15, 2019

Choose a reason for hiding this comment

dtaylor113 Aug 15, 2019

Choose a reason for hiding this comment

spadgett commented Aug 15, 2019

chancez commented Aug 15, 2019

spadgett commented Aug 15, 2019 • edited Loading

chancez commented Aug 15, 2019

dtaylor113 commented Aug 15, 2019

spadgett left a comment

Choose a reason for hiding this comment

spadgett commented Aug 15, 2019

openshift-ci-robot commented Aug 15, 2019

openshift-ci-robot commented Aug 15, 2019

openshift-ci-robot commented Aug 15, 2019

joshuawilson commented Aug 15, 2019

spadgett commented Aug 15, 2019

openshift-bot commented Aug 16, 2019

openshift-bot commented Aug 16, 2019

openshift-bot commented Aug 16, 2019

openshift-ci-robot commented Aug 16, 2019

dtaylor113 commented Aug 13, 2019 •

edited

Loading

spadgett commented Aug 13, 2019 •

edited

Loading

dtaylor113 commented Aug 13, 2019 •

edited

Loading

spadgett commented Aug 15, 2019 •

edited

Loading