Skip to content

Commit 8e8a0e6

Browse files
[Bugfix-795] Fix date comparison edge case (#816)
* Fix updatedAt and markedStaleOn date comparison * Delete accidental file * Refactor * Cleanup * cleanup
1 parent 80962c1 commit 8e8a0e6

File tree

4 files changed

+116
-7
lines changed

4 files changed

+116
-7
lines changed

dist/index.js

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -742,7 +742,9 @@ class IssuesProcessor {
742742
if (issue.markedStaleThisRun) {
743743
issueLogger.info(`marked stale this run, so don't check for updates`);
744744
}
745-
const issueHasUpdateSinceStale = new Date(issue.updated_at) > new Date(markedStaleOn);
745+
// The issue.updated_at and markedStaleOn are not always exactly in sync (they can be off by a second or 2)
746+
// isDateMoreRecentThan makes sure they are not the same date within a certain tolerance (15 seconds in this case)
747+
const issueHasUpdateSinceStale = is_date_more_recent_than_1.isDateMoreRecentThan(new Date(issue.updated_at), new Date(markedStaleOn), 15);
746748
issueLogger.info(`$$type has been updated since it was marked stale: ${logger_service_1.LoggerService.cyan(issueHasUpdateSinceStale)}`);
747749
// Should we un-stale this issue?
748750
if (shouldRemoveStaleWhenUpdated &&
@@ -1965,12 +1967,25 @@ exports.getHumanizedDate = getHumanizedDate;
19651967

19661968
"use strict";
19671969

1970+
/// returns false if the dates are equal within the `equalityToleranceInSeconds` number of seconds
1971+
/// otherwise returns true if `comparedDate` is after `date`
19681972
Object.defineProperty(exports, "__esModule", ({ value: true }));
1969-
exports.isDateMoreRecentThan = void 0;
1970-
function isDateMoreRecentThan(date, comparedDate) {
1973+
exports.isDateEqualTo = exports.isDateMoreRecentThan = void 0;
1974+
function isDateMoreRecentThan(date, comparedDate, equalityToleranceInSeconds = 0) {
1975+
if (equalityToleranceInSeconds > 0) {
1976+
const areDatesEqual = isDateEqualTo(date, comparedDate, equalityToleranceInSeconds);
1977+
return !areDatesEqual && date > comparedDate;
1978+
}
19711979
return date > comparedDate;
19721980
}
19731981
exports.isDateMoreRecentThan = isDateMoreRecentThan;
1982+
function isDateEqualTo(date, otherDate, toleranceInSeconds) {
1983+
const timestamp = date.getTime();
1984+
const otherTimestamp = otherDate.getTime();
1985+
const deltaInSeconds = Math.abs(timestamp - otherTimestamp) / 1000;
1986+
return deltaInSeconds <= toleranceInSeconds;
1987+
}
1988+
exports.isDateEqualTo = isDateEqualTo;
19741989

19751990

19761991
/***/ }),

src/classes/issues-processor.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -673,8 +673,13 @@ export class IssuesProcessor {
673673
issueLogger.info(`marked stale this run, so don't check for updates`);
674674
}
675675

676-
const issueHasUpdateSinceStale =
677-
new Date(issue.updated_at) > new Date(markedStaleOn);
676+
// The issue.updated_at and markedStaleOn are not always exactly in sync (they can be off by a second or 2)
677+
// isDateMoreRecentThan makes sure they are not the same date within a certain tolerance (15 seconds in this case)
678+
const issueHasUpdateSinceStale = isDateMoreRecentThan(
679+
new Date(issue.updated_at),
680+
new Date(markedStaleOn),
681+
15
682+
);
678683

679684
issueLogger.info(
680685
`$$type has been updated since it was marked stale: ${LoggerService.cyan(

src/functions/dates/is-date-more-recent-than.spec.ts

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {isDateMoreRecentThan} from './is-date-more-recent-than';
1+
import {isDateEqualTo, isDateMoreRecentThan} from './is-date-more-recent-than';
22

33
describe('isDateMoreRecentThan()', (): void => {
44
let date: Date;
@@ -48,4 +48,68 @@ describe('isDateMoreRecentThan()', (): void => {
4848
expect(result).toStrictEqual(true);
4949
});
5050
});
51+
52+
describe('date equality', (): void => {
53+
it('should correctly compare a before date outside tolerance', (): void => {
54+
const aDate = new Date('2022-09-09T13:00:00');
55+
const otherDate = new Date('2022-09-09T14:00:00');
56+
expect(isDateEqualTo(aDate, otherDate, 60)).toBe(false);
57+
});
58+
59+
it('should correctly compare a before date inside tolerance', (): void => {
60+
const aDate = new Date('2022-09-09T13:00:00');
61+
const otherDate = new Date('2022-09-09T13:00:42');
62+
expect(isDateEqualTo(aDate, otherDate, 60)).toBe(true);
63+
});
64+
65+
it('should correctly compare an after date outside tolerance', (): void => {
66+
const aDate = new Date('2022-09-09T13:00:00');
67+
const otherDate = new Date('2022-09-09T12:00:00');
68+
expect(isDateEqualTo(aDate, otherDate, 60)).toBe(false);
69+
});
70+
71+
it('should correctly compare an after date inside tolerance', (): void => {
72+
const aDate = new Date('2022-09-09T13:00:00');
73+
const otherDate = new Date('2022-09-09T12:59:42');
74+
expect(isDateEqualTo(aDate, otherDate, 60)).toBe(true);
75+
});
76+
77+
it('should correctly compare an exactly equal date', (): void => {
78+
const aDate = new Date('2022-09-09T13:00:00');
79+
const otherDate = new Date('2022-09-09T13:00:00');
80+
expect(isDateEqualTo(aDate, otherDate, 60)).toBe(true);
81+
});
82+
});
83+
84+
describe('date comparison with tolerances', (): void => {
85+
it('should correctly compare a before date outside tolerance', (): void => {
86+
const aDate = new Date('2022-09-09T13:00:00');
87+
const otherDate = new Date('2022-09-09T14:00:00');
88+
expect(isDateMoreRecentThan(aDate, otherDate)).toBe(false);
89+
});
90+
91+
it('should correctly compare a before date inside tolerance', (): void => {
92+
const aDate = new Date('2022-09-09T13:00:00');
93+
const otherDate = new Date('2022-09-09T13:00:42');
94+
expect(isDateMoreRecentThan(aDate, otherDate, 60)).toBe(false); // considered equal here
95+
});
96+
97+
it('should correctly compare an after date outside tolerance', (): void => {
98+
const aDate = new Date('2022-09-09T13:00:00');
99+
const otherDate = new Date('2022-09-09T12:00:00');
100+
expect(isDateMoreRecentThan(aDate, otherDate, 60)).toBe(true);
101+
});
102+
103+
it('should correctly compare an after date inside tolerance', (): void => {
104+
const aDate = new Date('2022-09-09T13:00:00');
105+
const otherDate = new Date('2022-09-09T12:59:42');
106+
expect(isDateMoreRecentThan(aDate, otherDate, 60)).toBe(false); // considered equal here
107+
});
108+
109+
it('should correctly compare an exactly equal date', (): void => {
110+
const aDate = new Date('2022-09-09T13:00:00');
111+
const otherDate = new Date('2022-09-09T13:00:00');
112+
expect(isDateMoreRecentThan(aDate, otherDate, 60)).toBe(false);
113+
});
114+
});
51115
});
Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,31 @@
1+
/// returns false if the dates are equal within the `equalityToleranceInSeconds` number of seconds
2+
/// otherwise returns true if `comparedDate` is after `date`
3+
14
export function isDateMoreRecentThan(
25
date: Readonly<Date>,
3-
comparedDate: Readonly<Date>
6+
comparedDate: Readonly<Date>,
7+
equalityToleranceInSeconds = 0
48
): boolean {
9+
if (equalityToleranceInSeconds > 0) {
10+
const areDatesEqual = isDateEqualTo(
11+
date,
12+
comparedDate,
13+
equalityToleranceInSeconds
14+
);
15+
16+
return !areDatesEqual && date > comparedDate;
17+
}
18+
519
return date > comparedDate;
620
}
21+
22+
export function isDateEqualTo(
23+
date: Date,
24+
otherDate: Date,
25+
toleranceInSeconds: number
26+
): boolean {
27+
const timestamp = date.getTime();
28+
const otherTimestamp = otherDate.getTime();
29+
const deltaInSeconds = Math.abs(timestamp - otherTimestamp) / 1000;
30+
return deltaInSeconds <= toleranceInSeconds;
31+
}

0 commit comments

Comments
 (0)