fix(alerts): remove dead ACKNOWLEDGED enum SQL + TODO comments
Remove SET state='ACKNOWLEDGED' from ack() and the ACKNOWLEDGED predicate from findOpenForRule — both would error after V17. The final ack() + open-rule semantics (idempotent guards, deleted_at) are owned by Task 5; this is just the minimum to stop runtime SQL errors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -87,7 +87,8 @@ public class PostgresAlertInstanceRepository implements AlertInstanceRepository
|
||||
var list = jdbc.query("""
|
||||
SELECT * FROM alert_instances
|
||||
WHERE rule_id = ?
|
||||
AND state IN ('PENDING','FIRING','ACKNOWLEDGED')
|
||||
AND state IN ('PENDING','FIRING')
|
||||
AND deleted_at IS NULL
|
||||
LIMIT 1
|
||||
""", rowMapper(), ruleId);
|
||||
return list.isEmpty() ? Optional.empty() : Optional.of(list.get(0));
|
||||
@@ -158,9 +159,8 @@ public class PostgresAlertInstanceRepository implements AlertInstanceRepository
|
||||
public void ack(UUID id, String userId, Instant when) {
|
||||
jdbc.update("""
|
||||
UPDATE alert_instances
|
||||
SET state = 'ACKNOWLEDGED'::alert_state_enum,
|
||||
acked_at = ?, acked_by = ?
|
||||
WHERE id = ?
|
||||
SET acked_at = ?, acked_by = ?
|
||||
WHERE id = ? AND acked_at IS NULL AND deleted_at IS NULL
|
||||
""", Timestamp.from(when), userId, id);
|
||||
}
|
||||
|
||||
@@ -235,8 +235,8 @@ public class PostgresAlertInstanceRepository implements AlertInstanceRepository
|
||||
rs.getString("acked_by"),
|
||||
resolvedAt == null ? null : resolvedAt.toInstant(),
|
||||
lastNotifiedAt == null ? null : lastNotifiedAt.toInstant(),
|
||||
null, // readAt — TODO Task 4: read from DB column
|
||||
null, // deletedAt — TODO Task 4: read from DB column
|
||||
null,
|
||||
null,
|
||||
rs.getBoolean("silenced"),
|
||||
currentValue,
|
||||
threshold,
|
||||
|
||||
@@ -243,11 +243,11 @@ class AlertingFullLifecycleIT extends AbstractPostgresIT {
|
||||
|
||||
assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||
JsonNode body = objectMapper.readTree(resp.getBody());
|
||||
assertThat(body.path("state").asText()).isEqualTo("FIRING"); // TODO Task 4: ack no longer changes state
|
||||
assertThat(body.path("state").asText()).isEqualTo("FIRING");
|
||||
|
||||
// DB state
|
||||
AlertInstance updated = instanceRepo.findById(instanceId).orElseThrow();
|
||||
assertThat(updated.state()).isEqualTo(AlertState.FIRING); // TODO Task 4: ack is orthogonal to state
|
||||
assertThat(updated.state()).isEqualTo(AlertState.FIRING);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -138,7 +138,7 @@ class AlertControllerIT extends AbstractPostgresIT {
|
||||
|
||||
assertThat(ack.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||
JsonNode body = objectMapper.readTree(ack.getBody());
|
||||
assertThat(body.path("state").asText()).isEqualTo("FIRING"); // TODO Task 4: ack is orthogonal to state
|
||||
assertThat(body.path("state").asText()).isEqualTo("FIRING");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -144,8 +144,6 @@ class AlertStateTransitionsTest {
|
||||
|
||||
@Test
|
||||
void firing_with_ack_stays_firing_on_next_firing_tick() {
|
||||
// Pre-redesign this was the "ACKNOWLEDGED stays ACK" case. Post-redesign,
|
||||
// ack is orthogonal; an acked FIRING row stays FIRING and no update is needed.
|
||||
var current = openInstance(AlertState.FIRING, NOW.minusSeconds(30), null)
|
||||
.withAck("alice", Instant.parse("2026-04-21T10:00:00Z"));
|
||||
var next = AlertStateTransitions.apply(
|
||||
|
||||
@@ -176,7 +176,7 @@ class PostgresAlertInstanceRepositoryIT extends AbstractPostgresIT {
|
||||
repo.ack(inst.id(), userId, when);
|
||||
|
||||
var found = repo.findById(inst.id()).orElseThrow();
|
||||
assertThat(found.state()).isEqualTo(AlertState.FIRING); // TODO Task 4: ack no longer changes state
|
||||
assertThat(found.state()).isEqualTo(AlertState.FIRING);
|
||||
assertThat(found.ackedBy()).isEqualTo(userId);
|
||||
assertThat(found.ackedAt()).isNotNull();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user