Skip to content

Commit f6661a8

Browse files
authored
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.
1 parent ceeec53 commit f6661a8

35 files changed

Lines changed: 1596 additions & 340 deletions

app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureActionId.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,19 @@ public enum FailureActionId {
1818

1919
DISMISS(Execution.SERVER, "Dismiss"),
2020

21+
/**
22+
* Open the failed operation in the client with its document, for the owner to run again
23+
* themselves. Not a re-run: the settings are theirs to check first.
24+
*/
25+
OPEN_IN_TOOL(Execution.CLIENT, "Retry"),
26+
27+
/**
28+
* Ask the owner for the password and unlock the document in their client. Re-running is implied
29+
* rather than named: an id says what a caller must supply, and a {@link
30+
* FailureActionSlot#RESOLUTION} runs the failed work again once it has it.
31+
*/
32+
DECRYPT(Execution.CLIENT, "Decrypt and retry"),
33+
2134
/** Open the document behind the incident, in whichever client can resolve its id. */
2235
VIEW_FILE(Execution.CLIENT, "View file"),
2336

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package stirling.software.proprietary.failure;
2+
3+
/** Placement intent, not layout: the client promotes, knowing what it can actually run. */
4+
public enum FailureActionSlot {
5+
6+
/** The action that resolves the failure. At most one per kind. */
7+
RESOLUTION,
8+
9+
/** Offered alongside the resolution, for a caller the resolution is not aimed at. */
10+
SECONDARY,
11+
12+
/** Available but folded away: correct, rarely what anyone wants to press next. */
13+
OVERFLOW
14+
}

app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureKind.java

Lines changed: 55 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
package stirling.software.proprietary.failure;
22

3+
import static stirling.software.proprietary.failure.FailureActionId.DECRYPT;
34
import static stirling.software.proprietary.failure.FailureActionId.DISMISS;
5+
import static stirling.software.proprietary.failure.FailureActionId.OPEN_IN_TOOL;
46
import static stirling.software.proprietary.failure.FailureActionId.VIEW_FILE;
57
import static stirling.software.proprietary.failure.FailureActionId.VIEW_IN_PROCESSOR;
8+
import static stirling.software.proprietary.failure.FailureActionSlot.OVERFLOW;
9+
import static stirling.software.proprietary.failure.FailureActionSlot.SECONDARY;
610
import static stirling.software.proprietary.failure.FailureAudience.ANYONE_WHO_SEES;
711
import static stirling.software.proprietary.failure.FailureAudience.OWNER;
812
import static stirling.software.proprietary.failure.FailureAudience.TEAM_REVIEWER;
@@ -21,11 +25,8 @@
2125
import lombok.Getter;
2226

2327
/**
24-
* The registry of failure kinds, described as data: a stable id, i18n keys and an English fallback
25-
* like {@code ExceptionUtils.ErrorCode}, plus the facets a review surface needs.
26-
*
27-
* <p>A new kind ships as a registry entry plus copy. Each offer also says who it is for, since one
28-
* incident is read both by whoever hit it and by whoever reviews after them.
28+
* The registry of failure kinds as data: id, i18n keys, English fallback, plus the facets a review
29+
* surface needs. A new kind ships as an entry plus copy; each offer says who it is for and where.
2930
*/
3031
@Getter
3132
public enum FailureKind {
@@ -36,9 +37,12 @@ public enum FailureKind {
3637
FailureScope.FILE,
3738
errorCodes("E004"),
3839
fallback("This document is password-protected, so the pipeline could not read it."),
39-
offer(VIEW_FILE, OWNER),
40-
offer(VIEW_IN_PROCESSOR, TEAM_REVIEWER),
41-
offer(DISMISS, ANYONE_WHO_SEES)),
40+
// The password is the fix; the owner's own document is the runner-up.
41+
resolution(DECRYPT, OWNER),
42+
global(VIEW_FILE, OWNER, SECONDARY),
43+
global(VIEW_IN_PROCESSOR, TEAM_REVIEWER, OVERFLOW),
44+
global(OPEN_IN_TOOL, OWNER, OVERFLOW),
45+
global(DISMISS, ANYONE_WHO_SEES, OVERFLOW)),
4246

4347
UNKNOWN(
4448
FailureStage.INTERNAL,
@@ -47,11 +51,11 @@ public enum FailureKind {
4751
FailureScope.RUN,
4852
noErrorCodes(),
4953
fallback("This run failed for a reason Stirling does not yet recognise."),
50-
// Same order as every other kind: declaration order is display order, so the document
51-
// leads wherever it is offered rather than moving between failures.
52-
offer(VIEW_FILE, OWNER),
53-
offer(VIEW_IN_PROCESSOR, TEAM_REVIEWER),
54-
offer(DISMISS, ANYONE_WHO_SEES));
54+
// No known fix to declare, so a plain retry leads: these are often one-offs.
55+
global(OPEN_IN_TOOL, OWNER, SECONDARY),
56+
global(VIEW_FILE, OWNER, SECONDARY),
57+
global(VIEW_IN_PROCESSOR, TEAM_REVIEWER, OVERFLOW),
58+
global(DISMISS, ANYONE_WHO_SEES, OVERFLOW));
5559

5660
private static final String KEY_PREFIX = "portal.failures.kind.";
5761
private static final String ACTION_KEY_PREFIX = "portal.failures.action.";
@@ -98,27 +102,37 @@ public enum FailureKind {
98102
this.offers = List.of(offers);
99103
}
100104

101-
/**
102-
* One ordered list rather than ids plus parallel maps of audiences and labels, which could
103-
* disagree with each other.
104-
*
105-
* @param labelKeySuffix key under {@code portal.failures.action.}, or null for the generic
106-
* label
107-
*/
108-
private record Offer(FailureActionId id, FailureAudience audience, String labelKeySuffix) {}
105+
/** One ordered list, not parallel maps of audiences, slots and labels that could disagree. */
106+
private record Offer(
107+
FailureActionId id,
108+
FailureAudience audience,
109+
FailureActionSlot slot,
110+
String labelKeySuffix) {}
109111

110-
/** Declaration order is display order. */
111-
private static Offer offer(FailureActionId id, FailureAudience audience) {
112-
return new Offer(id, audience, null);
112+
/** The action that fixes this kind. One per kind: needing two would make it two kinds. */
113+
private static Offer resolution(FailureActionId id, FailureAudience audience) {
114+
return new Offer(id, audience, FailureActionSlot.RESOLUTION, null);
113115
}
114116

115-
/**
116-
* As {@link #offer(FailureActionId, FailureAudience)}, but labelled by this kind's own wording
117-
* where the shared one reads badly.
118-
*/
119-
private static Offer offer(
117+
/** As {@link #resolution(FailureActionId, FailureAudience)}, with this kind's own wording. */
118+
private static Offer resolution(
120119
FailureActionId id, FailureAudience audience, String labelKeySuffix) {
121-
return new Offer(id, audience, labelKeySuffix);
120+
return new Offer(id, audience, FailureActionSlot.RESOLUTION, labelKeySuffix);
121+
}
122+
123+
/** Not this kind's fix: an offer any kind can make, with the shared wording. */
124+
private static Offer global(
125+
FailureActionId id, FailureAudience audience, FailureActionSlot slot) {
126+
return new Offer(id, audience, slot, null);
127+
}
128+
129+
/** As above, with this kind's own wording where the shared one reads badly. */
130+
private static Offer global(
131+
FailureActionId id,
132+
FailureAudience audience,
133+
FailureActionSlot slot,
134+
String labelKeySuffix) {
135+
return new Offer(id, audience, slot, labelKeySuffix);
122136
}
123137

124138
/**
@@ -157,21 +171,25 @@ public List<FailureActionId> getActions() {
157171
return offers.stream().map(Offer::id).toList();
158172
}
159173

160-
/**
161-
* What this kind offers, in declaration order, each with its label resolved. What a review
162-
* surface reads, so it never has to ask two separate questions about one offer.
163-
*/
174+
/** What this kind offers, in declaration order, each with label and placement resolved. */
164175
public List<OfferedAction> getOfferedActions() {
165176
return offers.stream()
166177
.map(
167178
offer ->
168179
new OfferedAction(
169-
offer.id(), labelKeyFor(offer.id()), offer.audience()))
180+
offer.id(),
181+
labelKeyFor(offer.id()),
182+
offer.audience(),
183+
offer.slot()))
170184
.toList();
171185
}
172186

173-
/** One action as a kind declares it: what to call it and who it is for. */
174-
public record OfferedAction(FailureActionId id, String labelKey, FailureAudience audience) {}
187+
/** One action as a kind declares it: what to call it, who it is for, where it wants to sit. */
188+
public record OfferedAction(
189+
FailureActionId id,
190+
String labelKey,
191+
FailureAudience audience,
192+
FailureActionSlot slot) {}
175193

176194
/** Whether this kind offers {@code action}. The dispatch guard: see {@code FailureActionId}. */
177195
public boolean declares(FailureActionId action) {

app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventRepository.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,16 +64,17 @@ List<FileRunEventEntity> findByTeamAndStatus(
6464
int fold(@Param("id") String id, @Param("now") Instant now, @Param("detail") String detail);
6565

6666
/**
67-
* Reopen a resolved incident whose failure has recurred. Guarded on the current status so only
68-
* {@code RESOLVED} flips; a concurrent dismiss is never overwritten back to {@code NEW}.
67+
* A recurrence reopens {@code RESOLVED} (the fix did not hold) and {@code FILE_REMOVED} (the
68+
* document is back). Guarded, so a reviewer's {@code DISMISSED} is never overwritten.
6969
*/
7070
@Modifying(clearAutomatically = true)
7171
@Transactional
7272
@Query(
7373
"update FileRunEventEntity e set"
7474
+ " e.status = stirling.software.proprietary.failure.FileRunEventStatus.NEW,"
75-
+ " e.statusActor = null, e.statusAt = null where e.id = :id and e.status ="
76-
+ " stirling.software.proprietary.failure.FileRunEventStatus.RESOLVED")
75+
+ " e.statusActor = null, e.statusAt = null where e.id = :id and e.status in"
76+
+ " (stirling.software.proprietary.failure.FileRunEventStatus.RESOLVED,"
77+
+ " stirling.software.proprietary.failure.FileRunEventStatus.FILE_REMOVED)")
7778
int reopenIfResolved(@Param("id") String id);
7879

7980
/**

app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventService.java

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
package stirling.software.proprietary.failure;
22

3+
import java.nio.charset.StandardCharsets;
4+
import java.security.MessageDigest;
5+
import java.security.NoSuchAlgorithmException;
6+
import java.util.HexFormat;
37
import java.util.List;
48
import java.util.Map;
59

@@ -171,6 +175,18 @@ public FileRunEvent dispatch(String eventId, String actionId, Map<String, String
171175
return action.execute(event, inputs == null ? Map.of() : inputs, currentActor());
172176
}
173177

178+
/** Mark an incident resolved after a client's own retry worked. Idempotent. */
179+
public FileRunEvent resolve(String eventId) {
180+
FileRunEvent event = requireVisible(eventId);
181+
// No terminal pre-check: the store's guarded UPDATE decides, rather than racing a read.
182+
return store.applyStatusOnce(
183+
event.id(),
184+
event.teamId(),
185+
FileRunEventStatus.RESOLVED,
186+
currentActor(),
187+
FileRunEventStatus.open());
188+
}
189+
174190
/** "No such event" rather than a refusal, so trying does not confirm a colleague's exists. */
175191
private FileRunEvent requireVisible(String eventId) {
176192
ReadScope scope = readScope();
@@ -222,7 +238,8 @@ private static AvailableAction availability(
222238
boolean unattended,
223239
boolean documentless) {
224240
String reason = disabledReasonFor(offer.audience(), closed, unattended, documentless);
225-
return new AvailableAction(offer.id(), offer.labelKey(), reason == null, reason);
241+
return new AvailableAction(
242+
offer.id(), offer.labelKey(), offer.slot(), reason == null, reason);
226243
}
227244

228245
/** Closed wins over everything, then the owner-only reasons, most specific first. */
@@ -251,11 +268,38 @@ private static boolean offeredTo(
251268
};
252269
}
253270

254-
/** Login disabled has no roles, so its one operator triages everything. */
255-
private boolean reviewsTeam() {
271+
/** Whether the caller triages the team's incidents, not only their own. Login disabled: all. */
272+
public boolean reviewsTeam() {
256273
return !enforced() || policyManagementAuthority.canEditPolicies();
257274
}
258275

276+
/**
277+
* An opaque, stable discriminator for the calling viewer, for a client scoping per-browser read
278+
* state. Hashed rather than the username itself: a client only needs to tell one viewer from
279+
* another, and the value ends up in that browser's own storage.
280+
*
281+
* <p>{@code "anonymous"} with login disabled, where the one operator is every viewer.
282+
*/
283+
public String viewerKey() {
284+
String actor = currentActor();
285+
return actor == null || actor.isBlank() ? "anonymous" : sha256Prefix(actor);
286+
}
287+
288+
/** First 8 bytes of SHA-256 as hex: stable, one-way, and collision-safe enough to key on. */
289+
private static String sha256Prefix(String value) {
290+
try {
291+
byte[] digest =
292+
MessageDigest.getInstance("SHA-256")
293+
.digest(value.getBytes(StandardCharsets.UTF_8));
294+
return HexFormat.of().formatHex(digest, 0, 8);
295+
} catch (NoSuchAlgorithmException e) {
296+
// Every JVM ships SHA-256; a constant here would silently merge two viewers' read
297+
// state, so the caller gets no key and the client falls back to showing everything.
298+
log.warn("SHA-256 unavailable, so notifications cannot be scoped to a viewer", e);
299+
return "";
300+
}
301+
}
302+
259303
private FailureActionId parseActionId(String actionId) {
260304
for (FailureActionId candidate : FailureActionId.values()) {
261305
if (candidate.name().equals(actionId)) {
@@ -326,6 +370,11 @@ private boolean enforced() {
326370
return applicationProperties.getSecurity().isEnableLogin();
327371
}
328372

373+
/** One action offered to one caller, availability resolved. */
329374
public record AvailableAction(
330-
FailureActionId id, String labelKey, boolean enabled, String disabledReasonKey) {}
375+
FailureActionId id,
376+
String labelKey,
377+
FailureActionSlot slot,
378+
boolean enabled,
379+
String disabledReasonKey) {}
331380
}

app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventStatus.java

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,16 @@
33
import java.util.Arrays;
44
import java.util.List;
55

6-
/**
7-
* Disposition of one recorded failure. {@code RESOLVED} is declared but not set yet (it becomes
8-
* system-set later); the rollup already defines what a repeat means for it, which is to reopen.
9-
*/
6+
/** Disposition of one recorded failure. {@code RESOLVED} is system-set; a repeat reopens it. */
107
public enum FileRunEventStatus {
118
NEW(false),
129
ACKNOWLEDGED(false),
1310
DISMISSED(true),
1411
RESOLVED(true),
1512

1613
/**
17-
* The document this incident was about was deleted from its owner's editor, so there is nothing
18-
* left to act on. Distinct from {@code DISMISSED}, which is a reviewer's decision, and from
19-
* {@code RESOLVED}, which reopens on recurrence: this one cannot recur, the file is gone.
14+
* The document was deleted, so there is nothing left to act on. A recurrence reopens it like
15+
* {@code RESOLVED}: a fresh failure is proof the document is back.
2016
*/
2117
FILE_REMOVED(true);
2218

app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventView.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,15 @@ public static FileRunEventView of(
6161
}
6262

6363
/**
64-
* {@code defaultLabel} and {@code execution} let a client render and route an action it was
65-
* never built with. Declaration order is display order.
64+
* {@code defaultLabel} and {@code execution} let a client render an action it was never built
65+
* with; {@code slot} is placement intent. See {@link FailureActionSlot}.
6666
*/
6767
public record ActionView(
6868
String id,
6969
String labelKey,
7070
String defaultLabel,
7171
FailureActionId.Execution execution,
72+
FailureActionSlot slot,
7273
boolean enabled,
7374
String disabledReasonKey) {
7475

@@ -78,6 +79,7 @@ public static ActionView of(FileRunEventService.AvailableAction action) {
7879
action.labelKey(),
7980
action.id().getDefaultLabel(),
8081
action.id().getExecution(),
82+
action.slot(),
8183
action.enabled(),
8284
action.disabledReasonKey());
8385
}

0 commit comments

Comments
 (0)