Skip to content

Commit

Permalink
Actually since we enable save we need to handle errors for all fields
Browse files Browse the repository at this point in the history
  • Loading branch information
PopDaph committed Jul 17, 2024
1 parent cbb22e2 commit 76365b7
Show file tree
Hide file tree
Showing 8 changed files with 137 additions and 102 deletions.
141 changes: 89 additions & 52 deletions front/components/assistant_builder/ActionsScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,29 +21,29 @@ import React, { useCallback, useEffect, useState } from "react";

import {
ActionDustAppRun,
isActionDustAppRunValid,
isActionDustAppRunValid as hasErrorActionDustAppRun,
} from "@app/components/assistant_builder/actions/DustAppRunAction";
import {
ActionProcess,
isActionProcessValid,
hasErrorActionProcess,
} from "@app/components/assistant_builder/actions/ProcessAction";
import {
ActionRetrievalExhaustive,
ActionRetrievalSearch,
isActionRetrievalExhaustiveValid,
isActionRetrievalSearchValid,
hasErrorActionRetrievalExhaustive,
hasErrorActionRetrievalSearch,
} from "@app/components/assistant_builder/actions/RetrievalAction";
import {
ActionTablesQuery,
isActionTablesQueryValid,
hasErrorActionTablesQuery,
} from "@app/components/assistant_builder/actions/TablesQueryAction";
import {
ActionVisualization,
isActionVisualizationValid,
hasErrorActionVisualization,
} from "@app/components/assistant_builder/actions/VisualizationAction";
import {
ActionWebNavigation,
isActionWebsearchValid as isActionWebNavigationValid,
hasErrorActionWebNavigation,
} from "@app/components/assistant_builder/actions/WebNavigationAction";
import { isLegacyAssistantBuilderConfiguration } from "@app/components/assistant_builder/legacy_agent";
import type {
Expand Down Expand Up @@ -85,24 +85,24 @@ function ActionModeSection({
return show && <div className="flex flex-col gap-6">{children}</div>;
}

export function isActionValid(
export function hasActionError(
action: AssistantBuilderActionConfiguration
): boolean {
): string | null {
switch (action.type) {
case "RETRIEVAL_SEARCH":
return isActionRetrievalSearchValid(action);
return hasErrorActionRetrievalSearch(action);
case "RETRIEVAL_EXHAUSTIVE":
return isActionRetrievalExhaustiveValid(action);
return hasErrorActionRetrievalExhaustive(action);
case "PROCESS":
return isActionProcessValid(action);
return hasErrorActionProcess(action);
case "DUST_APP_RUN":
return isActionDustAppRunValid(action);
return hasErrorActionDustAppRun(action);
case "TABLES_QUERY":
return isActionTablesQueryValid(action);
return hasErrorActionTablesQuery(action);
case "WEB_NAVIGATION":
return isActionWebNavigationValid(action);
return hasErrorActionWebNavigation(action);
case "VISUALIZATION":
return isActionVisualizationValid(action);
return hasErrorActionVisualization(action);
default:
assertNever(action);
}
Expand Down Expand Up @@ -388,8 +388,15 @@ function NewActionModal({
const [newAction, setNewAction] =
useState<AssistantBuilderActionConfiguration | null>(null);

const [showEmptyDescriptionError, setShowEmptyDescriptionError] =
useState(false);
const [showInvalidActionError, setShowInvalidActionError] = useState<
string | null
>(null);
const [showInvalidActionNameError, setShowInvalidActionNameError] = useState<
string | null
>(null);
const [showInvalidActionDescError, setShowInvalidActionDescError] = useState<
string | null
>(null);

useEffect(() => {
if (initialAction && !newAction) {
Expand All @@ -409,7 +416,9 @@ function NewActionModal({
onClose();
setTimeout(() => {
setNewAction(null);
setShowEmptyDescriptionError(false);
setShowInvalidActionNameError(null);
setShowInvalidActionDescError(null);
setShowInvalidActionError(null);
}, 500);
};

Expand All @@ -420,18 +429,31 @@ function NewActionModal({
hasChanged={true}
variant="side-md"
title=" "
onSave={
newAction && titleValid && descriptionValid && isActionValid(newAction)
? () => {
newAction.name = newAction.name.trim();
newAction.description = newAction.description.trim();
onSave(newAction);
onCloseLocal();
}
: () => {
setShowEmptyDescriptionError(!descriptionValid);
}
}
onSave={() => {
if (
newAction &&
titleValid &&
descriptionValid &&
!hasActionError(newAction)
) {
newAction.name = newAction.name.trim();
newAction.description = newAction.description.trim();
onSave(newAction);
onCloseLocal();
} else {
if (!titleValid) {
setShowInvalidActionNameError(
"This name is already used for another tool. Please use a different name."
);
}
if (!descriptionValid) {
setShowInvalidActionDescError("Description cannot be empty.");
}
if (newAction) {
setShowInvalidActionError(hasActionError(newAction));
}
}
}}
>
<div className="w-full pt-8">
<div className="flex flex-col gap-4">
Expand All @@ -442,23 +464,27 @@ function NewActionModal({
actionName,
actionDescription,
getNewActionConfig,
}) =>
}) => {
setNewAction({
...newAction,
configuration: getNewActionConfig(
newAction.configuration
) as any,
description: actionDescription,
name: actionName,
})
}
});
setShowInvalidActionError(null);
}}
owner={owner}
setEdited={setEdited}
dataSources={dataSources}
dustApps={dustApps}
builderState={builderState}
titleValid={titleValid}
showEmptyDescriptionError={showEmptyDescriptionError}
showInvalidActionNameError={showInvalidActionNameError}
showInvalidActionDescError={showInvalidActionDescError}
showInvalidActionError={showInvalidActionError}
setShowInvalidActionNameError={setShowInvalidActionNameError}
setShowInvalidActionDescError={setShowInvalidActionDescError}
/>
)}
</div>
Expand Down Expand Up @@ -537,7 +563,6 @@ function ActionConfigEditor({
setEdited,
description,
onDescriptionChange,
showEmptyDescriptionError,
}: {
owner: WorkspaceType;
action: AssistantBuilderActionConfiguration;
Expand All @@ -554,7 +579,6 @@ function ActionConfigEditor({
setEdited: (edited: boolean) => void;
description: string;
onDescriptionChange: (v: string) => void;
showEmptyDescriptionError: boolean;
}) {
switch (action.type) {
case "DUST_APP_RUN":
Expand Down Expand Up @@ -619,7 +643,6 @@ function ActionConfigEditor({
setEdited={setEdited}
description={description}
onDescriptionChange={onDescriptionChange}
showEmptyDescriptionError={showEmptyDescriptionError}
/>
);
case "TABLES_QUERY":
Expand Down Expand Up @@ -650,8 +673,11 @@ function ActionConfigEditor({

function ActionEditor({
action,
titleValid,
showEmptyDescriptionError,
showInvalidActionNameError,
showInvalidActionDescError,
showInvalidActionError,
setShowInvalidActionNameError,
setShowInvalidActionDescError,
updateAction,
owner,
setEdited,
Expand All @@ -660,8 +686,11 @@ function ActionEditor({
builderState,
}: {
action: AssistantBuilderActionConfiguration;
titleValid: boolean;
showEmptyDescriptionError: boolean;
showInvalidActionNameError: string | null;
showInvalidActionDescError: string | null;
showInvalidActionError: string | null;
setShowInvalidActionNameError: (error: string | null) => void;
setShowInvalidActionDescError: (error: string | null) => void;
updateAction: (args: {
actionName: string;
actionDescription: string;
Expand Down Expand Up @@ -728,19 +757,23 @@ function ActionEditor({
actionDescription: action.description,
getNewActionConfig: (old) => old,
});
setShowInvalidActionNameError(null);
}}
error={
!titleValid
? "This name is already used for another tool. Please use a different name."
: null
}
error={showInvalidActionNameError}
className="text-sm"
/>
</div>
</DropdownMenu.Items>
</DropdownMenu>
)}
</div>

{showInvalidActionNameError && (
<div className="text-sm text-warning-500">
{showInvalidActionNameError}
</div>
)}

<ActionConfigEditor
owner={owner}
action={action}
Expand All @@ -756,9 +789,14 @@ function ActionEditor({
actionDescription: v,
getNewActionConfig: (old) => old,
});
setShowInvalidActionDescError(null);
}}
showEmptyDescriptionError={showEmptyDescriptionError}
/>
{showInvalidActionError && (
<div className="text-sm text-warning-500">
{showInvalidActionError}
</div>
)}
</>
</ActionModeSection>
{shouldDisplayDescription && (
Expand Down Expand Up @@ -791,11 +829,10 @@ function ActionEditor({
actionDescription: v,
getNewActionConfig: (old) => old,
});
setShowInvalidActionDescError(null);
}
}}
error={
showEmptyDescriptionError ? "Description cannot be empty." : null
}
error={showInvalidActionDescError}
showErrorLabel={true}
/>
</div>
Expand Down
4 changes: 2 additions & 2 deletions front/components/assistant_builder/AssistantBuilder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import { useSWRConfig } from "swr";

import { SharingButton } from "@app/components/assistant/Sharing";
import ActionsScreen, {
isActionValid,
hasActionError,
} from "@app/components/assistant_builder/ActionsScreen";
import AssistantBuilderRightPanel from "@app/components/assistant_builder/AssistantBuilderPreviewDrawer";
import { BuilderLayout } from "@app/components/assistant_builder/BuilderLayout";
Expand Down Expand Up @@ -260,7 +260,7 @@ export default function AssistantBuilder({
setInstructionsError(null);
}

if (!builderState.actions.every((a) => isActionValid(a))) {
if (!builderState.actions.every((a) => hasActionError(a) === null)) {
valid = false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@ import {

export function isActionDustAppRunValid(
action: AssistantBuilderActionConfiguration
) {
return action.type === "DUST_APP_RUN" && !!action.configuration.app;
): string | null {
return action.type === "DUST_APP_RUN" && !!action.configuration.app
? null
: "Please select a Dust App.";
}

export function ActionDustAppRun({
Expand Down
Loading

0 comments on commit 76365b7

Please sign in to comment.