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 15 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
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
direction="row"
align="center"
className="text-red-500 font-medium mt-2"
>
Copy link
Member

Choose a reason for hiding this comment

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

If the component has props for doing what you want (text color, font weight), then please use this instead of using tailwindcss. You can just do <Text color="red" weight="medium"> for example.

Copy link
Member

Choose a reason for hiding this comment

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

Also, <Flex> has props for margin top: <Flex mt="2">.

<Text>
The previously selected secret
could not be found.
</Text>
</Flex>
)}
</Flex>
),
)}
Expand Down
45 changes: 44 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, addDeletedSecrets } =
useWorkflowStore();
const { setEditorActions } = useEditorActionStore();
const { errorToast } = useToast();
const { setSecrets, clear } = 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 () => clear();
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should rename clear from the secrets store to clearSecretsStore to be consistent with the workflow store naming. Also, makes it more clear here what's happening.

}
}, [encryptedSecrets, setSecrets, clear]);

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

useEffect(() => {
if (!workflow || !encryptedSecrets) {
return;
}
const secretIds = new Set(encryptedSecrets.map((s) => s.secretId));
const missingSecrets = workflow?.nodes
.filter((node) => node.type === "action")
.reduce((acc, node) => {
const secretsMapping = node.secretsMapping;
const secretsForNode = Object.keys(secretsMapping);
Copy link
Member

Choose a reason for hiding this comment

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

secretsForNode --> secretPlaceholders

secretsForNode.forEach((secretPlaceholder) => {
if (!secretIds.has(secretsMapping[secretPlaceholder])) {
if (!acc.has(node.id)) {
acc.set(node.id, new Set());
}
acc.get(node.id)?.add(secretPlaceholder);
}
});
return acc;
}, new Map<string, Set<string>>());
if (missingSecrets) {
addDeletedSecrets(missingSecrets);
}
if (missingSecrets && missingSecrets.size > 0) {
errorToast(
"Some secrets are missing. Please add them before running the workflow.",
);
}
}, [encryptedSecrets, workflow, addDeletedSecrets]);

// TODO(frontend): nicer loading screen
if (isLoadingEditorActions || (!isNew && isLoadingWorkflow)) {
return <Box>Loading...</Box>;
Expand Down
29 changes: 27 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,30 @@ 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 className="absolute inset-0 bg-[var(--red-9)] rounded-full" />
<Flex
className="relative rounded-full w-6 h-6"
justify="center"
align="center"
>
<ExclamationTriangleIcon className="text-white w-3.5 h-3.5" />
</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,
});
};
9 changes: 5 additions & 4 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 @@ -42,7 +42,7 @@ export function SaveWorkflowProvider({

const handleSaveWorkflow = async () => {
if (isPending) {
return;
return false;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix workflow name validation control flow.

The workflow name validation check doesn't return early, allowing invalid workflows to proceed with the save operation. This could lead to inconsistent state.

Apply this fix:

 if (!isValidWorkflowName(workflow.workflowName)) {
   errorToast(WORKFLOW_NAME_VALIDATION_ERROR_MESSAGE);
+  return false;
 }

Also applies to: 59-59, 60-60, 61-61, 62-62

Copy link
Member

Choose a reason for hiding this comment

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

I think it is okay to remove the if-condition. It is probably more dangerous to return false in this case because if we would do error handling based on the result, it would trigger an error although there is none.

}
setIsPending(true);

Expand All @@ -60,7 +60,6 @@ export function SaveWorkflowProvider({
}
if (!isValidWorkflowName(workflow.workflowName)) {
errorToast(WORKFLOW_NAME_VALIDATION_ERROR_MESSAGE);
return;
}

// Make sure that we only save args which are present in the current
Expand Down Expand Up @@ -102,6 +101,7 @@ export function SaveWorkflowProvider({
if (webhookId && webhookSecret) {
updateWebhookIdAndSecret(webhookId, webhookSecret);
}
return true;
} catch (error) {
if (
error instanceof AxiosError &&
Expand All @@ -115,6 +115,7 @@ export function SaveWorkflowProvider({
} else {
errorToast("Failed to save workflow. Please try again.");
}
return false;
} finally {
setIsPending(false);
}
Expand Down
Loading
Loading