;
sources: string[];
+ runsOnEditor: boolean;
scopeTypes: string[];
reviewerEmail: string;
folder: PolicyFolderSettings;
From f6661a8f87d81ced83ca0d9e14ae79a25d47ccf3 Mon Sep 17 00:00:00 2001
From: EthanHealy01 <80844253+EthanHealy01@users.noreply.github.com>
Date: Tue, 1 Sep 2026 13:25:19 +0000
Subject: [PATCH 34/37] Failure action slots, resolve transition, and the bell
that renders them (Review Flow PR 5a) (#7761)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Review Flow PR 5a — the first half of #7479, which stays open for
reference until both halves land. This PR is the ranking and the
bookkeeping; #7762 adds the retry handlers. Merging both reproduces
#7479's diff byte-for-byte.
## What's added
**The action slot model (backend).** `FailureActionSlot` ranks each of a
kind's offers as its `RESOLUTION`, `SECONDARY` or `OVERFLOW`.
`FailureKind` now declares placement per offer — the password-protected
kind names `DECRYPT_AND_RETRY` as its resolution, `UNKNOWN` leads with a
plain `RETRY` — and `FailureActionId` gains those two ids. The
declarations are data; their client handlers arrive in the follow-up, so
this build withholds them with a reason rather than rendering unwired
buttons (the same forward-compatibility #7478 relied on).
**A resolve transition.** `POST /api/v1/notifications/{id}/resolved`
lets a client report a failure fixed. `NotificationSource.parse` turns a
qualified notification id back into the source that owns it, and
`FileRunEventService` folds the resolution into the incident rather than
deleting it.
**`viewerReviewsTeam` on the list response.** A member sees only rows
whose document this browser holds — they can neither open nor fix
anything else — while a team reviewer keeps every row.
**The bell renders the ranking** (`promoteActions`): one primary button,
at most one secondary, the rest in an overflow menu beside **Copy log**.
The row's body is the kind's own sentence; the raw failure message moves
into the menu.
**Read state is a timestamp, not a row id.** `readThroughAt` replaces
`lastSeenId`: when a resolved or dismissed row leaves the list, the rows
below it stay read instead of re-lighting the badge.
## How to test
Needs a proprietary or SaaS build with login enabled (`task dev:all`,
sign in).
1. **Create a failure.** Add a password-protected PDF to the editor and
choose **Skip for now**; the upload's policy run fails on it.
2. **Open the bell.** The row reads the kind's sentence, not a stack
trace. Its primary button is **View file** — the server offers Decrypt
and retry as the resolution, but this build withholds it (handler lands
in the follow-up), so the best renderable offer is promoted instead.
3. **Open the row's ⋯ menu.** View in processor and Dismiss sit there,
along with **Copy log**, which copies the raw message.
4. **Check the read marker survives a departure.** With two failures,
open the bell (badge clears), dismiss the newer row, and refresh: the
badge stays dark. On main, the marker held the departed row's id and the
older row re-read as unread.
5. **Member visibility.** As a plain member, a failure recorded from
another browser does not appear in the bell; as a team reviewer it does.
6. **Resolve endpoint.** `POST
/api/v1/notifications/failure-{eventId}/resolved` as the owner removes
the row on the next poll; `NotificationResolveTest` pins refusal for a
non-owner, an unknown id, and a foreign prefix.
## Migration
None.
---
.../proprietary/failure/FailureActionId.java | 13 +
.../failure/FailureActionSlot.java | 14 +
.../proprietary/failure/FailureKind.java | 92 +++---
.../failure/FileRunEventRepository.java | 9 +-
.../failure/FileRunEventService.java | 57 +++-
.../failure/FileRunEventStatus.java | 10 +-
.../proprietary/failure/FileRunEventView.java | 6 +-
.../notification/NotificationController.java | 39 ++-
.../notification/NotificationService.java | 42 ++-
.../notification/NotificationSource.java | 22 ++
.../accountlink/ConnectServiceTest.java | 2 -
.../failure/CheckConstrainedEnumsTest.java | 1 +
.../proprietary/failure/FailureKindTest.java | 74 ++---
.../failure/FileRunEventControllerTest.java | 2 +
.../FileRunEventHttpIntegrationTest.java | 1 +
.../failure/FileRunEventServiceTest.java | 120 ++++++-
.../InMemoryFileRunEventRepository.java | 4 +-
.../failure/NotificationProjectionTest.java | 73 +++++
.../failure/NotificationResolveTest.java | 152 +++++++++
.../ConnectRequestServiceTest.java | 2 -
.../public/locales/en-US/translation.toml | 21 +-
.../notifications/NotificationBell.css | 51 ++-
.../notifications/NotificationBell.test.tsx | 174 +++++++---
.../notifications/NotificationItem.tsx | 156 ++++-----
.../notifications/NotificationPanel.tsx | 6 +-
.../notificationActionSlots.test.ts | 299 ++++++++++++++++++
.../notifications/notificationActionSlots.ts | 65 ++++
.../QuickNavRailNotifications.test.tsx | 16 +-
.../src/core/extensions/accountLogout.ts | 3 +
.../src/core/hooks/useNotifications.test.ts | 249 ++++++++++++---
.../editor/src/core/hooks/useNotifications.ts | 113 +++++--
.../editor/src/core/services/notifications.ts | 43 ++-
.../src/desktop/extensions/accountLogout.ts | 3 +
.../notificationActions.test.tsx | 1 +
.../proprietary/extensions/accountLogout.ts | 3 +
35 files changed, 1597 insertions(+), 341 deletions(-)
create mode 100644 app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureActionSlot.java
create mode 100644 app/proprietary/src/test/java/stirling/software/proprietary/failure/NotificationResolveTest.java
create mode 100644 frontend/editor/src/core/components/notifications/notificationActionSlots.test.ts
create mode 100644 frontend/editor/src/core/components/notifications/notificationActionSlots.ts
diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureActionId.java b/app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureActionId.java
index 8f60e9f4a0..968917ba4e 100644
--- a/app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureActionId.java
+++ b/app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureActionId.java
@@ -18,6 +18,19 @@ public enum FailureActionId {
DISMISS(Execution.SERVER, "Dismiss"),
+ /**
+ * Open the failed operation in the client with its document, for the owner to run again
+ * themselves. Not a re-run: the settings are theirs to check first.
+ */
+ OPEN_IN_TOOL(Execution.CLIENT, "Retry"),
+
+ /**
+ * Ask the owner for the password and unlock the document in their client. Re-running is implied
+ * rather than named: an id says what a caller must supply, and a {@link
+ * FailureActionSlot#RESOLUTION} runs the failed work again once it has it.
+ */
+ DECRYPT(Execution.CLIENT, "Decrypt and retry"),
+
/** Open the document behind the incident, in whichever client can resolve its id. */
VIEW_FILE(Execution.CLIENT, "View file"),
diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureActionSlot.java b/app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureActionSlot.java
new file mode 100644
index 0000000000..7a001dc8c1
--- /dev/null
+++ b/app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureActionSlot.java
@@ -0,0 +1,14 @@
+package stirling.software.proprietary.failure;
+
+/** Placement intent, not layout: the client promotes, knowing what it can actually run. */
+public enum FailureActionSlot {
+
+ /** The action that resolves the failure. At most one per kind. */
+ RESOLUTION,
+
+ /** Offered alongside the resolution, for a caller the resolution is not aimed at. */
+ SECONDARY,
+
+ /** Available but folded away: correct, rarely what anyone wants to press next. */
+ OVERFLOW
+}
diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureKind.java b/app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureKind.java
index fa2fd93448..9b507555e9 100644
--- a/app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureKind.java
+++ b/app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureKind.java
@@ -1,8 +1,12 @@
package stirling.software.proprietary.failure;
+import static stirling.software.proprietary.failure.FailureActionId.DECRYPT;
import static stirling.software.proprietary.failure.FailureActionId.DISMISS;
+import static stirling.software.proprietary.failure.FailureActionId.OPEN_IN_TOOL;
import static stirling.software.proprietary.failure.FailureActionId.VIEW_FILE;
import static stirling.software.proprietary.failure.FailureActionId.VIEW_IN_PROCESSOR;
+import static stirling.software.proprietary.failure.FailureActionSlot.OVERFLOW;
+import static stirling.software.proprietary.failure.FailureActionSlot.SECONDARY;
import static stirling.software.proprietary.failure.FailureAudience.ANYONE_WHO_SEES;
import static stirling.software.proprietary.failure.FailureAudience.OWNER;
import static stirling.software.proprietary.failure.FailureAudience.TEAM_REVIEWER;
@@ -21,11 +25,8 @@ import lombok.AccessLevel;
import lombok.Getter;
/**
- * The registry of failure kinds, described as data: a stable id, i18n keys and an English fallback
- * like {@code ExceptionUtils.ErrorCode}, plus the facets a review surface needs.
- *
- * A new kind ships as a registry entry plus copy. Each offer also says who it is for, since one
- * incident is read both by whoever hit it and by whoever reviews after them.
+ * The registry of failure kinds as data: id, i18n keys, English fallback, plus the facets a review
+ * surface needs. A new kind ships as an entry plus copy; each offer says who it is for and where.
*/
@Getter
public enum FailureKind {
@@ -36,9 +37,12 @@ public enum FailureKind {
FailureScope.FILE,
errorCodes("E004"),
fallback("This document is password-protected, so the pipeline could not read it."),
- offer(VIEW_FILE, OWNER),
- offer(VIEW_IN_PROCESSOR, TEAM_REVIEWER),
- offer(DISMISS, ANYONE_WHO_SEES)),
+ // The password is the fix; the owner's own document is the runner-up.
+ resolution(DECRYPT, OWNER),
+ global(VIEW_FILE, OWNER, SECONDARY),
+ global(VIEW_IN_PROCESSOR, TEAM_REVIEWER, OVERFLOW),
+ global(OPEN_IN_TOOL, OWNER, OVERFLOW),
+ global(DISMISS, ANYONE_WHO_SEES, OVERFLOW)),
UNKNOWN(
FailureStage.INTERNAL,
@@ -47,11 +51,11 @@ public enum FailureKind {
FailureScope.RUN,
noErrorCodes(),
fallback("This run failed for a reason Stirling does not yet recognise."),
- // Same order as every other kind: declaration order is display order, so the document
- // leads wherever it is offered rather than moving between failures.
- offer(VIEW_FILE, OWNER),
- offer(VIEW_IN_PROCESSOR, TEAM_REVIEWER),
- offer(DISMISS, ANYONE_WHO_SEES));
+ // No known fix to declare, so a plain retry leads: these are often one-offs.
+ global(OPEN_IN_TOOL, OWNER, SECONDARY),
+ global(VIEW_FILE, OWNER, SECONDARY),
+ global(VIEW_IN_PROCESSOR, TEAM_REVIEWER, OVERFLOW),
+ global(DISMISS, ANYONE_WHO_SEES, OVERFLOW));
private static final String KEY_PREFIX = "portal.failures.kind.";
private static final String ACTION_KEY_PREFIX = "portal.failures.action.";
@@ -98,27 +102,37 @@ public enum FailureKind {
this.offers = List.of(offers);
}
- /**
- * One ordered list rather than ids plus parallel maps of audiences and labels, which could
- * disagree with each other.
- *
- * @param labelKeySuffix key under {@code portal.failures.action.}, or null for the generic
- * label
- */
- private record Offer(FailureActionId id, FailureAudience audience, String labelKeySuffix) {}
+ /** One ordered list, not parallel maps of audiences, slots and labels that could disagree. */
+ private record Offer(
+ FailureActionId id,
+ FailureAudience audience,
+ FailureActionSlot slot,
+ String labelKeySuffix) {}
- /** Declaration order is display order. */
- private static Offer offer(FailureActionId id, FailureAudience audience) {
- return new Offer(id, audience, null);
+ /** The action that fixes this kind. One per kind: needing two would make it two kinds. */
+ private static Offer resolution(FailureActionId id, FailureAudience audience) {
+ return new Offer(id, audience, FailureActionSlot.RESOLUTION, null);
}
- /**
- * As {@link #offer(FailureActionId, FailureAudience)}, but labelled by this kind's own wording
- * where the shared one reads badly.
- */
- private static Offer offer(
+ /** As {@link #resolution(FailureActionId, FailureAudience)}, with this kind's own wording. */
+ private static Offer resolution(
FailureActionId id, FailureAudience audience, String labelKeySuffix) {
- return new Offer(id, audience, labelKeySuffix);
+ return new Offer(id, audience, FailureActionSlot.RESOLUTION, labelKeySuffix);
+ }
+
+ /** Not this kind's fix: an offer any kind can make, with the shared wording. */
+ private static Offer global(
+ FailureActionId id, FailureAudience audience, FailureActionSlot slot) {
+ return new Offer(id, audience, slot, null);
+ }
+
+ /** As above, with this kind's own wording where the shared one reads badly. */
+ private static Offer global(
+ FailureActionId id,
+ FailureAudience audience,
+ FailureActionSlot slot,
+ String labelKeySuffix) {
+ return new Offer(id, audience, slot, labelKeySuffix);
}
/**
@@ -157,21 +171,25 @@ public enum FailureKind {
return offers.stream().map(Offer::id).toList();
}
- /**
- * What this kind offers, in declaration order, each with its label resolved. What a review
- * surface reads, so it never has to ask two separate questions about one offer.
- */
+ /** What this kind offers, in declaration order, each with label and placement resolved. */
public List getOfferedActions() {
return offers.stream()
.map(
offer ->
new OfferedAction(
- offer.id(), labelKeyFor(offer.id()), offer.audience()))
+ offer.id(),
+ labelKeyFor(offer.id()),
+ offer.audience(),
+ offer.slot()))
.toList();
}
- /** One action as a kind declares it: what to call it and who it is for. */
- public record OfferedAction(FailureActionId id, String labelKey, FailureAudience audience) {}
+ /** One action as a kind declares it: what to call it, who it is for, where it wants to sit. */
+ public record OfferedAction(
+ FailureActionId id,
+ String labelKey,
+ FailureAudience audience,
+ FailureActionSlot slot) {}
/** Whether this kind offers {@code action}. The dispatch guard: see {@code FailureActionId}. */
public boolean declares(FailureActionId action) {
diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventRepository.java b/app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventRepository.java
index 316dd34925..ae6b7acb0c 100644
--- a/app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventRepository.java
+++ b/app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventRepository.java
@@ -64,16 +64,17 @@ public interface FileRunEventRepository extends JpaRepository{@code "anonymous"} with login disabled, where the one operator is every viewer.
+ */
+ public String viewerKey() {
+ String actor = currentActor();
+ return actor == null || actor.isBlank() ? "anonymous" : sha256Prefix(actor);
+ }
+
+ /** First 8 bytes of SHA-256 as hex: stable, one-way, and collision-safe enough to key on. */
+ private static String sha256Prefix(String value) {
+ try {
+ byte[] digest =
+ MessageDigest.getInstance("SHA-256")
+ .digest(value.getBytes(StandardCharsets.UTF_8));
+ return HexFormat.of().formatHex(digest, 0, 8);
+ } catch (NoSuchAlgorithmException e) {
+ // Every JVM ships SHA-256; a constant here would silently merge two viewers' read
+ // state, so the caller gets no key and the client falls back to showing everything.
+ log.warn("SHA-256 unavailable, so notifications cannot be scoped to a viewer", e);
+ return "";
+ }
+ }
+
private FailureActionId parseActionId(String actionId) {
for (FailureActionId candidate : FailureActionId.values()) {
if (candidate.name().equals(actionId)) {
@@ -326,6 +370,11 @@ public class FileRunEventService {
return applicationProperties.getSecurity().isEnableLogin();
}
+ /** One action offered to one caller, availability resolved. */
public record AvailableAction(
- FailureActionId id, String labelKey, boolean enabled, String disabledReasonKey) {}
+ FailureActionId id,
+ String labelKey,
+ FailureActionSlot slot,
+ boolean enabled,
+ String disabledReasonKey) {}
}
diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventStatus.java b/app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventStatus.java
index 9bf0e0b607..5e2499753a 100644
--- a/app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventStatus.java
+++ b/app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventStatus.java
@@ -3,10 +3,7 @@ package stirling.software.proprietary.failure;
import java.util.Arrays;
import java.util.List;
-/**
- * Disposition of one recorded failure. {@code RESOLVED} is declared but not set yet (it becomes
- * system-set later); the rollup already defines what a repeat means for it, which is to reopen.
- */
+/** Disposition of one recorded failure. {@code RESOLVED} is system-set; a repeat reopens it. */
public enum FileRunEventStatus {
NEW(false),
ACKNOWLEDGED(false),
@@ -14,9 +11,8 @@ public enum FileRunEventStatus {
RESOLVED(true),
/**
- * The document this incident was about was deleted from its owner's editor, so there is nothing
- * left to act on. Distinct from {@code DISMISSED}, which is a reviewer's decision, and from
- * {@code RESOLVED}, which reopens on recurrence: this one cannot recur, the file is gone.
+ * The document was deleted, so there is nothing left to act on. A recurrence reopens it like
+ * {@code RESOLVED}: a fresh failure is proof the document is back.
*/
FILE_REMOVED(true);
diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventView.java b/app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventView.java
index b88ba3d48d..3b7501e9e2 100644
--- a/app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventView.java
+++ b/app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventView.java
@@ -61,14 +61,15 @@ public record FileRunEventView(
}
/**
- * {@code defaultLabel} and {@code execution} let a client render and route an action it was
- * never built with. Declaration order is display order.
+ * {@code defaultLabel} and {@code execution} let a client render an action it was never built
+ * with; {@code slot} is placement intent. See {@link FailureActionSlot}.
*/
public record ActionView(
String id,
String labelKey,
String defaultLabel,
FailureActionId.Execution execution,
+ FailureActionSlot slot,
boolean enabled,
String disabledReasonKey) {
@@ -78,6 +79,7 @@ public record FileRunEventView(
action.labelKey(),
action.id().getDefaultLabel(),
action.id().getExecution(),
+ action.slot(),
action.enabled(),
action.disabledReasonKey());
}
diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationController.java b/app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationController.java
index f2bf36a8dc..4c51dd4e72 100644
--- a/app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationController.java
+++ b/app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationController.java
@@ -2,10 +2,14 @@ package stirling.software.proprietary.notification;
import java.util.List;
+import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
+import org.springframework.web.server.ResponseStatusException;
import io.swagger.v3.oas.annotations.Hidden;
import io.swagger.v3.oas.annotations.Operation;
@@ -13,9 +17,11 @@ import io.swagger.v3.oas.annotations.tags.Tag;
import lombok.RequiredArgsConstructor;
+import stirling.software.proprietary.failure.FailureActionException;
+
/**
- * Open to any authenticated user, unlike the failure endpoints it draws on: each source scopes its
- * own rows. Read-only, because every action a notification offers runs on the client's own device.
+ * Open to any authenticated user: each source scopes its own rows. Every action runs on the
+ * client's own device, so the only write is it reporting a fix.
*/
@RestController
@RequestMapping("/api/v1/notifications")
@@ -40,9 +46,34 @@ public class NotificationController {
+ " to mark read here yet: the client tracks what it has shown.")
public NotificationsResponse list(@RequestParam(required = false) Integer limit) {
int capped = Math.min(limit == null ? DEFAULT_LIMIT : Math.max(1, limit), MAX_LIMIT);
- return new NotificationsResponse(notifications.list(capped));
+ return new NotificationsResponse(
+ notifications.list(capped),
+ notifications.callerReviewsTeam(),
+ notifications.callerViewerKey());
+ }
+
+ @PostMapping("/{notificationId}/resolved")
+ @Operation(
+ summary = "Record that a client-side retry fixed what a notification was about",
+ description =
+ "Takes the prefixed notification id, not the producing row's id. Not an action:"
+ + " nobody is offered a resolve button, and a recurrence brings the"
+ + " notification back.")
+ public NotificationView resolved(@PathVariable String notificationId) {
+ try {
+ return notifications.resolve(notificationId);
+ } catch (IllegalArgumentException e) {
+ throw new ResponseStatusException(HttpStatus.BAD_REQUEST, e.getMessage(), e);
+ } catch (FailureActionException e) {
+ throw new ResponseStatusException(
+ FailureActionException.statusOf(e.getReason()), e.getMessage(), e);
+ }
}
/** Wrapped so paging or a total can be added without breaking clients. */
- public record NotificationsResponse(List notifications) {}
+ public record NotificationsResponse(
+ List notifications,
+ boolean viewerReviewsTeam,
+ /** Opaque; the client scopes its own read state on it. Empty means "cannot scope". */
+ String viewerKey) {}
}
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 f7bf3b8530..1922ed3e6b 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,47 @@ 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. */
+ public boolean callerReviewsTeam() {
+ return fileRunEvents.reviewsTeam();
+ }
+
+ /** Opaque and stable, so a shared browser can keep one viewer's read state off another's. */
+ public String callerViewerKey() {
+ return fileRunEvents.viewerKey();
+ }
+
+ /** Takes the prefixed id, so the bell cannot reach a failure endpoint even by accident. */
+ public NotificationView resolve(String notificationId) {
+ NotificationSource.QualifiedId qualified = qualify(notificationId);
+ return switch (qualified.source()) {
+ case FAILURE -> fromFailure(fileRunEvents.resolve(qualified.rowId()));
+ };
+ }
+
+ /** The source and row id behind a notification id, refusing anything that is not one. */
+ private static NotificationSource.QualifiedId qualify(String notificationId) {
+ return NotificationSource.parse(notificationId)
+ .orElseThrow(
+ () ->
+ new IllegalArgumentException(
+ "Not a notification id: " + notificationId));
}
/** Prefixes the row id on the way out, so it is never sent bare. */
diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationSource.java b/app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationSource.java
index 007e51616f..0dbe99cee6 100644
--- a/app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationSource.java
+++ b/app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationSource.java
@@ -1,6 +1,8 @@
package stirling.software.proprietary.notification;
+import java.util.Arrays;
import java.util.Locale;
+import java.util.Optional;
/**
* Which subsystem produced a notification. Every id is prefixed with it, so a client never holds
@@ -18,4 +20,24 @@ public enum NotificationSource {
public String qualify(String sourceRowId) {
return prefix() + sourceRowId;
}
+
+ /** Empty rather than throwing for an unprefixed or unknown id: both arrive from clients. */
+ public static Optional parse(String notificationId) {
+ if (notificationId == null) {
+ return Optional.empty();
+ }
+ int separator = notificationId.indexOf(SEPARATOR);
+ if (separator <= 0 || separator == notificationId.length() - 1) {
+ return Optional.empty();
+ }
+ String prefix = notificationId.substring(0, separator);
+ String rowId = notificationId.substring(separator + 1);
+ return Arrays.stream(values())
+ .filter(source -> source.name().equalsIgnoreCase(prefix))
+ .findFirst()
+ .map(source -> new QualifiedId(source, rowId));
+ }
+
+ /** A notification id split into the source that owns it and that source's own row id. */
+ public record QualifiedId(NotificationSource source, String rowId) {}
}
diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/ConnectServiceTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/ConnectServiceTest.java
index 5e040e4ad7..5af25d7250 100644
--- a/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/ConnectServiceTest.java
+++ b/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/ConnectServiceTest.java
@@ -357,8 +357,6 @@ class ConnectServiceTest {
assertThat(status.authorizeUrl()).isEqualTo("https://app.example.com/link?request=req-1");
}
- // ---------------------------------------------------------------------------------------
-
/** A start with nothing but the reconstructed request URL, as a headless caller would send. */
private static ConnectService.CallbackHint fromRequest(String derivedBaseUrl) {
return new ConnectService.CallbackHint(null, null, derivedBaseUrl);
diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/failure/CheckConstrainedEnumsTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/failure/CheckConstrainedEnumsTest.java
index 7eaeb01eb4..87d6e02e31 100644
--- a/app/proprietary/src/test/java/stirling/software/proprietary/failure/CheckConstrainedEnumsTest.java
+++ b/app/proprietary/src/test/java/stirling/software/proprietary/failure/CheckConstrainedEnumsTest.java
@@ -43,6 +43,7 @@ class CheckConstrainedEnumsTest {
assertThat(persisted)
.doesNotContain(
FailureAudience.class,
+ FailureActionSlot.class,
FailureActionId.class,
FailureActionId.Execution.class,
Ownership.class);
diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/failure/FailureKindTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/failure/FailureKindTest.java
index 03aa5c9402..4d5a76bbdd 100644
--- a/app/proprietary/src/test/java/stirling/software/proprietary/failure/FailureKindTest.java
+++ b/app/proprietary/src/test/java/stirling/software/proprietary/failure/FailureKindTest.java
@@ -1,6 +1,9 @@
package stirling.software.proprietary.failure;
import static org.assertj.core.api.Assertions.assertThat;
+import static stirling.software.proprietary.failure.FailureActionSlot.OVERFLOW;
+import static stirling.software.proprietary.failure.FailureActionSlot.RESOLUTION;
+import static stirling.software.proprietary.failure.FailureActionSlot.SECONDARY;
import static stirling.software.proprietary.failure.FailureAudience.ANYONE_WHO_SEES;
import static stirling.software.proprietary.failure.FailureAudience.OWNER;
import static stirling.software.proprietary.failure.FailureAudience.TEAM_REVIEWER;
@@ -34,9 +37,12 @@ class FailureKindTest {
/** In full, so a declaration pairing the right action with the wrong audience cannot pass. */
private static FailureKind.OfferedAction offered(
- FailureActionId id, FailureAudience audience, String labelKeySuffix) {
+ FailureActionId id,
+ FailureAudience audience,
+ FailureActionSlot slot,
+ String labelKeySuffix) {
return new FailureKind.OfferedAction(
- id, "portal.failures.action." + labelKeySuffix, audience);
+ id, "portal.failures.action." + labelKeySuffix, audience, slot);
}
@Nested
@@ -70,27 +76,6 @@ class FailureKindTest {
assertThat(kind.getId()).matches("^[A-Z][A-Z0-9_]*$");
}
- @ParameterizedTest
- @EnumSource(FailureKind.class)
- void declaresItsActionsInTheSameOrderAsEveryOtherKind(FailureKind kind) {
- // Declaration order is display order and the first usable offer is the row's primary,
- // so
- // two kinds disagreeing would flip the solid button between rows.
- List ranking =
- List.of(
- FailureActionId.VIEW_FILE,
- FailureActionId.VIEW_IN_PROCESSOR,
- FailureActionId.DISMISS);
-
- List declared = kind.getActions();
- assertThat(ranking)
- .as("%s declares an action the shared ranking does not rank", kind.getId())
- .containsAll(declared);
- assertThat(declared)
- .as("%s declares its actions out of the shared order", kind.getId())
- .isEqualTo(ranking.stream().filter(declared::contains).toList());
- }
-
@Test
void idsAreUnique() {
Set ids = new HashSet<>();
@@ -121,12 +106,13 @@ class FailureKindTest {
@ParameterizedTest
@EnumSource(FailureKind.class)
- void everyOfferSaysWhoItIsFor(FailureKind kind) {
- // Read per row to decide what a caller is shown, so a null would leak a button.
+ void everyOfferSaysWhoItIsForAndWhereItGoes(FailureKind kind) {
+ // Both decide what a caller is shown, so a missing one places a button by accident.
for (FailureKind.OfferedAction offer : kind.getOfferedActions()) {
assertThat(offer.audience())
.as("%s offers %s", kind.getId(), offer.id())
.isNotNull();
+ assertThat(offer.slot()).as("%s offers %s", kind.getId(), offer.id()).isNotNull();
}
}
@@ -138,6 +124,17 @@ class FailureKindTest {
assertThat(kind.getActions()).doesNotHaveDuplicates();
}
+ @ParameterizedTest
+ @EnumSource(FailureKind.class)
+ void declaresAtMostOneResolution(FailureKind kind) {
+ // Two things that both claim to fix it is a sign of two kinds wearing one id.
+ assertThat(
+ kind.getOfferedActions().stream()
+ .filter(offer -> offer.slot() == FailureActionSlot.RESOLUTION)
+ .toList())
+ .hasSizeLessThanOrEqualTo(1);
+ }
+
@Test
void noTwoKindsClaimTheSameErrorCode() {
// Computed independently of duplicateErrorCodes(), then checked against it: the boot
@@ -232,16 +229,18 @@ class FailureKindTest {
class Unknown {
@Test
- void offersItsOwnerTheirDocumentAndTheRunToWhoeverReviews() {
- // Nothing here is known to be fixable, so the offers are just the places to look.
+ void offersARetryToItsOwnerAndTheRunToWhoeverReviews() {
+ // No known fix, so no resolution; a retry is still worth offering for a one-off.
assertThat(FailureKind.UNKNOWN.getOfferedActions())
.containsExactly(
- offered(FailureActionId.VIEW_FILE, OWNER, "viewFile"),
+ offered(FailureActionId.OPEN_IN_TOOL, OWNER, SECONDARY, "openInTool"),
+ offered(FailureActionId.VIEW_FILE, OWNER, SECONDARY, "viewFile"),
offered(
FailureActionId.VIEW_IN_PROCESSOR,
TEAM_REVIEWER,
+ OVERFLOW,
"viewInProcessor"),
- offered(FailureActionId.DISMISS, ANYONE_WHO_SEES, "dismiss"));
+ offered(FailureActionId.DISMISS, ANYONE_WHO_SEES, OVERFLOW, "dismiss"));
}
@Test
@@ -294,16 +293,19 @@ class FailureKindTest {
}
@Test
- void offersTheDocumentToItsOwnerAndTheRunToItsReviewer() {
- // The point of the audiences: only the owner holds the document.
+ void aKindWithSomethingToFixOffersTheFixToItsOwnerAndTheRunToItsReviewer() {
+ // Only the owner has the password, so a reviewer is offered the run and a dismiss.
assertThat(FailureKind.INPUT_PASSWORD_PROTECTED.getOfferedActions())
.containsExactly(
- offered(FailureActionId.VIEW_FILE, OWNER, "viewFile"),
+ offered(FailureActionId.DECRYPT, OWNER, RESOLUTION, "decrypt"),
+ offered(FailureActionId.VIEW_FILE, OWNER, SECONDARY, "viewFile"),
offered(
FailureActionId.VIEW_IN_PROCESSOR,
TEAM_REVIEWER,
+ OVERFLOW,
"viewInProcessor"),
- offered(FailureActionId.DISMISS, ANYONE_WHO_SEES, "dismiss"));
+ offered(FailureActionId.OPEN_IN_TOOL, OWNER, OVERFLOW, "openInTool"),
+ offered(FailureActionId.DISMISS, ANYONE_WHO_SEES, OVERFLOW, "dismiss"));
}
@Test
@@ -333,10 +335,8 @@ class FailureKindTest {
assertThat(FailureKind.UNKNOWN.labelKeyFor(FailureActionId.DISMISS))
.isEqualTo(FailureKind.genericLabelKey(FailureActionId.DISMISS))
.isEqualTo("portal.failures.action.dismiss");
- assertThat(
- FailureKind.INPUT_PASSWORD_PROTECTED.labelKeyFor(
- FailureActionId.VIEW_IN_PROCESSOR))
- .isEqualTo("portal.failures.action.viewInProcessor");
+ assertThat(FailureKind.INPUT_PASSWORD_PROTECTED.labelKeyFor(FailureActionId.DECRYPT))
+ .isEqualTo("portal.failures.action.decrypt");
}
@Test
diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventControllerTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventControllerTest.java
index 3ece59f6af..4627e37bd4 100644
--- a/app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventControllerTest.java
+++ b/app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventControllerTest.java
@@ -153,6 +153,7 @@ class FileRunEventControllerTest {
action -> {
assertThat(action.defaultLabel()).isNotBlank();
assertThat(action.execution()).isNotNull();
+ assertThat(action.slot()).isNotNull();
})
.filteredOn(action -> "VIEW_IN_PROCESSOR".equals(action.id()))
.singleElement()
@@ -160,6 +161,7 @@ class FileRunEventControllerTest {
action -> {
assertThat(action.execution())
.isEqualTo(FailureActionId.Execution.CLIENT);
+ assertThat(action.slot()).isEqualTo(FailureActionSlot.OVERFLOW);
assertThat(action.defaultLabel()).isEqualTo("View in processor");
});
}
diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventHttpIntegrationTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventHttpIntegrationTest.java
index ae46a50c8a..4bf886dbc8 100644
--- a/app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventHttpIntegrationTest.java
+++ b/app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventHttpIntegrationTest.java
@@ -130,6 +130,7 @@ class FileRunEventHttpIntegrationTest {
assertThat(actions.get(0).get("defaultLabel").asString())
.isEqualTo("View in processor");
assertThat(actions.get(0).get("execution").asString()).isEqualTo("CLIENT");
+ assertThat(actions.get(0).get("slot").asString()).isEqualTo("OVERFLOW");
assertThat(actions.get(0).get("enabled").asBoolean()).isTrue();
assertThat(actions.get(0).get("disabledReasonKey").isNull()).isTrue();
assertThat(actions.get(1).get("id").asString()).isEqualTo("DISMISS");
diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventServiceTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventServiceTest.java
index 83ad173637..504ec92ba9 100644
--- a/app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventServiceTest.java
+++ b/app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventServiceTest.java
@@ -157,6 +157,103 @@ class FileRunEventServiceTest {
}
}
+ @Nested
+ @DisplayName("resolve")
+ class Resolve {
+
+ @Test
+ void marksTheRowResolvedWhenAClientReportsItsRetryWorked() {
+ FileRunEvent event = given(FailureKind.UNKNOWN, TEAM, "f1");
+
+ FileRunEvent resolved = service.resolve(event.id());
+
+ assertThat(resolved.status()).isEqualTo(FileRunEventStatus.RESOLVED);
+ assertThat(resolved.statusActor()).isEqualTo(ACTOR);
+ assertThat(service.list(null, null, 10)).as("resolved work is not open work").isEmpty();
+ }
+
+ @Test
+ void isNotAnActionAnyoneCanPress() {
+ // System-set on a client-side retry, so there is no id to dispatch and no button.
+ assertThat(Arrays.stream(FailureActionId.values()).map(Enum::name))
+ .doesNotContain("RESOLVE", "RESOLVED");
+ }
+
+ @Test
+ void reportingTheSameSuccessTwiceIsNotARefusal() {
+ // A client that retries, succeeds and reports twice is telling the truth twice.
+ FileRunEvent event = given(FailureKind.UNKNOWN, TEAM, "f1");
+ Instant first = service.resolve(event.id()).statusAt();
+
+ assertThat(service.resolve(event.id()).statusAt()).isEqualTo(first);
+ }
+
+ @Test
+ void aDismissedRowCannotBeResolvedBehindTheReviewersBack() {
+ FileRunEvent event = given(FailureKind.UNKNOWN, TEAM, "f1");
+ service.dispatch(event.id(), "DISMISS", Map.of());
+
+ assertThatThrownBy(() -> service.resolve(event.id()))
+ .isInstanceOf(FailureActionException.class)
+ .extracting(e -> ((FailureActionException) e).getReason())
+ .isEqualTo(FailureActionException.Reason.ALREADY_CLOSED);
+ }
+
+ @Test
+ void anotherTeamsRowIsNotFound() {
+ FileRunEvent theirs = given(FailureKind.UNKNOWN, 99L, "f1");
+
+ assertThatThrownBy(() -> service.resolve(theirs.id()))
+ .isInstanceOf(FailureActionException.class)
+ .extracting(e -> ((FailureActionException) e).getReason())
+ .isEqualTo(FailureActionException.Reason.EVENT_NOT_FOUND);
+ }
+
+ @Test
+ void aRecurrenceReopensIt() {
+ // RESOLVED claims one attempt worked, not that the problem is gone for good.
+ service.report(new EditorFailureReport("compress", "E004", List.of("f-1"), "boom"));
+ FileRunEvent event = service.list(null, null, 10).getFirst();
+ service.resolve(event.id());
+
+ service.report(new EditorFailureReport("compress", "E004", List.of("f-1"), "boom"));
+
+ assertThat(service.list(null, null, 10))
+ .singleElement()
+ .extracting(FileRunEvent::status)
+ .isEqualTo(FileRunEventStatus.NEW);
+ }
+
+ @Test
+ void aRecurrenceReopensAnIncidentClosedBecauseTheFileWasRemoved() {
+ // A library file comes back under the same id, so without this every repeat folds
+ // into the closed row and the queue never shows the failure again.
+ service.report(new EditorFailureReport("compress", "E001", List.of("f-1"), "boom"));
+ service.forgetFiles(List.of("f-1"));
+ assertThat(service.list(null, null, 10)).isEmpty();
+
+ service.report(new EditorFailureReport("compress", "E001", List.of("f-1"), "boom"));
+
+ assertThat(service.list(null, null, 10))
+ .singleElement()
+ .extracting(FileRunEvent::status)
+ .isEqualTo(FileRunEventStatus.NEW);
+ }
+
+ @Test
+ void aRecurrenceLeavesAReviewersDismissalAlone() {
+ // Dismiss is a decision about the incident, not a claim about the document, so it
+ // outlasts a repeat where FILE_REMOVED and RESOLVED do not.
+ service.report(new EditorFailureReport("compress", "E001", List.of("f-1"), "boom"));
+ FileRunEvent event = service.list(null, null, 10).getFirst();
+ service.dispatch(event.id(), "DISMISS", Map.of());
+
+ service.report(new EditorFailureReport("compress", "E001", List.of("f-1"), "boom"));
+
+ assertThat(service.list(null, null, 10)).isEmpty();
+ }
+ }
+
@Nested
@DisplayName("triage never touches the document")
class NeverTouchesTheDocument {
@@ -352,13 +449,17 @@ class FileRunEventServiceTest {
}
@Test
- void theOwnerIsOfferedTheirDocumentAndNotTheReviewersView() {
- // The document is theirs to open; the processor view is for whoever reviews the team.
+ void theOwnerIsOfferedTheFixAndNotTheReviewersView() {
+ // The unlock is the owner's to do; the processor view is for whoever reviews.
when(authority.canEditPolicies()).thenReturn(false);
FileRunEvent mine = givenHitBy(ACTOR, FailureKind.INPUT_PASSWORD_PROTECTED, TEAM, "f1");
assertThat(offeredFor(mine))
- .containsExactly(FailureActionId.VIEW_FILE, FailureActionId.DISMISS);
+ .containsExactly(
+ FailureActionId.DECRYPT,
+ FailureActionId.VIEW_FILE,
+ FailureActionId.OPEN_IN_TOOL,
+ FailureActionId.DISMISS);
assertThat(service.availableActions(mine))
.allMatch(FileRunEventService.AvailableAction::enabled);
}
@@ -385,8 +486,10 @@ class FileRunEventServiceTest {
assertThat(offeredFor(unattended))
.containsExactly(
+ FailureActionId.DECRYPT,
FailureActionId.VIEW_FILE,
FailureActionId.VIEW_IN_PROCESSOR,
+ FailureActionId.OPEN_IN_TOOL,
FailureActionId.DISMISS);
}
@@ -499,6 +602,17 @@ class FileRunEventServiceTest {
.equals(action.disabledReasonKey()));
}
+ @Test
+ void carriesTheKindsPlacementIntentForEachOffer() {
+ FileRunEvent mine = givenHitBy(ACTOR, FailureKind.INPUT_PASSWORD_PROTECTED, TEAM, "f1");
+
+ assertThat(service.availableActions(mine))
+ .filteredOn(action -> action.id() == FailureActionId.DECRYPT)
+ .singleElement()
+ .extracting(FileRunEventService.AvailableAction::slot)
+ .isEqualTo(FailureActionSlot.RESOLUTION);
+ }
+
@Test
void carriesTheLabelKeyForEachOffer() {
FileRunEvent event = given(FailureKind.UNKNOWN, TEAM, "f1");
diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/failure/InMemoryFileRunEventRepository.java b/app/proprietary/src/test/java/stirling/software/proprietary/failure/InMemoryFileRunEventRepository.java
index 4f429fe9b1..7bf386c3b1 100644
--- a/app/proprietary/src/test/java/stirling/software/proprietary/failure/InMemoryFileRunEventRepository.java
+++ b/app/proprietary/src/test/java/stirling/software/proprietary/failure/InMemoryFileRunEventRepository.java
@@ -96,7 +96,9 @@ class InMemoryFileRunEventRepository implements FileRunEventRepository {
@Override
public int reopenIfResolved(String id) {
FileRunEventEntity entity = rows.get(id);
- if (entity == null || entity.getStatus() != FileRunEventStatus.RESOLVED) {
+ if (entity == null
+ || (entity.getStatus() != FileRunEventStatus.RESOLVED
+ && entity.getStatus() != FileRunEventStatus.FILE_REMOVED)) {
return 0;
}
entity.setStatus(FileRunEventStatus.NEW);
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 e76a8b96ee..06df6df843 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
@@ -2,6 +2,7 @@ package stirling.software.proprietary.failure;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.lenient;
+import static org.mockito.Mockito.when;
import java.util.List;
@@ -120,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
@@ -162,7 +189,53 @@ class NotificationProjectionTest {
assertThat(action.labelKey()).startsWith("portal.failures.action.");
assertThat(action.defaultLabel()).isNotBlank();
assertThat(action.execution()).isNotNull();
+ assertThat(action.slot()).isNotNull();
});
}
}
+
+ @Nested
+ @DisplayName("the response says whether the caller reviews the team")
+ class ReviewerFlag {
+
+ @Test
+ void trueForAReviewerSoTheClientFiltersNothing() {
+ when(authority.canEditPolicies()).thenReturn(true);
+
+ assertThat(controller.list(null).viewerReviewsTeam()).isTrue();
+ }
+
+ @Test
+ void falseForAMemberSoTheClientHidesRowsForFilesItDoesNotHold() {
+ when(authority.canEditPolicies()).thenReturn(false);
+
+ assertThat(controller.list(null).viewerReviewsTeam()).isFalse();
+ }
+ }
+
+ @Nested
+ @DisplayName("the response names the viewer, opaquely, for a client to scope read state on")
+ class ViewerKey {
+
+ @Test
+ void steadyForOneViewerAcrossReads() {
+ assertThat(controller.list(null).viewerKey())
+ .isEqualTo(controller.list(null).viewerKey())
+ .isNotBlank();
+ }
+
+ @Test
+ void differentForAnotherViewerSoOneCannotInheritTheOthersMarker() {
+ String mine = controller.list(null).viewerKey();
+ when(userService.getCurrentUsername()).thenReturn("someone.else@example.com");
+
+ assertThat(controller.list(null).viewerKey()).isNotEqualTo(mine);
+ }
+
+ @Test
+ void neverTheUsernameItself() {
+ // It lands in that browser's storage, and a client only needs to tell viewers apart.
+ assertThat(controller.list(null).viewerKey()).doesNotContain(ACTOR);
+ }
+ }
}
diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/failure/NotificationResolveTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/failure/NotificationResolveTest.java
new file mode 100644
index 0000000000..b6cddd6a71
--- /dev/null
+++ b/app/proprietary/src/test/java/stirling/software/proprietary/failure/NotificationResolveTest.java
@@ -0,0 +1,152 @@
+package stirling.software.proprietary.failure;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.lenient;
+import static org.mockito.Mockito.when;
+
+import java.util.List;
+import java.util.Map;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.springframework.http.HttpStatus;
+import org.springframework.web.server.ResponseStatusException;
+
+import stirling.software.common.model.ApplicationProperties;
+import stirling.software.common.service.UserServiceInterface;
+import stirling.software.proprietary.notification.NotificationController;
+import stirling.software.proprietary.notification.NotificationService;
+import stirling.software.proprietary.notification.NotificationView;
+import stirling.software.proprietary.policy.config.PolicyManagementAuthority;
+
+/** Reporting a client-side retry that worked: the bell's one write. */
+@ExtendWith(MockitoExtension.class)
+@DisplayName("reporting a client-side retry that worked")
+class NotificationResolveTest {
+
+ private static final Long TEAM = 7L;
+ private static final String ACTOR = "reviewer@example.com";
+
+ @Mock private PolicyManagementAuthority authority;
+ @Mock private UserServiceInterface userService;
+
+ private FileRunEventStore store;
+ private FileRunEventService failures;
+ private NotificationController controller;
+
+ @BeforeEach
+ void setUp() {
+ ApplicationProperties props = new ApplicationProperties();
+ props.getSecurity().setEnableLogin(true);
+ store = new FileRunEventStore(new InMemoryFileRunEventRepository());
+ failures =
+ new FileRunEventService(
+ store,
+ new FailureActionRegistry(
+ List.of(new AcknowledgeAction(store), new DismissAction(store))),
+ authority,
+ userService,
+ props);
+ controller = new NotificationController(new NotificationService(failures));
+
+ lenient().when(authority.currentUserTeamId()).thenReturn(TEAM);
+ lenient().when(authority.canEditPolicies()).thenReturn(true);
+ lenient().when(userService.getCurrentUsername()).thenReturn(ACTOR);
+ }
+
+ private FileRunEvent given(FailureKind kind, String actor, String fileId) {
+ return store.record(RecordFailure.forEditor(kind, TEAM, actor, fileId, "boom"));
+ }
+
+ /** The status a refused call came back with. Fails the test if the call was allowed. */
+ private HttpStatus statusOf(Runnable call) {
+ try {
+ call.run();
+ } catch (ResponseStatusException e) {
+ return HttpStatus.valueOf(e.getStatusCode().value());
+ }
+ throw new AssertionError("expected the call to be refused");
+ }
+
+ @Test
+ void closesTheRowBehindThePrefixedId() {
+ // Why the route exists: the bell has no raw id to close its own row with.
+ FileRunEvent event = given(FailureKind.UNKNOWN, ACTOR, "f-1");
+
+ NotificationView resolved = controller.resolved("failure:" + event.id());
+
+ assertThat(resolved.status()).isEqualTo(FileRunEventStatus.RESOLVED);
+ assertThat(store.find(event.id(), TEAM).orElseThrow().status())
+ .isEqualTo(FileRunEventStatus.RESOLVED);
+ }
+
+ @Test
+ void theRowsOwnIdIsNotANotificationId() {
+ // Refused outright rather than left to work by accident for whichever source it reaches.
+ FileRunEvent event = given(FailureKind.UNKNOWN, ACTOR, "f-1");
+
+ assertThat(statusOf(() -> controller.resolved(event.id())))
+ .isEqualTo(HttpStatus.BAD_REQUEST);
+ assertThat(store.find(event.id(), TEAM).orElseThrow().status())
+ .isEqualTo(FileRunEventStatus.NEW);
+ }
+
+ @Test
+ void anUnknownSourcePrefixIsABadRequest() {
+ // Not a 404: it was never a notification id, so there is no row to report missing.
+ FileRunEvent event = given(FailureKind.UNKNOWN, ACTOR, "f-1");
+
+ assertThat(statusOf(() -> controller.resolved("quota:" + event.id())))
+ .isEqualTo(HttpStatus.BAD_REQUEST);
+ assertThat(statusOf(() -> controller.resolved("failure:")))
+ .isEqualTo(HttpStatus.BAD_REQUEST);
+ }
+
+ @Test
+ void reportingTheSameSuccessTwiceIsNotARefusal() {
+ FileRunEvent event = given(FailureKind.UNKNOWN, ACTOR, "f-1");
+ NotificationView first = controller.resolved("failure:" + event.id());
+
+ assertThat(controller.resolved("failure:" + event.id()))
+ .isEqualTo(first)
+ .extracting(NotificationView::status)
+ .isEqualTo(FileRunEventStatus.RESOLVED);
+ }
+
+ @Test
+ void aRowAReviewerHasDismissedIsAConflict() {
+ // Their decision stands: a retry reporting in afterwards does not overwrite it.
+ FileRunEvent event = given(FailureKind.UNKNOWN, ACTOR, "f-1");
+ failures.dispatch(event.id(), "DISMISS", Map.of());
+
+ assertThat(statusOf(() -> controller.resolved("failure:" + event.id())))
+ .isEqualTo(HttpStatus.CONFLICT);
+ assertThat(store.find(event.id(), TEAM).orElseThrow().status())
+ .isEqualTo(FileRunEventStatus.DISMISSED);
+ }
+
+ @Test
+ void aColleaguesNotificationIsNotFoundForAMember() {
+ FileRunEvent theirs = given(FailureKind.UNKNOWN, "colleague@example.com", "f-1");
+ when(authority.canEditPolicies()).thenReturn(false);
+
+ assertThat(statusOf(() -> controller.resolved("failure:" + theirs.id())))
+ .isEqualTo(HttpStatus.NOT_FOUND);
+ }
+
+ @Test
+ void aReviewerClosesAColleaguesRowTheyFixed() {
+ // Visibility decides, not ownership: a reviewer reads the team's incidents, so a reviewer
+ // who fixes one closes it. The member's own row is unreachable to them the other way round.
+ FileRunEvent theirs = given(FailureKind.UNKNOWN, "colleague@example.com", "f-1");
+
+ controller.resolved("failure:" + theirs.id());
+
+ assertThat(store.find(theirs.id(), TEAM).orElseThrow().status())
+ .isEqualTo(FileRunEventStatus.RESOLVED);
+ }
+}
diff --git a/app/saas/src/test/java/stirling/software/saas/accountlink/ConnectRequestServiceTest.java b/app/saas/src/test/java/stirling/software/saas/accountlink/ConnectRequestServiceTest.java
index f75c96becb..e15d9749f2 100644
--- a/app/saas/src/test/java/stirling/software/saas/accountlink/ConnectRequestServiceTest.java
+++ b/app/saas/src/test/java/stirling/software/saas/accountlink/ConnectRequestServiceTest.java
@@ -324,8 +324,6 @@ class ConnectRequestServiceTest {
assertThat(service.claim("nope", CLAIM_SECRET).outcome()).isEqualTo(ClaimOutcome.REJECTED);
}
- // ---------------------------------------------------------------------------------------
-
private static ConnectRequest pending() {
ConnectRequest row = new ConnectRequest();
row.setRequestId("req");
diff --git a/frontend/editor/public/locales/en-US/translation.toml b/frontend/editor/public/locales/en-US/translation.toml
index 345cf07ab2..d4a028388b 100644
--- a/frontend/editor/public/locales/en-US/translation.toml
+++ b/frontend/editor/public/locales/en-US/translation.toml
@@ -5506,25 +5506,22 @@ count = "{{remaining}} of {{total}}"
label = "Free credits"
[notifications]
-empty = "Nothing to report."
+empty = "You're all caught up."
handoffUnavailable = "This browser will not let the processor pass the document to the editor. Open it from the editor instead."
-noDocumentLinked = "This failure is not linked to a specific document, so there is nothing to open here."
-notOnThisDevice = "This document is not on this device, so it cannot be opened here."
+noDocumentLinked = "This failure is not linked to a specific document, so it cannot be opened or retried here."
+notOnThisDevice = "This document is not on this device, so it cannot be opened or retried here."
occurrences = "{{count}} times"
open = "Notifications"
title = "Notifications"
unread = "Unread"
[notifications.action]
+copiedLog = "Copied"
+copyLog = "Copy log"
failed = "That did not work. Try again in a moment."
+more = "More options"
unavailable = "Not available for this notification."
-[notifications.detail]
-copied = "Copied"
-copy = "Copy error"
-less = "Show less"
-more = "Show full message"
-
[notifications.section]
earlier = "Earlier"
new = "New"
@@ -7896,8 +7893,10 @@ title = "Failures"
[portal.failures.action]
acknowledge = "Acknowledge"
confirm = "Are you sure?"
+decrypt = "Decrypt and retry"
dismiss = "Dismiss"
dismissSkipFile = "Skip this file"
+openInTool = "Retry"
viewFile = "View file"
viewInProcessor = "View in processor"
@@ -7920,11 +7919,11 @@ description = "Policy runs that fail will appear here with the actions you can t
title = "No failures recorded"
[portal.failures.kind.inputPasswordProtected]
-description = "The pipeline could not open the document because it is password-protected. Unlock it and run it again, or skip this file."
+description = "Your file is password protected, so the run could not read it."
title = "Password-protected document"
[portal.failures.kind.unknown]
-description = "This run failed for a reason Stirling does not yet recognise. The raw message is shown below."
+description = "Something went wrong that Stirling does not recognise yet."
title = "Unrecognised failure"
[portal.failures.origin]
diff --git a/frontend/editor/src/core/components/notifications/NotificationBell.css b/frontend/editor/src/core/components/notifications/NotificationBell.css
index 46367a0b2b..fe8fe54fed 100644
--- a/frontend/editor/src/core/components/notifications/NotificationBell.css
+++ b/frontend/editor/src/core/components/notifications/NotificationBell.css
@@ -35,6 +35,25 @@
text-align: center;
}
+/* Scoped to the bell, so the shared DividerWithText is untouched elsewhere. */
+.notification-bell__divider.text-divider {
+ margin-top: 0.125rem;
+ margin-bottom: 0.125rem;
+}
+
+/* Gray by default, because the shared rule is near-invisible here. */
+.notification-bell__divider .text-divider__rule {
+ background-color: var(--c-border-strong);
+}
+
+.notification-bell__divider--new .text-divider__rule {
+ background-color: var(--c-danger);
+}
+
+.notification-bell__divider--new .text-divider__label {
+ color: var(--c-danger);
+}
+
.notification-bell__panel {
position: fixed;
z-index: var(--z-popover, 60);
@@ -128,38 +147,6 @@
overflow-wrap: anywhere;
}
-/* Expanded, the message is the point of the row, so let it run and scroll rather than clamp. */
-.notification-bell__detail--full {
- display: block;
- max-height: 10rem;
- overflow-y: auto;
- -webkit-line-clamp: none;
-}
-
-.notification-bell__chrome {
- grid-column: 2;
- display: flex;
- gap: var(--sp-1, 0.25rem);
- margin-top: var(--sp-1, 0.25rem);
-}
-
-/* Reading aids for the message, tinted rather than filled: they sit next to the row's real actions
- and must not read as one of them. */
-.notification-bell__chip {
- padding: 0.0625rem 0.375rem;
- border: none;
- border-radius: var(--radius-sm, 0.25rem);
- background: var(--c-primary-subtle);
- color: var(--c-accent-fg, var(--c-primary));
- font-size: 0.6875rem;
- cursor: pointer;
-}
-
-.notification-bell__chip:hover,
-.notification-bell__chip:focus-visible {
- background: var(--c-hover);
-}
-
/* Why the actions this row could have had are absent. Muted: it explains, it does not warn. */
.notification-bell__note {
grid-column: 2;
diff --git a/frontend/editor/src/core/components/notifications/NotificationBell.test.tsx b/frontend/editor/src/core/components/notifications/NotificationBell.test.tsx
index 8753a5880b..5e0d14e998 100644
--- a/frontend/editor/src/core/components/notifications/NotificationBell.test.tsx
+++ b/frontend/editor/src/core/components/notifications/NotificationBell.test.tsx
@@ -9,21 +9,25 @@ import { MantineProvider } from "@mantine/core";
import type {
AppNotification,
NotificationActionOffer,
+ NotificationActionSlot,
} from "@app/services/notifications";
// @app/ui Button is a Mantine wrapper, so it needs the provider in the tree.
const render = (ui: Parameters[0]) =>
baseRender(ui, { wrapper: MantineProvider });
-/**
- * Two things are the bell's own and worth pinning: which notifications the user has already looked
- * at, and how a row behaves around an action.
- */
+// The bell's own two jobs: what counts as read, and how a row behaves around an action.
const fetchNotifications = vi.fn();
+// A bare array is wrapped as a reviewer's response; member filtering is the hook's own test.
vi.mock("@app/services/notifications", () => ({
- fetchNotifications: (...args: unknown[]) => fetchNotifications(...args),
+ fetchNotifications: async (...args: unknown[]) => {
+ const value = await fetchNotifications(...args);
+ return Array.isArray(value)
+ ? { notifications: value, viewerReviewsTeam: true, viewerKey: "viewer-a" }
+ : value;
+ },
}));
// IndexedDB, which jsdom has none of. Answered here so availability is a fact of the test.
@@ -35,7 +39,7 @@ const h = vi.hoisted(() => ({
string,
{
available: (context: unknown) => boolean;
- run: (context: unknown, password?: string) => unknown;
+ run: (context: unknown) => unknown;
closesPanel?: boolean;
}
>,
@@ -58,6 +62,8 @@ vi.mock("react-i18next", () => ({
useTranslation: () => ({
// A string fallback, or an options object with defaultValue plus what it interpolates.
t: (key: string, fallback?: unknown) => {
+ // The kinds' sentences live in the locale files, so one stands in here.
+ if (key.endsWith(".description")) return "Kind description";
if (typeof fallback === "string") return fallback;
if (fallback && typeof fallback === "object") {
const options = fallback as Record;
@@ -77,18 +83,33 @@ const { NotificationBell } =
function offer(
id: string,
+ slot: NotificationActionSlot = "SECONDARY",
overrides: Partial = {},
): NotificationActionOffer {
return {
id,
labelKey: `portal.failures.action.${id.toLowerCase()}`,
defaultLabel: id,
+ slot,
enabled: true,
disabledReasonKey: null,
...overrides,
};
}
+// Read state watermarks the ordering time, so rows need distinct ones. "a" is the newest.
+const AT: Record = {
+ a: "2026-08-05T02:00:00Z",
+ b: "2026-08-05T01:00:00Z",
+};
+
+/** Scoped to the viewer the mocked response names, as the store writes it. */
+const READ_THROUGH_KEY = "stirling.notifications.readThroughAt.viewer-a";
+
+function markReadThrough(iso: string): void {
+ window.localStorage.setItem(READ_THROUGH_KEY, String(Date.parse(iso)));
+}
+
function notification(
id: string,
title = "Unrecognised failure",
@@ -109,8 +130,8 @@ function notification(
sourceId: null,
policyId: null,
occurrences: 1,
- createdAt: "2026-08-05T00:00:00Z",
- lastSeenAt: "2026-08-05T00:00:00Z",
+ createdAt: AT[id] ?? "2026-08-05T00:00:00Z",
+ lastSeenAt: AT[id] ?? "2026-08-05T00:00:00Z",
actions: [],
...overrides,
};
@@ -172,7 +193,7 @@ describe("NotificationBell", () => {
it("divides what is new from what the user has already seen", async () => {
// "b" was the newest last time, so "a" is the only new one.
- window.localStorage.setItem("stirling.notifications.lastSeenId", "b");
+ markReadThrough(AT.b);
fetchNotifications.mockResolvedValue([
notification("a"),
notification("b"),
@@ -186,7 +207,7 @@ describe("NotificationBell", () => {
it("keeps the division on screen after opening marks them read", async () => {
// Frozen on open: read live it would collapse the moment the badge cleared.
- window.localStorage.setItem("stirling.notifications.lastSeenId", "b");
+ markReadThrough(AT.b);
fetchNotifications.mockResolvedValue([
notification("a"),
notification("b"),
@@ -200,7 +221,7 @@ describe("NotificationBell", () => {
});
it("does not divide a list with nothing new in it", async () => {
- window.localStorage.setItem("stirling.notifications.lastSeenId", "a");
+ markReadThrough(AT.a);
fetchNotifications.mockResolvedValue([notification("a")]);
render();
await openPanel();
@@ -231,29 +252,26 @@ describe("NotificationBell", () => {
first.unmount();
// A newer one arrives above the one already seen.
- fetchNotifications.mockResolvedValue([
- notification("b"),
- notification("a"),
- ]);
+ const arrived = notification("c", "Unrecognised failure", {
+ lastSeenAt: "2026-08-05T03:00:00Z",
+ });
+ fetchNotifications.mockResolvedValue([arrived, notification("a")]);
render();
expect(await screen.findByText("1")).toBeTruthy();
});
- it("treats everything as unread when the last seen one is gone", async () => {
- // We cannot tell how far the user got, so show them rather than marking the lot read.
- window.localStorage.setItem(
- "stirling.notifications.lastSeenId",
- "vanished",
- );
- fetchNotifications.mockResolvedValue([
- notification("a"),
- notification("b"),
- ]);
+ it("leaves the rest read when the row that was newest has gone", async () => {
+ // The newest row leaves; marking read by id would then relight the badge for the older one.
+ markReadThrough(AT.a);
+ fetchNotifications.mockResolvedValue([notification("b")]);
render();
+ await openPanel();
- expect(await screen.findByText("2")).toBeTruthy();
+ // Nothing is new, so nothing is labelled new: by id, this row would have counted as unread.
+ expect(await screen.findByText("Unrecognised failure")).toBeTruthy();
+ expect(screen.queryByText("New")).toBeNull();
});
it("renders the server's title and repeat count without knowing the source", async () => {
@@ -291,6 +309,49 @@ describe("NotificationBell", () => {
).toBeTruthy();
});
+ it("tucks overflow actions into a menu, not a row of buttons", async () => {
+ h.specs = {
+ DECRYPT: { available: () => true, run: vi.fn() },
+ VIEW_FILE: { available: () => true, run: vi.fn() },
+ VIEW_IN_PROCESSOR: { available: () => true, run: vi.fn() },
+ };
+ fetchNotifications.mockResolvedValue([
+ notification("a", "Unrecognised failure", {
+ actions: [
+ offer("DECRYPT", "RESOLUTION"),
+ offer("VIEW_FILE", "SECONDARY"),
+ offer("VIEW_IN_PROCESSOR", "OVERFLOW"),
+ ],
+ }),
+ ]);
+ render();
+ await openPanel();
+
+ // Two real buttons; the overflow one is off screen until the menu is opened.
+ expect(
+ screen.getByRole("button", {
+ name: "DECRYPT: Unrecognised failure",
+ }),
+ ).toBeTruthy();
+ expect(
+ screen.getByRole("button", { name: "VIEW_FILE: Unrecognised failure" }),
+ ).toBeTruthy();
+ expect(
+ screen.queryByRole("button", {
+ name: "VIEW_IN_PROCESSOR: Unrecognised failure",
+ }),
+ ).toBeNull();
+
+ fireEvent.click(
+ screen.getByRole("button", {
+ name: "More options: Unrecognised failure",
+ }),
+ );
+ expect(
+ await screen.findByRole("menuitem", { name: "VIEW_IN_PROCESSOR" }),
+ ).toBeTruthy();
+ });
+
it("runs whichever of the row's actions is pressed", async () => {
const run = vi.fn();
h.specs = {
@@ -370,7 +431,7 @@ describe("NotificationBell", () => {
await waitFor(() =>
expect(
screen.getByText(
- "This document is not on this device, so it cannot be opened here.",
+ "This document is not on this device, so it cannot be opened or retried here.",
),
).toBeTruthy(),
);
@@ -387,7 +448,7 @@ describe("NotificationBell", () => {
expect(
await screen.findByText(
- "This failure is not linked to a specific document, so there is nothing to open here.",
+ "This failure is not linked to a specific document, so it cannot be opened or retried here.",
),
).toBeTruthy();
});
@@ -422,7 +483,7 @@ describe("NotificationBell", () => {
notification("a", "Unrecognised failure", {
ownership: "UNOWNED",
actions: [
- offer("VIEW_FILE", {
+ offer("VIEW_FILE", "SECONDARY", {
enabled: false,
disabledReasonKey: "portal.failures.disabled.unattended",
}),
@@ -452,11 +513,11 @@ describe("NotificationBell", () => {
fetchNotifications.mockResolvedValue([
notification("a", "Unrecognised failure", {
actions: [
- offer("VIEW_IN_PROCESSOR", {
+ offer("VIEW_IN_PROCESSOR", "SECONDARY", {
enabled: false,
disabledReasonKey: "portal.failures.disabled.closed",
}),
- offer("VIEW_FILE", {
+ offer("VIEW_FILE", "SECONDARY", {
enabled: false,
disabledReasonKey: "portal.failures.disabled.closed",
}),
@@ -474,7 +535,12 @@ describe("NotificationBell", () => {
expect(
screen.queryByRole("button", { name: /VIEW_IN_PROCESSOR|VIEW_FILE/ }),
).toBeNull();
- expect(document.querySelector(".notification-bell__actions")).toBeNull();
+ // The error log stays reachable: a row with nothing left to do still owns its detail.
+ expect(
+ screen.getByRole("button", {
+ name: "More options: Unrecognised failure",
+ }),
+ ).toBeTruthy();
});
it("shows a failed action in the row instead of leaving the user guessing", async () => {
@@ -506,25 +572,43 @@ describe("NotificationBell", () => {
expect(screen.getByText("Password-protected document")).toBeTruthy();
});
- it("expands the message without touching the row's actions", async () => {
+ it("reads the kind's own words rather than the raw failure", async () => {
+ // A bell is not a log: the row gets a sentence, the message goes in the menu.
+ const stack = "org.apache.pdfbox.InvalidPasswordException";
fetchNotifications.mockResolvedValue([
- notification("a", "Unrecognised failure", {
- detail: "org.apache.pdfbox.InvalidPasswordException",
+ notification("a", "Password-protected document", {
+ titleKey: "portal.failures.kind.inputPasswordProtected.title",
+ detail: stack,
}),
]);
render();
await openPanel();
- const expand = screen.getByRole("button", {
- name: "Show full message: Unrecognised failure",
- });
- fireEvent.click(expand);
+ expect(await screen.findByText("Kind description")).toBeTruthy();
+ expect(screen.queryByText(stack)).toBeNull();
+ });
- expect(
- screen.getByRole("button", { name: "Show less: Unrecognised failure" }),
- ).toBeTruthy();
- expect(
- screen.getByRole("button", { name: "Copy error: Unrecognised failure" }),
- ).toBeTruthy();
+ it("keeps the log one click away, for a row whose only extra is the log", async () => {
+ h.specs = { VIEW_FILE: { available: () => true, run: vi.fn() } };
+ const stack = "org.apache.pdfbox.InvalidPasswordException";
+ const clipboard = vi.fn().mockResolvedValue(undefined);
+ Object.assign(navigator, { clipboard: { writeText: clipboard } });
+ fetchNotifications.mockResolvedValue([
+ notification("a", "Unrecognised failure", {
+ detail: stack,
+ actions: [offer("VIEW_FILE", "SECONDARY")],
+ }),
+ ]);
+ render();
+ await openPanel();
+
+ fireEvent.click(
+ await screen.findByRole("button", {
+ name: "More options: Unrecognised failure",
+ }),
+ );
+ fireEvent.click(await screen.findByRole("menuitem", { name: "Copy log" }));
+
+ await waitFor(() => expect(clipboard).toHaveBeenCalledWith(stack));
});
});
diff --git a/frontend/editor/src/core/components/notifications/NotificationItem.tsx b/frontend/editor/src/core/components/notifications/NotificationItem.tsx
index b53ca7894c..cbb17048d2 100644
--- a/frontend/editor/src/core/components/notifications/NotificationItem.tsx
+++ b/frontend/editor/src/core/components/notifications/NotificationItem.tsx
@@ -1,18 +1,26 @@
import { useState } from "react";
import type { TFunction } from "i18next";
import { useTranslation } from "react-i18next";
-import { Button } from "@app/ui";
+import { Menu, Tooltip } from "@mantine/core";
+import { ActionIcon, Button } from "@app/ui";
+import LocalIcon from "@app/components/shared/LocalIcon";
import { isResolvableHere } from "@app/hooks/useNotifications";
import type { NotificationDocumentState } from "@app/hooks/useNotifications";
import type {
ClientActionRegistry,
NotificationActionContext,
} from "@app/components/notifications/notificationActions";
+import { promoteActions } from "@app/components/notifications/notificationActionSlots";
import type {
AppNotification,
NotificationActionOffer,
} from "@app/services/notifications";
+/** The kind's own sentence, sharing the portal's copy. */
+function summaryKeyOf(titleKey: string): string {
+ return titleKey.replace(/\.title$/, ".description");
+}
+
/**
* The server's reason wins, being about the failure rather than this browser. Otherwise only what we
* actually looked up, so a row we never probed is never called absent.
@@ -35,12 +43,12 @@ function noteFor(
if (!notification.fileId)
return t(
"notifications.noDocumentLinked",
- "This failure is not linked to a specific document, so there is nothing to open here.",
+ "This failure is not linked to a specific document, so it cannot be opened or retried here.",
);
return isResolvableHere(notification)
? t(
"notifications.notOnThisDevice",
- "This document is not on this device, so it cannot be opened here.",
+ "This document is not on this device, so it cannot be opened or retried here.",
)
: null;
}
@@ -53,7 +61,7 @@ interface NotificationItemProps {
onDismissPanel: () => void;
}
-/** Its own component because the last attempt's message and its expanded state are per-row. */
+/** Its own component because the last attempt's message and the copy state are per-row. */
export function NotificationItem({
notification,
unread,
@@ -64,7 +72,6 @@ export function NotificationItem({
const { t } = useTranslation();
const [message, setMessage] = useState(null);
const [busy, setBusy] = useState(null);
- const [expanded, setExpanded] = useState(false);
const [copied, setCopied] = useState(false);
const title = t(notification.titleKey, notification.defaultTitle);
@@ -73,23 +80,17 @@ export function NotificationItem({
hasLocalFile: documentState.hasLocalFile,
};
- // An id this build has never heard of is skipped rather than rendered unwired: the server ships
- // new kinds, and new actions, ahead of the clients that understand them.
- const usable = notification.actions.filter((offer) => {
- if (!offer.enabled) return false;
- const spec = registry[offer.id];
- return spec ? spec.available(context) : false;
- });
-
- // Only from an action this build would otherwise have rendered: a reason about one it cannot
- // perform anyway is not this row's explanation.
- const withheldReasonKey =
- notification.actions.find(
- (offer) =>
- !offer.enabled &&
- offer.disabledReasonKey !== null &&
- registry[offer.id] !== undefined,
- )?.disabledReasonKey ?? null;
+ const { primary, secondary, overflow, withheldReasonKey } = promoteActions(
+ notification.actions,
+ (offer) => {
+ const spec = registry[offer.id];
+ // An id this build has never heard of: skipped rather than rendered unwired.
+ if (!spec) return false;
+ return spec.available(context);
+ },
+ // A reason from an action this build could not have rendered explains nothing.
+ (offer) => registry[offer.id] !== undefined,
+ );
const labelOf = (offer: NotificationActionOffer) =>
t(offer.labelKey, offer.defaultLabel);
@@ -129,6 +130,7 @@ export function NotificationItem({
};
const note = noteFor(notification, documentState, withheldReasonKey, t);
+ const summary = t(summaryKeyOf(notification.titleKey), { defaultValue: "" });
return (
)}
- {notification.detail && (
- <>
-
- {notification.detail}
-
-
-
-
-
-
- )}
+ {summary && {summary}}
{note && {note}}
- {/* In the kind's declared order, the first leading. */}
- {usable.length > 0 && (
+ {/* The menu is not gated on a button existing: a row with no action still owns its log. */}
+ {(primary || notification.detail) && (
- {usable.map((offer, index) => (
+ {primary && (
void run(offer)}
+ label={labelOf(primary)}
+ busy={busy === primary.id}
+ onRun={() => void run(primary)}
/>
- ))}
+ )}
+ {secondary && (
+ void run(secondary)}
+ />
+ )}
+ {(overflow.length > 0 || notification.detail) && (
+
+ )}
)}
@@ -220,7 +231,8 @@ export function NotificationItem({
}
interface ActionButtonProps {
- variant: "primary" | "secondary";
+ /** Solid for the row's answer, outlined for its runner-up, ghost for the rest. */
+ variant: "primary" | "secondary" | "tertiary";
rowTitle: string;
label: string;
busy: boolean;
diff --git a/frontend/editor/src/core/components/notifications/NotificationPanel.tsx b/frontend/editor/src/core/components/notifications/NotificationPanel.tsx
index f3ff0c621e..b2b4581b5a 100644
--- a/frontend/editor/src/core/components/notifications/NotificationPanel.tsx
+++ b/frontend/editor/src/core/components/notifications/NotificationPanel.tsx
@@ -65,6 +65,8 @@ export function NotificationPanel({
if (panel.current?.contains(target)) return;
// A trigger closes this itself; counting it as outside would reopen it.
if (target.closest?.("[data-notifications-trigger]")) return;
+ // The overflow menu is portaled out, so a click in it would read as outside the panel.
+ if (target.closest?.(".notification-bell__menu")) return;
onClose();
};
const closeOnEscape = (event: KeyboardEvent) => {
@@ -98,7 +100,7 @@ export function NotificationPanel({
{notifications.length === 0 ? (
- {t("notifications.empty", "Nothing to report.")}
+ {t("notifications.empty", "You're all caught up.")}
) : (
@@ -107,6 +109,7 @@ export function NotificationPanel({
{index === 0 && dividedAt > 0 && (
-
@@ -115,6 +118,7 @@ export function NotificationPanel({
{index === dividedAt && dividedAt > 0 && (
-
diff --git a/frontend/editor/src/core/components/notifications/notificationActionSlots.test.ts b/frontend/editor/src/core/components/notifications/notificationActionSlots.test.ts
new file mode 100644
index 0000000000..6f1d8cc7c5
--- /dev/null
+++ b/frontend/editor/src/core/components/notifications/notificationActionSlots.test.ts
@@ -0,0 +1,299 @@
+import { describe, expect, it } from "vitest";
+import { promoteActions } from "@app/components/notifications/notificationActionSlots";
+import type {
+ NotificationActionOffer,
+ NotificationActionSlot,
+} from "@app/services/notifications";
+
+// Pinned against the shapes the server sends: what is left over depends on what won the buttons.
+
+/** The offers as `FailureKind` declares them for an unrecognised failure. */
+const UNKNOWN_OFFERS: Record = {
+ OPEN_IN_TOOL: offer("OPEN_IN_TOOL", "SECONDARY"),
+ VIEW_IN_PROCESSOR: offer("VIEW_IN_PROCESSOR", "SECONDARY"),
+ VIEW_FILE: offer("VIEW_FILE", "OVERFLOW"),
+};
+
+const PASSWORD_OFFERS: Record = {
+ DECRYPT: offer("DECRYPT", "RESOLUTION"),
+ OPEN_IN_TOOL: offer("OPEN_IN_TOOL", "OVERFLOW"),
+ VIEW_FILE: offer("VIEW_FILE", "OVERFLOW"),
+ VIEW_IN_PROCESSOR: offer("VIEW_IN_PROCESSOR", "SECONDARY"),
+};
+
+/** The reasons the server sends with an action it would refuse. */
+const NO_DOCUMENT = "portal.failures.disabled.noDocument";
+const UNATTENDED = "portal.failures.disabled.unattended";
+const CLOSED = "portal.failures.disabled.closed";
+
+function offer(
+ id: string,
+ slot: NotificationActionSlot,
+ overrides: Partial = {},
+): NotificationActionOffer {
+ return {
+ id,
+ labelKey: `portal.failures.action.${id.toLowerCase()}`,
+ defaultLabel: id,
+ slot,
+ enabled: true,
+ disabledReasonKey: null,
+ ...overrides,
+ };
+}
+
+function from(
+ declared: Record,
+ ids: string[],
+): NotificationActionOffer[] {
+ return ids.map((id) => {
+ const found = declared[id];
+ if (!found) throw new Error(`That kind offers no ${id}`);
+ return found;
+ });
+}
+
+const unknown = (...ids: string[]) => from(UNKNOWN_OFFERS, ids);
+
+const password = (...ids: string[]) => from(PASSWORD_OFFERS, ids);
+
+/** The same offers, with the named ones refused as the server would refuse them. */
+function refusing(
+ offers: NotificationActionOffer[],
+ reasonKey: string,
+ ...ids: string[]
+): NotificationActionOffer[] {
+ return offers.map((action) =>
+ ids.includes(action.id)
+ ? { ...action, enabled: false, disabledReasonKey: reasonKey }
+ : action,
+ );
+}
+
+/** Everything this client can do, with the file on this device. */
+const RUNNABLE = new Set([
+ "OPEN_IN_TOOL",
+ "DECRYPT",
+ "VIEW_FILE",
+ "VIEW_IN_PROCESSOR",
+]);
+
+/** The predicate the bell supplies: a known id, on a device that can act on it. */
+const canRun = (action: NotificationActionOffer) => RUNNABLE.has(action.id);
+
+/** The build's knowledge alone, which is what gates a withheld reason. */
+const knowsAction = (action: NotificationActionOffer) =>
+ RUNNABLE.has(action.id);
+
+function promoted(list: NotificationActionOffer[]) {
+ const { primary, secondary, overflow, withheldReasonKey } = promoteActions(
+ list,
+ canRun,
+ knowsAction,
+ );
+ return {
+ primary: primary?.id ?? null,
+ secondary: secondary?.id ?? null,
+ overflow: overflow.map((action) => action.id),
+ withheldReasonKey,
+ };
+}
+
+describe("promoteActions", () => {
+ it("gives the owner the retry, and keeps the rest quiet behind it", () => {
+ // No portal access, so the server never offered the processor link.
+ expect(promoted(unknown("OPEN_IN_TOOL", "VIEW_FILE"))).toEqual({
+ primary: "OPEN_IN_TOOL",
+ secondary: null,
+ overflow: ["VIEW_FILE"],
+ withheldReasonKey: null,
+ });
+ });
+
+ it("leads an attended policy failure with the queue, and states what was refused", () => {
+ // Not the reader's document, so a greyed unlock would be false hope: the note stays instead.
+ expect(
+ promoted(
+ refusing(
+ unknown("OPEN_IN_TOOL", "VIEW_IN_PROCESSOR", "VIEW_FILE"),
+ NO_DOCUMENT,
+ "OPEN_IN_TOOL",
+ "VIEW_FILE",
+ ),
+ ),
+ ).toEqual({
+ primary: "VIEW_IN_PROCESSOR",
+ secondary: null,
+ overflow: [],
+ withheldReasonKey: NO_DOCUMENT,
+ });
+ });
+
+ it("leads an unattended failure with the queue, and says retrying is not available", () => {
+ // Nobody holds the document: one reason for the row, from the best thing it lost.
+ expect(
+ promoted(
+ refusing(
+ unknown("OPEN_IN_TOOL", "VIEW_IN_PROCESSOR", "VIEW_FILE"),
+ UNATTENDED,
+ "OPEN_IN_TOOL",
+ "VIEW_FILE",
+ ),
+ ),
+ ).toEqual({
+ primary: "VIEW_IN_PROCESSOR",
+ secondary: null,
+ overflow: [],
+ withheldReasonKey: UNATTENDED,
+ });
+ });
+
+ it("explains nothing on a colleague's failure, having taken nothing away", () => {
+ // Nothing needing the bytes was offered, so there is no loss to account for.
+ expect(promoted(unknown("VIEW_IN_PROCESSOR"))).toEqual({
+ primary: "VIEW_IN_PROCESSOR",
+ secondary: null,
+ overflow: [],
+ withheldReasonKey: null,
+ });
+ });
+
+ it("leads a password failure with the unlock, not the plain retry", () => {
+ // Running it again unchanged is a second answer to the same problem, so it drops behind.
+ expect(promoted(password("DECRYPT", "OPEN_IN_TOOL", "VIEW_FILE"))).toEqual({
+ primary: "DECRYPT",
+ secondary: null,
+ overflow: ["OPEN_IN_TOOL", "VIEW_FILE"],
+ withheldReasonKey: null,
+ });
+ });
+
+ it("gives a reviewer their own password failure the unlock plus the queue", () => {
+ expect(
+ promoted(
+ password("DECRYPT", "OPEN_IN_TOOL", "VIEW_FILE", "VIEW_IN_PROCESSOR"),
+ ),
+ ).toEqual({
+ primary: "DECRYPT",
+ secondary: "VIEW_IN_PROCESSOR",
+ overflow: ["OPEN_IN_TOOL", "VIEW_FILE"],
+ withheldReasonKey: null,
+ });
+ });
+
+ it("leaves a closed row no buttons at all, only its reason", () => {
+ // Already closed elsewhere: every offer refused, so the row is its message plus one line.
+ expect(
+ promoted(
+ refusing(
+ unknown("OPEN_IN_TOOL", "VIEW_FILE"),
+ CLOSED,
+ "OPEN_IN_TOOL",
+ "VIEW_FILE",
+ ),
+ ),
+ ).toEqual({
+ primary: null,
+ secondary: null,
+ overflow: [],
+ withheldReasonKey: CLOSED,
+ });
+ });
+
+ it("promotes past a resolution the shell cannot deliver", () => {
+ // Read from the processor, which has no FileContext, so the unlock reports itself unavailable.
+ const inProcessor = (action: NotificationActionOffer) =>
+ action.id !== "DECRYPT" && canRun(action);
+
+ const { primary, secondary, overflow } = promoteActions(
+ password("DECRYPT", "OPEN_IN_TOOL", "VIEW_FILE", "VIEW_IN_PROCESSOR"),
+ inProcessor,
+ knowsAction,
+ );
+
+ expect(primary?.id).toBe("VIEW_IN_PROCESSOR");
+ expect(secondary).toBeNull();
+ expect(overflow.map((action) => action.id)).toEqual([
+ "OPEN_IN_TOOL",
+ "VIEW_FILE",
+ ]);
+ });
+
+ it("drops a client action this device cannot perform, without inventing a reason", () => {
+ // The document is gone from this browser: the actions disappear rather than fail on click.
+ const { primary, overflow, withheldReasonKey } = promoteActions(
+ unknown("OPEN_IN_TOOL", "VIEW_FILE"),
+ () => false,
+ knowsAction,
+ );
+
+ expect(primary).toBeNull();
+ expect(overflow).toEqual([]);
+ expect(withheldReasonKey).toBeNull();
+ });
+
+ it("skips an action id it has never heard of without touching the rest", () => {
+ // The server ships a kind with a new action before this build knows what it means.
+ const list = [
+ offer("QUARANTINE", "RESOLUTION"),
+ ...unknown("OPEN_IN_TOOL"),
+ ];
+
+ expect(promoted(list)).toEqual({
+ primary: "OPEN_IN_TOOL",
+ secondary: null,
+ overflow: [],
+ withheldReasonKey: null,
+ });
+ });
+
+ it("has nothing to promote when nothing survives", () => {
+ expect(
+ promoteActions(
+ [],
+ () => true,
+ () => true,
+ ),
+ ).toEqual({
+ primary: null,
+ secondary: null,
+ overflow: [],
+ withheldReasonKey: null,
+ });
+ });
+
+ it("never explains the row with an action this build has never heard of", () => {
+ // A client that could never have drawn the button is not explained by its reason.
+ const list = [
+ offer("QUARANTINE", "RESOLUTION", {
+ enabled: false,
+ disabledReasonKey: NO_DOCUMENT,
+ }),
+ ...unknown("VIEW_IN_PROCESSOR"),
+ ];
+
+ expect(promoted(list)).toEqual({
+ primary: "VIEW_IN_PROCESSOR",
+ secondary: null,
+ overflow: [],
+ withheldReasonKey: null,
+ });
+ });
+
+ it("takes the reason from the best action lost, not the first declared", () => {
+ // Two refusals, one row: the reader gets the one they would have reached for first.
+ const list = [
+ offer("VIEW_FILE", "OVERFLOW", {
+ enabled: false,
+ disabledReasonKey: CLOSED,
+ }),
+ offer("DECRYPT", "RESOLUTION", {
+ enabled: false,
+ disabledReasonKey: NO_DOCUMENT,
+ }),
+ ...password("VIEW_IN_PROCESSOR"),
+ ];
+
+ expect(promoted(list).withheldReasonKey).toBe(NO_DOCUMENT);
+ });
+});
diff --git a/frontend/editor/src/core/components/notifications/notificationActionSlots.ts b/frontend/editor/src/core/components/notifications/notificationActionSlots.ts
new file mode 100644
index 0000000000..636e9d7b77
--- /dev/null
+++ b/frontend/editor/src/core/components/notifications/notificationActionSlots.ts
@@ -0,0 +1,65 @@
+import type {
+ NotificationActionOffer,
+ NotificationActionSlot,
+} from "@app/services/notifications";
+
+// The server says what an action does and what it has earned; this turns that into an order.
+
+const SLOT_RANK: Record = {
+ RESOLUTION: 0,
+ SECONDARY: 1,
+ OVERFLOW: 2,
+};
+
+export interface PromotedActions {
+ /** The row's own button. Null when nothing survived the filter. */
+ primary: NotificationActionOffer | null;
+ /** A second button, only ever an action the server marked SECONDARY. */
+ secondary: NotificationActionOffer | null;
+ /** Everything else, in the server's order, for the row to render quietly after those two. */
+ overflow: NotificationActionOffer[];
+ /** The reason for the best action withheld, for the row to state once. */
+ withheldReasonKey: string | null;
+}
+
+/** One primary, at most one secondary, and the quiet rest. A disabled action is dropped. */
+export function promoteActions(
+ offers: readonly NotificationActionOffer[],
+ canRenderClientAction: (offer: NotificationActionOffer) => boolean,
+ knowsAction: (offer: NotificationActionOffer) => boolean,
+): PromotedActions {
+ const ranked = offers
+ .map((offer, declaredAt) => ({ offer, declaredAt }))
+ // Slot first, then declaration order, so two actions in one slot keep the server's ranking.
+ .sort(
+ (a, b) =>
+ SLOT_RANK[a.offer.slot] - SLOT_RANK[b.offer.slot] ||
+ a.declaredAt - b.declaredAt,
+ )
+ .map(({ offer }) => offer);
+
+ // The best one withheld, so a row explains itself once rather than once per lost action.
+ const withheldReasonKey =
+ ranked.find(
+ (offer) =>
+ !offer.enabled && offer.disabledReasonKey && knowsAction(offer),
+ )?.disabledReasonKey ?? null;
+
+ const renderable = ranked.filter(
+ (offer) => offer.enabled && canRenderClientAction(offer),
+ );
+
+ const [primary, next, ...rest] = renderable;
+ if (!primary)
+ return { primary: null, secondary: null, overflow: [], withheldReasonKey };
+
+ // A second RESOLUTION would read as two answers to one problem; OVERFLOW was ranked below.
+ const secondary = next?.slot === "SECONDARY" ? next : null;
+
+ return {
+ primary,
+ secondary,
+ overflow: secondary ? rest : next ? [next, ...rest] : rest,
+ withheldReasonKey,
+ };
+}
diff --git a/frontend/editor/src/core/components/shared/quickNav/QuickNavRailNotifications.test.tsx b/frontend/editor/src/core/components/shared/quickNav/QuickNavRailNotifications.test.tsx
index 800eb55b5b..db9772fad2 100644
--- a/frontend/editor/src/core/components/shared/quickNav/QuickNavRailNotifications.test.tsx
+++ b/frontend/editor/src/core/components/shared/quickNav/QuickNavRailNotifications.test.tsx
@@ -35,7 +35,11 @@ function notification(id: string): AppNotification {
describe("QuickNavRailNotifications", () => {
beforeEach(() => {
window.localStorage.clear();
- fetchNotifications.mockReset().mockResolvedValue([]);
+ fetchNotifications.mockReset().mockResolvedValue({
+ notifications: [],
+ viewerReviewsTeam: true,
+ viewerKey: "viewer-a",
+ });
h.notificationsAvailable = true;
});
@@ -53,10 +57,12 @@ describe("QuickNavRailNotifications", () => {
});
it("carries the unread count on the icon", async () => {
- fetchNotifications.mockResolvedValue([
- notification("a"),
- notification("b"),
- ]);
+ // A reviewer's response, so nothing is filtered for want of a local document.
+ fetchNotifications.mockResolvedValue({
+ notifications: [notification("a"), notification("b")],
+ viewerReviewsTeam: true,
+ viewerKey: "viewer-a",
+ });
render( {}} />);
diff --git a/frontend/editor/src/core/extensions/accountLogout.ts b/frontend/editor/src/core/extensions/accountLogout.ts
index df4e8f4273..1e82dafdc1 100644
--- a/frontend/editor/src/core/extensions/accountLogout.ts
+++ b/frontend/editor/src/core/extensions/accountLogout.ts
@@ -1,3 +1,4 @@
+import { clearNotificationReadState } from "@app/hooks/useNotifications";
import { suspendWorkbenchSession } from "@app/services/workbenchSession";
type SignOutFn = () => Promise;
@@ -27,6 +28,8 @@ export function useAccountLogout() {
// inherit this workbench. Suspends writing too - signing out unmounts the
// editor, and its flush would otherwise write the record straight back.
suspendWorkbenchSession();
+ // Same reason: the next person's own failures must not arrive pre-read.
+ clearNotificationReadState();
await signOut();
} finally {
redirectToLogin();
diff --git a/frontend/editor/src/core/hooks/useNotifications.test.ts b/frontend/editor/src/core/hooks/useNotifications.test.ts
index 746db77390..e9544b88ab 100644
--- a/frontend/editor/src/core/hooks/useNotifications.test.ts
+++ b/frontend/editor/src/core/hooks/useNotifications.test.ts
@@ -1,6 +1,9 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import { act, renderHook, waitFor } from "@testing-library/react";
-import type { AppNotification } from "@app/services/notifications";
+import type {
+ AppNotification,
+ FetchedNotifications,
+} from "@app/services/notifications";
/**
* The bell is mounted several times over, so what is pinned here is that they share one read: one
@@ -20,8 +23,26 @@ vi.mock("@app/services/localFilePresence", () => ({
hasLocalFile: (fileId: string) => hasLocalFile(fileId),
}));
-const { useNotifications, refreshNotificationsNow } =
- await import("@app/hooks/useNotifications");
+const {
+ useNotifications,
+ refreshNotificationsNow,
+ clearNotificationReadState,
+} = await import("@app/hooks/useNotifications");
+
+/** A fetch result. Reviewer by default, so a test says nothing about filtering unless it means to. */
+function feed(
+ notifications: AppNotification[],
+ viewerReviewsTeam = true,
+ viewerKey: string | null = VIEWER,
+): FetchedNotifications {
+ return { notifications, viewerReviewsTeam, viewerKey };
+}
+
+const VIEWER = "viewer-a";
+
+function readThroughKeyFor(viewerKey: string): string {
+ return `stirling.notifications.readThroughAt.${viewerKey}`;
+}
function notification(
id: string,
@@ -52,12 +73,12 @@ function notification(
describe("useNotifications", () => {
beforeEach(() => {
window.localStorage.clear();
- fetchNotifications.mockReset().mockResolvedValue([]);
- hasLocalFile.mockClear();
+ fetchNotifications.mockReset().mockResolvedValue(feed([]));
+ hasLocalFile.mockReset().mockResolvedValue(true);
});
it("reads the list once however many bells are mounted", async () => {
- fetchNotifications.mockResolvedValue([notification("a")]);
+ fetchNotifications.mockResolvedValue(feed([notification("a")]));
const first = renderHook(() => useNotifications());
const second = renderHook(() => useNotifications());
@@ -70,11 +91,13 @@ describe("useNotifications", () => {
});
it("looks a document up once for the list, not once per row", async () => {
- fetchNotifications.mockResolvedValue([
- notification("a", { fileId: "f-1" }),
- notification("b", { fileId: "f-1" }),
- notification("c", { fileId: "f-2" }),
- ]);
+ fetchNotifications.mockResolvedValue(
+ feed([
+ notification("a", { fileId: "f-1" }),
+ notification("b", { fileId: "f-1" }),
+ notification("c", { fileId: "f-2" }),
+ ]),
+ );
const { result } = renderHook(() => useNotifications());
@@ -83,20 +106,21 @@ describe("useNotifications", () => {
});
it("looks up an attended run's document but never an unattended run's", async () => {
- // Asking storage about a source's hash can only miss, and would then be shown as "not on this
- // device" about a document that never was.
- fetchNotifications.mockResolvedValue([
- notification("attended", {
- origin: "POLICY",
- sourceId: null,
- fileId: "editor-file-1",
- }),
- notification("unattended", {
- origin: "POLICY",
- sourceId: "src-s3-invoices",
- fileId: "hashed-identity",
- }),
- ]);
+ // Only an attended row names a reference this browser could resolve; a source's hash misses.
+ fetchNotifications.mockResolvedValue(
+ feed([
+ notification("attended", {
+ origin: "POLICY",
+ sourceId: null,
+ fileId: "editor-file-1",
+ }),
+ notification("unattended", {
+ origin: "POLICY",
+ sourceId: "src-s3-invoices",
+ fileId: "hashed-identity",
+ }),
+ ]),
+ );
const { result } = renderHook(() => useNotifications());
@@ -113,6 +137,70 @@ describe("useNotifications", () => {
).toBe(false);
});
+ it("hides a member's row whose document is not in this browser, keeps the one that is", async () => {
+ hasLocalFile.mockImplementation((id: string) =>
+ Promise.resolve(id === "here"),
+ );
+ fetchNotifications.mockResolvedValue(
+ feed(
+ [
+ notification("gone", { fileId: "gone" }),
+ notification("kept", { fileId: "here" }),
+ ],
+ false,
+ ),
+ );
+
+ const { result } = renderHook(() => useNotifications());
+
+ await waitFor(() => expect(result.current.notifications).toHaveLength(1));
+ expect(result.current.notifications[0].id).toBe("kept");
+ // The hidden row is not news either: it must not light the badge.
+ expect(result.current.unreadCount).toBe(1);
+ });
+
+ it("hides a member's unattended row even when its id happens to be stored here", async () => {
+ // A source-fed row's fileId is a content hash from another id space. Storage answering for it
+ // is a collision, not the document, so the row must go on being filtered as unresolvable.
+ hasLocalFile.mockResolvedValue(true);
+ fetchNotifications.mockResolvedValue(
+ feed(
+ [
+ notification("unattended", {
+ sourceId: "src-s3-invoices",
+ fileId: "collides-with-a-local-id",
+ }),
+ ],
+ false,
+ ),
+ );
+
+ const { result } = renderHook(() => useNotifications());
+
+ await waitFor(() => expect(fetchNotifications).toHaveBeenCalled());
+ expect(result.current.notifications).toHaveLength(0);
+ });
+
+ it("shows a reviewer both rows, document here or not", async () => {
+ // A reviewer keeps a row for a file they cannot open: it is how they see a policy needs fixing.
+ hasLocalFile.mockImplementation((id: string) =>
+ Promise.resolve(id === "here"),
+ );
+ fetchNotifications.mockResolvedValue(
+ feed(
+ [
+ notification("gone", { fileId: "gone" }),
+ notification("kept", { fileId: "here" }),
+ ],
+ true,
+ ),
+ );
+
+ const { result } = renderHook(() => useNotifications());
+
+ await waitFor(() => expect(result.current.notifications).toHaveLength(2));
+ });
+
it("polls on one timer and stops it when the last bell unmounts", async () => {
vi.useFakeTimers();
try {
@@ -144,10 +232,9 @@ describe("useNotifications", () => {
});
it("marks every bell read, not just the one the user opened", async () => {
- fetchNotifications.mockResolvedValue([
- notification("b"),
- notification("a"),
- ]);
+ fetchNotifications.mockResolvedValue(
+ feed([notification("b"), notification("a")]),
+ );
const first = renderHook(() => useNotifications());
const second = renderHook(() => useNotifications());
await waitFor(() => expect(first.result.current.unreadCount).toBe(2));
@@ -158,19 +245,95 @@ describe("useNotifications", () => {
expect(first.result.current.unreadCount).toBe(0);
expect(second.result.current.unreadCount).toBe(0);
+ expect(window.localStorage.getItem(readThroughKeyFor(VIEWER))).toBe(
+ String(Date.parse("2026-08-05T00:00:00Z")),
+ );
+ });
+
+ it("keeps one viewer's read state off another's on a shared browser", async () => {
+ // A timestamp is legible to whoever reads it next, so an unscoped marker would leave the
+ // incoming user's older failures silently pre-read.
+ fetchNotifications.mockResolvedValue(feed([notification("a")]));
+ const first = renderHook(() => useNotifications());
+ await waitFor(() => expect(first.result.current.unreadCount).toBe(1));
+ await act(async () => first.result.current.markAllSeen());
+ expect(first.result.current.unreadCount).toBe(0);
+ first.unmount();
+
+ // Same browser, same rows, different signed-in viewer.
+ fetchNotifications.mockResolvedValue(
+ feed([notification("a")], true, "viewer-b"),
+ );
+ const second = renderHook(() => useNotifications());
+
+ await waitFor(() => expect(second.result.current.unreadCount).toBe(1));
+ });
+
+ it("marks nothing when the server names no viewer", async () => {
+ // Unscoped would be worse than unsaved: the next viewer here would inherit it.
+ fetchNotifications.mockResolvedValue(feed([notification("a")], true, null));
+ const { result } = renderHook(() => useNotifications());
+ await waitFor(() => expect(result.current.unreadCount).toBe(1));
+
+ await act(async () => result.current.markAllSeen());
+
expect(
- window.localStorage.getItem("stirling.notifications.lastSeenId"),
- ).toBe("b");
+ Object.keys(window.localStorage).filter((key) =>
+ key.startsWith("stirling.notifications.readThroughAt"),
+ ),
+ ).toEqual([]);
+ });
+
+ it("keeps earlier rows read once the row that was newest has gone", async () => {
+ fetchNotifications.mockResolvedValue(
+ feed([
+ notification("new", { lastSeenAt: "2026-08-05T01:00:00Z" }),
+ notification("old"),
+ ]),
+ );
+ const { result } = renderHook(() => useNotifications());
+ await waitFor(() => expect(result.current.unreadCount).toBe(2));
+ await act(async () => result.current.markAllSeen());
+ expect(result.current.unreadCount).toBe(0);
+
+ // It leaves the list; a marker holding its id would make the row below read as unread.
+ fetchNotifications.mockResolvedValue(feed([notification("old")]));
+ await act(async () => result.current.refresh());
+
+ await waitFor(() => expect(result.current.notifications).toHaveLength(1));
+ expect(result.current.unreadCount).toBe(0);
+ });
+
+ it("forgets the marker on sign-out, so the next user's failures are not pre-read", async () => {
+ // A time is parseable whoever left it, so an inherited marker would silently mark the
+ // incoming user's older rows read - the direction the id-based marker never failed in.
+ fetchNotifications.mockResolvedValue(feed([notification("theirs")]));
+ const leaving = renderHook(() => useNotifications());
+ await waitFor(() => expect(leaving.result.current.unreadCount).toBe(1));
+ await act(async () => leaving.result.current.markAllSeen());
+ expect(leaving.result.current.unreadCount).toBe(0);
+
+ clearNotificationReadState();
+ leaving.unmount();
+ expect(window.localStorage.getItem(readThroughKeyFor(VIEWER))).toBeNull();
+
+ // The next user's own row is older than the marker that was just cleared.
+ fetchNotifications.mockResolvedValue(
+ feed([notification("mine", { lastSeenAt: "2026-08-04T00:00:00Z" })]),
+ );
+ const arriving = renderHook(() => useNotifications());
+
+ await waitFor(() => expect(arriving.result.current.unreadCount).toBe(1));
});
it("chains one fresh read behind the read in flight rather than joining it", async () => {
// A refresh exists to observe a write the caller just made. The read in flight may have
// started before that write, so joining it would report the world without it - and the
// caller would wait a whole poll interval for news of their own action.
- let release: (listed: AppNotification[]) => void = () => {};
+ let release: (fetched: FetchedNotifications) => void = () => {};
fetchNotifications.mockImplementationOnce(
() =>
- new Promise((resolve) => {
+ new Promise((resolve) => {
release = resolve;
}),
);
@@ -180,7 +343,7 @@ describe("useNotifications", () => {
// A refresh from a row, twice over, and a second bell mounting - all mid-read. The
// refreshes share ONE chained read; the mount joins what is already there.
- fetchNotifications.mockResolvedValue([notification("a")]);
+ fetchNotifications.mockResolvedValue(feed([notification("a")]));
act(() => {
first.result.current.refresh();
first.result.current.refresh();
@@ -189,7 +352,7 @@ describe("useNotifications", () => {
expect(fetchNotifications).toHaveBeenCalledTimes(1);
// The stale read lands empty; the chained fresh read is what delivers the row.
- await act(async () => release([]));
+ await act(async () => release(feed([])));
await waitFor(() => expect(fetchNotifications).toHaveBeenCalledTimes(2));
await waitFor(() =>
expect(first.result.current.notifications).toHaveLength(1),
@@ -203,17 +366,17 @@ describe("useNotifications", () => {
expect(hook.result.current.unreadCount).toBe(0);
// The failure report chain: row recorded server-side, then the re-read.
- fetchNotifications.mockResolvedValue([notification("a")]);
+ fetchNotifications.mockResolvedValue(feed([notification("a")]));
act(() => refreshNotificationsNow());
await waitFor(() => expect(hook.result.current.unreadCount).toBe(1));
});
it("still lands the row when the refresh races a poll read already in flight", async () => {
- let releaseStale: (listed: AppNotification[]) => void = () => {};
+ let releaseStale: (fetched: FetchedNotifications) => void = () => {};
fetchNotifications.mockImplementationOnce(
() =>
- new Promise((resolve) => {
+ new Promise((resolve) => {
releaseStale = resolve;
}),
);
@@ -223,23 +386,23 @@ describe("useNotifications", () => {
// The failure is recorded while a poll's read is still in flight, then its refresh fires.
// Joining that stale read would miss the row until the next poll interval.
- fetchNotifications.mockResolvedValue([notification("a")]);
+ fetchNotifications.mockResolvedValue(feed([notification("a")]));
act(() => refreshNotificationsNow());
- await act(async () => releaseStale([]));
+ await act(async () => releaseStale(feed([])));
await waitFor(() => expect(hook.result.current.unreadCount).toBe(1));
});
it("keeps its own list rather than one left by a bell that has gone", async () => {
- fetchNotifications.mockResolvedValue([notification("a")]);
+ fetchNotifications.mockResolvedValue(feed([notification("a")]));
const first = renderHook(() => useNotifications());
await waitFor(() =>
expect(first.result.current.notifications).toHaveLength(1),
);
first.unmount();
- // It must not show the old row while its own read is in flight.
- fetchNotifications.mockResolvedValue([]);
+ // The next bell must not show the old row while its own read is still in flight.
+ fetchNotifications.mockResolvedValue(feed([]));
const second = renderHook(() => useNotifications());
expect(second.result.current.notifications).toHaveLength(0);
diff --git a/frontend/editor/src/core/hooks/useNotifications.ts b/frontend/editor/src/core/hooks/useNotifications.ts
index 9f307a9a42..79856f5046 100644
--- a/frontend/editor/src/core/hooks/useNotifications.ts
+++ b/frontend/editor/src/core/hooks/useNotifications.ts
@@ -12,25 +12,62 @@ import { hasLocalFile } from "@app/services/localFilePresence";
// TODO: read state is per-browser. Move it server-side when notifications get their own table.
const POLL_INTERVAL_MS = 30_000;
-const SEEN_STORAGE_KEY = "stirling.notifications.lastSeenId";
+const SEEN_STORAGE_KEY_PREFIX = "stirling.notifications.readThroughAt";
-function readLastSeenId(): string | null {
+/**
+ * Scoped to the viewer the server named, because a timestamp is legible to whoever reads it next: an
+ * unscoped marker left by the previous user of a shared browser would silently pre-read the
+ * incoming user's older failures. Null while the viewer is unknown, which reads as nothing marked.
+ */
+function seenStorageKey(viewerKey: string | null): string | null {
+ return viewerKey ? `${SEEN_STORAGE_KEY_PREFIX}.${viewerKey}` : null;
+}
+
+/** A time, not an id: an id points at nothing once its row leaves the list. */
+function orderedAt(notification: AppNotification): number {
+ return Date.parse(notification.lastSeenAt);
+}
+
+function readReadThrough(viewerKey: string | null): number | null {
+ const key = seenStorageKey(viewerKey);
+ if (!key) return null;
try {
- return window.localStorage.getItem(SEEN_STORAGE_KEY);
+ const stored = Number(window.localStorage.getItem(key));
+ return Number.isFinite(stored) && stored > 0 ? stored : null;
} catch {
// Private mode: everything reads as unseen, which errs towards showing failures.
return null;
}
}
-function writeLastSeenId(id: string): void {
+function writeReadThrough(viewerKey: string | null, at: number): void {
+ const key = seenStorageKey(viewerKey);
+ // Unscoped would be worse than unsaved: the next viewer here would inherit it.
+ if (!key) return;
try {
- window.localStorage.setItem(SEEN_STORAGE_KEY, id);
+ window.localStorage.setItem(key, String(at));
} catch {
// The marker just will not survive a reload.
}
}
+/**
+ * Forget how far the departing reader got. The marker is scoped to its viewer, so this is belt to
+ * that brace: it also covers a sign-out on a build where the server names no viewer, and it drops
+ * the in-memory marker so the bell does not answer for them until the next read says who is here.
+ */
+export function clearNotificationReadState(): void {
+ const key = seenStorageKey(snapshot.viewerKey);
+ if (key) {
+ try {
+ window.localStorage.removeItem(key);
+ } catch {
+ // Nothing to clear that a read could trust anyway.
+ }
+ }
+ publish({ ...snapshot, readThroughAt: null, viewerKey: null });
+}
+
export interface NotificationDocumentState {
hasLocalFile: boolean;
}
@@ -51,13 +88,17 @@ interface NotificationsSnapshot {
notifications: AppNotification[];
/** Keyed by fileId, so several rows about one document cost one lookup. */
documents: Record;
- lastSeenId: string | null;
+ /** Everything up to and including this time has been read. Epoch millis, never a row id. */
+ readThroughAt: number | null;
+ /** Who the marker belongs to. Null until a read says, so nothing is marked on their behalf. */
+ viewerKey: string | null;
}
const NOTHING_LOADED: NotificationsSnapshot = {
notifications: [],
documents: {},
- lastSeenId: null,
+ readThroughAt: null,
+ viewerKey: null,
};
let snapshot: NotificationsSnapshot = NOTHING_LOADED;
@@ -87,7 +128,11 @@ function publish(next: NotificationsSnapshot): void {
}
async function read(forCycle: number): Promise {
- const listed = await fetchNotifications();
+ const {
+ notifications: listed,
+ viewerReviewsTeam,
+ viewerKey,
+ } = await fetchNotifications();
if (forCycle !== cycle) return;
const fileIds = [
@@ -111,10 +156,27 @@ async function read(forCycle: number): Promise {
);
if (forCycle !== cycle) return;
+ const documents = Object.fromEntries(resolved);
+ // Presentation, not access: the server has already scoped these rows to the reader. Hidden
+ // because every offer a member gets needs the document, so the row would only say so.
+ const visible = viewerReviewsTeam
+ ? listed
+ : listed.filter(
+ // Asked rather than left to the lookup missing: an unattended row's fileId comes from
+ // another id space, so a hit on one would be a collision and not the document.
+ (n) =>
+ isResolvableHere(n) &&
+ Boolean(n.fileId && documents[n.fileId]?.hasLocalFile),
+ );
+
+ // Read per read, not once at startup: the marker belongs to whoever the server says is
+ // reading, and signing in or out changes who that is without remounting the bell.
publish({
...snapshot,
- notifications: listed,
- documents: Object.fromEntries(resolved),
+ notifications: visible,
+ documents,
+ viewerKey,
+ readThroughAt: readReadThrough(viewerKey),
});
}
@@ -155,8 +217,9 @@ function loadFresh(): void {
function startPolling(): void {
cycle += 1;
- // From disk, not memory: another tab may have moved the marker on.
- snapshot = { ...NOTHING_LOADED, lastSeenId: readLastSeenId() };
+ // Nothing read until the first read names the viewer, since the marker is theirs and not this
+ // browser's. Everything counts as unread until then, which errs towards showing failures.
+ snapshot = NOTHING_LOADED;
pollTimer = window.setInterval(() => void load(), POLL_INTERVAL_MS);
void load();
}
@@ -183,10 +246,15 @@ function subscribe(onStoreChange: () => void): () => void {
}
function markAllSeen(): void {
- const newest = snapshot.notifications[0];
- if (!newest || snapshot.lastSeenId === newest.id) return;
- writeLastSeenId(newest.id);
- publish({ ...snapshot, lastSeenId: newest.id });
+ // The newest time in the list, not the first row's, so a re-sorted list cannot under-mark.
+ const newest = Math.max(
+ ...snapshot.notifications.map(orderedAt).filter(Number.isFinite),
+ );
+ if (!Number.isFinite(newest)) return;
+ if (snapshot.readThroughAt !== null && newest <= snapshot.readThroughAt)
+ return;
+ writeReadThrough(snapshot.viewerKey, newest);
+ publish({ ...snapshot, readThroughAt: newest });
}
function refresh(): void {
@@ -215,18 +283,17 @@ export interface NotificationsState {
}
export function useNotifications(): NotificationsState {
- const { notifications, documents, lastSeenId } = useSyncExternalStore(
+ const { notifications, documents, readThroughAt } = useSyncExternalStore(
subscribe,
getSnapshot,
getSnapshot,
);
- // A marker no longer in the list means we cannot tell how far the user got, so everything reads
- // as unread rather than being silently marked seen.
- const seenIndex = lastSeenId
- ? notifications.findIndex((n) => n.id === lastSeenId)
- : -1;
- const unreadCount = seenIndex === -1 ? notifications.length : seenIndex;
+ // A resolved row leaves without dragging the rest back into unread. Unparseable counts as new.
+ const unreadCount =
+ readThroughAt === null
+ ? notifications.length
+ : notifications.filter((n) => !(orderedAt(n) <= readThroughAt)).length;
return {
notifications,
diff --git a/frontend/editor/src/core/services/notifications.ts b/frontend/editor/src/core/services/notifications.ts
index 3101c6056e..d56cefcd2a 100644
--- a/frontend/editor/src/core/services/notifications.ts
+++ b/frontend/editor/src/core/services/notifications.ts
@@ -12,12 +12,16 @@ export type NotificationOrigin = "TOOL" | "POLICY" | "PIPELINE";
/** From this reader's point of view. `UNOWNED` is an unattended run: nobody holds the file. */
export type NotificationOwnership = "MINE" | "THEIRS" | "UNOWNED";
+/** How much of the row an action has earned; `promoteActions` turns it into a place. */
+export type NotificationActionSlot = "RESOLUTION" | "SECONDARY" | "OVERFLOW";
+
/** `id` is an open string, not a union: the server may know actions this build does not. */
export interface NotificationActionOffer {
id: string;
labelKey: string;
/** English fallback, for a build with no copy for `labelKey`. */
defaultLabel: string;
+ slot: NotificationActionSlot;
/** False renders no button in the bell, and a disabled one in the portal's queue. */
enabled: boolean;
disabledReasonKey: string | null;
@@ -49,18 +53,49 @@ export interface AppNotification {
interface NotificationsResponse {
notifications: AppNotification[];
+ viewerReviewsTeam: boolean;
+ viewerKey: string;
}
-/** Newest first. Empty rather than throwing: a bell that cannot load is an empty bell, not an error. */
+export interface FetchedNotifications {
+ notifications: AppNotification[];
+ /** A reviewer keeps rows whose document this browser does not hold; a member does not. */
+ viewerReviewsTeam: boolean;
+ /**
+ * Opaque id for the signed-in viewer, for scoping this browser's read state. Null when the
+ * server did not say, which must read as "cannot scope" rather than as a viewer of its own.
+ */
+ viewerKey: string | null;
+}
+
+/** Newest first. Empty rather than throwing, and defaulting to the least hiding. */
export async function fetchNotifications(
limit = 20,
-): Promise {
+): Promise {
try {
const response = await apiClient.get(
`${NOTIFICATIONS_PATH}?limit=${limit}`,
);
- return response?.data?.notifications ?? [];
+ return {
+ notifications: response?.data?.notifications ?? [],
+ viewerReviewsTeam: response?.data?.viewerReviewsTeam ?? true,
+ viewerKey: response?.data?.viewerKey || null,
+ };
} catch {
- return [];
+ return { notifications: [], viewerReviewsTeam: true, viewerKey: null };
+ }
+}
+
+/** Never throws: a refusal is not worth interrupting a user whose document is already fixed. */
+export async function reportNotificationResolved(
+ notificationId: string,
+): Promise {
+ try {
+ await apiClient.post(
+ `${NOTIFICATIONS_PATH}/${encodeURIComponent(notificationId)}/resolved`,
+ );
+ return true;
+ } catch {
+ return false;
}
}
diff --git a/frontend/editor/src/desktop/extensions/accountLogout.ts b/frontend/editor/src/desktop/extensions/accountLogout.ts
index ca75c9c5e9..76eab02f07 100644
--- a/frontend/editor/src/desktop/extensions/accountLogout.ts
+++ b/frontend/editor/src/desktop/extensions/accountLogout.ts
@@ -1,3 +1,4 @@
+import { clearNotificationReadState } from "@app/hooks/useNotifications";
import { connectionModeService } from "@app/services/connectionModeService";
import { suspendWorkbenchSession } from "@app/services/workbenchSession";
@@ -21,6 +22,8 @@ export function useAccountLogout() {
// inherit this workbench. Suspends writing too - signing out unmounts the
// editor, and its flush would otherwise write the record straight back.
suspendWorkbenchSession();
+ // Same reason: the next person's own failures must not arrive pre-read.
+ clearNotificationReadState();
await signOut();
const currentConfig = await connectionModeService.getCurrentConfig();
diff --git a/frontend/editor/src/proprietary/components/notifications/notificationActions.test.tsx b/frontend/editor/src/proprietary/components/notifications/notificationActions.test.tsx
index 950a8d41ff..c0aad4490b 100644
--- a/frontend/editor/src/proprietary/components/notifications/notificationActions.test.tsx
+++ b/frontend/editor/src/proprietary/components/notifications/notificationActions.test.tsx
@@ -85,6 +85,7 @@ function offer(id: string): NotificationActionOffer {
id,
labelKey: `portal.failures.action.${id.toLowerCase()}`,
defaultLabel: id,
+ slot: "SECONDARY",
enabled: true,
disabledReasonKey: null,
};
diff --git a/frontend/editor/src/proprietary/extensions/accountLogout.ts b/frontend/editor/src/proprietary/extensions/accountLogout.ts
index 8c4d9f15d7..1952ae7172 100644
--- a/frontend/editor/src/proprietary/extensions/accountLogout.ts
+++ b/frontend/editor/src/proprietary/extensions/accountLogout.ts
@@ -1,3 +1,4 @@
+import { clearNotificationReadState } from "@app/hooks/useNotifications";
import { suspendWorkbenchSession } from "@app/services/workbenchSession";
type SignOutFn = () => Promise;
@@ -27,6 +28,8 @@ export function useAccountLogout() {
// inherit this workbench. Suspends writing too - signing out unmounts the
// editor, and its flush would otherwise write the record straight back.
suspendWorkbenchSession();
+ // Same reason: the next person's own failures must not arrive pre-read.
+ clearNotificationReadState();
await signOut();
} finally {
redirectToLogin();
From d30faf246b7f169d83083eec20abc91e282857f2 Mon Sep 17 00:00:00 2001
From: ConnorYoh <40631091+ConnorYoh@users.noreply.github.com>
Date: Tue, 1 Sep 2026 19:09:57 +0000
Subject: [PATCH 35/37] fix(billing): the paid tier is Team, and it is not
unlimited users (#7730)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Copy only. No behaviour, no lookup keys, no licence semantics, no
backend.
## Current state
Every surface that sells the paid self-hosted tier offers **"unlimited
seats"** for **"$99/server/mo"**, and the portal's free plan badges
**"Unlimited users"** and **"SSO included"** as free-tier facts.
## Problem
Both claims are now enforceably false.
[#7492](https://github.com/Stirling-Tools/Stirling-PDF/pull/7492) makes
the licence carry a real user cap, and
[Stirling-PDF-SaaS#325](https://github.com/Stirling-Tools/Stirling-PDF-SaaS/pull/325)
sells capacity in blocks of 100 users. An admin reading "unlimited
seats" and then hitting a 409 at the invite screen is the worst version
of this.
The demo has already dropped both claims; ours were the last ones
standing.
## Solution
| Surface | Was | Now |
|---|---|---|
| Onboarding licence slide | "Stirling Server plan, **unlimited seats**
… $99/server/mo" | "Stirling Team plan, **100 users** … $99/mo" |
| Plan comparison table | `unlimitedUsers` = "Unlimited users" |
`usersIncluded` = "100 users included" |
| Plan card highlights | "Unlimited users" | "100 users included" |
| Static plan section | `name: "Server"`, `maxUsers: "Unlimited users"`
| `plan.team.name`, `plan.team.maxUsers` |
| Upgrade banner | "Upgrade to Server Plan" / "unlimited users" |
"Upgrade to the Team plan" / "100 users, SSO" |
| Portal free plan | "Editor" + "SSO included" + "Unlimited users" |
"Editor" + "Every PDF tool" + "Web, desktop & self-hosted" |
The i18n keys are **renamed** (`unlimitedUsers` to `usersIncluded`)
rather than just revalued, so the key name cannot outlive the claim.
Also drops "per server" from `plan.licenseWarning` — we price a block of
100 users and count the provisioned roster, never nodes. And deletes the
orphaned `[settings.planBilling.tier]` block: zero source references,
and it described a retired model (50 credits/mo free, 500 included plus
overage billing).
## Deliberately unchanged
**"Processor" stays the name of the product surface.** The demo names
each plan for its price tier (Editor = $0, Team = $99/mo, Credits = 1¢
each) while keeping Processor as the surface a plan unlocks. Renaming
the surface here would conflate the two, so the plan-name split is left
for the explicit plan catalogue. The free plan also gains no "500 free
credits monthly" badge yet: that is true in the demo but not in our
backend, which still grants a one-time lifetime pool.
## How to test
Self-hosted, as an admin over the free user limit: Settings → Plan
should offer the Team plan at "100 users included", and the onboarding
licence slide should no longer promise unlimited seats. On the portal
billing page, the free plan should read "Free" with no SSO or
unlimited-users badge.
Green locally: 4/4 i18n audits (missing, unused, structure,
translation), 876 tests across 110 files, oxlint, prettier, and all four
typecheck variants (core, proprietary, saas, portal).
---
.../public/locales/en-US/translation.toml | 46 ++++++++-----------
.../onboarding/slides/ServerLicenseSlide.tsx | 8 ++--
.../tools/certSign/CertificateSelector.tsx | 2 +-
.../components/billing/FreePlanView.tsx | 7 ++-
.../components/shared/UpgradeBanner.tsx | 4 +-
.../configSections/AdminPlanSection.tsx | 4 +-
.../config/configSections/plan/PlanCard.tsx | 4 +-
.../configSections/plan/StaticPlanSection.tsx | 12 ++---
.../proprietary/constants/planConstants.ts | 21 +++++----
9 files changed, 53 insertions(+), 55 deletions(-)
diff --git a/frontend/editor/public/locales/en-US/translation.toml b/frontend/editor/public/locales/en-US/translation.toml
index d4a028388b..eea64d65b8 100644
--- a/frontend/editor/public/locales/en-US/translation.toml
+++ b/frontend/editor/public/locales/en-US/translation.toml
@@ -5756,10 +5756,10 @@ rolePlaceholder = "Confirm your role"
roleUser = "User"
[onboarding.serverLicense]
-freeBody = "Our Open-Core licensing permits up to {{freeTierLimit}} users for free per server. To scale uninterrupted, we recommend the Stirling Server plan - unlimited seats and SSO support for $99/server/mo."
-freeTitle = "Server License"
-overLimitBody = "Our licensing permits up to {{freeTierLimit}} users for free per server. You have {{overLimitUserCopy}} Stirling users. To continue uninterrupted, upgrade to the Stirling Server plan - unlimited seats, PDF text editing, and full admin control for $99/server/mo."
-overLimitTitle = "Server License Needed"
+freeBody = "Our Open-Core licensing permits up to {{freeTierLimit}} users for free. To scale uninterrupted, we recommend the Stirling Team plan - 100 users and SSO support for $99/mo."
+freeTitle = "Team plan"
+overLimitBody = "Our licensing permits up to {{freeTierLimit}} users for free. You have {{overLimitUserCopy}} Stirling users. To continue uninterrupted, upgrade to the Stirling Team plan - 100 users, PDF text editing, and full admin control for $99/mo."
+overLimitTitle = "Team plan needed"
seePlans = "See Plans →"
upgrade = "Upgrade now →"
@@ -6570,7 +6570,7 @@ popular = "Popular"
selectPlan = "Select Plan"
showComparison = "Compare All Features"
upgrade = "Upgrade"
-withServer = "+ Server Plan"
+withServer = "+ Team plan"
[plan.api]
large = "5,000 Credits"
@@ -6588,8 +6588,8 @@ highlight1 = "Custom pricing"
highlight2 = "Dedicated support"
highlight3 = "Latest features"
name = "Enterprise"
-requiresServer = "Requires Server"
-requiresServerMessage = "Please upgrade to the Server plan first before upgrading to Enterprise."
+requiresServer = "Requires Team plan"
+requiresServerMessage = "Please upgrade to the Team plan first before upgrading to Enterprise."
[plan.feature]
api = "API Access"
@@ -6614,10 +6614,10 @@ saml = "SAML"
secureLoginSupport = "Secure Login Support"
selfHostedDeployment = "Self-hosted deployment"
sso = "SSO"
-unlimitedUsers = "Unlimited users"
upToFiveUsers = "Up to 5 users"
upToFiveUsersLowercase = "up to 5 users"
usageTracking = "Usage tracking"
+usersIncluded = "100 users included"
usersLimitedToSeats = "Users limited to seats"
[plan.free]
@@ -6646,12 +6646,12 @@ saveWithAnnualBilling = "Save with annual billing"
selfHosted = "Self-hosted"
selfHostedOnInfrastructure = "Self-hosted on your infrastructure"
ssoOAuth = "SSO (OAuth2/OIDC)"
-unlimitedUsers = "Unlimited users"
upToFiveUsers = "Up to 5 users"
usageTrackingPrometheus = "Usage tracking & Prometheus"
+usersIncluded = "100 users included"
[plan.licenseWarning]
-body = "You have {{total}} users but the free tier only supports {{limit}} per server. Upgrade to keep Stirling PDF running smoothly."
+body = "You have {{total}} users but the free tier only supports {{limit}}. Upgrade to keep Stirling PDF running smoothly."
cta = "See plans"
overLimit = "more than {{limit}}"
title = "Free self-hosted limit reached"
@@ -6674,7 +6674,7 @@ title = "You're on a Roll!"
[plan.static]
activateLicense = "Activate Your License"
contactToUpgrade = "Contact us to upgrade or customize your plan"
-getLicense = "Get Server License"
+getLicense = "Get the Team plan"
monthlyBilling = "Monthly Billing"
selectPeriod = "Select Billing Period"
upgradeToEnterprise = "Upgrade to Enterprise"
@@ -6695,6 +6695,10 @@ keyDescription = "Paste the license key from your email"
success = "License Activated!"
successMessage = "Your license has been successfully activated. You can now close this window."
+[plan.team]
+maxUsers = "100 users"
+name = "Team"
+
[policies.activity]
outputsUnavailable = "Policy outputs are no longer available to download."
partialOutputsUnavailable = "Some policy outputs are no longer available to download."
@@ -7000,16 +7004,16 @@ subtitle = "Deploy anywhere, for your whole team."
title = "Free PDF Editors"
[portal.billing.freePlan]
+anywhere = "Web, desktop & self-hosted"
checkoutErrorTitle = "Couldn't start checkout"
currentPlan = "Current plan"
+everyPdfTool = "Every PDF tool"
freeForever = "Free forever"
noTeamResolved = "No team is resolved on your wallet yet — refresh and try again."
ownerOnly = "Only the team owner can switch on the Processor plan."
payInvoice = "Pay invoice to complete"
planName = "Editor"
-ssoIncluded = "SSO included"
switchOnProcessor = "Switch on the Processor →"
-unlimitedUsers = "Unlimited users"
viewQuote = "View quote"
[portal.billing.invoices]
@@ -10444,18 +10448,6 @@ memberCount_one = "{{count}} team member"
memberCount_other = "{{count}} team members"
memberCount_zero = "no team members"
-[settings.planBilling.tier]
-enterprise = "Enterprise"
-enterpriseDescription = "Custom enterprise features and support"
-free = "Free"
-freeDescription = "50 credits per month"
-team = "Team"
-teamBadge = "Team"
-teamDescription = "500 credits/month included, automatic overage billing for uninterrupted service"
-teamTooltipCredits = "Team plan includes {{credits}} credits/month."
-teamTooltipFineprint = "Only pay for what you use beyond included credits."
-teamTooltipOverage = "Automatic overage billing at {{price}}/credit ensures uninterrupted service."
-
[settings.planBilling.trial]
daysRemaining = "{{days}} days remaining"
daysRemainingFull = "Your trial ends in {{days}} days"
@@ -11414,9 +11406,9 @@ urgent = "Urgent"
attentionBody = "Your admin needs to sign in to see more info. Please contact them immediately."
attentionBodyAdmin = "Review the license requirements to keep this server compliant."
attentionTitle = "This server needs admin attention"
-message = "Get the most out of Stirling PDF with unlimited users and advanced features"
+message = "Get the most out of Stirling PDF with 100 users, SSO, and advanced features"
seeInfo = "See info"
-title = "Upgrade to Server Plan"
+title = "Upgrade to the Team plan"
upgradeButton = "Upgrade Now"
[URLToPDF]
diff --git a/frontend/editor/src/core/components/onboarding/slides/ServerLicenseSlide.tsx b/frontend/editor/src/core/components/onboarding/slides/ServerLicenseSlide.tsx
index ac32244e85..09265fb3cf 100644
--- a/frontend/editor/src/core/components/onboarding/slides/ServerLicenseSlide.tsx
+++ b/frontend/editor/src/core/components/onboarding/slides/ServerLicenseSlide.tsx
@@ -20,8 +20,8 @@ export default function ServerLicenseSlide({
totalUsers != null ? totalUsers.toLocaleString() : null;
const overLimitUserCopy = formattedTotalUsers ?? `more than ${freeTierLimit}`;
const title = isOverLimit
- ? i18n.t("onboarding.serverLicense.overLimitTitle", "Server License Needed")
- : i18n.t("onboarding.serverLicense.freeTitle", "Server License");
+ ? i18n.t("onboarding.serverLicense.overLimitTitle", "Team plan needed")
+ : i18n.t("onboarding.serverLicense.freeTitle", "Team plan");
const key = isOverLimit ? "server-license-over-limit" : "server-license";
const overLimitBody = (
@@ -31,7 +31,7 @@ export default function ServerLicenseSlide({
components={{
strong: ,
}}
- defaults="Our licensing permits up to {{freeTierLimit}} users for free per server. You have {{overLimitUserCopy}} Stirling users. To continue uninterrupted, upgrade to the Stirling Server plan - unlimited seats, PDF text editing, and full admin control for $99/server/mo."
+ defaults="Our licensing permits up to {{freeTierLimit}} users for free. You have {{overLimitUserCopy}} Stirling users. To continue uninterrupted, upgrade to the Stirling Team plan - 100 users, PDF text editing, and full admin control for $99/mo."
/>
);
@@ -42,7 +42,7 @@ export default function ServerLicenseSlide({
components={{
strong: ,
}}
- defaults="Our Open-Core licensing permits up to {{freeTierLimit}} users for free per server. To scale uninterrupted, we recommend the Stirling Server plan - unlimited seats and SSO support for $99/server/mo."
+ defaults="Our Open-Core licensing permits up to {{freeTierLimit}} users for free. To scale uninterrupted, we recommend the Stirling Team plan - 100 users and SSO support for $99/mo."
/>
);
diff --git a/frontend/editor/src/core/components/tools/certSign/CertificateSelector.tsx b/frontend/editor/src/core/components/tools/certSign/CertificateSelector.tsx
index 0c09b8c5e6..0cccbe4a40 100644
--- a/frontend/editor/src/core/components/tools/certSign/CertificateSelector.tsx
+++ b/frontend/editor/src/core/components/tools/certSign/CertificateSelector.tsx
@@ -70,7 +70,7 @@ export const CertificateSelector: React.FC = ({
return (
- {/* Managed certificate options — server plan only */}
+ {/* Managed certificate options — Team plan only */}
{isServerPlan && (
- {t("portal.billing.freePlan.ssoIncluded", "SSO included")}
+ {t("portal.billing.freePlan.everyPdfTool", "Every PDF tool")}
- {t("portal.billing.freePlan.unlimitedUsers", "Unlimited users")}
+ {t(
+ "portal.billing.freePlan.anywhere",
+ "Web, desktop & self-hosted",
+ )}