Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Secrets list as dropdown from existing secrets and frontend warnings for deleted secrets #37

Merged
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
40321e7
feat: add new SaveWorkflowProvider and use it in save-workflow-button
leon-schmid Oct 23, 2024
32e7fc5
refactor: populate SecretsStore in workflow-editor and wrap content i…
leon-schmid Oct 23, 2024
e65deca
feat: add dropdown to select stored secrets or redirect to set one to…
leon-schmid Oct 23, 2024
371a375
feat: show warning in frontend for particular action node if a previo…
leon-schmid Oct 28, 2024
5f87f24
fix: linting errors
leon-schmid Oct 28, 2024
8955122
refactor: change store interface for missing secrets for action node
leon-schmid Oct 30, 2024
13deda5
feat: improve UI design for missing secrets alert for action node
leon-schmid Oct 30, 2024
95bd9ed
refactor: change store interface for deleted secrets
leon-schmid Oct 30, 2024
e16ef49
fix: don't cache workflow
leon-schmid Oct 30, 2024
25ed337
refactor: change notification banner for missing secrets for action
leon-schmid Oct 30, 2024
3b631ff
fix: remove unused import
leon-schmid Oct 30, 2024
da9bb9c
fix: use updated provider
leon-schmid Nov 4, 2024
faadc26
refactor: store interface for deleted secrets
leon-schmid Nov 4, 2024
8cd605f
refactor: magic number for disabling cache
leon-schmid Nov 4, 2024
f9cd92b
feat: return boolean to see if saving workflow was a success
leon-schmid Nov 4, 2024
4b59595
chore: implement requested changes
leon-schmid Nov 4, 2024
32e10d4
refactor: use Flex componen properties instead of tailwindcss
leon-schmid Nov 4, 2024
013e8ae
fix: throw error when workflow name is invalid
leon-schmid Nov 4, 2024
ad89e44
chore: implement requested changes
leon-schmid Nov 4, 2024
df638cd
chore: implement requested changes
leon-schmid Nov 4, 2024
4f62392
fix: workflow exist at given point in function
leon-schmid Nov 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions web/src/components/secrets/secrets.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,20 @@ export default function Secrets() {
isPending: isListingSecretsLoading,
error: listingSecretsError,
} = useListSecretsApi();
const { setSecrets, getNumberOfSecrets, addNewSecret, isNewSecret, clear } =
useSecretsStore();
const {
setSecrets,
getNumberOfSecrets,
addNewSecret,
isNewSecret,
clearSecretsStore,
} = useSecretsStore();

useEffect(() => {
if (encryptedSecrets) {
setSecrets(encryptedSecrets);
return () => clear();
return () => clearSecretsStore();
}
}, [encryptedSecrets, setSecrets, clear]);
}, [encryptedSecrets, setSecrets, clearSecretsStore]);

