Skip to content

Commit 6539b98

Browse files
committed
Fix decrypt-and-retry, and give the bell a hierarchy to read
Decrypt and retry unlocked the document but left the user holding two copies of it, and the retry restarted the upload chain from the front rather than rejoining it where it stopped. The unlock now versions the original in place via consumeFiles, and the run resumes at the first policy that has not already been applied. Read state moves from the id of the newest notification to a watermark on the time the list is ordered by. Resolving the newest row used to leave the marker pointing at nothing, which read every remaining row as unread again. Each row now shows two buttons and an overflow menu instead of a line of near-equal ones, and the failure message sits in its own box with copy and expand as corner glyphs.
1 parent 1410c4d commit 6539b98

25 files changed

Lines changed: 1007 additions & 360 deletions

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,12 @@ public enum FailureKind {
4646
FailureScope.FILE,
4747
errorCodes("E004"),
4848
fallback("This document is password-protected, so the pipeline could not read it."),
49-
// The password is the fix and only the owner has it, so everyone else is
50-
// offered the run and a way to close the row.
49+
// The password is the fix and the owner's own document the runner-up; the rest go to
50+
// the overflow menu.
5151
resolution(DECRYPT_AND_RETRY, OWNER),
52+
global(VIEW_FILE, OWNER, SECONDARY),
53+
global(VIEW_IN_PROCESSOR, TEAM_REVIEWER, OVERFLOW),
5254
global(RETRY, OWNER, OVERFLOW),
53-
global(VIEW_FILE, OWNER, OVERFLOW),
54-
global(VIEW_IN_PROCESSOR, TEAM_REVIEWER, SECONDARY),
5555
global(DISMISS, ANYONE_WHO_SEES, OVERFLOW)),
5656

5757
UNKNOWN(
@@ -61,11 +61,11 @@ public enum FailureKind {
6161
FailureScope.RUN,
6262
noErrorCodes(),
6363
fallback("This run failed for a reason Stirling does not yet recognise."),
64-
// Nothing here is known to be fixable, so there is no resolution to declare. A plain
65-
// retry is still worth offering: an unrecognised failure is often a one-off.
64+
// Nothing known to be fixable, so no resolution to declare: a plain retry leads
65+
// instead, an unrecognised failure often being a one-off.
6666
global(RETRY, OWNER, SECONDARY),
67-
global(VIEW_IN_PROCESSOR, TEAM_REVIEWER, SECONDARY),
68-
global(VIEW_FILE, OWNER, OVERFLOW),
67+
global(VIEW_FILE, OWNER, SECONDARY),
68+
global(VIEW_IN_PROCESSOR, TEAM_REVIEWER, OVERFLOW),
6969
global(DISMISS, ANYONE_WHO_SEES, OVERFLOW));
7070

7171
private static final String KEY_PREFIX = "portal.failures.kind.";

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -289,8 +289,8 @@ private static boolean offeredTo(
289289
};
290290
}
291291

292-
/** Whether the caller triages the whole team's incidents. Login disabled has no roles. */
293-
private boolean reviewsTeam() {
292+
/** Whether the caller triages the whole team's incidents, rather than only their own. */
293+
public boolean reviewsTeam() {
294294
return !enforced() || policyManagementAuthority.canEditPolicies();
295295
}
296296

‎app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationController.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ public class NotificationController {
5050
+ " to mark read here yet: the client tracks what it has shown.")
5151
public NotificationsResponse list(@RequestParam(required = false) Integer limit) {
5252
int capped = Math.min(limit == null ? DEFAULT_LIMIT : Math.max(1, limit), MAX_LIMIT);
53-
return new NotificationsResponse(notifications.list(capped));
53+
return new NotificationsResponse(
54+
notifications.list(capped), notifications.callerReviewsTeam());
5455
}
5556

5657
/**
@@ -81,6 +82,9 @@ public NotificationView resolved(@PathVariable String notificationId) {
8182

8283
/**
8384
* Wrapped rather than a bare array so paging or a total can be added without breaking clients.
85+
* {@code viewerReviewsTeam} lets the client filter a member's list; see {@link
86+
* NotificationService#callerReviewsTeam()}.
8487
*/
85-
public record NotificationsResponse(List<NotificationView> notifications) {}
88+
public record NotificationsResponse(
89+
List<NotificationView> notifications, boolean viewerReviewsTeam) {}
8690
}

‎app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationService.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,14 @@ public List<NotificationView> list(int limit) {
3434
return fileRunEvents.list(null, null, limit).stream().map(this::fromFailure).toList();
3535
}
3636

