Refactor isImportantForNotification: remove DB dependency and add tests
- Make function synchronous and remove database dependency - Accept lastNonPendingStatus as parameter instead of monitorID - Simplify function logic for better testability - Update call sites to fetch lastNonPendingStatus only when needed - Add comprehensive test suite with 17 test cases covering all status transitions Addresses code review feedback about function complexity and testability
This commit is contained in:
saber04414
parent
a529ad888f
commit
f0c17ab9e8
@ -1009,7 +1009,13 @@ class Monitor extends BeanModel {
|
||||
if (isImportant) {
|
||||
bean.important = true;
|
||||
|
||||
if (await Monitor.isImportantForNotification(isFirstBeat, previousBeat?.status, bean.status, this.id)) {
|
||||
// Get lastNonPendingStatus only for PENDING -> UP transitions
|
||||
let lastNonPendingStatus = null;
|
||||
if (previousBeat?.status === PENDING && bean.status === UP) {
|
||||
lastNonPendingStatus = await Monitor.getLastNonPendingStatus(this.id);
|
||||
}
|
||||
|
||||
if (Monitor.isImportantForNotification(isFirstBeat, previousBeat?.status, bean.status, lastNonPendingStatus)) {
|
||||
log.debug("monitor", `[${this.name}] sendNotification`);
|
||||
await Monitor.sendNotification(isFirstBeat, this, bean);
|
||||
} else {
|
||||
@ -1450,42 +1456,22 @@ class Monitor extends BeanModel {
|
||||
* @param {boolean} isFirstBeat Is this the first beat of this monitor?
|
||||
* @param {const} previousBeatStatus Status of the previous beat
|
||||
* @param {const} currentBeatStatus Status of the current beat
|
||||
* @param {number} monitorID Optional monitor ID to check history for PENDING->UP transitions
|
||||
* @returns {Promise<boolean>} True if is an important beat else false
|
||||
* @returns {boolean} True if is an important beat else false
|
||||
>>>>>>> d4c2e5fc (Refactor isImportantForNotification: remove DB dependency and add tests)
|
||||
*/
|
||||
static async isImportantForNotification(isFirstBeat, previousBeatStatus, currentBeatStatus, monitorID = null) {
|
||||
// * ? -> ANY STATUS = important [isFirstBeat]
|
||||
// UP -> PENDING = not important
|
||||
// * UP -> DOWN = important
|
||||
// UP -> UP = not important
|
||||
// PENDING -> PENDING = not important
|
||||
// * PENDING -> DOWN = important
|
||||
// PENDING -> UP = important if monitor was DOWN before PENDING (fix for issue #6025)
|
||||
// DOWN -> PENDING = this case not exists
|
||||
// DOWN -> DOWN = not important
|
||||
// * DOWN -> UP = important
|
||||
// MAINTENANCE -> MAINTENANCE = not important
|
||||
// MAINTENANCE -> UP = not important
|
||||
// * MAINTENANCE -> DOWN = important
|
||||
// DOWN -> MAINTENANCE = not important
|
||||
// UP -> MAINTENANCE = not important
|
||||
|
||||
// Check for PENDING -> UP transition
|
||||
if (previousBeatStatus === PENDING && currentBeatStatus === UP) {
|
||||
// If monitorID is provided, check if the monitor was DOWN before entering PENDING
|
||||
if (monitorID) {
|
||||
const lastNonPendingStatus = await Monitor.getLastNonPendingStatus(monitorID);
|
||||
// If the last non-PENDING status was DOWN, this transition is important
|
||||
if (lastNonPendingStatus === DOWN) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
// Otherwise, PENDING -> UP is not important (original behavior)
|
||||
return false;
|
||||
static isImportantForNotification(isFirstBeat, previousBeatStatus, currentBeatStatus, lastNonPendingStatus = null) {
|
||||
// First beat is always important
|
||||
if (isFirstBeat) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// PENDING -> UP is important only if monitor was DOWN before entering PENDING (fix for issue #6025)
|
||||
if (previousBeatStatus === PENDING && currentBeatStatus === UP) {
|
||||
return lastNonPendingStatus === DOWN;
|
||||
}
|
||||
|
||||
// Important transitions
|
||||
return (
|
||||
isFirstBeat ||
|
||||
(previousBeatStatus === MAINTENANCE && currentBeatStatus === DOWN) ||
|
||||
(previousBeatStatus === UP && currentBeatStatus === DOWN) ||
|
||||
(previousBeatStatus === DOWN && currentBeatStatus === UP) ||
|
||||
|
||||
@ -99,7 +99,13 @@ router.all("/api/push/:pushToken", async (request, response) => {
|
||||
|
||||
bean.important = Monitor.isImportantBeat(isFirstBeat, previousHeartbeat?.status, bean.status);
|
||||
|
||||
if (await Monitor.isImportantForNotification(isFirstBeat, previousHeartbeat?.status, bean.status, monitor.id)) {
|
||||
// Get lastNonPendingStatus only for PENDING -> UP transitions
|
||||
let lastNonPendingStatus = null;
|
||||
if (previousHeartbeat?.status === PENDING && bean.status === UP) {
|
||||
lastNonPendingStatus = await Monitor.getLastNonPendingStatus(monitor.id);
|
||||
}
|
||||
|
||||
if (Monitor.isImportantForNotification(isFirstBeat, previousHeartbeat?.status, bean.status, lastNonPendingStatus)) {
|
||||
// Reset down count
|
||||
bean.downCount = 0;
|
||||
|
||||
|
||||
96
test/backend-test/test-monitor-notification.js
Normal file
96
test/backend-test/test-monitor-notification.js
Normal file
@ -0,0 +1,96 @@
|
||||
const { describe, test } = require("node:test");
|
||||
const assert = require("node:assert");
|
||||
const Monitor = require("../../server/model/monitor");
|
||||
const { UP, DOWN, PENDING, MAINTENANCE } = require("../../src/util");
|
||||
|
||||
describe("Monitor.isImportantForNotification", () => {
|
||||
// First beat is always important
|
||||
test("first beat is always important", () => {
|
||||
assert.strictEqual(Monitor.isImportantForNotification(true, null, UP), true);
|
||||
assert.strictEqual(Monitor.isImportantForNotification(true, null, DOWN), true);
|
||||
assert.strictEqual(Monitor.isImportantForNotification(true, null, PENDING), true);
|
||||
assert.strictEqual(Monitor.isImportantForNotification(true, null, MAINTENANCE), true);
|
||||
});
|
||||
|
||||
// UP -> PENDING = not important
|
||||
test("UP -> PENDING is not important", () => {
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, UP, PENDING), false);
|
||||
});
|
||||
|
||||
// UP -> DOWN = important
|
||||
test("UP -> DOWN is important", () => {
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, UP, DOWN), true);
|
||||
});
|
||||
|
||||
// UP -> UP = not important
|
||||
test("UP -> UP is not important", () => {
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, UP, UP), false);
|
||||
});
|
||||
|
||||
// PENDING -> PENDING = not important
|
||||
test("PENDING -> PENDING is not important", () => {
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, PENDING, PENDING), false);
|
||||
});
|
||||
|
||||
// PENDING -> DOWN = important
|
||||
test("PENDING -> DOWN is important", () => {
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, PENDING, DOWN), true);
|
||||
});
|
||||
|
||||
// PENDING -> UP = important if monitor was DOWN before PENDING (fix for issue #6025)
|
||||
test("PENDING -> UP is important when lastNonPendingStatus was DOWN", () => {
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, PENDING, UP, DOWN), true);
|
||||
});
|
||||
|
||||
test("PENDING -> UP is not important when lastNonPendingStatus was not DOWN", () => {
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, PENDING, UP, UP), false);
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, PENDING, UP, MAINTENANCE), false);
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, PENDING, UP, null), false);
|
||||
});
|
||||
|
||||
// DOWN -> DOWN = not important
|
||||
test("DOWN -> DOWN is not important", () => {
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, DOWN, DOWN), false);
|
||||
});
|
||||
|
||||
// DOWN -> UP = important
|
||||
test("DOWN -> UP is important", () => {
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, DOWN, UP), true);
|
||||
});
|
||||
|
||||
// MAINTENANCE -> MAINTENANCE = not important
|
||||
test("MAINTENANCE -> MAINTENANCE is not important", () => {
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, MAINTENANCE, MAINTENANCE), false);
|
||||
});
|
||||
|
||||
// MAINTENANCE -> UP = not important
|
||||
test("MAINTENANCE -> UP is not important", () => {
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, MAINTENANCE, UP), false);
|
||||
});
|
||||
|
||||
// MAINTENANCE -> DOWN = important
|
||||
test("MAINTENANCE -> DOWN is important", () => {
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, MAINTENANCE, DOWN), true);
|
||||
});
|
||||
|
||||
// DOWN -> MAINTENANCE = not important
|
||||
test("DOWN -> MAINTENANCE is not important", () => {
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, DOWN, MAINTENANCE), false);
|
||||
});
|
||||
|
||||
// UP -> MAINTENANCE = not important
|
||||
test("UP -> MAINTENANCE is not important", () => {
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, UP, MAINTENANCE), false);
|
||||
});
|
||||
|
||||
// Additional edge cases
|
||||
test("PENDING -> UP with undefined lastNonPendingStatus defaults to not important", () => {
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, PENDING, UP, undefined), false);
|
||||
});
|
||||
|
||||
test("non-PENDING -> UP transitions ignore lastNonPendingStatus parameter", () => {
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, DOWN, UP, DOWN), true);
|
||||
assert.strictEqual(Monitor.isImportantForNotification(false, UP, UP, DOWN), false);
|
||||
});
|
||||
});
|
||||
|
||||
Loading…
Reference in New Issue
Block a user