Skip to content
This repository has been archived by the owner on May 28, 2024. It is now read-only.

Tech Debt: DST sensitivity in Auto-Approver #5603

Open
jbum opened this issue Nov 7, 2021 · 1 comment
Open

Tech Debt: DST sensitivity in Auto-Approver #5603

jbum opened this issue Nov 7, 2021 · 1 comment
Assignees

Comments

@jbum
Copy link
Collaborator

jbum commented Nov 7, 2021

In the Cron repo, the file CovidTranslationPrApproval/AutoApprover.js contains a DST-sensitive bug that triggers on the two Sundays of the year when DST kicks in.

The expression moment().startOf('day') will produce a timestamp from before DST kicked in. Adding a fixed number of hours to that will produce a time that is 1 hour different than desired. For example, 10am on Nov 7 occurs 11 hours after midnight.

Suggested fix is to formulate a complete date/time rather than doing an offset off of midnight.

        .map(x => ({
            label: x.label, diff: moment()
                .diff(moment().startOf('day')
                    .add(x.hour, 'hours')
                    .add(x.minute, 'minutes'), 'minutes')
        }))
        .filter(x => x.diff > 0 && x.diff < 15);
@jbum
Copy link
Collaborator Author

jbum commented Nov 15, 2021

Suggested method -- the basic idea is to pass a fully formed date and time string to moment() rather than initializing the timestamp at midnight and adding hours.

    const todayDateStr = moment().format("YYYY-MM-DD");

    const ActiveLabels = AutoApproverLabels.timeLabels
        .map(x => ({
            label: x.label, diff: moment()
                .diff(moment(todayDateStr + ' ' + 
                      (x.hour < 10? '0' : '') + x.hour + 
                      ':' + 
                      (x.minute < 10? '0' : '') + x.minute))
}))
        .filter(x => x.diff > 0 && x.diff < 15);

@jbum jbum assigned jbum and carterm Nov 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants