Adding date and time to test queue and new test plan dropdown #397

evmiguel · 2022-01-12T17:38:02Z

In this PR, I did not include weeks or months as a unit of time. The only units I included were days and hours, mostly because when I started to add weeks and months, the calculations were not precise enough. In this implementation, if the number of days passes 30 days, the date as a string will be used instead; this approach is just like GitHub. See how React commits look like

This PR adds the days and git SHAs to the Test Plan rows. It also adds the days and git SHAs to the version picker in the Add Test Plan picker.

alflennik · 2022-01-12T18:34:35Z

client/utils/dateInterval.js

+    }
+
+    return `${intervalNumber} ${intervalUnit} ago`;
+};


Nicely done!

agreed with @alflennik that this is really slick! However, Matt prefers that we use absolute dates instead of relative times, always (I mentioned that GitHub uses this approach, but he felt consistency and no relative times was clearer). Can we change the output to always be the formatted date string?

Also, can we add the time in order to disambiguate between two commits on the same day?

What do you think of something like this?

`${startDate.toLocaleString('default', { month: 'short' })} ${startDate.getDate()}, ${startDate.getFullYear()} ${startDate.toLocaleTimeString()}`

alflennik · 2022-01-12T18:35:07Z

client/utils/dateInterval.test.js

+        const dateIntervalString = calculateDateInterval(startDate, endDate);
+        expect(dateIntervalString).toBe('Jan 1, 2022');
+    });
+});


Great job with the tests too 👍

… picker dropdown

s3ththompson

@evmiguel this PR is awesome!

I received some feedback from Matt in a 1:1 last night that changes the requirements slightly. Sorry for the churn!

s3ththompson · 2022-01-12T19:19:14Z

client/utils/dateInterval.js

+    }
+
+    return `${intervalNumber} ${intervalUnit} ago`;
+};


agreed with @alflennik that this is really slick! However, Matt prefers that we use absolute dates instead of relative times, always (I mentioned that GitHub uses this approach, but he felt consistency and no relative times was clearer). Can we change the output to always be the formatted date string?

Also, can we add the time in order to disambiguate between two commits on the same day?

What do you think of something like this?

`${startDate.toLocaleString('default', { month: 'short' })} ${startDate.getDate()}, ${startDate.getFullYear()} ${startDate.toLocaleTimeString()}`

s3ththompson · 2022-01-12T19:22:07Z

client/components/TestQueueRow/index.jsx

