Skip to content

Commit

Permalink
[Bugfix-795] Fix date comparison edge case (#816)
Browse files Browse the repository at this point in the history
* Fix updatedAt and markedStaleOn date comparison

* Delete accidental file

* Refactor

* Cleanup

* cleanup
  • Loading branch information
luketomlinson authored and JoannaaKL committed Sep 12, 2022
1 parent e200968 commit c750aa3
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 7 deletions.
21 changes: 18 additions & 3 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,9 @@ class IssuesProcessor {
if (issue.markedStaleThisRun) {
issueLogger.info(`marked stale this run, so don't check for updates`);
}
const issueHasUpdateSinceStale = new Date(issue.updated_at) > new Date(markedStaleOn);
// The issue.updated_at and markedStaleOn are not always exactly in sync (they can be off by a second or 2)
// isDateMoreRecentThan makes sure they are not the same date within a certain tolerance (15 seconds in this case)
const issueHasUpdateSinceStale = is_date_more_recent_than_1.isDateMoreRecentThan(new Date(issue.updated_at), new Date(markedStaleOn), 15);
issueLogger.info(`$$type has been updated since it was marked stale: ${logger_service_1.LoggerService.cyan(issueHasUpdateSinceStale)}`);
// Should we un-stale this issue?
if (shouldRemoveStaleWhenUpdated &&
Expand Down Expand Up @@ -1965,12 +1967,25 @@ exports.getHumanizedDate = getHumanizedDate;

"use strict";

/// returns false if the dates are equal within the `equalityToleranceInSeconds` number of seconds
/// otherwise returns true if `comparedDate` is after `date`
Object.defineProperty(exports, "__esModule", ({ value: true }));
exports.isDateMoreRecentThan = void 0;
function isDateMoreRecentThan(date, comparedDate) {
exports.isDateEqualTo = exports.isDateMoreRecentThan = void 0;
function isDateMoreRecentThan(date, comparedDate, equalityToleranceInSeconds = 0) {
if (equalityToleranceInSeconds > 0) {
const areDatesEqual = isDateEqualTo(date, comparedDate, equalityToleranceInSeconds);
return !areDatesEqual && date > comparedDate;
}
return date > comparedDate;
}
exports.isDateMoreRecentThan = isDateMoreRecentThan;
function isDateEqualTo(date, otherDate, toleranceInSeconds) {
const timestamp = date.getTime();
const otherTimestamp = otherDate.getTime();
const deltaInSeconds = Math.abs(timestamp - otherTimestamp) / 1000;
return deltaInSeconds <= toleranceInSeconds;
}
exports.isDateEqualTo = isDateEqualTo;


/***/ }),
Expand Down
9 changes: 7 additions & 2 deletions src/classes/issues-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -673,8 +673,13 @@ export class IssuesProcessor {
issueLogger.info(`marked stale this run, so don't check for updates`);
}

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

issueLogger.info(
`$$type has been updated since it was marked stale: ${LoggerService.cyan(
Expand Down
66 changes: 65 additions & 1 deletion src/functions/dates/is-date-more-recent-than.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {isDateMoreRecentThan} from './is-date-more-recent-than';
import {isDateEqualTo, isDateMoreRecentThan} from './is-date-more-recent-than';

describe('isDateMoreRecentThan()', (): void => {
let date: Date;
Expand Down Expand Up @@ -48,4 +48,68 @@ describe('isDateMoreRecentThan()', (): void => {
expect(result).toStrictEqual(true);
});
});

describe('date equality', (): void => {
it('should correctly compare a before date outside tolerance', (): void => {
const aDate = new Date('2022-09-09T13:00:00');
const otherDate = new Date('2022-09-09T14:00:00');
expect(isDateEqualTo(aDate, otherDate, 60)).toBe(false);
});

it('should correctly compare a before date inside tolerance', (): void => {
const aDate = new Date('2022-09-09T13:00:00');
const otherDate = new Date('2022-09-09T13:00:42');
expect(isDateEqualTo(aDate, otherDate, 60)).toBe(true);
});

it('should correctly compare an after date outside tolerance', (): void => {
const aDate = new Date('2022-09-09T13:00:00');
const otherDate = new Date('2022-09-09T12:00:00');
expect(isDateEqualTo(aDate, otherDate, 60)).toBe(false);
});

it('should correctly compare an after date inside tolerance', (): void => {
const aDate = new Date('2022-09-09T13:00:00');
const otherDate = new Date('2022-09-09T12:59:42');
expect(isDateEqualTo(aDate, otherDate, 60)).toBe(true);
});

it('should correctly compare an exactly equal date', (): void => {
const aDate = new Date('2022-09-09T13:00:00');
const otherDate = new Date('2022-09-09T13:00:00');
expect(isDateEqualTo(aDate, otherDate, 60)).toBe(true);
});
});

describe('date comparison with tolerances', (): void => {
it('should correctly compare a before date outside tolerance', (): void => {
const aDate = new Date('2022-09-09T13:00:00');
const otherDate = new Date('2022-09-09T14:00:00');
expect(isDateMoreRecentThan(aDate, otherDate)).toBe(false);
});

it('should correctly compare a before date inside tolerance', (): void => {
const aDate = new Date('2022-09-09T13:00:00');
const otherDate = new Date('2022-09-09T13:00:42');
expect(isDateMoreRecentThan(aDate, otherDate, 60)).toBe(false); // considered equal here
});

it('should correctly compare an after date outside tolerance', (): void => {
const aDate = new Date('2022-09-09T13:00:00');
const otherDate = new Date('2022-09-09T12:00:00');
expect(isDateMoreRecentThan(aDate, otherDate, 60)).toBe(true);
});

it('should correctly compare an after date inside tolerance', (): void => {
const aDate = new Date('2022-09-09T13:00:00');
const otherDate = new Date('2022-09-09T12:59:42');
expect(isDateMoreRecentThan(aDate, otherDate, 60)).toBe(false); // considered equal here
});

it('should correctly compare an exactly equal date', (): void => {
const aDate = new Date('2022-09-09T13:00:00');
const otherDate = new Date('2022-09-09T13:00:00');
expect(isDateMoreRecentThan(aDate, otherDate, 60)).toBe(false);
});
});
});
27 changes: 26 additions & 1 deletion src/functions/dates/is-date-more-recent-than.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,31 @@
/// returns false if the dates are equal within the `equalityToleranceInSeconds` number of seconds
/// otherwise returns true if `comparedDate` is after `date`

export function isDateMoreRecentThan(
date: Readonly<Date>,
comparedDate: Readonly<Date>
comparedDate: Readonly<Date>,
equalityToleranceInSeconds = 0
): boolean {
if (equalityToleranceInSeconds > 0) {
const areDatesEqual = isDateEqualTo(
date,
comparedDate,
equalityToleranceInSeconds
);

return !areDatesEqual && date > comparedDate;
}

return date > comparedDate;
}

export function isDateEqualTo(
date: Date,
otherDate: Date,
toleranceInSeconds: number
): boolean {
const timestamp = date.getTime();
const otherTimestamp = otherDate.getTime();
const deltaInSeconds = Math.abs(timestamp - otherTimestamp) / 1000;
return deltaInSeconds <= toleranceInSeconds;
}

0 comments on commit c750aa3

Please sign in to comment.