mirror of
https://github.com/Stirling-Tools/Stirling-PDF.git
synced 2026-09-02 21:03:34 +03:00
Failure action slots, resolve transition, and the bell that renders them (Review Flow PR 5a) (#7761)
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.
This commit is contained in:
+13
@@ -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"),
|
||||
|
||||
|
||||
+14
@@ -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
|
||||
}
|
||||
+55
-37
@@ -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.
|
||||
*
|
||||
* <p>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<OfferedAction> 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) {
|
||||
|
||||
+5
-4
@@ -64,16 +64,17 @@ public interface FileRunEventRepository extends JpaRepository<FileRunEventEntity
|
||||
int fold(@Param("id") String id, @Param("now") Instant now, @Param("detail") String detail);
|
||||
|
||||
/**
|
||||
* Reopen a resolved incident whose failure has recurred. Guarded on the current status so only
|
||||
* {@code RESOLVED} flips; a concurrent dismiss is never overwritten back to {@code NEW}.
|
||||
* A recurrence reopens {@code RESOLVED} (the fix did not hold) and {@code FILE_REMOVED} (the
|
||||
* document is back). Guarded, so a reviewer's {@code DISMISSED} is never overwritten.
|
||||
*/
|
||||
@Modifying(clearAutomatically = true)
|
||||
@Transactional
|
||||
@Query(
|
||||
"update FileRunEventEntity e set"
|
||||
+ " e.status = stirling.software.proprietary.failure.FileRunEventStatus.NEW,"
|
||||
+ " e.statusActor = null, e.statusAt = null where e.id = :id and e.status ="
|
||||
+ " stirling.software.proprietary.failure.FileRunEventStatus.RESOLVED")
|
||||
+ " e.statusActor = null, e.statusAt = null where e.id = :id and e.status in"
|
||||
+ " (stirling.software.proprietary.failure.FileRunEventStatus.RESOLVED,"
|
||||
+ " stirling.software.proprietary.failure.FileRunEventStatus.FILE_REMOVED)")
|
||||
int reopenIfResolved(@Param("id") String id);
|
||||
|
||||
/**
|
||||
|
||||
+53
-4
@@ -1,5 +1,9 @@
|
||||
package stirling.software.proprietary.failure;
|
||||
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.security.MessageDigest;
|
||||
import java.security.NoSuchAlgorithmException;
|
||||
import java.util.HexFormat;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
@@ -171,6 +175,18 @@ public class FileRunEventService {
|
||||
return action.execute(event, inputs == null ? Map.of() : inputs, currentActor());
|
||||
}
|
||||
|
||||
/** Mark an incident resolved after a client's own retry worked. Idempotent. */
|
||||
public FileRunEvent resolve(String eventId) {
|
||||
FileRunEvent event = requireVisible(eventId);
|
||||
// No terminal pre-check: the store's guarded UPDATE decides, rather than racing a read.
|
||||
return store.applyStatusOnce(
|
||||
event.id(),
|
||||
event.teamId(),
|
||||
FileRunEventStatus.RESOLVED,
|
||||
currentActor(),
|
||||
FileRunEventStatus.open());
|
||||
}
|
||||
|
||||
/** "No such event" rather than a refusal, so trying does not confirm a colleague's exists. */
|
||||
private FileRunEvent requireVisible(String eventId) {
|
||||
ReadScope scope = readScope();
|
||||
@@ -222,7 +238,8 @@ public class FileRunEventService {
|
||||
boolean unattended,
|
||||
boolean documentless) {
|
||||
String reason = disabledReasonFor(offer.audience(), closed, unattended, documentless);
|
||||
return new AvailableAction(offer.id(), offer.labelKey(), reason == null, reason);
|
||||
return new AvailableAction(
|
||||
offer.id(), offer.labelKey(), offer.slot(), reason == null, reason);
|
||||
}
|
||||
|
||||
/** Closed wins over everything, then the owner-only reasons, most specific first. */
|
||||
@@ -251,11 +268,38 @@ public class FileRunEventService {
|
||||
};
|
||||
}
|
||||
|
||||
/** Login disabled has no roles, so its one operator triages everything. */
|
||||
private boolean reviewsTeam() {
|
||||
/** Whether the caller triages the team's incidents, not only their own. Login disabled: all. */
|
||||
public boolean reviewsTeam() {
|
||||
return !enforced() || policyManagementAuthority.canEditPolicies();
|
||||
}
|
||||
|
||||
/**
|
||||
* An opaque, stable discriminator for the calling viewer, for a client scoping per-browser read
|
||||
* state. Hashed rather than the username itself: a client only needs to tell one viewer from
|
||||
* another, and the value ends up in that browser's own storage.
|
||||
*
|
||||
* <p>{@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) {}
|
||||
}
|
||||
|
||||
+3
-7
@@ -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);
|
||||
|
||||
|
||||
+4
-2
@@ -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());
|
||||
}
|
||||
|
||||
+35
-4
@@ -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<NotificationView> notifications) {}
|
||||
public record NotificationsResponse(
|
||||
List<NotificationView> notifications,
|
||||
boolean viewerReviewsTeam,
|
||||
/** Opaque; the client scopes its own read state on it. Empty means "cannot scope". */
|
||||
String viewerKey) {}
|
||||
}
|
||||
|
||||
+40
-2
@@ -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.
|
||||
*
|
||||
* <p>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<NotificationView> 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. */
|
||||
|
||||
+22
@@ -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<QualifiedId> 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) {}
|
||||
}
|
||||
|
||||
-2
@@ -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);
|
||||
|
||||
+1
@@ -43,6 +43,7 @@ class CheckConstrainedEnumsTest {
|
||||
assertThat(persisted)
|
||||
.doesNotContain(
|
||||
FailureAudience.class,
|
||||
FailureActionSlot.class,
|
||||
FailureActionId.class,
|
||||
FailureActionId.Execution.class,
|
||||
Ownership.class);
|
||||
|
||||
+37
-37
@@ -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<FailureActionId> ranking =
|
||||
List.of(
|
||||
FailureActionId.VIEW_FILE,
|
||||
FailureActionId.VIEW_IN_PROCESSOR,
|
||||
FailureActionId.DISMISS);
|
||||
|
||||
List<FailureActionId> 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<String> 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
|
||||
|
||||
+2
@@ -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");
|
||||
});
|
||||
}
|
||||
|
||||
+1
@@ -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");
|
||||
|
||||
+117
-3
@@ -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");
|
||||
|
||||
+3
-1
@@ -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);
|
||||
|
||||
+73
@@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+152
@@ -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);
|
||||
}
|
||||
}
|
||||
-2
@@ -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");
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<typeof baseRender>[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<string, unknown>;
|
||||
@@ -77,18 +83,33 @@ const { NotificationBell } =
|
||||
|
||||
function offer(
|
||||
id: string,
|
||||
slot: NotificationActionSlot = "SECONDARY",
|
||||
overrides: Partial<NotificationActionOffer> = {},
|
||||
): 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<string, string> = {
|
||||
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(<NotificationBell />);
|
||||
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(<NotificationBell />);
|
||||
|
||||
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(<NotificationBell />);
|
||||
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(<NotificationBell />);
|
||||
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(<NotificationBell />);
|
||||
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(<NotificationBell />);
|
||||
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));
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string | null>(null);
|
||||
const [busy, setBusy] = useState<string | null>(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 (
|
||||
<li
|
||||
@@ -151,62 +153,71 @@ export function NotificationItem({
|
||||
</span>
|
||||
)}
|
||||
|
||||
{notification.detail && (
|
||||
<>
|
||||
<span
|
||||
className={
|
||||
expanded
|
||||
? "notification-bell__detail notification-bell__detail--full"
|
||||
: "notification-bell__detail"
|
||||
}
|
||||
>
|
||||
{notification.detail}
|
||||
</span>
|
||||
<span className="notification-bell__chrome">
|
||||
<button
|
||||
type="button"
|
||||
className="notification-bell__chip"
|
||||
aria-label={`${t("notifications.detail.copy", "Copy error")}: ${title}`}
|
||||
onClick={() => void copyDetail()}
|
||||
>
|
||||
{copied
|
||||
? t("notifications.detail.copied", "Copied")
|
||||
: t("notifications.detail.copy", "Copy error")}
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
className="notification-bell__chip"
|
||||
aria-expanded={expanded}
|
||||
aria-label={`${
|
||||
expanded
|
||||
? t("notifications.detail.less", "Show less")
|
||||
: t("notifications.detail.more", "Show full message")
|
||||
}: ${title}`}
|
||||
onClick={() => setExpanded((wasExpanded) => !wasExpanded)}
|
||||
>
|
||||
{expanded
|
||||
? t("notifications.detail.less", "Show less")
|
||||
: t("notifications.detail.more", "Show full message")}
|
||||
</button>
|
||||
</span>
|
||||
</>
|
||||
)}
|
||||
{summary && <span className="notification-bell__detail">{summary}</span>}
|
||||
|
||||
{note && <span className="notification-bell__note">{note}</span>}
|
||||
|
||||
{/* 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) && (
|
||||
<span className="notification-bell__actions">
|
||||
{usable.map((offer, index) => (
|
||||
{primary && (
|
||||
<ActionButton
|
||||
key={offer.id}
|
||||
variant={index === 0 ? "primary" : "secondary"}
|
||||
variant="primary"
|
||||
rowTitle={title}
|
||||
label={labelOf(offer)}
|
||||
busy={busy === offer.id}
|
||||
onRun={() => void run(offer)}
|
||||
label={labelOf(primary)}
|
||||
busy={busy === primary.id}
|
||||
onRun={() => void run(primary)}
|
||||
/>
|
||||
))}
|
||||
)}
|
||||
{secondary && (
|
||||
<ActionButton
|
||||
variant="secondary"
|
||||
rowTitle={title}
|
||||
label={labelOf(secondary)}
|
||||
busy={busy === secondary.id}
|
||||
onRun={() => void run(secondary)}
|
||||
/>
|
||||
)}
|
||||
{(overflow.length > 0 || notification.detail) && (
|
||||
<Menu withinPortal position="bottom-end" shadow="md" width={180}>
|
||||
<Menu.Target>
|
||||
<Tooltip
|
||||
label={t("notifications.action.more", "More options")}
|
||||
withinPortal
|
||||
>
|
||||
<ActionIcon
|
||||
variant="tertiary"
|
||||
size="sm"
|
||||
className="notification-bell__more"
|
||||
aria-label={`${t("notifications.action.more", "More options")}: ${title}`}
|
||||
>
|
||||
<LocalIcon icon="more-horiz" width={14} height={14} />
|
||||
</ActionIcon>
|
||||
</Tooltip>
|
||||
</Menu.Target>
|
||||
<Menu.Dropdown className="notification-bell__menu">
|
||||
{overflow.map((offer) => (
|
||||
<Menu.Item
|
||||
key={offer.id}
|
||||
disabled={busy === offer.id}
|
||||
onClick={() => void run(offer)}
|
||||
>
|
||||
{labelOf(offer)}
|
||||
</Menu.Item>
|
||||
))}
|
||||
{notification.detail && (
|
||||
<Menu.Item
|
||||
closeMenuOnClick={false}
|
||||
onClick={() => void copyDetail()}
|
||||
>
|
||||
{copied
|
||||
? t("notifications.action.copiedLog", "Copied")
|
||||
: t("notifications.action.copyLog", "Copy log")}
|
||||
</Menu.Item>
|
||||
)}
|
||||
</Menu.Dropdown>
|
||||
</Menu>
|
||||
)}
|
||||
</span>
|
||||
)}
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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 ? (
|
||||
<p className="notification-bell__empty">
|
||||
{t("notifications.empty", "Nothing to report.")}
|
||||
{t("notifications.empty", "You're all caught up.")}
|
||||
</p>
|
||||
) : (
|
||||
<ul className="notification-bell__list">
|
||||
@@ -107,6 +109,7 @@ export function NotificationPanel({
|
||||
{index === 0 && dividedAt > 0 && (
|
||||
<li aria-hidden>
|
||||
<DividerWithText
|
||||
className="notification-bell__divider notification-bell__divider--new"
|
||||
text={t("notifications.section.new", "New")}
|
||||
/>
|
||||
</li>
|
||||
@@ -115,6 +118,7 @@ export function NotificationPanel({
|
||||
{index === dividedAt && dividedAt > 0 && (
|
||||
<li aria-hidden>
|
||||
<DividerWithText
|
||||
className="notification-bell__divider"
|
||||
text={t("notifications.section.earlier", "Earlier")}
|
||||
/>
|
||||
</li>
|
||||
|
||||
@@ -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<string, NotificationActionOffer> = {
|
||||
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<string, NotificationActionOffer> = {
|
||||
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> = {},
|
||||
): NotificationActionOffer {
|
||||
return {
|
||||
id,
|
||||
labelKey: `portal.failures.action.${id.toLowerCase()}`,
|
||||
defaultLabel: id,
|
||||
slot,
|
||||
enabled: true,
|
||||
disabledReasonKey: null,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function from(
|
||||
declared: Record<string, NotificationActionOffer>,
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -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<NotificationActionSlot, number> = {
|
||||
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,
|
||||
};
|
||||
}
|
||||
+11
-5
@@ -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(<QuickNavRailNotifications onToggle={() => {}} />);
|
||||
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { clearNotificationReadState } from "@app/hooks/useNotifications";
|
||||
import { suspendWorkbenchSession } from "@app/services/workbenchSession";
|
||||
|
||||
type SignOutFn = () => Promise<void>;
|
||||
@@ -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();
|
||||
|
||||
@@ -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<AppNotification[]>((resolve) => {
|
||||
new Promise<FetchedNotifications>((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<AppNotification[]>((resolve) => {
|
||||
new Promise<FetchedNotifications>((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);
|
||||
|
||||
@@ -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<string, NotificationDocumentState>;
|
||||
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<void> {
|
||||
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<void> {
|
||||
);
|
||||
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,
|
||||
|
||||
@@ -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<AppNotification[]> {
|
||||
): Promise<FetchedNotifications> {
|
||||
try {
|
||||
const response = await apiClient.get<NotificationsResponse>(
|
||||
`${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<boolean> {
|
||||
try {
|
||||
await apiClient.post(
|
||||
`${NOTIFICATIONS_PATH}/${encodeURIComponent(notificationId)}/resolved`,
|
||||
);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -85,6 +85,7 @@ function offer(id: string): NotificationActionOffer {
|
||||
id,
|
||||
labelKey: `portal.failures.action.${id.toLowerCase()}`,
|
||||
defaultLabel: id,
|
||||
slot: "SECONDARY",
|
||||
enabled: true,
|
||||
disabledReasonKey: null,
|
||||
};
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { clearNotificationReadState } from "@app/hooks/useNotifications";
|
||||
import { suspendWorkbenchSession } from "@app/services/workbenchSession";
|
||||
|
||||
type SignOutFn = () => Promise<void>;
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user