diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationService.java b/app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationService.java index 9bf1bd8f3b..ac7cbda307 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationService.java @@ -20,9 +20,20 @@ public class NotificationService { private final FileRunEventService fileRunEvents; - /** Newest first, and only open failures: one already dealt with is not news. */ + /** + * Newest first, and only open failures about a document: one already dealt with is not news, + * and a row naming no file has nothing the bell can offer beyond saying so. + * + *

Filtered on the named file rather than the kind's scope, because a RUN-scoped kind still + * names one when the editor reported it: a failed tool run belongs here. Applied after the + * limit, so a page can come back short while unattributed rows exist - the review surface is + * where those are meant to be read, and it lists them unfiltered. + */ public List list(int limit) { - return fileRunEvents.list(null, null, limit).stream().map(this::fromFailure).toList(); + return fileRunEvents.list(null, null, limit).stream() + .filter(event -> event.fileId() != null && !event.fileId().isBlank()) + .map(this::fromFailure) + .toList(); } /** Whether the caller sees the whole team's incidents rather than only their own. */ diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/failure/NotificationProjectionTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/failure/NotificationProjectionTest.java index 379bbc1f4c..5b4ce6f1a4 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/failure/NotificationProjectionTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/failure/NotificationProjectionTest.java @@ -121,6 +121,32 @@ class NotificationProjectionTest { .allMatch(action -> action.execution() == FailureActionId.Execution.CLIENT); } + @Test + void holdsBackAFailureNamingNoDocumentBecauseTheBellCouldOnlySaySo() { + // The only row the bell can offer nothing for. The review surface still lists it. + given(FailureKind.UNKNOWN, ACTOR, null); + given(FailureKind.INPUT_PASSWORD_PROTECTED, ACTOR, "f-1"); + + assertThat(controller.list(null).notifications()) + .singleElement() + .satisfies(row -> assertThat(row.fileId()).isEqualTo("f-1")); + } + + @Test + void keepsARunScopedFailureThatStillNamesADocument() { + // An editor-reported tool failure is RUN-scoped but names the file it ran on, so + // filtering on the kind's scope rather than the row would have dropped it. + given(FailureKind.UNKNOWN, ACTOR, "f-2"); + + assertThat(controller.list(null).notifications()) + .singleElement() + .satisfies( + row -> { + assertThat(row.kindId()).isEqualTo("UNKNOWN"); + assertThat(row.fileId()).isEqualTo("f-2"); + }); + } + @Test void namesTheSourceThatFedAnUnattendedRunSoItsFileIdIsNotMistakenForAClientsOwn() { // Without the source a client looks up a hash it can never resolve and calls it