@@ -114,25 +115,44 @@ const TestQueueRow = ({
    };

    const renderAssignedUserToTestPlan = () => {
+        const gitShaDateDifference = (


Matt felt strongly that the Test Queue rows should just show the date, not the Git SHA... Can we remove the Git SHA here, (but keep it where it appears in the NewTestPlanReportModal picker)?

If it is just the date displayed then what would be a delineation if there are two tests published the same day?

Yep, had the same thought--see sibling comment about adding time to date string...

+1 on adding time in addition to absolute date format. SHAs don't really mean anything whereas time solves the delineation issue brought up by Rich and does mean something for the user.

alflennik · 2022-01-12T19:31:05Z

Just to throw more variables in to the mix, I think including the word "revised" will help both sighted and unsighted users understand the contextual meaning of the date.

We could put (commit d1abe0) in parentheses following the text, if we decide it is needed.

Visually, I think we should bump up the font size of the test plan link to 1.1rem and lower the font size of the revision text to 0.9rem, and make it italic, as pictured.

richnoah · 2022-01-12T19:35:48Z

@alflennik would it still be revised if it was an initial test? Maybe the use of Published would fit all situations.

alflennik · 2022-01-12T19:38:25Z

@richnoah here's what that looks like. I also threw in a more specific date assuming that's what we go with. I think published is good, maybe it's better than revised haha. I like it.

Note... I forgot the space between 1:31 and pm.

@s3ththompson do you think this is something Matt would sign off on?

alflennik · 2022-01-12T18:37:30Z

server/resources/commands.json

+    {
+        "id": "TWO",
+        "text": "2"
+    },
    {
        "id": "T_THEN_DOWN",
        "text": "T followed by Down Arrow"


Do you know why these changes are here? We don't necessarily need to get rid of them, but I'm just sort of curious what's going on here...?

I took this out. Sneaky file!

alflennik · 2022-01-12T18:40:56Z

client/components/NewTestPlanReport/NewTestPlanReportModal.jsx

+                                    new Date(item.updatedAt),
+                                    new Date()
+                                )}{' '}
+                                ({item.gitSha.substring(0, 5)})


Typically the shortened git shas are seven characters, not five, I think.

Assuming we keep the shas at all. Seems like we've decided to drop them.

@s3ththompson are we keeping the SHAs?

Sorry for not being clearer... Matt's feedback was to remove SHAs from the TestQueueRows, but keep them in the NewTestPlanReportModal:

TestQueueRows:

Nov 18, 2021 at 1:31pm

NewTestPlanReportModal:

Nov 18, 2021 at 1:31pm (sha1234) or

Nov 18, 2021 at 1:31pm "Create tests for APG design pattern example: Main Landmark" (sha1234)

Matt expressed that if we could add commit messages to the modal that would be even better, but I think we should do whatever is easier here since this is a temporary fix.

isaacdurazo · 2022-01-12T20:02:54Z

I like the use of the label "Published". should we consider other labels like "Last modified"?

@alflennik good job on the styling. I agree with your font-treatment suggestions

evmiguel · 2022-01-12T21:09:03Z

@s3ththompson @alflennik @richnoah @isaacdurazo - Here's what I did:

Absolute date in TestQueueRow
Absolute date, git commit message, and git SHA in NewTestPlanReportModal
Styling suggested by @alflennik
Use of "Published" or "Revised"

client/utils/gitUtils.js

alflennik

I did make one note about the capitalization, but I think it's optional. This looks great, and good job rolling with the punches.

… message to test plan version picker

alflennik · 2022-01-13T20:35:45Z

Confirmed the PM is now pm, nice.

evmiguel requested review from alflennik and howard-e January 12, 2022 17:38

alflennik reviewed Jan 12, 2022

View reviewed changes

Adding days and date (for over 30 days) to Test Plan cell and version…

539587b

… picker dropdown

evmiguel force-pushed the testplan-with-time branch from 207974e to 539587b Compare January 12, 2022 19:03

s3ththompson requested changes Jan 12, 2022

View reviewed changes

alflennik reviewed Jan 12, 2022

View reviewed changes

evmiguel requested review from s3ththompson, isaacdurazo, alflennik and richnoah January 12, 2022 20:58

evmiguel force-pushed the testplan-with-time branch 2 times, most recently from c6a2607 to 0b08083 Compare January 12, 2022 21:04

evmiguel force-pushed the testplan-with-time branch 2 times, most recently from 8a78f68 to 67aed2f Compare January 12, 2022 21:43

s3ththompson approved these changes Jan 13, 2022

View reviewed changes

evmiguel force-pushed the testplan-with-time branch from 67aed2f to 06fc951 Compare January 13, 2022 18:58

alflennik reviewed Jan 13, 2022

View reviewed changes

client/utils/gitUtils.js Show resolved Hide resolved

alflennik approved these changes Jan 13, 2022

View reviewed changes

Removing date intervals in favor of absolute dates. Adding git commit…

713f852

… message to test plan version picker

evmiguel force-pushed the testplan-with-time branch from 06fc951 to 713f852 Compare January 13, 2022 20:32

evmiguel merged commit 97f2497 into main Jan 13, 2022

jkva mentioned this pull request Feb 2, 2022

Edge Case Bugfix: User Locale Date and Time Rendering #400

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding date and time to test queue and new test plan dropdown #397

Adding date and time to test queue and new test plan dropdown #397

evmiguel commented Jan 12, 2022

alflennik Jan 12, 2022

s3ththompson Jan 12, 2022 •

edited

alflennik Jan 12, 2022

s3ththompson left a comment

s3ththompson Jan 12, 2022 •

edited

s3ththompson Jan 12, 2022

richnoah Jan 12, 2022

s3ththompson Jan 12, 2022 •

edited

isaacdurazo Jan 12, 2022 •

edited

alflennik commented Jan 12, 2022

richnoah commented Jan 12, 2022

alflennik commented Jan 12, 2022 •

edited

alflennik Jan 12, 2022

evmiguel Jan 12, 2022

alflennik Jan 12, 2022

alflennik Jan 12, 2022

evmiguel Jan 12, 2022

s3ththompson Jan 12, 2022

isaacdurazo commented Jan 12, 2022 •

edited

evmiguel commented Jan 12, 2022 •

edited

alflennik left a comment

alflennik commented Jan 13, 2022

Adding date and time to test queue and new test plan dropdown #397

Adding date and time to test queue and new test plan dropdown #397

Conversation

evmiguel commented Jan 12, 2022

Choose a reason for hiding this comment

s3ththompson Jan 12, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

s3ththompson left a comment

Choose a reason for hiding this comment

s3ththompson Jan 12, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

s3ththompson Jan 12, 2022 • edited

Choose a reason for hiding this comment

isaacdurazo Jan 12, 2022 • edited

Choose a reason for hiding this comment

alflennik commented Jan 12, 2022

richnoah commented Jan 12, 2022

alflennik commented Jan 12, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isaacdurazo commented Jan 12, 2022 • edited

evmiguel commented Jan 12, 2022 • edited

alflennik left a comment

Choose a reason for hiding this comment

alflennik commented Jan 13, 2022

s3ththompson Jan 12, 2022 •

edited

s3ththompson Jan 12, 2022 •

edited

s3ththompson Jan 12, 2022 •

edited

isaacdurazo Jan 12, 2022 •

edited

alflennik commented Jan 12, 2022 •

edited

isaacdurazo commented Jan 12, 2022 •

edited

evmiguel commented Jan 12, 2022 •

edited