cagov / covid19

This is the official COVID19 response website for the state of California.
https://covid19.ca.gov/
44 stars 22 forks source link

Tech Debt: DST sensitivity in Auto-Approver #5603

Open jbum opened 2 years ago

jbum commented 2 years ago

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 commented 2 years ago

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);