From ceeec53df4b72bd13576cd5146c07adf1e5ba96b Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Date: Tue, 1 Sep 2026 13:12:06 +0000 Subject: [PATCH] Let a pipeline run on the editor, on upload or export (#7581) Redesigns the policies system so that the backend has an understanding of policies running over the Editor. The Editor is not set up as a source for the backend because the backend can't actively get files from it, they come in via the frontend sending them to the backend, so instead pipelines have a specific editor key in them to encode whether the pipeline is triggered on file upload/export in the editor. Also make a big effort in the frontend code towards genericising policy running. Previously, there was specific support in the main policy executor for each policy that it had to run, which was not going to be appropriate long-term, especially when users can run any pipeline in the editor. There's more work needed here for me to really be happy with it but this PR is plenty large on its own and moves it in the right direction. All of the above was required to allow arbitrary user pipelines to run in the editor. This PR makes it so that the user can select Editor as a source in the pipeline creator, along with whether it should run on upload or export. image --------- Co-authored-by: James Brunton --- .../policy/controller/PolicyController.java | 12 +- .../policy/model/EditorConfig.java | 34 ++ .../proprietary/policy/model/Policy.java | 36 ++- .../overview/PolicyOverviewService.java | 6 +- .../DefaultClassificationPolicySeeder.java | 9 +- .../policy/source/SourceOverviewService.java | 10 +- .../policy/store/InProcessPolicyStore.java | 3 +- .../policy/store/JpaPolicyStore.java | 67 +++- .../overview/PolicyOverviewServiceTest.java | 39 +++ ...DefaultClassificationPolicySeederTest.java | 20 +- .../source/SourceOverviewServiceTest.java | 10 +- .../policy/store/JpaPolicyStoreTest.java | 124 ++++++++ .../public/locales/en-US/translation.toml | 9 + .../fileManager/CompactFileDetails.tsx | 3 +- .../components/filesPage/VersionTimeline.tsx | 10 +- .../src/core/components/shared/ToolChain.tsx | 35 +-- .../core/contexts/file/FileReducer.test.ts | 35 ++- .../src/core/contexts/file/FileReducer.ts | 20 +- .../src/core/services/fileStubHelpers.ts | 4 +- .../classification-heuristic-upload.spec.ts | 14 +- .../stubbed/editor-pipeline-auto-run.spec.ts | 85 +++++ frontend/editor/src/core/types/file.ts | 3 + .../src/core/utils/toolOperationLabel.test.ts | 29 ++ .../src/core/utils/toolOperationLabel.ts | 17 + frontend/editor/src/portal/api/pipelines.ts | 2 + frontend/editor/src/portal/api/policies.ts | 7 + .../pipelines/PipelineInputTrigger.tsx | 140 +++++++++ .../pipelines/graph/PipelineGraph.tsx | 6 +- .../components/policies/PolicyDetailPanel.tsx | 16 +- .../components/policies/PolicySetupWizard.tsx | 22 +- .../src/portal/views/PipelineBuilder.css | 12 + .../src/portal/views/PipelineBuilder.test.tsx | 39 +++ .../src/portal/views/PipelineBuilder.tsx | 213 +++++++------ .../policies/PolicyAutoRunController.tsx | 7 +- .../policies/classificationLocalPass.ts | 169 ++++++++++ .../components/policies/policyLocalPass.ts | 34 ++ .../policies/policyRunStore.test.ts | 23 -- .../components/policies/policyRunStore.ts | 22 +- .../policies/useClientSideClassification.ts | 251 --------------- .../policies/usePolicyAutoRun.batch.test.tsx | 129 ++++++-- .../policies/usePolicyAutoRun.chain.test.tsx | 297 ++++++------------ .../usePolicyAutoRun.escalation.test.tsx | 155 --------- .../policies/usePolicyAutoRun.import.test.tsx | 1 + .../policies/usePolicyAutoRun.race.test.tsx | 28 +- .../usePolicyAutoRun.reentry.test.tsx | 1 + .../policies/usePolicyAutoRun.retry.test.tsx | 1 + .../components/policies/usePolicyAutoRun.ts | 158 ++-------- ...test.tsx => usePolicyLocalPasses.test.tsx} | 150 +++++++-- .../policies/usePolicyLocalPasses.ts | 144 +++++++++ .../data/classificationPolicy.test.ts | 96 +++--- .../proprietary/data/classificationPolicy.ts | 60 ++-- .../src/proprietary/hooks/usePolicies.test.ts | 75 ++++- .../src/proprietary/hooks/usePolicies.ts | 22 +- .../src/proprietary/policies/codec.test.ts | 63 +++- .../editor/src/proprietary/policies/codec.ts | 12 +- .../editor/src/proprietary/policies/types.ts | 17 + .../services/policyBackend.test.ts | 135 ++++++++ .../src/proprietary/services/policyBackend.ts | 12 +- .../proprietary/services/policyDispatch.ts | 97 ++++++ .../proprietary/services/policyExport.test.ts | 129 ++++++++ .../src/proprietary/services/policyExport.ts | 38 ++- .../services/policyPipeline.test.ts | 6 +- .../proprietary/services/policyPipeline.ts | 22 +- .../services/policyStorage.test.ts | 37 +++ .../src/proprietary/services/policyStorage.ts | 35 ++- .../editor/src/proprietary/types/policies.ts | 6 + 66 files changed, 2405 insertions(+), 1118 deletions(-) create mode 100644 app/proprietary/src/main/java/stirling/software/proprietary/policy/model/EditorConfig.java create mode 100644 frontend/editor/src/core/tests/stubbed/editor-pipeline-auto-run.spec.ts create mode 100644 frontend/editor/src/core/utils/toolOperationLabel.test.ts create mode 100644 frontend/editor/src/core/utils/toolOperationLabel.ts create mode 100644 frontend/editor/src/portal/components/pipelines/PipelineInputTrigger.tsx create mode 100644 frontend/editor/src/proprietary/components/policies/classificationLocalPass.ts create mode 100644 frontend/editor/src/proprietary/components/policies/policyLocalPass.ts delete mode 100644 frontend/editor/src/proprietary/components/policies/useClientSideClassification.ts delete mode 100644 frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.escalation.test.tsx rename frontend/editor/src/proprietary/components/policies/{useClientSideClassification.test.tsx => usePolicyLocalPasses.test.tsx} (62%) create mode 100644 frontend/editor/src/proprietary/components/policies/usePolicyLocalPasses.ts create mode 100644 frontend/editor/src/proprietary/services/policyBackend.test.ts create mode 100644 frontend/editor/src/proprietary/services/policyDispatch.ts create mode 100644 frontend/editor/src/proprietary/services/policyExport.test.ts diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/policy/controller/PolicyController.java b/app/proprietary/src/main/java/stirling/software/proprietary/policy/controller/PolicyController.java index 5a5a3018b1..3dac4e284e 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/policy/controller/PolicyController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/policy/controller/PolicyController.java @@ -336,6 +336,15 @@ public class PolicyController { * nothing to check. */ private void requireAccessibleOutput(Policy policy) { + // An editor policy hands its results back to the workspace the file came from. A stored + // destination would send the run to a folder or bucket instead, leaving the editor's copy + // untouched - and the editor's import would then have nothing to collect. + if (policy.editor().allowed() && !policy.outputIds().isEmpty()) { + throw new ResponseStatusException( + HttpStatus.BAD_REQUEST, + "An editor policy delivers back to the editor and can't also have a" + + " destination"); + } for (String outputId : policy.outputIds()) { Source destination = sourceStore @@ -393,7 +402,8 @@ public class PolicyController { policy.steps(), policy.output(), policy.outputIds(), - teamId); + teamId, + policy.editor()); } /** Output secrets never leave the server: reads return the redaction sentinel instead. */ diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/policy/model/EditorConfig.java b/app/proprietary/src/main/java/stirling/software/proprietary/policy/model/EditorConfig.java new file mode 100644 index 0000000000..9b15adea2d --- /dev/null +++ b/app/proprietary/src/main/java/stirling/software/proprietary/policy/model/EditorConfig.java @@ -0,0 +1,34 @@ +package stirling.software.proprietary.policy.model; + +/** + * How a policy participates in the editor: it fires in the browser as each file passes through, + * rather than being swept from a stored {@code Source} on a trigger. + * + *

An object rather than a bare flag so the moment it fires ({@code runOn}) travels with the + * decision, and so later editor-only settings have somewhere to live. + * + * @param allowed whether the editor may run this policy at all + * @param runOn which moment it fires on: {@code "upload"} or {@code "export"} + */ +public record EditorConfig(boolean allowed, String runOn) { + + public static final String UPLOAD = "upload"; + public static final String EXPORT = "export"; + + public EditorConfig { + runOn = EXPORT.equals(runOn) ? EXPORT : UPLOAD; + } + + /** Not an editor policy: swept server-side, or run only on demand. */ + public static EditorConfig disabled() { + return new EditorConfig(false, UPLOAD); + } + + public static EditorConfig onUpload() { + return new EditorConfig(true, UPLOAD); + } + + public static EditorConfig onExport() { + return new EditorConfig(true, EXPORT); + } +} diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/policy/model/Policy.java b/app/proprietary/src/main/java/stirling/software/proprietary/policy/model/Policy.java index 14b1eb325c..63b26f380c 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/policy/model/Policy.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/policy/model/Policy.java @@ -1,6 +1,7 @@ package stirling.software.proprietary.policy.model; import java.util.List; +import java.util.Optional; /** * A stored automation: ordered tool steps, input bindings, and output destinations. @@ -24,13 +25,29 @@ public record Policy( List steps, OutputSpec output, List outputIds, - Long teamId) { + Long teamId, + EditorConfig editor) { public Policy { inputs = inputs == null ? List.of() : List.copyOf(inputs); steps = steps == null ? List.of() : steps; output = output == null ? OutputSpec.inline() : output; outputIds = outputIds == null ? List.of() : List.copyOf(outputIds); + editor = editor == null ? EditorConfig.disabled() : editor; + } + + /** Without editor participation: a swept or on-demand policy. */ + public Policy( + String id, + String name, + String owner, + boolean enabled, + List inputs, + List steps, + OutputSpec output, + List outputIds, + Long teamId) { + this(id, name, owner, enabled, inputs, steps, output, outputIds, teamId, null); } /** @@ -70,6 +87,14 @@ public record Policy( return inputs.stream().map(PipelineInput::sourceId).toList(); } + /** + * The moment this policy fires in the editor ("upload" / "export"), or empty when the editor + * does not run it. Legacy blobs are lifted onto {@link EditorConfig} when they are read. + */ + public Optional editorRunOn() { + return editor.allowed() ? Optional.of(editor.runOn()) : Optional.empty(); + } + /** The distinct trigger types configured across this policy's inputs (manual inputs aside). */ public List triggerTypes() { return inputs.stream() @@ -82,17 +107,20 @@ public record Policy( /** A copy with the inline output replaced (e.g. resolved for the engine, or migrated). */ public Policy withOutput(OutputSpec resolved) { - return new Policy(id, name, owner, enabled, inputs, steps, resolved, outputIds, teamId); + return new Policy( + id, name, owner, enabled, inputs, steps, resolved, outputIds, teamId, editor); } /** A copy under a different owner (e.g. moving a seed off a placeholder name). */ public Policy withOwner(String newOwner) { - return new Policy(id, name, newOwner, enabled, inputs, steps, output, outputIds, teamId); + return new Policy( + id, name, newOwner, enabled, inputs, steps, output, outputIds, teamId, editor); } /** A copy referencing the given saved output destinations. */ public Policy withOutputIds(List newOutputIds) { - return new Policy(id, name, owner, enabled, inputs, steps, output, newOutputIds, teamId); + return new Policy( + id, name, owner, enabled, inputs, steps, output, newOutputIds, teamId, editor); } /** diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/policy/overview/PolicyOverviewService.java b/app/proprietary/src/main/java/stirling/software/proprietary/policy/overview/PolicyOverviewService.java index b2be7b668e..0d7845a209 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/policy/overview/PolicyOverviewService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/policy/overview/PolicyOverviewService.java @@ -114,10 +114,14 @@ public class PolicyOverviewService { /** * Summarise a policy's triggers for the overview row: "manual" when no input is triggered, * otherwise the distinct trigger types across its inputs (e.g. "folder-watch, schedule"). + * + *

An editor policy has no wire input to trigger, but it is not manual either - it fires in + * the editor on every upload or export, so it reports that rather than reading as on-demand. */ private static String triggerSummary(Policy policy) { List types = policy.triggerTypes(); - return types.isEmpty() ? "manual" : String.join(", ", types); + if (!types.isEmpty()) return String.join(", ", types); + return policy.editorRunOn().map(runOn -> "editor-" + runOn).orElse("manual"); } private static String outputSummary(OutputSpec output) { diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/policy/seed/DefaultClassificationPolicySeeder.java b/app/proprietary/src/main/java/stirling/software/proprietary/policy/seed/DefaultClassificationPolicySeeder.java index 9b347366bc..7d35198633 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/policy/seed/DefaultClassificationPolicySeeder.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/policy/seed/DefaultClassificationPolicySeeder.java @@ -14,6 +14,7 @@ import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import stirling.software.proprietary.model.TeamCreatedEvent; +import stirling.software.proprietary.policy.model.EditorConfig; import stirling.software.proprietary.policy.model.OutputSpec; import stirling.software.proprietary.policy.model.PipelineStep; import stirling.software.proprietary.policy.model.Policy; @@ -98,9 +99,8 @@ public class DefaultClassificationPolicySeeder { static Policy defaultPolicy(Long teamId) { Map options = new HashMap<>(); options.put("categoryId", CATEGORY); - options.put("runOn", "upload"); options.put("mode", "new_version"); - options.put("sources", List.of("editor")); + options.put("sources", List.of()); options.put("scopeTypes", List.of()); options.put("reviewerEmail", ""); return new Policy( @@ -113,6 +113,9 @@ public class DefaultClassificationPolicySeeder { List.of(), List.of(new PipelineStep(CLASSIFY_ENDPOINT, Map.of())), new OutputSpec("inline", options), - teamId); + List.of(), + teamId, + // Classification runs in the editor on every upload. + EditorConfig.onUpload()); } } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/policy/source/SourceOverviewService.java b/app/proprietary/src/main/java/stirling/software/proprietary/policy/source/SourceOverviewService.java index 0f9df21440..10792591d9 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/policy/source/SourceOverviewService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/policy/source/SourceOverviewService.java @@ -107,14 +107,12 @@ public class SourceOverviewService { } /** - * Whether a policy runs from the editor. Editor membership is carried in the policy's output - * metadata ({@code output.options.sources}) - a client-side list the editor writes when a - * policy targets it - rather than as a persisted {@code sourceId}, because the editor is - * virtual and has no stored source to reference. + * Whether a policy runs from the editor. Read from the policy's first-class {@link + * stirling.software.proprietary.policy.model.EditorConfig}, never inferred from a sources list + * (the editor is not a real source). */ private static boolean runsFromEditor(Policy policy) { - Object sources = policy.output().options().get("sources"); - return sources instanceof List list && list.contains(EditorSource.ID); + return policy.editor().allowed(); } /** diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/policy/store/InProcessPolicyStore.java b/app/proprietary/src/main/java/stirling/software/proprietary/policy/store/InProcessPolicyStore.java index 70d67bba0f..08bc253863 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/policy/store/InProcessPolicyStore.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/policy/store/InProcessPolicyStore.java @@ -38,7 +38,8 @@ public class InProcessPolicyStore implements PolicyStore { policy.steps(), policy.output(), policy.outputIds(), - policy.teamId()); + policy.teamId(), + policy.editor()); policies.put(id, stored); // Existing policy keeps its position; a new one appends to the end of its team's queue. sortOrders.computeIfAbsent(id, key -> nextSortOrder(stored.teamId())); diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/policy/store/JpaPolicyStore.java b/app/proprietary/src/main/java/stirling/software/proprietary/policy/store/JpaPolicyStore.java index 6edaa76c78..f335a4d754 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/policy/store/JpaPolicyStore.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/policy/store/JpaPolicyStore.java @@ -3,6 +3,7 @@ package stirling.software.proprietary.policy.store; import java.util.List; import java.util.Objects; import java.util.Optional; +import java.util.Set; import java.util.UUID; import org.springframework.stereotype.Service; @@ -11,8 +12,10 @@ import org.springframework.transaction.annotation.Transactional; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import stirling.software.proprietary.policy.model.EditorConfig; import stirling.software.proprietary.policy.model.Policy; import stirling.software.proprietary.policy.model.PolicyBinding; +import stirling.software.proprietary.policy.source.EditorSource; import tools.jackson.databind.JsonNode; import tools.jackson.databind.ObjectMapper; @@ -48,7 +51,8 @@ public class JpaPolicyStore implements PolicyStore { policy.steps(), policy.output(), policy.outputIds(), - policy.teamId()); + policy.teamId(), + policy.editor()); PolicyEntity entity = new PolicyEntity(); entity.setId(id); @@ -148,7 +152,9 @@ public class JpaPolicyStore implements PolicyStore { // One unreadable row must never abort a bulk read or crash startup. private Optional toPolicy(PolicyEntity entity) { try { - JsonNode node = upgradeLegacyShape(objectMapper.readTree(entity.getPolicyJson())); + JsonNode node = + liftEditorConfig( + upgradeLegacyShape(objectMapper.readTree(entity.getPolicyJson()))); return Optional.of(objectMapper.treeToValue(node, Policy.class)); } catch (Exception e) { log.error( @@ -191,4 +197,61 @@ public class JpaPolicyStore implements PolicyStore { obj.remove("sourceIds"); return obj; } + + /** Categories whose editor moment defaulted to export before it was stored (see runOn.ts). */ + private static final Set EXPORT_BY_DEFAULT = Set.of("security"); + + /** + * Derive {@code editor} for a blob written before editor participation had its own field, from + * its {@code output.options}: allowed when {@code sources} lists {@code "editor"}, or - for a + * catalogue policy - when there is no {@code sources} list at all (an unnarrowed catalogue + * policy runs in the editor). + * + *

Runs on every read, deliberately outside {@link #upgradeLegacyShape}'s early return: a + * blob written after triggers moved onto {@code inputs} but before this field existed still + * needs lifting, and that early return would skip exactly those rows. + */ + private JsonNode liftEditorConfig(JsonNode root) { + if (!(root instanceof ObjectNode obj) || obj.hasNonNull("editor")) { + return root; + } + JsonNode options = obj.path("output").path("options"); + String categoryId = text(options, "categoryId"); + JsonNode sources = options.get("sources"); + boolean listed = sources != null && sources.isArray() && !sources.isEmpty(); + boolean allowed; + if (listed) { + // An explicit scope list decides: only the editor's own id puts it on the editor. + allowed = false; + for (JsonNode source : sources) { + if (source.isValueNode() && EditorSource.ID.equals(source.asString())) { + allowed = true; + break; + } + } + } else { + // No list: a catalogue policy ran in the editor by default, but a builder pipeline + // (no category) could not reach the editor at all, so silence is not consent there. + allowed = !categoryId.isBlank(); + } + ObjectNode editor = objectMapper.createObjectNode(); + editor.put("allowed", allowed); + editor.put("runOn", legacyRunOn(options, categoryId)); + obj.set("editor", editor); + return obj; + } + + /** The stored moment, or the category default the client applied when none was stored. */ + private static String legacyRunOn(JsonNode options, String categoryId) { + String stored = text(options, "runOn"); + if (EditorConfig.EXPORT.equals(stored) || EditorConfig.UPLOAD.equals(stored)) { + return stored; + } + return EXPORT_BY_DEFAULT.contains(categoryId) ? EditorConfig.EXPORT : EditorConfig.UPLOAD; + } + + private static String text(JsonNode parent, String field) { + JsonNode node = parent.path(field); + return node.isValueNode() ? node.asString() : ""; + } } diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/policy/overview/PolicyOverviewServiceTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/policy/overview/PolicyOverviewServiceTest.java index 373b596136..4d0830f9c5 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/policy/overview/PolicyOverviewServiceTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/policy/overview/PolicyOverviewServiceTest.java @@ -15,6 +15,7 @@ import stirling.software.common.model.ApplicationProperties; import stirling.software.common.service.UserServiceInterface; import stirling.software.proprietary.policy.config.PolicyAccessGuard; import stirling.software.proprietary.policy.config.PolicyManagementAuthority; +import stirling.software.proprietary.policy.model.EditorConfig; import stirling.software.proprietary.policy.model.OutputSpec; import stirling.software.proprietary.policy.model.PipelineInput; import stirling.software.proprietary.policy.model.PipelineStep; @@ -223,6 +224,44 @@ class PolicyOverviewServiceTest { teamId)); } + @Test + void editorPolicyReportsItsRunMomentRatherThanReadingAsManual() { + policyStore.save( + new Policy( + null, + "Editor flatten", + "owner", + true, + List.of(), + List.of(new PipelineStep("/api/v1/misc/flatten", Map.of())), + OutputSpec.inline(), + List.of(), + 1L, + EditorConfig.onUpload())); + + PolicyView view = find(service.overview(), "Editor flatten"); + + assertEquals("editor-upload", view.trigger()); + } + + @Test + void sweptPolicyWithNoTriggeredInputIsStillManual() { + policyStore.save( + new Policy( + null, + "Swept compress", + "owner", + true, + List.of(), + List.of(new PipelineStep("/api/v1/misc/compress-pdf", Map.of())), + OutputSpec.inline(), + 1L)); + + PolicyView view = find(service.overview(), "Swept compress"); + + assertEquals("manual", view.trigger()); + } + private static PolicyView find(PoliciesOverviewResponse response, String name) { return response.pipelines().stream() .filter(view -> view.name().equals(name)) diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/policy/seed/DefaultClassificationPolicySeederTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/policy/seed/DefaultClassificationPolicySeederTest.java index f6e82bd011..fb4fa6d419 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/policy/seed/DefaultClassificationPolicySeederTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/policy/seed/DefaultClassificationPolicySeederTest.java @@ -64,14 +64,30 @@ class DefaultClassificationPolicySeederTest { assertThat(policy.teamId()).isEqualTo(7L); assertThat(policy.output().type()).isEqualTo("inline"); assertThat(policy.output().options().get("categoryId")).isEqualTo("classification"); - assertThat(policy.output().options().get("runOn")).isEqualTo("upload"); assertThat(policy.output().options().get("mode")).isEqualTo("new_version"); - assertThat(policy.output().options().get("sources")).isEqualTo(List.of("editor")); + // Editor participation is the policy's own flag, not a marker in the output options. + assertThat(policy.editor().allowed()).isTrue(); + assertThat(policy.editor().runOn()).isEqualTo("upload"); assertThat(policy.steps()).hasSize(1); assertThat(policy.steps().get(0).operation()) .isEqualTo("/api/v1/ai/tools/classify-and-label"); } + @Test + void marksEditorParticipationOnEditorConfigAndSeedsNoSources() { + when(policyStore.findByTeam(7L)).thenReturn(List.of()); + + seeder().onTeamCreated(new TeamCreatedEvent(7L, "Acme")); + + ArgumentCaptor saved = ArgumentCaptor.forClass(Policy.class); + verify(policyStore).save(saved.capture()); + Policy policy = saved.getValue(); + // Editor participation is on EditorConfig, not the sources list; the seed carries no + // sources. + assertThat(policy.editor().allowed()).isTrue(); + assertThat(policy.output().options().get("sources")).isEqualTo(List.of()); + } + @Test void doesNotSeedWhenAClassificationPolicyAlreadyExists() { when(policyStore.findByTeam(7L)).thenReturn(List.of(classificationPolicy(7L))); diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/policy/source/SourceOverviewServiceTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/policy/source/SourceOverviewServiceTest.java index a8c295acc8..66d75f8be0 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/policy/source/SourceOverviewServiceTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/policy/source/SourceOverviewServiceTest.java @@ -15,6 +15,7 @@ import stirling.software.common.model.ApplicationProperties; import stirling.software.common.service.UserServiceInterface; import stirling.software.proprietary.policy.config.PolicyAccessGuard; import stirling.software.proprietary.policy.config.PolicyManagementAuthority; +import stirling.software.proprietary.policy.model.EditorConfig; import stirling.software.proprietary.policy.model.OutputSpec; import stirling.software.proprietary.policy.model.PipelineInput; import stirling.software.proprietary.policy.model.PipelineStep; @@ -222,9 +223,7 @@ class SourceOverviewServiceTest { OutputSpec.inline())); } - /** - * A policy that targets the editor: membership rides in its output metadata, not a sourceId. - */ + /** A policy that targets the editor: membership on its {@link EditorConfig}, not a sourceId. */ private void editorPolicy(String name) { policyStore.save( new Policy( @@ -234,7 +233,10 @@ class SourceOverviewServiceTest { true, List.of(), List.of(new PipelineStep("/api/v1/misc/compress-pdf", Map.of())), - new OutputSpec("inline", Map.of("sources", List.of("editor"))))); + OutputSpec.inline(), + List.of(), + null, + EditorConfig.onUpload())); } private void teamPolicy(String name, Long teamId, String... sourceIds) { diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/policy/store/JpaPolicyStoreTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/policy/store/JpaPolicyStoreTest.java index 2a1d2b4f11..ae95b3a3ce 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/policy/store/JpaPolicyStoreTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/policy/store/JpaPolicyStoreTest.java @@ -18,6 +18,7 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import stirling.software.proprietary.policy.model.EditorConfig; import stirling.software.proprietary.policy.model.OutputSpec; import stirling.software.proprietary.policy.model.PipelineInput; import stirling.software.proprietary.policy.model.PipelineStep; @@ -113,6 +114,129 @@ class JpaPolicyStoreTest { upgraded.inputs()); } + /** + * The regression this guards: before the editor lift, a blob written by the pre-{@code editor} + * seeder deserialized straight onto {@link EditorConfig#disabled()}, silently taking every + * upgraded install's Classification policy off the editor. + * + *

The {@code inputs} variant is the important one - {@link + * JpaPolicyStore#upgradeLegacyShape} returns early on it, so a lift living inside that method + * would miss exactly the rows written between the trigger migration and this field. + */ + @Test + void getLiftsALegacyEditorSourceOntoEditorConfigWhenInputsArePresent() { + Policy lifted = readLegacy(legacyJson("\"inputs\":[],", "\"sources\":[\"editor\"],")); + + assertEquals(EditorConfig.onUpload(), lifted.editor()); + assertEquals(Optional.of("upload"), lifted.editorRunOn()); + } + + @Test + void getLiftsALegacyEditorSourceOnThePreInputsShapeToo() { + // Oldest shape: policy-level trigger + sourceIds, so both migrations have to compose. + Policy lifted = + readLegacy( + legacyJson( + "\"trigger\":{\"type\":\"schedule\",\"options\":{}}," + + "\"sourceIds\":[\"s1\"],", + "\"sources\":[\"editor\"],")); + + assertEquals(EditorConfig.onUpload(), lifted.editor()); + assertEquals( + List.of(new PipelineInput("s1", new TriggerConfig("schedule", Map.of()))), + lifted.inputs()); + } + + @Test + void getTreatsAnUnnarrowedCataloguePolicyAsEditorRun() { + // Empty and absent both meant "nobody narrowed it", which the editor read as its own. + assertTrue(readLegacy(legacyJson("\"inputs\":[],", "\"sources\":[],")).editor().allowed()); + assertTrue(readLegacy(legacyJson("\"inputs\":[],", "")).editor().allowed()); + } + + @Test + void getLeavesACataloguePolicyScopedElsewhereOffTheEditor() { + Policy lifted = readLegacy(legacyJson("\"inputs\":[],", "\"sources\":[\"sharepoint\"],")); + + assertFalse(lifted.editor().allowed()); + assertEquals(Optional.empty(), lifted.editorRunOn()); + } + + @Test + void getLeavesASourcelessBuilderPipelineOffTheEditor() { + // No categoryId: a pipeline built on the Pipelines page, which never reached the editor. + String json = + "{\"id\":\"p1\",\"name\":\"legacy\",\"enabled\":true,\"inputs\":[]," + + "\"steps\":[],\"output\":{\"type\":\"inline\",\"options\":{}}}"; + + assertFalse(readLegacy(json).editor().allowed()); + } + + @Test + void getKeepsTheCategoryDefaultMomentWhenNoRunOnWasStored() { + // Security enforced on export before runOn was persisted (frontend runOn.ts + // DEFAULT_RUN_ON). + String json = + "{\"id\":\"p1\",\"name\":\"legacy\",\"enabled\":true,\"inputs\":[]," + + "\"steps\":[],\"output\":{\"type\":\"inline\",\"options\":{" + + "\"categoryId\":\"security\",\"sources\":[\"editor\"]}}}"; + + assertEquals(EditorConfig.onExport(), readLegacy(json).editor()); + } + + @Test + void getNeverOverridesAnExplicitlyStoredEditorBlock() { + // A deliberate opt-out survives, so the lift stays safe to leave in permanently. + String json = + "{\"id\":\"p1\",\"name\":\"legacy\",\"enabled\":true,\"inputs\":[]," + + "\"steps\":[],\"editor\":{\"allowed\":false,\"runOn\":\"upload\"}," + + "\"output\":{\"type\":\"inline\",\"options\":{" + + "\"categoryId\":\"classification\",\"sources\":[\"editor\"]}}}"; + + assertFalse(readLegacy(json).editor().allowed()); + } + + /** + * Pins the wire shape the stubbed Playwright spec hardcodes: the derived block is additive, so + * a real response carries it alongside the untouched legacy options bag. + */ + @Test + void getLeavesTheLegacyOptionsBagIntactSoTheResponseCarriesBoth() { + Policy lifted = readLegacy(legacyJson("\"inputs\":[],", "\"sources\":[\"editor\"],")); + + assertEquals(List.of("editor"), lifted.output().options().get("sources")); + String wire = objectMapper.writeValueAsString(lifted); + assertTrue( + wire.contains("\"editor\":{\"allowed\":true,\"runOn\":\"upload\"}"), + "expected the derived editor block on the wire, got: " + wire); + } + + /** + * The blob main's DefaultClassificationPolicySeeder wrote, with the shape bits parameterised. + */ + private static String legacyJson(String shapeFields, String sourcesField) { + return "{\"id\":\"p1\",\"name\":\"Classification Policy\",\"owner\":\"system\"," + + "\"enabled\":true," + + shapeFields + + "\"steps\":[{\"operation\":\"/api/v1/ai/tools/classify-and-label\"," + + "\"parameters\":{}}]," + + "\"output\":{\"type\":\"inline\",\"options\":{" + + "\"categoryId\":\"classification\",\"runOn\":\"upload\"," + + "\"mode\":\"new_version\"," + + sourcesField + + "\"scopeTypes\":[],\"reviewerEmail\":\"\"}},\"teamId\":1}"; + } + + private Policy readLegacy(String policyJson) { + PolicyEntity entity = new PolicyEntity(); + entity.setId("p1"); + entity.setName("legacy"); + entity.setEnabled(true); + entity.setPolicyJson(policyJson); + when(repository.findById("p1")).thenReturn(Optional.of(entity)); + return store.get("p1").orElseThrow(); + } + @Test void saveDenormalizesTeamIdForScopedQueries() { store.save( diff --git a/frontend/editor/public/locales/en-US/translation.toml b/frontend/editor/public/locales/en-US/translation.toml index df996b23c0..345cf07ab2 100644 --- a/frontend/editor/public/locales/en-US/translation.toml +++ b/frontend/editor/public/locales/en-US/translation.toml @@ -8220,6 +8220,9 @@ chooseDestination = "Choose a destination" chooseOperation = "Choose what this step does" chooseSource = "Choose a source" discard = "Discard changes" +editorDestination = "Editor" +editorDestinationDetail = "Replaces the file you ran it on" +editorDestinationHelp = "This pipeline runs on the files in your workspace, and its results replace the file it ran on. There is nowhere else to send them." inputs = "Input" inputSource = "Input source" inputTrigger = "Trigger" @@ -8231,6 +8234,10 @@ needsSource = "No source chosen" noToolMatches = "No tools match your search." pause = "Pause" rename = "Rename pipeline" +runOn = "Runs on" +runOnExport = "Every export" +runOnTooltip = "Choose when this pipeline runs on your files: when you add them, or when you export them." +runOnUpload = "Every upload" searchTools = "Search tools" sendToSystem = "Send to another system" stepsIncompatible = "These steps can't run on what their prior step produces: {{tools}}." @@ -8371,6 +8378,8 @@ steps = "Steps" trigger = "Trigger" [portal.pipelines.trigger] +editor-export = "Every export" +editor-upload = "Every upload" folder-watch = "Folder watch" manual = "Manual" schedule = "Scheduled" diff --git a/frontend/editor/src/core/components/fileManager/CompactFileDetails.tsx b/frontend/editor/src/core/components/fileManager/CompactFileDetails.tsx index 988ecf789c..27f55ce060 100644 --- a/frontend/editor/src/core/components/fileManager/CompactFileDetails.tsx +++ b/frontend/editor/src/core/components/fileManager/CompactFileDetails.tsx @@ -7,6 +7,7 @@ import ChevronLeftIcon from "@mui/icons-material/ChevronLeft"; import ChevronRightIcon from "@mui/icons-material/ChevronRight"; import { useTranslation } from "react-i18next"; import { getFileSize } from "@app/utils/fileUtils"; +import { toolOperationLabel } from "@app/utils/toolOperationLabel"; import { StirlingFileStub } from "@app/types/fileContext"; import { PrivateContent } from "@app/components/shared/PrivateContent"; @@ -115,7 +116,7 @@ const CompactFileDetails: React.FC = ({ {currentFile?.toolHistory && currentFile.toolHistory.length > 0 && ( {currentFile.toolHistory - .map((tool) => t(`home.${tool.toolId}.title`, tool.toolId)) + .map((tool) => toolOperationLabel(tool, t)) .join(" → ")} )} diff --git a/frontend/editor/src/core/components/filesPage/VersionTimeline.tsx b/frontend/editor/src/core/components/filesPage/VersionTimeline.tsx index f484f936ef..77d8808c17 100644 --- a/frontend/editor/src/core/components/filesPage/VersionTimeline.tsx +++ b/frontend/editor/src/core/components/filesPage/VersionTimeline.tsx @@ -10,7 +10,7 @@ import HistoryIcon from "@mui/icons-material/History"; import MoreVertIcon from "@mui/icons-material/MoreVert"; import { FileId, ToolOperation } from "@app/types/file"; -import { ToolId } from "@app/types/toolId"; +import { toolOperationLabel } from "@app/utils/toolOperationLabel"; import { StirlingFileStub } from "@app/types/fileContext"; import { formatFileSize, getFileDate } from "@app/utils/fileUtils"; import { downloadFileFromStorage } from "@app/utils/downloadUtils"; @@ -64,10 +64,10 @@ function deltaToolFor( return curr[priorLen] ?? null; } -/** Translated tool name via `home.{toolId}.title`. */ -function ToolLabel({ toolId }: { toolId: ToolId }) { +/** The operation's own label when it has one, else its translated tool name. */ +function ToolLabel({ operation }: { operation: ToolOperation }) { const { t } = useTranslation(); - return {t(`home.${toolId}.title`, toolId)}; + return {toolOperationLabel(operation, t)}; } export interface VersionTimelineProps { @@ -242,7 +242,7 @@ export function VersionTimeline({ style={{ color: "var(--c-text)" }} > {delta ? ( - + ) : ( t("filesPage.versionOrigin", "Original upload") )} diff --git a/frontend/editor/src/core/components/shared/ToolChain.tsx b/frontend/editor/src/core/components/shared/ToolChain.tsx index 249e7802cc..7974614759 100644 --- a/frontend/editor/src/core/components/shared/ToolChain.tsx +++ b/frontend/editor/src/core/components/shared/ToolChain.tsx @@ -6,8 +6,8 @@ import React from "react"; import { Text, Tooltip, Badge, Group } from "@mantine/core"; import { ToolOperation } from "@app/types/file"; +import { toolOperationLabel } from "@app/utils/toolOperationLabel"; import { useTranslation } from "react-i18next"; -import { ToolId } from "@app/types/toolId"; interface ToolChainProps { toolChain: ToolOperation[]; @@ -29,11 +29,7 @@ const ToolChain: React.FC = ({ const { t } = useTranslation(); if (!toolChain || toolChain.length === 0) return null; - const toolIds = toolChain.map((tool) => tool.toolId); - - const getToolName = (toolId: ToolId) => { - return t(`home.${toolId}.title`, toolId); - }; + const getToolName = (tool: ToolOperation) => toolOperationLabel(tool, t); // Create full tool chain for tooltip const fullChainDisplay = @@ -42,7 +38,7 @@ const ToolChain: React.FC = ({ {toolChain.map((tool, index) => ( - {getToolName(tool.toolId)} + {getToolName(tool)} {index < toolChain.length - 1 && ( @@ -53,18 +49,21 @@ const ToolChain: React.FC = ({ ))} ) : ( - {toolIds.map(getToolName).join(" → ")} + {toolChain.map(getToolName).join(" → ")} ); // Create truncated display based on available space const getTruncatedDisplay = () => { - if (toolIds.length <= 2) { + if (toolChain.length <= 2) { // Show all tools if 2 or fewer - return { text: toolIds.map(getToolName).join(" → "), isTruncated: false }; + return { + text: toolChain.map(getToolName).join(" → "), + isTruncated: false, + }; } else { // Show first tool ... last tool for longer chains return { - text: `${getToolName(toolIds[0])} → +${toolIds.length - 2} → ${getToolName(toolIds[toolIds.length - 1])}`, + text: `${getToolName(toolChain[0])} → +${toolChain.length - 2} → ${getToolName(toolChain[toolChain.length - 1])}`, isTruncated: true, }; } @@ -75,10 +74,10 @@ const ToolChain: React.FC = ({ // Compact style for very small spaces if (displayStyle === "compact") { const compactText = - toolIds.length === 1 - ? getToolName(toolIds[0]) - : `${toolIds.length} tools`; - const isCompactTruncated = toolIds.length > 1; + toolChain.length === 1 + ? getToolName(toolChain[0]) + : `${toolChain.length} tools`; + const isCompactTruncated = toolChain.length > 1; const compactElement = ( = ({ {toolChain.slice(0, 3).map((tool, index) => ( - {getToolName(tool.toolId)} + {getToolName(tool)} {index < Math.min(toolChain.length - 1, 2) && ( @@ -131,7 +130,7 @@ const ToolChain: React.FC = ({ ... - {getToolName(toolChain[toolChain.length - 1].toolId)} + {getToolName(toolChain[toolChain.length - 1])} )} @@ -140,7 +139,7 @@ const ToolChain: React.FC = ({ ); return isBadgesTruncated ? ( - + {badgesElement} ) : ( diff --git a/frontend/editor/src/core/contexts/file/FileReducer.test.ts b/frontend/editor/src/core/contexts/file/FileReducer.test.ts index cdba68b013..92f82f3ed0 100644 --- a/frontend/editor/src/core/contexts/file/FileReducer.test.ts +++ b/frontend/editor/src/core/contexts/file/FileReducer.test.ts @@ -192,6 +192,39 @@ describe("fileContextReducer — silent CONSUME_FILES (background enforcement)", ]); }); + it("carries a no-label [] verdict forward (classified, not unclassified)", () => { + // "a" was classified and found nothing ([]) - distinct from null (never classified). The output + // must inherit [] so the local pass treats it as already-classified and never re-classifies (or + // re-bills) it. + const start = stateWith([stub("a", { classificationLabels: [] })]); + const next = fileContextReducer(start, { + type: "CONSUME_FILES", + payload: { + inputFileIds: ["a" as FileId], + outputStirlingFileStubs: [stub("b")], + }, + }); + expect(next.files.byId["b" as FileId].classificationLabels).toEqual([]); + }); + + it("prefers a real label over a merge input's no-label [] verdict", () => { + // Merge of a labelled file and a no-label one: the output should keep the real label. + const start = stateWith([ + stub("a", { classificationLabels: [] }), + stub("b", { classificationLabels: ["Invoice"] }), + ]); + const next = fileContextReducer(start, { + type: "CONSUME_FILES", + payload: { + inputFileIds: ["a" as FileId, "b" as FileId], + outputStirlingFileStubs: [stub("c")], + }, + }); + expect(next.files.byId["c" as FileId].classificationLabels).toEqual([ + "Invoice", + ]); + }); + it("an output's own classificationLabels win over the input's", () => { // A re-classify produces an output that already carries (fresher) labels. const start = stateWith([stub("a", { classificationLabels: ["Invoice"] })]); @@ -211,7 +244,7 @@ describe("fileContextReducer — silent CONSUME_FILES (background enforcement)", it("carries classificationConfidence forward with the labels", () => { // The confidence is part of the verdict: without it the escalation decision - // (shouldDispatchToAi) dies at the version boundary and a chained + // (localVerdictNeedsEscalation) dies at the version boundary and a chained // classification never runs. const start = stateWith([ stub("a", { diff --git a/frontend/editor/src/core/contexts/file/FileReducer.ts b/frontend/editor/src/core/contexts/file/FileReducer.ts index 1ed23b11d6..ba69492b23 100644 --- a/frontend/editor/src/core/contexts/file/FileReducer.ts +++ b/frontend/editor/src/core/contexts/file/FileReducer.ts @@ -389,16 +389,16 @@ export function fileContextReducer( // Carry the document's classification verdict forward across the edit: any // tool that versions/derives a classified file keeps it in its label // groups instead of dropping to "Other" and waiting on a PDF re-read. - // Inherited from the first input that has labels, together with that - // verdict's confidence - the escalation decision (shouldDispatchToAi) is - // about the document, not about which step produced the current bytes, so - // it must survive the version boundary. An output that already carries its - // own verdict (e.g. a fresh classify result) keeps it. - const verdictDonor = inputFileIds - .map((id) => state.files.byId[id]) - .find( + // Inherited together with that verdict's confidence - the escalation + // decision (localVerdictNeedsEscalation) is about the document, not about + // which step produced the current bytes, so it must survive the version + // boundary. An output that already carries its own verdict (e.g. a fresh + // classify result) keeps it. + const inputStubs = inputFileIds.map((id) => state.files.byId[id]); + const verdictDonor = + inputStubs.find( (s) => s?.classificationLabels && s.classificationLabels.length > 0, - ); + ) ?? inputStubs.find((s) => s?.classificationLabels !== undefined); // Mark every consume output as tool-produced (the single chokepoint for // both versioned edits and independent artifacts like convert/split/merge) @@ -409,7 +409,7 @@ export function fileContextReducer( ...stub, derivedFromTool: true, sourceFileIds, - ...(stub.classificationLabels == null && verdictDonor + ...(stub.classificationLabels === undefined && verdictDonor ? { classificationLabels: verdictDonor.classificationLabels, classificationConfidence: diff --git a/frontend/editor/src/core/services/fileStubHelpers.ts b/frontend/editor/src/core/services/fileStubHelpers.ts index 60489c8c94..836d000af7 100644 --- a/frontend/editor/src/core/services/fileStubHelpers.ts +++ b/frontend/editor/src/core/services/fileStubHelpers.ts @@ -14,6 +14,8 @@ export async function createStirlingFilesAndStubs( files: File[], parentStub: StirlingFileStub, toolId: ToolId, + /** Shown instead of the tool's name in version history (a policy passes its pipeline name). */ + label?: string, ): Promise<{ stirlingFiles: StirlingFile[]; stubs: StirlingFileStub[] }> { const stirlingFiles: StirlingFile[] = []; const stubs: StirlingFileStub[] = []; @@ -22,7 +24,7 @@ export async function createStirlingFilesAndStubs( const processedFileMetadata = await generateProcessedFileMetadata(file); const childStub = createChildStub( parentStub, - { toolId, timestamp: Date.now() }, + { toolId, timestamp: Date.now(), ...(label ? { label } : {}) }, file, processedFileMetadata?.thumbnailUrl, processedFileMetadata, diff --git a/frontend/editor/src/core/tests/stubbed/classification-heuristic-upload.spec.ts b/frontend/editor/src/core/tests/stubbed/classification-heuristic-upload.spec.ts index 856ddd58f5..7d42434385 100644 --- a/frontend/editor/src/core/tests/stubbed/classification-heuristic-upload.spec.ts +++ b/frontend/editor/src/core/tests/stubbed/classification-heuristic-upload.spec.ts @@ -12,14 +12,20 @@ const FIXTURES = path.join( "../test-fixtures/classification/unlabelled", ); -/** The stored policy DefaultClassificationPolicySeeder writes for a new team. */ +/** + * What GET /api/v1/policies returns for the row an older + * DefaultClassificationPolicySeeder wrote - i.e. one stored before editor + * participation had its own field. `JpaPolicyStore.liftEditorConfig` derives the + * `editor` block from the legacy `output.options` on read; the lift is additive, + * so a real response carries both. Migration of the stored shape itself is + * covered by JpaPolicyStoreTest, which exercises the Java the stub stands in for. + */ const SEEDED_POLICY = { id: "seeded-classification", name: "Classification Policy", owner: "system", enabled: true, - trigger: null, - sourceIds: [], + inputs: [], steps: [{ operation: "/api/v1/ai/tools/classify-and-label", parameters: {} }], output: { type: "inline", @@ -32,7 +38,9 @@ const SEEDED_POLICY = { reviewerEmail: "", }, }, + outputIds: [], teamId: 1, + editor: { allowed: true, runOn: "upload" }, }; test("a 10-file upload wave classifies every file into its group", async ({ diff --git a/frontend/editor/src/core/tests/stubbed/editor-pipeline-auto-run.spec.ts b/frontend/editor/src/core/tests/stubbed/editor-pipeline-auto-run.spec.ts new file mode 100644 index 0000000000..a71bc98f33 --- /dev/null +++ b/frontend/editor/src/core/tests/stubbed/editor-pipeline-auto-run.spec.ts @@ -0,0 +1,85 @@ +import path from "path"; +import { test, expect } from "@app/tests/helpers/stub-test-base"; +import { uploadFiles } from "@app/tests/helpers/ui-helpers"; + +// A pipeline reaches the editor auto-run through its own editor flag; a swept one must not. + +test.use({ autoGoto: false }); + +const SAMPLE = path.join( + import.meta.dirname, + "../test-fixtures/classification/unlabelled/invoice_acme.pdf", +); + +/** A builder-made pipeline: no categoryId, one harmless step. */ +function builderPipeline(editor: { allowed: boolean; runOn: string }) { + return { + id: "builder-pipeline-1", + name: "Flatten everything", + owner: "system", + enabled: true, + trigger: null, + sourceIds: [], + steps: [{ operation: "/api/v1/misc/flatten", parameters: {} }], + output: { type: "inline", options: { mode: "new_version" } }, + editor, + teamId: 1, + }; +} + +/** Install the policy list + capture every stored-policy run dispatch. */ +async function armed(page: import("@playwright/test").Page, policy: unknown) { + const dispatched: string[] = []; + await page.route("**/api/v1/policies", (route) => + route.fulfill({ json: [policy] }), + ); + await page.route("**/api/v1/policies/*/run", (route) => { + dispatched.push(new URL(route.request().url()).pathname); + return route.fulfill({ json: { jobId: "job-1" } }); + }); + return dispatched; +} + +test("an editor pipeline set to run on upload dispatches when a file is added", async ({ + page, +}) => { + const dispatched = await armed( + page, + builderPipeline({ allowed: true, runOn: "upload" }), + ); + + await page.goto("/editor", { waitUntil: "domcontentloaded" }); + await uploadFiles(page, SAMPLE); + + await expect + .poll(() => dispatched, { timeout: 15_000 }) + .toContain("/api/v1/policies/builder-pipeline-1/run"); +}); + +test("a swept pipeline never runs on editor upload", async ({ page }) => { + const dispatched = await armed( + page, + builderPipeline({ allowed: false, runOn: "upload" }), + ); + + await page.goto("/editor", { waitUntil: "domcontentloaded" }); + await uploadFiles(page, SAMPLE); + + await page.waitForTimeout(5_000); + expect(dispatched).toEqual([]); +}); + +test("an editor pipeline set to run on export does not fire on upload", async ({ + page, +}) => { + const dispatched = await armed( + page, + builderPipeline({ allowed: true, runOn: "export" }), + ); + + await page.goto("/editor", { waitUntil: "domcontentloaded" }); + await uploadFiles(page, SAMPLE); + + await page.waitForTimeout(5_000); + expect(dispatched).toEqual([]); +}); diff --git a/frontend/editor/src/core/types/file.ts b/frontend/editor/src/core/types/file.ts index 98a0094f43..c6ec1898cb 100644 --- a/frontend/editor/src/core/types/file.ts +++ b/frontend/editor/src/core/types/file.ts @@ -16,6 +16,9 @@ export type FileId = string & { readonly [tag]: "FileId" }; export interface ToolOperation { toolId: ToolId; timestamp: number; + /** Overrides the tool's own name in history. Set by a policy run to its pipeline's name, since + * every policy records the same "automate" toolId. */ + label?: string; } /** diff --git a/frontend/editor/src/core/utils/toolOperationLabel.test.ts b/frontend/editor/src/core/utils/toolOperationLabel.test.ts new file mode 100644 index 0000000000..9b21f6e65f --- /dev/null +++ b/frontend/editor/src/core/utils/toolOperationLabel.test.ts @@ -0,0 +1,29 @@ +import { describe, it, expect } from "vitest"; +import type { TFunction } from "i18next"; +import { toolOperationLabel } from "@app/utils/toolOperationLabel"; +import type { ToolOperation } from "@app/types/file"; + +// Stands in for i18next: echoes the key so the assertions show which lookup ran. +const t = ((key: string, fallback?: string) => + key === "home.automate.title" ? "Automate" : (fallback ?? key)) as TFunction; + +const op = (over: Partial): ToolOperation => + ({ toolId: "automate", timestamp: 0, ...over }) as ToolOperation; + +describe("toolOperationLabel", () => { + it("prefers the operation's own label", () => { + expect(toolOperationLabel(op({ label: "add-page-numbers" }), t)).toBe( + "add-page-numbers", + ); + }); + + // Every policy records the same "automate" toolId, so without a label each automated version + // reads identically no matter which pipeline produced it. + it("falls back to the tool's name when unlabelled", () => { + expect(toolOperationLabel(op({}), t)).toBe("Automate"); + }); + + it("keeps the fallback for an empty label rather than rendering a blank", () => { + expect(toolOperationLabel(op({ label: "" }), t)).toBe("Automate"); + }); +}); diff --git a/frontend/editor/src/core/utils/toolOperationLabel.ts b/frontend/editor/src/core/utils/toolOperationLabel.ts new file mode 100644 index 0000000000..1f29d02390 --- /dev/null +++ b/frontend/editor/src/core/utils/toolOperationLabel.ts @@ -0,0 +1,17 @@ +import type { TFunction } from "i18next"; +import type { ToolOperation } from "@app/types/file"; + +/** + * What produced a version, for the history surfaces. A policy run carries its own label (the + * pipeline's name) because every policy records the same "automate" toolId, which would otherwise + * render every automated version identically. + */ +export function toolOperationLabel( + operation: ToolOperation, + t: TFunction, +): string { + // Truthiness, not nullish: a blank label would otherwise render as an empty history entry. + return ( + operation.label || t(`home.${operation.toolId}.title`, operation.toolId) + ); +} diff --git a/frontend/editor/src/portal/api/pipelines.ts b/frontend/editor/src/portal/api/pipelines.ts index e441fcdac8..2be269c1ac 100644 --- a/frontend/editor/src/portal/api/pipelines.ts +++ b/frontend/editor/src/portal/api/pipelines.ts @@ -70,6 +70,8 @@ export interface Policy { * output} is used. */ outputIds: string[]; + /** Whether the editor runs this policy per file, and on which moment. */ + editor?: { allowed: boolean; runOn: "upload" | "export" }; teamId?: number | null; } diff --git a/frontend/editor/src/portal/api/policies.ts b/frontend/editor/src/portal/api/policies.ts index da545b98c4..6963ccc1b1 100644 --- a/frontend/editor/src/portal/api/policies.ts +++ b/frontend/editor/src/portal/api/policies.ts @@ -76,6 +76,8 @@ export interface PolicyState { configured: boolean; status: PolicyStatus; sources: string[]; + /** Whether the editor runs this policy per file; stored, not derived from `sources`. */ + runsOnEditor?: boolean; scopeTypes: string[]; reviewerEmail: string; fieldValues: Record; @@ -92,6 +94,7 @@ export interface PolicyState { export interface PolicySetupResult { fieldValues: Record; sources: string[]; + runsOnEditor: boolean; scopeTypes: string[]; reviewerEmail: string; outputMode: "new_file" | "new_version"; @@ -433,6 +436,7 @@ function decoratePolicy( configured: true, status, sources: decoded.sources, + runsOnEditor: decoded.runsOnEditor, scopeTypes: decoded.scopeTypes, reviewerEmail: decoded.reviewerEmail, fieldValues: decoded.fieldValues, @@ -595,6 +599,7 @@ export function buildWireFromSetup( enabled, categoryId: entry.category.id, sources: result.sources, + runsOnEditor: result.runsOnEditor, scopeTypes: result.scopeTypes, reviewerEmail: result.reviewerEmail, fieldValues: result.fieldValues, @@ -625,6 +630,8 @@ export function buildWireFromState( enabled, categoryId: entry.category.id, sources: s.sources, + // Carry the stored value through: pause/resume must not re-derive it. + runsOnEditor: s.runsOnEditor === true, scopeTypes: s.scopeTypes, reviewerEmail: s.reviewerEmail, fieldValues: s.fieldValues, diff --git a/frontend/editor/src/portal/components/pipelines/PipelineInputTrigger.tsx b/frontend/editor/src/portal/components/pipelines/PipelineInputTrigger.tsx new file mode 100644 index 0000000000..eb5af9cd21 --- /dev/null +++ b/frontend/editor/src/portal/components/pipelines/PipelineInputTrigger.tsx @@ -0,0 +1,140 @@ +// Swept sources are scheduled or triggered server-side; the editor runs client-side as each file +// passes through, so the two get different controls. + +import { useTranslation } from "react-i18next"; +import { Tooltip } from "@mantine/core"; +import InfoOutlinedIcon from "@mui/icons-material/InfoOutlined"; +import { FormField, Input, Select } from "@app/ui"; + +export type ScheduleUnit = "MINUTES" | "HOURS" | "DAYS"; +export type EditorRunOn = "upload" | "export"; + +const SCHEDULE_UNITS: ScheduleUnit[] = ["MINUTES", "HOURS", "DAYS"]; + +/** Empty trigger type = manual-only (no automatic trigger). */ +export const MANUAL = ""; +/** Sentinel for manual: Mantine's Select reads "" as no selection. Maps to {@link MANUAL}. */ +export const MANUAL_OPTION = "manual"; + +/** One input row in the builder: a source paired with its own trigger config. */ +export interface WorkingInput { + sourceId: string; + triggerType: string; + scheduleCount: string; + scheduleUnit: ScheduleUnit; +} + +export interface PipelineInputTriggerProps { + input: WorkingInput; + onInputChange: (patch: Partial) => void; + /** Trigger types offered for this row's source (manual first). */ + triggerOptions: { value: string; label: string }[]; + /** The chosen source is the editor, so the pipeline runs in the browser. */ + isEditorInput: boolean; + runOn: EditorRunOn; + onRunOnChange: (runOn: EditorRunOn) => void; +} + +export function PipelineInputTrigger({ + input, + onInputChange, + triggerOptions, + isEditorInput, + runOn, + onRunOnChange, +}: PipelineInputTriggerProps) { + const { t } = useTranslation(); + + if (isEditorInput) { + const label = t("portal.pipelines.builder.runOn", "Runs on"); + return ( + + + {label} + + + + } + > + + onInputChange({ + triggerType: value && value !== MANUAL_OPTION ? value : MANUAL, + }) + } + options={triggerOptions} + /> + + + {input.triggerType === "schedule" && ( +

+ + {t("portal.pipelines.composer.scheduleEvery")} + + onInputChange({ scheduleCount: e.target.value })} + className="portal-builder__schedule-count" + /> + - updateInput({ - triggerType: - value && value !== MANUAL_OPTION ? value : MANUAL, - }) - } - options={triggerOptionsFor(input.sourceId)} - /> - - - {input.triggerType === "schedule" && ( -
- - {t("portal.pipelines.composer.scheduleEvery")} - - - updateInput({ scheduleCount: e.target.value }) - } - className="portal-builder__schedule-count" - /> -