Skip to content

Update date-fns#9483

Closed
andaley wants to merge 23 commits into
masterfrom
ui/fix-transit-key-versions
Closed

Update date-fns#9483
andaley wants to merge 23 commits into
masterfrom
ui/fix-transit-key-versions

Conversation

@andaley
Copy link
Copy Markdown
Contributor

@andaley andaley commented Jul 15, 2020

Update date-fns

Update date-fns to 2.0.0 which requires us update all of our date-related helpers. The most notable change we had to account for is that all of the date-fns functions no longer accept strings as arguments. You'll notice we also had to switch some of the formatting arguments from YYYY to yyyy as a result. See the full list of breaking changes when upgrading to date-fns 2.0.0 here.

Testing

This PR requires a considerable QA as it changes the behavior of our date-from-now and date-format helpers. We now return an empty string and console log an error if the formatting doesn't work, so there's a chance some dates may be missing in the UI.

Navigate to the various pages of the UI where date-format or date-from-now is used, such as the replication secondaries add modal which displays the expiration date. These pages should still display the date as expected.

Here's an example of the replication secondaries add modal:
image

Comment thread ui/app/helpers/date-from-now.js Outdated
try {
dateString = formatDistanceToNow(date, { ...options });
} catch (e) {
console.log(date);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe also console log the error, or is this just for testing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is just for testing -- we haven't solved the issue yet 😭

export default DS.Model.extend({
type: attr('string', {
defaultValue: 'aes256-gcm96',
defaultValue: 'aes128-gcm96',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

curious why we're lowing the bit count here (assuming the 256 stands for bits). I'm also guessing, the higher the bit count the more secure?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

great question! in this case we're only changing the default value so it automatically selects the first item in the dropdown. the other option is still available. this way we're listing all options in alphabetic and numeric order:

image

<AlertBanner @type="warning" @message="We've stopped auto-renewing your token due to inactivity.
It will expire in {{date-from-now auth.tokenExpirationDate interval=1000 hideSuffix=true}}.
on {{date-format auth.tokenExpirationDate 'MMMM Do YYYY, h:mm:ss a'}}" />
on {{date-format auth.tokenExpirationDate 'MMMM Do yyyy, h:mm:ss a'}}" />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MMMM is capitalized, but yyyy is not?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

another excellent question! in the newest version of date-fnsyyyy (lowercase) represents the calendar year, which is what we want, whereas YYYY represents the week numbering year. see the examples here:

https://github.com/date-fns/date-fns/blob/master/docs/unicodeTokens.md#popular-mistakes

@andaley andaley marked this pull request as ready for review July 17, 2020 17:29
@andaley andaley changed the title Ui/fix transit key versions Update date-fns Jul 17, 2020
@andaley andaley closed this Jul 17, 2020
@andaley andaley deleted the ui/fix-transit-key-versions branch July 17, 2020 20:15
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.

4 participants