Retry and decrypt-and-retry for recorded failures (Review Flow PR 5)

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

@@ -0,0 +1,21 @@
+package stirling.software.proprietary.failure;
+
+/**
+ * Where a kind would like one of its actions to sit: the thing that fixes it, a supporting action,
+ * or one folded away in a menu.
+ *
+ * <p>Intent, not layout. The client does the final promotion, because only it knows whether the
+ * document is still in its own file store, and a resolution it cannot run is worth less than a
+ * secondary action it can. Declaration order in {@link FailureKind} breaks a tie within a slot.
+ */
+public enum FailureActionSlot {
+
+    /** The action that resolves the failure. At most one per kind is worth declaring here. */
+    RESOLUTION,
+
+    /** Offered alongside the resolution, for a caller the resolution is not aimed at. */
+    SECONDARY,
+
+    /** Available but folded away: correct, rarely the next thing anyone wants to press. */
+    OVERFLOW
+}

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

@@ -5,6 +5,8 @@
 import static stirling.software.proprietary.failure.FailureActionId.RETRY;
 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;
@@ -31,9 +33,9 @@
  * ships as a registry entry plus copy. Two members today: {@link #UNKNOWN} gives every failed run a
  * record, and kinds get promoted out of it as production shows what occurs.
  *
- * <p>Each offer also says who it is for, because the same incident is read by the person who hit it
- * and by whoever reviews after them: only the owner holds the document, only a reviewer wants the
- * run.
+ * <p>Each offer also says who it is for and where the kind wants it, because the same incident is
+ * read by the person who hit it and by whoever reviews after them: only the owner can supply a
+ * password, only a reviewer wants the run.
  */
 @Getter
 public enum FailureKind {
@@ -46,11 +48,11 @@ public enum FailureKind {
             fallback("This document is password-protected, so the pipeline could not read it."),
             // The password is the fix and only the owner has it, so everyone else is
             // offered the run and a way to close the row.
-            offer(DECRYPT_AND_RETRY, OWNER),
-            offer(RETRY, OWNER),
-            offer(VIEW_FILE, OWNER),
-            offer(VIEW_IN_PROCESSOR, TEAM_REVIEWER),
-            offer(DISMISS, ANYONE_WHO_SEES)),
+            resolution(DECRYPT_AND_RETRY, OWNER),
+            global(RETRY, OWNER, OVERFLOW),
+            global(VIEW_FILE, OWNER, OVERFLOW),
+            global(VIEW_IN_PROCESSOR, TEAM_REVIEWER, SECONDARY),
+            global(DISMISS, ANYONE_WHO_SEES, OVERFLOW)),
 
     UNKNOWN(
             FailureStage.INTERNAL,
@@ -59,12 +61,12 @@ public enum FailureKind {
             FailureScope.RUN,
             noErrorCodes(),
             fallback("This run failed for a reason Stirling does not yet recognise."),
-            // Nothing here is known to be fixable, but a plain retry is still worth offering: an
-            // unrecognised failure is often a one-off.
-            offer(RETRY, OWNER),
-            offer(VIEW_IN_PROCESSOR, TEAM_REVIEWER),
-            offer(VIEW_FILE, OWNER),
-            offer(DISMISS, ANYONE_WHO_SEES));
+            // Nothing here is known to be fixable, so there is no resolution to declare. A plain
+            // retry is still worth offering: an unrecognised failure is often a one-off.
+            global(RETRY, OWNER, SECONDARY),
+            global(VIEW_IN_PROCESSOR, TEAM_REVIEWER, SECONDARY),
+            global(VIEW_FILE, OWNER, 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.";
@@ -112,30 +114,52 @@ public enum FailureKind {
     }
 
     /**
-     * One action this kind offers: who it is for, and the key to label it by. One ordered list
-     * rather than ids plus parallel maps of audiences and label overrides, which could disagree
-     * with each other.
+     * One action this kind offers: who it is for, where it wants to sit, and the key to label it
+     * by. One ordered list rather than ids plus parallel maps of audiences, slots and label
+     * overrides, 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) {}
+    private record Offer(
+            FailureActionId id,
+            FailureAudience audience,
+            FailureActionSlot slot,
+            String labelKeySuffix) {}
 
     /**
-     * An action this kind offers, for whoever can actually take it, labelled by the shared wording.
-     * Declaration order is display order.
+     * The action that fixes this kind, for whoever can actually apply it. In the resolution slot by
+     * definition: a kind needing two of these would be two kinds.
      */
-    private static Offer offer(FailureActionId id, FailureAudience audience) {
-        return new Offer(id, audience, null);
+    private static Offer resolution(FailureActionId id, FailureAudience audience) {
+        return new Offer(id, audience, FailureActionSlot.RESOLUTION, null);
+    }
+
+    /** 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, FailureActionSlot.RESOLUTION, labelKeySuffix);
     }
 
     /**
-     * As {@link #offer(FailureActionId, FailureAudience)}, but labelled by this kind's own wording
-     * where the shared one reads badly.
+     * An action that is not this kind's fix: the same offer any kind can make, placed where this
+     * kind wants it and labelled by the shared wording.
      */
-    private static Offer offer(
-            FailureActionId id, FailureAudience audience, String labelKeySuffix) {
-        return new Offer(id, audience, labelKeySuffix);
+    private static Offer global(
+            FailureActionId id, FailureAudience audience, FailureActionSlot slot) {
+        return new Offer(id, audience, slot, null);
+    }
+
+    /**
+     * As {@link #global(FailureActionId, FailureAudience, FailureActionSlot)}, but labelled by 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);
     }
 
     /**
@@ -175,20 +199,27 @@ public List<FailureActionId> getActions() {
     }
 
     /**
-     * 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 its label and placement resolved. What
+     * a review surface reads, so it never has to ask three separate questions about one offer.
      */
     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) {

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

@@ -252,7 +252,8 @@ private static AvailableAction availability(
             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);
     }
 
     /**
@@ -363,7 +364,14 @@ private boolean enforced() {
         return applicationProperties.getSecurity().isEnableLogin();
     }
 
-    /** One action as offered to one caller about one event, with its availability resolved. */
+    /**
+     * One action as offered to one caller about one event, with its availability resolved. {@code
+     * slot} is the kind's placement intent, carried through for the client to make the final call.
+     */
     public record AvailableAction(
-            FailureActionId id, String labelKey, boolean enabled, String disabledReasonKey) {}
+            FailureActionId id,
+            String labelKey,
+            FailureActionSlot slot,
+            boolean enabled,
+            String disabledReasonKey) {}
 }

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

@@ -63,13 +63,15 @@ public static FileRunEventView of(
     /**
      * One button, as offered to this caller about this row. {@code defaultLabel} and {@code
      * execution} are here for the reason {@code defaultTitle} is on the row: a client can then
-     * render, and route, an action it was never built with. Declaration order is display order.
+     * render, and route, 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) {
 
@@ -80,6 +82,7 @@ public static ActionView of(FileRunEventService.AvailableAction action) {
                     action.labelKey(),
                     action.id().getDefaultLabel(),
                     action.id().getExecution(),
+                    action.slot(),
                     action.enabled(),
                     action.disabledReasonKey());
         }

app/proprietary/src/test/java/stirling/software/proprietary/failure/CheckConstrainedEnumsTest.java

@@ -47,6 +47,7 @@ void nothingAddedToTheModelReachedTheTable() throws Exception {
         assertThat(persisted)
                 .doesNotContain(
                         FailureAudience.class,
+                        FailureActionSlot.class,
                         FailureActionId.class,
                         FailureActionId.Execution.class,
                         Ownership.class);

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

@@ -1,6 +1,9 @@
 package stirling.software.proprietary.failure;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static stirling.software.proprietary.failure.FailureActionSlot.OVERFLOW;
+import static stirling.software.proprietary.failure.FailureActionSlot.RESOLUTION;
+import static stirling.software.proprietary.failure.FailureActionSlot.SECONDARY;
 import static stirling.software.proprietary.failure.FailureAudience.ANYONE_WHO_SEES;
 import static stirling.software.proprietary.failure.FailureAudience.OWNER;
 import static stirling.software.proprietary.failure.FailureAudience.TEAM_REVIEWER;
@@ -37,9 +40,12 @@ class FailureKindTest {
      * declaration that pairs 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
@@ -103,13 +109,14 @@ void everyDeclaredActionResolvesToALabelKey(FailureKind kind) {
 
         @ParameterizedTest
         @EnumSource(FailureKind.class)
-        void everyOfferSaysWhoItIsFor(FailureKind kind) {
-            // Read per row to decide what a caller is shown, so a missing one would be a button
-            // offered to whoever the null case happened to let through.
+        void everyOfferSaysWhoItIsForAndWhereItGoes(FailureKind kind) {
+            // Both are read per row to decide what a caller is shown, so a missing one would be a
+            // button placed by whatever the null case happened to do.
             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();
             }
         }
 
@@ -121,6 +128,17 @@ void offersEachActionAtMostOnce(FailureKind kind) {
             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
@@ -220,13 +238,14 @@ void offersARetryToItsOwnerAndTheRunToWhoeverReviews() {
             // worth offering the person who hit it: an unrecognised failure is often a one-off.
             assertThat(FailureKind.UNKNOWN.getOfferedActions())
                     .containsExactly(
-                            offered(FailureActionId.RETRY, OWNER, "retry"),
+                            offered(FailureActionId.RETRY, OWNER, SECONDARY, "retry"),
                             offered(
                                     FailureActionId.VIEW_IN_PROCESSOR,
                                     TEAM_REVIEWER,
+                                    SECONDARY,
                                     "viewInProcessor"),
-                            offered(FailureActionId.VIEW_FILE, OWNER, "viewFile"),
-                            offered(FailureActionId.DISMISS, ANYONE_WHO_SEES, "dismiss"));
+                            offered(FailureActionId.VIEW_FILE, OWNER, OVERFLOW, "viewFile"),
+                            offered(FailureActionId.DISMISS, ANYONE_WHO_SEES, OVERFLOW, "dismiss"));
         }
 
         @Test
@@ -284,14 +303,19 @@ void aKindWithSomethingToFixOffersTheFixToItsOwnerAndTheRunToItsReviewer() {
             // so a reviewer is offered the run and a way to close the row instead.
             assertThat(FailureKind.INPUT_PASSWORD_PROTECTED.getOfferedActions())
                     .containsExactly(
-                            offered(FailureActionId.DECRYPT_AND_RETRY, OWNER, "decryptAndRetry"),
-                            offered(FailureActionId.RETRY, OWNER, "retry"),
-                            offered(FailureActionId.VIEW_FILE, OWNER, "viewFile"),
+                            offered(
+                                    FailureActionId.DECRYPT_AND_RETRY,
+                                    OWNER,
+                                    RESOLUTION,
+                                    "decryptAndRetry"),
+                            offered(FailureActionId.RETRY, OWNER, OVERFLOW, "retry"),
+                            offered(FailureActionId.VIEW_FILE, OWNER, OVERFLOW, "viewFile"),
                             offered(
                                     FailureActionId.VIEW_IN_PROCESSOR,
                                     TEAM_REVIEWER,
+                                    SECONDARY,
                                     "viewInProcessor"),
-                            offered(FailureActionId.DISMISS, ANYONE_WHO_SEES, "dismiss"));
+                            offered(FailureActionId.DISMISS, ANYONE_WHO_SEES, OVERFLOW, "dismiss"));
         }
 
         @Test

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

@@ -153,13 +153,15 @@ void carriesEnoughForAClientToRenderAndRouteAnActionItDoesNotKnow() {
                             action -> {
                                 assertThat(action.defaultLabel()).isNotBlank();
                                 assertThat(action.execution()).isNotNull();
+                                assertThat(action.slot()).isNotNull();
                             })
                     .filteredOn(action -> "VIEW_IN_PROCESSOR".equals(action.id()))
                     .singleElement()
                     .satisfies(
                             action -> {
                                 assertThat(action.execution())
                                         .isEqualTo(FailureActionId.Execution.CLIENT);
+                                assertThat(action.slot()).isEqualTo(FailureActionSlot.SECONDARY);
                                 assertThat(action.defaultLabel()).isEqualTo("View in processor");
                             });
         }

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

@@ -130,6 +130,7 @@ void serialisesAnEventWithItsFacetsCopyKeysAndResolvedActions() throws Exception
             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("SECONDARY");
             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");

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

@@ -585,6 +585,17 @@ void closedRowOffersThemDisabledWithAReasonRatherThanHidingThem() {
                                             .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_AND_RETRY)
+                    .singleElement()
+                    .extracting(FileRunEventService.AvailableAction::slot)
+                    .isEqualTo(FailureActionSlot.RESOLUTION);
+        }
+
         @Test
         void carriesTheLabelKeyForEachOffer() {
             FileRunEvent event = given(FailureKind.UNKNOWN, TEAM, "f1");

app/proprietary/src/test/java/stirling/software/proprietary/failure/NotificationActionDispatchTest.java

@@ -311,6 +311,7 @@ void carriesWhatAClientNeedsToRenderAnActionItDoesNotKnow() {
                                 assertThat(action.labelKey()).startsWith("portal.failures.action.");
                                 assertThat(action.defaultLabel()).isNotBlank();
                                 assertThat(action.execution()).isNotNull();
+                                assertThat(action.slot()).isNotNull();
                             });
         }
     }