37+
/**
38+
* Whether the caller sees the whole team's incidents rather than only their own. The client
39+
* uses it to hide a member's rows whose document is not in this browser, which it alone knows.
40+
*/
41+
public boolean callerReviewsTeam() {
42+
return fileRunEvents.reviewsTeam();
43+
}
44+
3745
/**
3846
* Record that the client's own retry of this notification worked, and return it as it now
3947
* stands. Takes the prefixed id even though nobody pressed a button: it is still a call made

‎app/proprietary/src/test/java/stirling/software/proprietary/failure/FailureKindTest.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -239,12 +239,12 @@ void offersARetryToItsOwnerAndTheRunToWhoeverReviews() {
239239
assertThat(FailureKind.UNKNOWN.getOfferedActions())
240240
.containsExactly(
241241
offered(FailureActionId.RETRY, OWNER, SECONDARY, "retry"),
242+
offered(FailureActionId.VIEW_FILE, OWNER, SECONDARY, "viewFile"),
242243
offered(
243244
FailureActionId.VIEW_IN_PROCESSOR,
244245
TEAM_REVIEWER,
245-
SECONDARY,
246+
OVERFLOW,
246247
"viewInProcessor"),
247-
offered(FailureActionId.VIEW_FILE, OWNER, OVERFLOW, "viewFile"),
248248
offered(FailureActionId.DISMISS, ANYONE_WHO_SEES, OVERFLOW, "dismiss"));
249249
}
250250

@@ -308,13 +308,13 @@ void aKindWithSomethingToFixOffersTheFixToItsOwnerAndTheRunToItsReviewer() {
308308
OWNER,
309309
RESOLUTION,
310310
"decryptAndRetry"),
311-
offered(FailureActionId.RETRY, OWNER, OVERFLOW, "retry"),
312-
offered(FailureActionId.VIEW_FILE, OWNER, OVERFLOW, "viewFile"),
311+
offered(FailureActionId.VIEW_FILE, OWNER, SECONDARY, "viewFile"),
313312
offered(
314313
FailureActionId.VIEW_IN_PROCESSOR,
315314
TEAM_REVIEWER,
316-
SECONDARY,
315+
OVERFLOW,
317316
"viewInProcessor"),
317+
offered(FailureActionId.RETRY, OWNER, OVERFLOW, "retry"),
318318
offered(FailureActionId.DISMISS, ANYONE_WHO_SEES, OVERFLOW, "dismiss"));
319319
}
320320

‎app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventControllerTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ void carriesEnoughForAClientToRenderAndRouteAnActionItDoesNotKnow() {
161161
action -> {
162162
assertThat(action.execution())
163163
.isEqualTo(FailureActionId.Execution.CLIENT);
164-
assertThat(action.slot()).isEqualTo(FailureActionSlot.SECONDARY);
164+
assertThat(action.slot()).isEqualTo(FailureActionSlot.OVERFLOW);
165165
assertThat(action.defaultLabel()).isEqualTo("View in processor");
166166
});
167167
}

‎app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventHttpIntegrationTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ void serialisesAnEventWithItsFacetsCopyKeysAndResolvedActions() throws Exception
130130
assertThat(actions.get(0).get("defaultLabel").asString())
131131
.isEqualTo("View in processor");
132132
assertThat(actions.get(0).get("execution").asString()).isEqualTo("CLIENT");
133-
assertThat(actions.get(0).get("slot").asString()).isEqualTo("SECONDARY");
133+
assertThat(actions.get(0).get("slot").asString()).isEqualTo("OVERFLOW");
134134
assertThat(actions.get(0).get("enabled").asBoolean()).isTrue();
135135
assertThat(actions.get(0).get("disabledReasonKey").isNull()).isTrue();
136136
assertThat(actions.get(1).get("id").asString()).isEqualTo("DISMISS");

‎app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventServiceTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -437,8 +437,8 @@ void theOwnerIsOfferedTheFixAndNotTheReviewersView() {
437437
assertThat(offeredFor(mine))
438438
.containsExactly(
439439
FailureActionId.DECRYPT_AND_RETRY,
440-
FailureActionId.RETRY,
441440
FailureActionId.VIEW_FILE,
441+
FailureActionId.RETRY,
442442
FailureActionId.DISMISS);
443443
assertThat(service.availableActions(mine))
444444
.allMatch(FileRunEventService.AvailableAction::enabled);
@@ -468,9 +468,9 @@ void aReviewerInheritsTheOwnerActionsOnAnUnattendedRow() {
468468
assertThat(offeredFor(unattended))
469469
.containsExactly(
470470
FailureActionId.DECRYPT_AND_RETRY,
471-
FailureActionId.RETRY,
472471
FailureActionId.VIEW_FILE,
473472
FailureActionId.VIEW_IN_PROCESSOR,
473+
FailureActionId.RETRY,
474474
FailureActionId.DISMISS);
475475
}
476476

‎app/proprietary/src/test/java/stirling/software/proprietary/failure/NotificationProjectionTest.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import static org.assertj.core.api.Assertions.assertThat;
44
import static org.mockito.Mockito.lenient;
5+
import static org.mockito.Mockito.when;
56

67
import java.util.List;
78

@@ -171,4 +172,23 @@ void carriesWhatAClientNeedsToRenderAnActionItDoesNotKnow() {
171172
});
172173
}
173174
}
175+
176+
@Nested
177+
@DisplayName("the response says whether the caller reviews the team")
178+
class ReviewerFlag {
179+
180+
@Test
181+
void trueForAReviewerSoTheClientFiltersNothing() {
182+
when(authority.canEditPolicies()).thenReturn(true);
183+
184+
assertThat(controller.list(null).viewerReviewsTeam()).isTrue();
185+
}
186+
187+
@Test
188+
void falseForAMemberSoTheClientHidesRowsForFilesItDoesNotHold() {
189+
when(authority.canEditPolicies()).thenReturn(false);
190+
191+
assertThat(controller.list(null).viewerReviewsTeam()).isFalse();
192+
}
193+
}
174194
}

‎frontend/editor/public/locales/en-US/translation.toml‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5083,6 +5083,7 @@ unread = "Unread"
50835083

50845084
[notifications.action]
50855085
failed = "That did not work. Try again in a moment."
5086+
more = "More options"
50865087
unavailable = "Not available for this notification."
50875088

50885089
[notifications.detail]
@@ -5091,11 +5092,6 @@ copy = "Copy error"
50915092
less = "Show less"
50925093
more = "Show full message"
50935094

5094-
[notifications.password]
5095-
cancel = "Cancel"
5096-
label = "Document password"
5097-
working = "Unlocking..."
5098-
50995095
[notifications.section]
51005096
earlier = "Earlier"
51015097
new = "New"

0 commit comments

Comments
 (0)