feat(alerts): controller — DELETE/bulk-delete/bulk-ack/restore + acked/read filters + readAt on DTO
- GET /alerts gains tri-state acked + read query params
- new endpoints: DELETE /{id} (soft-delete), POST /bulk-delete, POST /bulk-ack, POST /{id}/restore
- requireLiveInstance 404s on soft-deleted rows; restore() reads the row regardless
- BulkReadRequest → BulkIdsRequest (shared body for bulk read/ack/delete)
- AlertDto gains readAt; deletedAt stays off the wire
- InAppInboxQuery.listInbox threads acked/read through to the repo (7-arg, no more null placeholders)
- SecurityConfig: new matchers for bulk-ack (VIEWER+), DELETE/bulk-delete/restore (OPERATOR+)
- AlertControllerIT: persistence assertions on /read + /bulk-read; full coverage for new endpoints
- InAppInboxQueryTest: updated to 7-arg listInbox signature
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
hsiegeln
@@ -69,6 +69,10 @@ class AlertControllerIT extends AbstractPostgresIT {
|
||||
jdbcTemplate.update("DELETE FROM users WHERE user_id IN ('test-operator','test-viewer')");
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// Existing tests (baseline)
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
@Test
|
||||
void listReturnsAlertsForEnv() throws Exception {
|
||||
AlertInstance instance = seedInstance(envIdA);
|
||||
@@ -82,7 +86,6 @@ class AlertControllerIT extends AbstractPostgresIT {
|
||||
assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||
JsonNode body = objectMapper.readTree(resp.getBody());
|
||||
assertThat(body.isArray()).isTrue();
|
||||
// The alert we seeded should be present
|
||||
boolean found = false;
|
||||
for (JsonNode node : body) {
|
||||
if (node.path("id").asText().equals(instance.id().toString())) {
|
||||
@@ -95,10 +98,8 @@ class AlertControllerIT extends AbstractPostgresIT {
|
||||
|
||||
@Test
|
||||
void envIsolation() throws Exception {
|
||||
// Seed an alert in env-A
|
||||
AlertInstance instanceA = seedInstance(envIdA);
|
||||
|
||||
// env-B inbox should NOT see env-A's alert
|
||||
ResponseEntity<String> resp = restTemplate.exchange(
|
||||
"/api/v1/environments/" + envSlugB + "/alerts",
|
||||
HttpMethod.GET,
|
||||
@@ -150,6 +151,10 @@ class AlertControllerIT extends AbstractPostgresIT {
|
||||
String.class);
|
||||
|
||||
assertThat(read.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||
|
||||
// Verify persistence — readAt must now be set
|
||||
AlertInstance updated = instanceRepo.findById(instance.id()).orElseThrow();
|
||||
assertThat(updated.readAt()).as("readAt must be set after /read").isNotNull();
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -168,6 +173,12 @@ class AlertControllerIT extends AbstractPostgresIT {
|
||||
String.class);
|
||||
|
||||
assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||
|
||||
// Verify persistence — both must have readAt set
|
||||
assertThat(instanceRepo.findById(i1.id()).orElseThrow().readAt())
|
||||
.as("i1 readAt must be set after bulk-read").isNotNull();
|
||||
assertThat(instanceRepo.findById(i2.id()).orElseThrow().readAt())
|
||||
.as("i2 readAt must be set after bulk-read").isNotNull();
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -180,6 +191,257 @@ class AlertControllerIT extends AbstractPostgresIT {
|
||||
assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// New endpoint tests
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
@Test
|
||||
void delete_softDeletes_and_subsequent_get_returns_404() {
|
||||
AlertInstance instance = seedInstance(envIdA);
|
||||
|
||||
// OPERATOR deletes
|
||||
ResponseEntity<String> del = restTemplate.exchange(
|
||||
"/api/v1/environments/" + envSlugA + "/alerts/" + instance.id(),
|
||||
HttpMethod.DELETE,
|
||||
new HttpEntity<>(securityHelper.authHeadersNoBody(operatorJwt)),
|
||||
String.class);
|
||||
assertThat(del.getStatusCode()).isEqualTo(HttpStatus.NO_CONTENT);
|
||||
|
||||
// Subsequent GET returns 404
|
||||
ResponseEntity<String> get = restTemplate.exchange(
|
||||
"/api/v1/environments/" + envSlugA + "/alerts/" + instance.id(),
|
||||
HttpMethod.GET,
|
||||
new HttpEntity<>(securityHelper.authHeadersNoBody(operatorJwt)),
|
||||
String.class);
|
||||
assertThat(get.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
|
||||
}
|
||||
|
||||
@Test
|
||||
void delete_non_operator_returns_403() {
|
||||
AlertInstance instance = seedInstance(envIdA);
|
||||
|
||||
ResponseEntity<String> del = restTemplate.exchange(
|
||||
"/api/v1/environments/" + envSlugA + "/alerts/" + instance.id(),
|
||||
HttpMethod.DELETE,
|
||||
new HttpEntity<>(securityHelper.authHeadersNoBody(viewerJwt)),
|
||||
String.class);
|
||||
assertThat(del.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN);
|
||||
}
|
||||
|
||||
@Test
|
||||
void bulkDelete_only_affects_matching_env() {
|
||||
AlertInstance inEnvA = seedInstance(envIdA);
|
||||
AlertInstance inEnvA2 = seedInstance(envIdA);
|
||||
AlertInstance inEnvB = seedInstance(envIdB);
|
||||
|
||||
String body = """
|
||||
{"instanceIds":["%s","%s","%s"]}
|
||||
""".formatted(inEnvA.id(), inEnvA2.id(), inEnvB.id());
|
||||
|
||||
ResponseEntity<String> resp = restTemplate.exchange(
|
||||
"/api/v1/environments/" + envSlugA + "/alerts/bulk-delete",
|
||||
HttpMethod.POST,
|
||||
new HttpEntity<>(body, securityHelper.authHeaders(operatorJwt)),
|
||||
String.class);
|
||||
assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||
|
||||
// env-A alerts should be soft-deleted
|
||||
assertThat(instanceRepo.findById(inEnvA.id()).orElseThrow().deletedAt())
|
||||
.as("inEnvA should be soft-deleted").isNotNull();
|
||||
assertThat(instanceRepo.findById(inEnvA2.id()).orElseThrow().deletedAt())
|
||||
.as("inEnvA2 should be soft-deleted").isNotNull();
|
||||
|
||||
// env-B alert must NOT be soft-deleted
|
||||
assertThat(instanceRepo.findById(inEnvB.id()).orElseThrow().deletedAt())
|
||||
.as("inEnvB must not be soft-deleted via env-A bulk-delete").isNull();
|
||||
}
|
||||
|
||||
@Test
|
||||
void bulkAck_only_touches_unacked_rows() {
|
||||
AlertInstance i1 = seedInstance(envIdA);
|
||||
AlertInstance i2 = seedInstance(envIdA);
|
||||
|
||||
// Pre-ack i1 with an existing user (must be in users table due to FK)
|
||||
instanceRepo.ack(i1.id(), "test-viewer", Instant.now().minusSeconds(60));
|
||||
|
||||
String body = """
|
||||
{"instanceIds":["%s","%s"]}
|
||||
""".formatted(i1.id(), i2.id());
|
||||
|
||||
ResponseEntity<String> resp = restTemplate.exchange(
|
||||
"/api/v1/environments/" + envSlugA + "/alerts/bulk-ack",
|
||||
HttpMethod.POST,
|
||||
new HttpEntity<>(body, securityHelper.authHeaders(operatorJwt)),
|
||||
String.class);
|
||||
assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||
|
||||
// i1's ackedBy must remain "test-viewer" (bulk-ack skips already-acked rows)
|
||||
AlertInstance refreshed1 = instanceRepo.findById(i1.id()).orElseThrow();
|
||||
assertThat(refreshed1.ackedBy()).as("previously-acked row must keep original ackedBy").isEqualTo("test-viewer");
|
||||
|
||||
// i2 must now be acked
|
||||
AlertInstance refreshed2 = instanceRepo.findById(i2.id()).orElseThrow();
|
||||
assertThat(refreshed2.ackedAt()).as("i2 must be acked after bulk-ack").isNotNull();
|
||||
}
|
||||
|
||||
@Test
|
||||
void restore_clears_deleted_at_and_reappears_in_inbox() throws Exception {
|
||||
AlertInstance instance = seedInstance(envIdA);
|
||||
|
||||
// Soft-delete first
|
||||
restTemplate.exchange(
|
||||
"/api/v1/environments/" + envSlugA + "/alerts/" + instance.id(),
|
||||
HttpMethod.DELETE,
|
||||
new HttpEntity<>(securityHelper.authHeadersNoBody(operatorJwt)),
|
||||
String.class);
|
||||
assertThat(instanceRepo.findById(instance.id()).orElseThrow().deletedAt()).isNotNull();
|
||||
|
||||
// Restore
|
||||
ResponseEntity<String> restoreResp = restTemplate.exchange(
|
||||
"/api/v1/environments/" + envSlugA + "/alerts/" + instance.id() + "/restore",
|
||||
HttpMethod.POST,
|
||||
new HttpEntity<>(securityHelper.authHeaders(operatorJwt)),
|
||||
String.class);
|
||||
assertThat(restoreResp.getStatusCode()).isEqualTo(HttpStatus.NO_CONTENT);
|
||||
|
||||
// deletedAt must be cleared
|
||||
assertThat(instanceRepo.findById(instance.id()).orElseThrow().deletedAt())
|
||||
.as("deletedAt must be null after restore").isNull();
|
||||
|
||||
// Alert reappears in inbox list
|
||||
ResponseEntity<String> listResp = restTemplate.exchange(
|
||||
"/api/v1/environments/" + envSlugA + "/alerts",
|
||||
HttpMethod.GET,
|
||||
new HttpEntity<>(securityHelper.authHeadersNoBody(operatorJwt)),
|
||||
String.class);
|
||||
assertThat(listResp.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||
JsonNode body = objectMapper.readTree(listResp.getBody());
|
||||
boolean found = false;
|
||||
for (JsonNode node : body) {
|
||||
if (node.path("id").asText().equals(instance.id().toString())) {
|
||||
found = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
assertThat(found).as("restored alert must reappear in inbox").isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
void list_respects_acked_filter_tristate() throws Exception {
|
||||
AlertInstance unacked = seedInstance(envIdA);
|
||||
AlertInstance acked = seedInstance(envIdA);
|
||||
instanceRepo.ack(acked.id(), "test-operator", Instant.now());
|
||||
|
||||
// ?acked=false — only unacked
|
||||
ResponseEntity<String> falseResp = restTemplate.exchange(
|
||||
"/api/v1/environments/" + envSlugA + "/alerts?acked=false",
|
||||
HttpMethod.GET,
|
||||
new HttpEntity<>(securityHelper.authHeadersNoBody(operatorJwt)),
|
||||
String.class);
|
||||
assertThat(falseResp.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||
JsonNode falseBody = objectMapper.readTree(falseResp.getBody());
|
||||
boolean unackedFound = false, ackedFoundInFalse = false;
|
||||
for (JsonNode node : falseBody) {
|
||||
String id = node.path("id").asText();
|
||||
if (id.equals(unacked.id().toString())) unackedFound = true;
|
||||
if (id.equals(acked.id().toString())) ackedFoundInFalse = true;
|
||||
}
|
||||
assertThat(unackedFound).as("unacked alert must appear with ?acked=false").isTrue();
|
||||
assertThat(ackedFoundInFalse).as("acked alert must NOT appear with ?acked=false").isFalse();
|
||||
|
||||
// ?acked=true — only acked
|
||||
ResponseEntity<String> trueResp = restTemplate.exchange(
|
||||
"/api/v1/environments/" + envSlugA + "/alerts?acked=true",
|
||||
HttpMethod.GET,
|
||||
new HttpEntity<>(securityHelper.authHeadersNoBody(operatorJwt)),
|
||||
String.class);
|
||||
assertThat(trueResp.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||
JsonNode trueBody = objectMapper.readTree(trueResp.getBody());
|
||||
boolean ackedFound = false, unackedFoundInTrue = false;
|
||||
for (JsonNode node : trueBody) {
|
||||
String id = node.path("id").asText();
|
||||
if (id.equals(acked.id().toString())) ackedFound = true;
|
||||
if (id.equals(unacked.id().toString())) unackedFoundInTrue = true;
|
||||
}
|
||||
assertThat(ackedFound).as("acked alert must appear with ?acked=true").isTrue();
|
||||
assertThat(unackedFoundInTrue).as("unacked alert must NOT appear with ?acked=true").isFalse();
|
||||
|
||||
// no param — both visible
|
||||
ResponseEntity<String> allResp = restTemplate.exchange(
|
||||
"/api/v1/environments/" + envSlugA + "/alerts",
|
||||
HttpMethod.GET,
|
||||
new HttpEntity<>(securityHelper.authHeadersNoBody(operatorJwt)),
|
||||
String.class);
|
||||
assertThat(allResp.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||
JsonNode allBody = objectMapper.readTree(allResp.getBody());
|
||||
boolean bothUnacked = false, bothAcked = false;
|
||||
for (JsonNode node : allBody) {
|
||||
String id = node.path("id").asText();
|
||||
if (id.equals(unacked.id().toString())) bothUnacked = true;
|
||||
if (id.equals(acked.id().toString())) bothAcked = true;
|
||||
}
|
||||
assertThat(bothUnacked).as("unacked must appear with no acked filter").isTrue();
|
||||
assertThat(bothAcked).as("acked must appear with no acked filter").isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
void list_respects_read_filter_tristate() throws Exception {
|
||||
AlertInstance unread = seedInstance(envIdA);
|
||||
AlertInstance read = seedInstance(envIdA);
|
||||
instanceRepo.markRead(read.id(), Instant.now());
|
||||
|
||||
// ?read=false — only unread
|
||||
ResponseEntity<String> falseResp = restTemplate.exchange(
|
||||
"/api/v1/environments/" + envSlugA + "/alerts?read=false",
|
||||
HttpMethod.GET,
|
||||
new HttpEntity<>(securityHelper.authHeadersNoBody(operatorJwt)),
|
||||
String.class);
|
||||
assertThat(falseResp.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||
JsonNode falseBody = objectMapper.readTree(falseResp.getBody());
|
||||
boolean unreadFound = false, readFoundInFalse = false;
|
||||
for (JsonNode node : falseBody) {
|
||||
String id = node.path("id").asText();
|
||||
if (id.equals(unread.id().toString())) unreadFound = true;
|
||||
if (id.equals(read.id().toString())) readFoundInFalse = true;
|
||||
}
|
||||
assertThat(unreadFound).as("unread alert must appear with ?read=false").isTrue();
|
||||
assertThat(readFoundInFalse).as("read alert must NOT appear with ?read=false").isFalse();
|
||||
|
||||
// ?read=true — only read
|
||||
ResponseEntity<String> trueResp = restTemplate.exchange(
|
||||
"/api/v1/environments/" + envSlugA + "/alerts?read=true",
|
||||
HttpMethod.GET,
|
||||
new HttpEntity<>(securityHelper.authHeadersNoBody(operatorJwt)),
|
||||
String.class);
|
||||
assertThat(trueResp.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||
JsonNode trueBody = objectMapper.readTree(trueResp.getBody());
|
||||
boolean readFound = false, unreadFoundInTrue = false;
|
||||
for (JsonNode node : trueBody) {
|
||||
String id = node.path("id").asText();
|
||||
if (id.equals(read.id().toString())) readFound = true;
|
||||
if (id.equals(unread.id().toString())) unreadFoundInTrue = true;
|
||||
}
|
||||
assertThat(readFound).as("read alert must appear with ?read=true").isTrue();
|
||||
assertThat(unreadFoundInTrue).as("unread alert must NOT appear with ?read=true").isFalse();
|
||||
|
||||
// no param — both visible
|
||||
ResponseEntity<String> allResp = restTemplate.exchange(
|
||||
"/api/v1/environments/" + envSlugA + "/alerts",
|
||||
HttpMethod.GET,
|
||||
new HttpEntity<>(securityHelper.authHeadersNoBody(operatorJwt)),
|
||||
String.class);
|
||||
assertThat(allResp.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||
JsonNode allBody = objectMapper.readTree(allResp.getBody());
|
||||
boolean bothUnread = false, bothRead = false;
|
||||
for (JsonNode node : allBody) {
|
||||
String id = node.path("id").asText();
|
||||
if (id.equals(unread.id().toString())) bothUnread = true;
|
||||
if (id.equals(read.id().toString())) bothRead = true;
|
||||
}
|
||||
assertThat(bothUnread).as("unread must appear with no read filter").isTrue();
|
||||
assertThat(bothRead).as("read must appear with no read filter").isTrue();
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// Helpers
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
@@ -80,7 +80,7 @@ class InAppInboxQueryTest {
|
||||
eq(USER_ID), eq(List.of("OPERATOR")), isNull(), isNull(), isNull(), isNull(), eq(20)))
|
||||
.thenReturn(List.of());
|
||||
|
||||
List<AlertInstance> result = query.listInbox(ENV_ID, USER_ID, 20);
|
||||
List<AlertInstance> result = query.listInbox(ENV_ID, USER_ID, null, null, null, null, 20);
|
||||
assertThat(result).isEmpty();
|
||||
verify(instanceRepo).listForInbox(ENV_ID, List.of(groupId.toString()),
|
||||
USER_ID, List.of("OPERATOR"), null, null, null, null, 20);
|
||||
@@ -99,7 +99,7 @@ class InAppInboxQueryTest {
|
||||
eq(states), eq(severities), isNull(), isNull(), eq(25)))
|
||||
.thenReturn(List.of());
|
||||
|
||||
query.listInbox(ENV_ID, USER_ID, states, severities, 25);
|
||||
query.listInbox(ENV_ID, USER_ID, states, severities, null, null, 25);
|
||||
verify(instanceRepo).listForInbox(ENV_ID, List.of(), USER_ID, List.of(),
|
||||
states, severities, null, null, 25);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user