if (isListingSecretsLoading) {
return null;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
import { Code, Flex, Text, TextField } from "@radix-ui/themes";
import { Code, Flex, Text, TextField, Select } from "@radix-ui/themes";
import ActionIcon from "../action-icon";
import WorkflowEditorRightPanelBase from "../right-panel-base";
import { useWorkflowStore } from "@/stores/workflow-store";
import { useSecretsStore } from "@/stores/secrets-store";
import { useEditorActionStore } from "@/stores/editor-action-store";
import { TEditorWorkflowActionNode } from "@/types/react-flow";
import { produce } from "immer";
import { ChangeEvent, useEffect } from "react";
import { useImmer } from "use-immer";
import { TActionMetadata } from "@/types/editor-actions";
import CodeEditorWithDialog from "@/components/code-editor-with-dialog/code-editor-with-dialog";
import useSaveWorkflow from "@/providers/save-workflow";
import { useRouter } from "next/navigation";

function buildInitialArgs(
action: TEditorWorkflowActionNode,
Expand All @@ -24,8 +27,17 @@ function buildInitialArgs(
}

export default function ActionEditPanel() {
const { nodes, selectedNodeIdx, updateNodeData } = useWorkflowStore();
const router = useRouter();
const {
nodes,
selectedNodeIdx,
updateNodeData,
hasDeletedSecret,
removeDeletedSecretByPlaceholder,
} = useWorkflowStore();
const { actionsIndex } = useEditorActionStore();
const { secrets } = useSecretsStore();
const { saveWorkflow } = useSaveWorkflow();

const [args, updateArgs] = useImmer<string[]>([]);

Expand Down Expand Up @@ -55,16 +67,30 @@ export default function ActionEditPanel() {
);
};

const saveWorkflowAndRedirect = async () => {
const saveSuccessful = await saveWorkflow();
if (saveSuccessful) {
router.push("/settings");
}
};

const onChangeSecretsMapping = (
secretPlaceholder: string,
event: ChangeEvent<HTMLInputElement>,
value: string,
) => {
if (value === "$$$save_and_redirect$$$") {
saveWorkflowAndRedirect();
return;
}

updateNodeData(
selectedNodeIdx,
produce(actionData, (draft) => {
draft.secretsMapping[secretPlaceholder] = event.target.value;
draft.secretsMapping[secretPlaceholder] = value;
}),
);

removeDeletedSecretByPlaceholder(action.id, secretPlaceholder);
};

const onChangeActionArgument = (
Expand Down Expand Up @@ -134,20 +160,62 @@ export default function ActionEditPanel() {
<Flex>
<Code>{secretPlaceholder}</Code>
</Flex>
<TextField.Root
variant="surface"
<Select.Root
value={
actionData.secretsMapping[
secretPlaceholder
]
}
onChange={(event) =>
onValueChange={(value) =>
onChangeSecretsMapping(
secretPlaceholder,
event,
value,
)
}
/>
>
<Select.Trigger />
<Select.Content>
<Select.Group>
<Select.Label>
Select a secret
</Select.Label>
{secrets.map((_, idx) => (
<Select.Item
key={
secrets[idx]
.secretId
}
value={
secrets[idx]
.secretId
}
>
{secrets[idx].secretId}
</Select.Item>
))}
<Select.Separator />
<Select.Item value="$$$save_and_redirect$$$">
Save Workflow and Redirect
to Secrets
</Select.Item>
</Select.Group>
</Select.Content>
</Select.Root>
{hasDeletedSecret(
action.id,
secretPlaceholder,
) && (
<Flex
mt="2"
direction="row"
align="center"
>
<Text color="red" weight="medium">
The previously selected secret
could not be found.
</Text>
</Flex>
)}
</Flex>
),
)}
Expand Down
50 changes: 49 additions & 1 deletion web/src/components/workflow-editor/workflow-editor.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
"use client";

import { useGetWorkflowApi } from "@/hooks/use-get-workflow-api";
import { useListSecretsApi } from "@/hooks/use-list-credentials-api";
import { useListEditorActionsApi } from "@/hooks/use-list-editor-actions-api";
import { useEditorActionStore } from "@/stores/editor-action-store";
import { useWorkflowStore } from "@/stores/workflow-store";
import { useSecretsStore } from "@/stores/secrets-store";
import { Box, Flex, Grid, Tabs, Text } from "@radix-ui/themes";
import Link from "next/link";
import { useEffect, useState } from "react";
Expand Down Expand Up @@ -36,9 +38,11 @@ export default function WorkflowEditor({

const [view, setView] = useState<View>("workflowBuilder");

const { isNew, setWorkflow, clearWorkflowStore } = useWorkflowStore();
const { isNew, setWorkflow, clearWorkflowStore, setDeletedSecrets } =
useWorkflowStore();
const { setEditorActions } = useEditorActionStore();
const { errorToast } = useToast();
const { setSecrets, clearSecretsStore } = useSecretsStore();

// Loading Actions
const {
Expand All @@ -56,6 +60,15 @@ export default function WorkflowEditor({
}
}, [editorActions, setEditorActions, editActionsError]);

const { data: encryptedSecrets } = useListSecretsApi();

useEffect(() => {
if (encryptedSecrets) {
setSecrets(encryptedSecrets);
return () => clearSecretsStore();
}
}, [encryptedSecrets, setSecrets, clearSecretsStore]);

// Load Workflow
// Note: we only load from the DB if the workflow is not newly created
const {
Expand Down Expand Up @@ -98,6 +111,41 @@ export default function WorkflowEditor({
router,
]);

useEffect(() => {
if (!workflow || !encryptedSecrets) {
return;
}
const secretIds = new Set(encryptedSecrets.map((s) => s.secretId));
const deletedSecrets = new Map(
workflow?.nodes
.filter((node) => node.type === "action")
.map((node) => {
const secretsMapping = node.secretsMapping;
const deletedSecrets = new Set(
Object.keys(secretsMapping).filter(
(secretPlaceholder) =>
!secretIds.has(
secretsMapping[secretPlaceholder],
),
),
);
return [node.id, deletedSecrets] as [string, Set<string>];
})
.filter(
(nodeIdAndDeletedSecrets) =>
nodeIdAndDeletedSecrets[1].size > 0,
),
);
if (deletedSecrets) {
setDeletedSecrets(deletedSecrets);
}
if (deletedSecrets && deletedSecrets.size > 0) {
errorToast(
"Some secrets are missing. Please add them before running the workflow.",
);
}
}, [encryptedSecrets, workflow, setDeletedSecrets]);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve deleted secrets detection implementation.

The logic for detecting deleted secrets can be simplified and improved:

  1. Remove the redundant deletedSecrets check at line 139.
  2. Combine the conditions for setting deleted secrets and showing the toast.
  3. Enhance the error message to be more specific.
  4. Use proper typing to avoid type assertion.

Here's the suggested implementation:

 useEffect(() => {
   if (!workflow || !encryptedSecrets) {
     return;
   }
   const secretIds = new Set(encryptedSecrets.map((s) => s.secretId));
   const deletedSecrets = new Map(
     workflow?.nodes
       .filter((node) => node.type === "action")
       .map((node) => {
         const secretsMapping = node.secretsMapping;
         const deletedSecrets = new Set(
           Object.keys(secretsMapping).filter(
             (secretPlaceholder) =>
               !secretIds.has(
                 secretsMapping[secretPlaceholder],
               ),
           ),
         );
-        return [node.id, deletedSecrets] as [string, Set<string>];
+        return [node.id, deletedSecrets];
       })
       .filter(
         (nodeIdAndDeletedSecrets) =>
           nodeIdAndDeletedSecrets[1].size > 0,
       ),
   );
-  if (deletedSecrets) {
-    setDeletedSecrets(deletedSecrets);
-  }
-  if (deletedSecrets && deletedSecrets.size > 0) {
+  setDeletedSecrets(deletedSecrets);
+  if (deletedSecrets.size > 0) {
+    const nodeCount = deletedSecrets.size;
     errorToast(
-      "Some secrets are missing. Please add them before running the workflow.",
+      `Missing secrets detected in ${nodeCount} ${
+        nodeCount === 1 ? 'node' : 'nodes'
+      }. Please add them before running the workflow.`,
     );
   }
 }, [encryptedSecrets, workflow, setDeletedSecrets]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
useEffect(() => {
if (!workflow || !encryptedSecrets) {
return;
}
const secretIds = new Set(encryptedSecrets.map((s) => s.secretId));
const deletedSecrets = new Map(
workflow?.nodes
.filter((node) => node.type === "action")
.map((node) => {
const secretsMapping = node.secretsMapping;
const deletedSecrets = new Set(
Object.keys(secretsMapping).filter(
(secretPlaceholder) =>
!secretIds.has(
secretsMapping[secretPlaceholder],
),
),
);
return [node.id, deletedSecrets] as [string, Set<string>];
})
.filter(
(nodeIdAndDeletedSecrets) =>
nodeIdAndDeletedSecrets[1].size > 0,
),
);
if (deletedSecrets) {
setDeletedSecrets(deletedSecrets);
}
if (deletedSecrets && deletedSecrets.size > 0) {
errorToast(
"Some secrets are missing. Please add them before running the workflow.",
);
}
}, [encryptedSecrets, workflow, setDeletedSecrets]);
useEffect(() => {
if (!workflow || !encryptedSecrets) {
return;
}
const secretIds = new Set(encryptedSecrets.map((s) => s.secretId));
const deletedSecrets = new Map(
workflow?.nodes
.filter((node) => node.type === "action")
.map((node) => {
const secretsMapping = node.secretsMapping;
const deletedSecrets = new Set(
Object.keys(secretsMapping).filter(
(secretPlaceholder) =>
!secretIds.has(
secretsMapping[secretPlaceholder],
),
),
);
return [node.id, deletedSecrets];
})
.filter(
(nodeIdAndDeletedSecrets) =>
nodeIdAndDeletedSecrets[1].size > 0,
),
);
setDeletedSecrets(deletedSecrets);
if (deletedSecrets.size > 0) {
const nodeCount = deletedSecrets.size;
errorToast(
`Missing secrets detected in ${nodeCount} ${
nodeCount === 1 ? 'node' : 'nodes'
}. Please add them before running the workflow.`,
);
}
}, [encryptedSecrets, workflow, setDeletedSecrets]);


// TODO(frontend): nicer loading screen
if (isLoadingEditorActions || (!isNew && isLoadingWorkflow)) {
return <Box>Loading...</Box>;
Expand Down
40 changes: 38 additions & 2 deletions web/src/components/workflow-graph/node-base.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { NodeToolbar, Position } from "reactflow";
import * as Toolbar from "@radix-ui/react-toolbar";
import DuplicateIcon from "../icons/duplicate-icon";
import { useWorkflowStore } from "@/stores/workflow-store";
import { TrashIcon } from "@radix-ui/react-icons";
import { TrashIcon, ExclamationTriangleIcon } from "@radix-ui/react-icons";

export interface NodeBaseProps {
nodeId: string;
Expand All @@ -20,7 +20,8 @@ export default function NodeBase({
name,
selected,
}: NodeBaseProps) {
const { nodes, duplicateNodeByIdx, deleteNodeByIdx } = useWorkflowStore();
const { nodes, duplicateNodeByIdx, deleteNodeByIdx, hasDeletedSecret } =
useWorkflowStore();

const nodeIdx = nodes.findIndex((node) => node.id === nodeId);
if (nodeIdx === -1) {
Expand Down Expand Up @@ -62,6 +63,41 @@ export default function NodeBase({
</Flex>
</Card>

<NodeToolbar
isVisible={hasDeletedSecret(nodeId)}
position={Position.Top}
offset={8}
align="end"
>
<Tooltip content="Missing Secret">
<Flex
style={{
transform: "translate(10px, 24px)",
}}
>
<Flex
inset="0"
position="absolute"
className="bg-[var(--red-9)] rounded-full"
/>
<Flex
position="relative"
className="rounded-full"
width="24px"
height="24px"
justify="center"
align="center"
>
<ExclamationTriangleIcon
color="white"
width="14px"
height="14px"
/>
</Flex>
</Flex>
</Tooltip>
</NodeToolbar>

<NodeToolbar
isVisible={selected && nodeId !== "start"}
position={Position.Right}
Expand Down
3 changes: 3 additions & 0 deletions web/src/hooks/use-get-workflow-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import { withSnakeCaseTransform } from "@/types/utils";
import { EditorWorkflowGraph } from "@/types/react-flow";
import { HTTPMethod } from "@/types/api";

const DISABLE_CACHE = 0;

// GET /editor/workflow
const GetWorkflowRequest = withSnakeCaseTransform(
z.object({
Expand All @@ -29,5 +31,6 @@ export const useGetWorkflowApi = (workflowId: string, doApiCall: boolean) => {
queryKey: ["workflow", workflowId],
queryFn: () => getWorkflow({ workflowId }),
enabled: doApiCall,
gcTime: DISABLE_CACHE,
});
};
14 changes: 7 additions & 7 deletions web/src/providers/save-workflow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ class WorkflowValidationError extends Error {
}

interface SaveWorkflowContextType {
saveWorkflow: () => Promise<void>;
saveWorkflow: () => Promise<boolean>;
isPending: boolean;
}

const SaveWorkflowContext = createContext<SaveWorkflowContextType>({
saveWorkflow: async () => {},
saveWorkflow: async () => false,
isPending: false,
});

Expand All @@ -41,9 +41,6 @@ export function SaveWorkflowProvider({
const [isPending, setIsPending] = useState(false);

const handleSaveWorkflow = async () => {
if (isPending) {
return;
}
setIsPending(true);

try {
Expand All @@ -59,8 +56,9 @@ export function SaveWorkflowProvider({
);
}
if (!isValidWorkflowName(workflow.workflowName)) {
errorToast(WORKFLOW_NAME_VALIDATION_ERROR_MESSAGE);
return;
throw new WorkflowValidationError(
WORKFLOW_NAME_VALIDATION_ERROR_MESSAGE,
);
}

// Make sure that we only save args which are present in the current
Expand Down Expand Up @@ -102,6 +100,7 @@ export function SaveWorkflowProvider({
if (webhookId && webhookSecret) {
updateWebhookIdAndSecret(webhookId, webhookSecret);
}
return true;
} catch (error) {
if (
error instanceof AxiosError &&
Expand All @@ -115,6 +114,7 @@ export function SaveWorkflowProvider({
} else {
errorToast("Failed to save workflow. Please try again.");
}
return false;
} finally {
setIsPending(false);
}
Expand Down
4 changes: 2 additions & 2 deletions web/src/stores/secrets-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { produce } from "immer";

type SecretsStore = {
secrets: (TSecret | TSecretMetadata)[];
clear: () => void;
clearSecretsStore: () => void;
setSecrets: (secrets: (TSecret | TSecretMetadata)[]) => void;
getSecret: (idx: number) => TSecret | TSecretMetadata;
getNumberOfSecrets: () => number;
Expand All @@ -16,7 +16,7 @@ type SecretsStore = {

export const useSecretsStore = create<SecretsStore>((set, get) => ({
secrets: [],
clear: () =>
clearSecretsStore: () =>
set({
secrets: [],
}),
Expand Down
Loading
Loading