From d3b8f6cd7e1d653a567824e8aa1e9b642d30d38c Mon Sep 17 00:00:00 2001 From: anovazzi1 Date: Mon, 9 Sep 2024 10:48:04 -0300 Subject: [PATCH] feature: add edit option to global variables table (#3712) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * first attempt to edit variables on the data table * refactor: Rename AddNewVariableButton to GlobalVariableModal and update its usage The component AddNewVariableButton has been renamed to GlobalVariableModal to better reflect its purpose. The component is now used in multiple places, including the GlobalVariablesPage and InputGlobalComponent. This change improves code clarity and consistency. * fix: fix apply to fields on table edit option * refactor: Trim field names before checking for availability in GlobalVariableModal and GlobalVariablesPage Trim field names before checking for availability in GlobalVariableModal and GlobalVariablesPage to ensure consistent comparison and avoid any potential issues with leading or trailing spaces. * Refactor GlobalVariablesPage to remove unused cellRenderer in the "value" field * [autofix.ci] apply automated fixes * Add validation for 'value' field in VariableRead model and import CREDENTIAL_TYPE - Introduced a field validator for the 'value' field in the VariableRead model to handle cases where the variable type is CREDENTIAL_TYPE. - Added necessary import for CREDENTIAL_TYPE. - Removed an unnecessary blank line in the variable API. * Add constants for credential and generic types in variable service * Refactor import statements in `kubernetes.py` to improve module organization * Refactor imports in test_service.py for better organization * refactor: Update import statements in variable.py for better organization * Refactor import and reorder fields in VariableRead model - Changed import of `CREDENTIAL_TYPE` from `service` to `constants` module. - Reordered fields in `VariableRead` model to place `type` before `value`. * ✨ (userSettings.spec.ts): Add additional randomName variables for testing purposes 📝 (userSettings.spec.ts): Update test to interact with global variables and improve readability and maintainability of the code * test: fix test_create_variable --------- Co-authored-by: Gabriel Luiz Freitas Almeida Co-authored-by: cristhianzl Co-authored-by: italojohnny --- src/backend/base/langflow/api/v1/variable.py | 4 +- .../database/models/variable/model.py | 11 +++ .../langflow/services/variable/constants.py | 2 + .../langflow/services/variable/kubernetes.py | 2 +- .../langflow/services/variable/service.py | 4 +- .../tests/unit/api/v1/test_variable.py | 2 +- .../unit/services/variable/test_service.py | 13 ++-- .../GlobalVariableModal.tsx} | 70 ++++++++++++++----- .../utils/sort-by-name.tsx | 0 .../components/inputGlobalComponent/index.tsx | 6 +- .../components/tableAutoCellRender/index.tsx | 6 +- .../variables/use-patch-global-variables.ts | 22 +++--- .../pages/GlobalVariablesPage/index.tsx | 58 ++++++++++----- .../src/types/global_variables/index.ts | 1 + .../scheduled-end-to-end/userSettings.spec.ts | 53 +++++++++++--- 15 files changed, 186 insertions(+), 68 deletions(-) create mode 100644 src/backend/base/langflow/services/variable/constants.py rename src/frontend/src/components/{addNewVariableButtonComponent/addNewVariableButton.tsx => GlobalVariableModal/GlobalVariableModal.tsx} (70%) rename src/frontend/src/components/{addNewVariableButtonComponent => GlobalVariableModal}/utils/sort-by-name.tsx (100%) diff --git a/src/backend/base/langflow/api/v1/variable.py b/src/backend/base/langflow/api/v1/variable.py index d8fe33957..e27392eb4 100644 --- a/src/backend/base/langflow/api/v1/variable.py +++ b/src/backend/base/langflow/api/v1/variable.py @@ -9,7 +9,8 @@ from langflow.services.database.models.user.model import User from langflow.services.database.models.variable import VariableCreate, VariableRead, VariableUpdate from langflow.services.deps import get_session, get_settings_service, get_variable_service from langflow.services.variable.base import VariableService -from langflow.services.variable.service import GENERIC_TYPE, DatabaseVariableService +from langflow.services.variable.constants import GENERIC_TYPE +from langflow.services.variable.service import DatabaseVariableService router = APIRouter(prefix="/variables", tags=["Variables"]) @@ -61,7 +62,6 @@ def read_variables( """Read all variables.""" try: return variable_service.get_all(user_id=current_user.id, session=session) - except Exception as e: raise HTTPException(status_code=500, detail=str(e)) from e diff --git a/src/backend/base/langflow/services/database/models/variable/model.py b/src/backend/base/langflow/services/database/models/variable/model.py index 376c1c92e..fa1a0b264 100644 --- a/src/backend/base/langflow/services/database/models/variable/model.py +++ b/src/backend/base/langflow/services/database/models/variable/model.py @@ -2,8 +2,11 @@ from datetime import datetime, timezone from typing import TYPE_CHECKING, List, Optional from uuid import UUID, uuid4 +from pydantic import ValidationInfo, field_validator from sqlmodel import JSON, Column, DateTime, Field, Relationship, SQLModel, func +from langflow.services.variable.constants import CREDENTIAL_TYPE + if TYPE_CHECKING: from langflow.services.database.models.user.model import User @@ -52,8 +55,16 @@ class VariableRead(SQLModel): id: UUID name: Optional[str] = Field(None, description="Name of the variable") type: Optional[str] = Field(None, description="Type of the variable") + value: Optional[str] = Field(None, description="Encrypted value of the variable") default_fields: Optional[List[str]] = Field(None, description="Default fields for the variable") + @field_validator("value") + @classmethod + def validate_value(cls, value: str, info: ValidationInfo): + if info.data.get("type") == CREDENTIAL_TYPE: + return None + return value + class VariableUpdate(SQLModel): id: UUID # Include the ID for updating diff --git a/src/backend/base/langflow/services/variable/constants.py b/src/backend/base/langflow/services/variable/constants.py new file mode 100644 index 000000000..3318e7a54 --- /dev/null +++ b/src/backend/base/langflow/services/variable/constants.py @@ -0,0 +1,2 @@ +CREDENTIAL_TYPE = "Credential" +GENERIC_TYPE = "Generic" diff --git a/src/backend/base/langflow/services/variable/kubernetes.py b/src/backend/base/langflow/services/variable/kubernetes.py index 62a044d53..460b84dcc 100644 --- a/src/backend/base/langflow/services/variable/kubernetes.py +++ b/src/backend/base/langflow/services/variable/kubernetes.py @@ -10,8 +10,8 @@ from langflow.services.base import Service from langflow.services.database.models.variable.model import Variable, VariableCreate from langflow.services.settings.service import SettingsService from langflow.services.variable.base import VariableService +from langflow.services.variable.constants import CREDENTIAL_TYPE, GENERIC_TYPE from langflow.services.variable.kubernetes_secrets import KubernetesSecretManager, encode_user_id -from langflow.services.variable.service import CREDENTIAL_TYPE, GENERIC_TYPE class KubernetesSecretService(VariableService, Service): diff --git a/src/backend/base/langflow/services/variable/service.py b/src/backend/base/langflow/services/variable/service.py index 740ff4f1a..b8cac4136 100644 --- a/src/backend/base/langflow/services/variable/service.py +++ b/src/backend/base/langflow/services/variable/service.py @@ -12,13 +12,11 @@ from langflow.services.base import Service from langflow.services.database.models.variable.model import Variable, VariableCreate, VariableUpdate from langflow.services.deps import get_session from langflow.services.variable.base import VariableService +from langflow.services.variable.constants import CREDENTIAL_TYPE, GENERIC_TYPE if TYPE_CHECKING: from langflow.services.settings.service import SettingsService -CREDENTIAL_TYPE = "Credential" -GENERIC_TYPE = "Generic" - class DatabaseVariableService(VariableService, Service): def __init__(self, settings_service: "SettingsService"): diff --git a/src/backend/tests/unit/api/v1/test_variable.py b/src/backend/tests/unit/api/v1/test_variable.py index 4445e7b10..3b983fd2a 100644 --- a/src/backend/tests/unit/api/v1/test_variable.py +++ b/src/backend/tests/unit/api/v1/test_variable.py @@ -24,7 +24,7 @@ def test_create_variable(client, body, active_user, logged_in_headers): assert body["type"] == result["type"] assert body["default_fields"] == result["default_fields"] assert "id" in result.keys() - assert "value" not in result.keys() + assert body["value"] != result["value"] def test_create_variable__variable_name_alread_exists(client, body, active_user, logged_in_headers): diff --git a/src/backend/tests/unit/services/variable/test_service.py b/src/backend/tests/unit/services/variable/test_service.py index 62e4914ab..deec85dab 100644 --- a/src/backend/tests/unit/services/variable/test_service.py +++ b/src/backend/tests/unit/services/variable/test_service.py @@ -1,11 +1,14 @@ -from langflow.services.database.models.variable.model import VariableUpdate -import pytest +from datetime import datetime from unittest.mock import patch from uuid import uuid4 -from datetime import datetime -from sqlmodel import SQLModel, Session, create_engine + +import pytest +from sqlmodel import Session, SQLModel, create_engine + +from langflow.services.database.models.variable.model import VariableUpdate from langflow.services.deps import get_settings_service -from langflow.services.variable.service import GENERIC_TYPE, CREDENTIAL_TYPE, DatabaseVariableService +from langflow.services.variable.constants import CREDENTIAL_TYPE, GENERIC_TYPE +from langflow.services.variable.service import DatabaseVariableService @pytest.fixture diff --git a/src/frontend/src/components/addNewVariableButtonComponent/addNewVariableButton.tsx b/src/frontend/src/components/GlobalVariableModal/GlobalVariableModal.tsx similarity index 70% rename from src/frontend/src/components/addNewVariableButtonComponent/addNewVariableButton.tsx rename to src/frontend/src/components/GlobalVariableModal/GlobalVariableModal.tsx index 90a14b005..8be21bec8 100644 --- a/src/frontend/src/components/addNewVariableButtonComponent/addNewVariableButton.tsx +++ b/src/frontend/src/components/GlobalVariableModal/GlobalVariableModal.tsx @@ -1,8 +1,10 @@ import { useGetGlobalVariables, + usePatchGlobalVariables, usePostGlobalVariables, } from "@/controllers/API/queries/variables"; import getUnavailableFields from "@/stores/globalVariablesStore/utils/get-unavailable-fields"; +import { GlobalVariable } from "@/types/global_variables"; import { useEffect, useState } from "react"; import BaseModal from "../../modals/baseModal"; import useAlertStore from "../../stores/alertStore"; @@ -17,21 +19,33 @@ import sortByName from "./utils/sort-by-name"; //TODO IMPLEMENT FORM LOGIC -export default function AddNewVariableButton({ +export default function GlobalVariableModal({ children, asChild, + initialData, + open: myOpen, + setOpen: mySetOpen, }: { - children: JSX.Element; + children?: JSX.Element; asChild?: boolean; + initialData?: GlobalVariable; + open?: boolean; + setOpen?: (a: boolean | ((o?: boolean) => boolean)) => void; }): JSX.Element { - const [key, setKey] = useState(""); - const [value, setValue] = useState(""); - const [type, setType] = useState("Generic"); - const [fields, setFields] = useState([]); - const [open, setOpen] = useState(false); + const [key, setKey] = useState(initialData?.name ?? ""); + const [value, setValue] = useState(initialData?.value ?? ""); + const [type, setType] = useState(initialData?.type ?? "Generic"); + const [fields, setFields] = useState( + initialData?.default_fields ?? [], + ); + const [open, setOpen] = + mySetOpen !== undefined && myOpen !== undefined + ? [myOpen, mySetOpen] + : useState(false); const setErrorData = useAlertStore((state) => state.setErrorData); const componentFields = useTypesStore((state) => state.ComponentFields); const { mutate: mutateAddGlobalVariable } = usePostGlobalVariables(); + const { mutate: updateVariable } = usePatchGlobalVariables(); const { data: globalVariables } = useGetGlobalVariables(); const [availableFields, setAvailableFields] = useState([]); @@ -39,12 +53,13 @@ export default function AddNewVariableButton({ if (globalVariables && componentFields.size > 0) { const unavailableFields = getUnavailableFields(globalVariables); const fields = Array.from(componentFields).filter( - (field) => !unavailableFields.hasOwnProperty(field), + (field) => !unavailableFields.hasOwnProperty(field.trim()), + ); + setAvailableFields( + sortByName(fields.concat(initialData?.default_fields ?? [])), ); - - setAvailableFields(sortByName(fields)); } - }, [globalVariables, componentFields]); + }, [globalVariables, componentFields, initialData]); const setSuccessData = useAlertStore((state) => state.setSuccessData); @@ -71,35 +86,52 @@ export default function AddNewVariableButton({ setOpen(false); setSuccessData({ - title: `Variable ${name} created successfully`, + title: `Variable ${name} ${initialData ? "updated" : "created"} successfully`, }); }, onError: (error) => { let responseError = error as ResponseErrorDetailAPI; setErrorData({ - title: "Error creating variable", + title: `Error ${initialData ? "updating" : "creating"} variable`, list: [ responseError?.response?.data?.detail ?? - "An unexpected error occurred while adding a new variable. Please try again.", + `An unexpected error occurred while ${initialData ? "updating a new" : "creating"} variable. Please try again.`, ], }); }, }); } + function submitForm() { + if (!initialData) { + handleSaveVariable(); + } else { + updateVariable({ + id: initialData.id, + name: key, + value: value, + default_fields: fields, + }); + setOpen(false); + } + } + return ( - Create Variable + + {" "} + {initialData ? "Update" : "Create"} Variable{" "} + { setType(e); }} @@ -161,7 +194,10 @@ export default function AddNewVariableButton({ ); diff --git a/src/frontend/src/components/addNewVariableButtonComponent/utils/sort-by-name.tsx b/src/frontend/src/components/GlobalVariableModal/utils/sort-by-name.tsx similarity index 100% rename from src/frontend/src/components/addNewVariableButtonComponent/utils/sort-by-name.tsx rename to src/frontend/src/components/GlobalVariableModal/utils/sort-by-name.tsx diff --git a/src/frontend/src/components/inputGlobalComponent/index.tsx b/src/frontend/src/components/inputGlobalComponent/index.tsx index 0a505f207..23c1a50f6 100644 --- a/src/frontend/src/components/inputGlobalComponent/index.tsx +++ b/src/frontend/src/components/inputGlobalComponent/index.tsx @@ -7,7 +7,7 @@ import DeleteConfirmationModal from "../../modals/deleteConfirmationModal"; import useAlertStore from "../../stores/alertStore"; import { InputGlobalComponentType } from "../../types/components"; import { cn } from "../../utils/utils"; -import AddNewVariableButton from "../addNewVariableButtonComponent/addNewVariableButton"; +import GlobalVariableModal from "../GlobalVariableModal/GlobalVariableModal"; import ForwardedIconComponent from "../genericIconComponent"; import InputComponent from "../inputComponent"; import { CommandItem } from "../ui/command"; @@ -72,7 +72,7 @@ export default function InputGlobalComponent({ optionsPlaceholder={"Global Variables"} optionsIcon="Globe" optionsButton={ - + Add New Variable - + } optionButton={(option) => ( diff --git a/src/frontend/src/controllers/API/queries/variables/use-patch-global-variables.ts b/src/frontend/src/controllers/API/queries/variables/use-patch-global-variables.ts index a34ee2863..b5728f145 100644 --- a/src/frontend/src/controllers/API/queries/variables/use-patch-global-variables.ts +++ b/src/frontend/src/controllers/API/queries/variables/use-patch-global-variables.ts @@ -1,13 +1,15 @@ import { useMutationFunctionType } from "@/types/api"; +import { GlobalVariable } from "@/types/global_variables"; import { UseMutationResult } from "@tanstack/react-query"; import { api } from "../../api"; import { getURL } from "../../helpers/constants"; import { UseRequestProcessor } from "../../services/request-processor"; interface PatchGlobalVariablesParams { - name: string; - value: string; + name?: string; + value?: string; id: string; + default_fields?: string[]; } export const usePatchGlobalVariables: useMutationFunctionType< @@ -16,15 +18,13 @@ export const usePatchGlobalVariables: useMutationFunctionType< > = (options?) => { const { mutate, queryClient } = UseRequestProcessor(); - async function patchGlobalVariables({ - name, - value, - id, - }: PatchGlobalVariablesParams): Promise { - const res = await api.patch(`${getURL("VARIABLES")}/${id}`, { - name, - value, - }); + async function patchGlobalVariables( + GlobalVariable: PatchGlobalVariablesParams, + ): Promise { + const res = await api.patch( + `${getURL("VARIABLES")}/${GlobalVariable.id}`, + GlobalVariable, + ); return res.data; } diff --git a/src/frontend/src/pages/SettingsPage/pages/GlobalVariablesPage/index.tsx b/src/frontend/src/pages/SettingsPage/pages/GlobalVariablesPage/index.tsx index 46a5c0559..8ec1eb4e4 100644 --- a/src/frontend/src/pages/SettingsPage/pages/GlobalVariablesPage/index.tsx +++ b/src/frontend/src/pages/SettingsPage/pages/GlobalVariablesPage/index.tsx @@ -1,13 +1,22 @@ import IconComponent from "../../../../components/genericIconComponent"; import { Button } from "../../../../components/ui/button"; +import TableAutoCellRender from "@/components/tableComponent/components/tableAutoCellRender"; import { useDeleteGlobalVariables, useGetGlobalVariables, } from "@/controllers/API/queries/variables"; -import { ColDef, ColGroupDef, SelectionChangedEvent } from "ag-grid-community"; -import { useState } from "react"; -import AddNewVariableButton from "../../../../components/addNewVariableButtonComponent/addNewVariableButton"; +import { useTypesStore } from "@/stores/typesStore"; +import { GlobalVariable } from "@/types/global_variables"; +import { + ColDef, + ColGroupDef, + RowClickedEvent, + RowDoubleClickedEvent, + SelectionChangedEvent, +} from "ag-grid-community"; +import { useEffect, useRef, useState } from "react"; +import GlobalVariableModal from "../../../../components/GlobalVariableModal/GlobalVariableModal"; import Dropdown from "../../../../components/dropdownComponent"; import ForwardedIconComponent from "../../../../components/genericIconComponent"; import TableComponent from "../../../../components/tableComponent"; @@ -16,6 +25,9 @@ import useAlertStore from "../../../../stores/alertStore"; export default function GlobalVariablesPage() { const setErrorData = useAlertStore((state) => state.setErrorData); + const [openModal, setOpenModal] = useState(false); + const initialData = useRef(undefined); + const getTypes = useTypesStore((state) => state.getTypes); const BadgeRenderer = (props) => { return props.value !== "" ? (
@@ -28,6 +40,11 @@ export default function GlobalVariablesPage() { ); }; + useEffect(() => { + //get the components to build the Aplly To Fields dropdown + getTypes(true); + }, []); + const DropdownEditor = ({ options, value, onValueChange }) => { return ( @@ -36,7 +53,7 @@ export default function GlobalVariablesPage() { ); }; // Column Definitions: Defines the columns to be displayed. - const [colDefs, setColDefs] = useState<(ColDef | ColGroupDef)[]>([ + const colDefs: ColDef[] = [ { headerName: "Variable Name", field: "name", @@ -51,25 +68,20 @@ export default function GlobalVariablesPage() { options: ["Generic", "Credential"], }, flex: 1, - editable: false, }, - // { - // field: "value", - // cellEditor: "agLargeTextCellEditor", - // flex: 2, - // editable: false, - // }, + { + field: "value", + }, { headerName: "Apply To Fields", field: "default_fields", valueFormatter: (params) => { - return params.value.join(", "); + return params.value?.join(", ") ?? ""; }, flex: 1, - editable: false, resizable: false, }, - ]); + ]; const [selectedRows, setSelectedRows] = useState([]); @@ -93,6 +105,11 @@ export default function GlobalVariablesPage() { }); } + function updateVariables(event: RowClickedEvent) { + initialData.current = event.data; + setOpenModal(true); + } + return (
@@ -109,12 +126,12 @@ export default function GlobalVariablesPage() {

- + - +
@@ -126,12 +143,21 @@ export default function GlobalVariablesPage() { setSelectedRows(event.api.getSelectedRows().map((row) => row.name)); }} rowSelection="multiple" + onRowClicked={updateVariables} suppressRowClickSelection={true} pagination={true} columnDefs={colDefs} rowData={globalVariables ?? []} onDelete={removeVariables} /> + {initialData.current && ( + + )}
); diff --git a/src/frontend/src/types/global_variables/index.ts b/src/frontend/src/types/global_variables/index.ts index ca1fcb826..dd9e0086b 100644 --- a/src/frontend/src/types/global_variables/index.ts +++ b/src/frontend/src/types/global_variables/index.ts @@ -3,4 +3,5 @@ export type GlobalVariable = { type: string; default_fields: string[]; name: string; + value?: string; }; diff --git a/src/frontend/tests/scheduled-end-to-end/userSettings.spec.ts b/src/frontend/tests/scheduled-end-to-end/userSettings.spec.ts index 0b8b32d04..a0dd1f4dd 100644 --- a/src/frontend/tests/scheduled-end-to-end/userSettings.spec.ts +++ b/src/frontend/tests/scheduled-end-to-end/userSettings.spec.ts @@ -21,6 +21,8 @@ test("should see general profile gradient", async ({ page }) => { test("should interact with global variables", async ({ page }) => { const randomName = Math.random().toString(36).substring(2); + const randomName2 = Math.random().toString(36).substring(2); + const randomName3 = Math.random().toString(36).substring(2); await page.goto("/"); await page.waitForTimeout(1000); @@ -50,29 +52,64 @@ test("should interact with global variables", async ({ page }) => { .fill("testtesttesttesttesttesttesttest"); await page.getByTestId("popover-anchor-apply-to-fields").click(); - await page.waitForTimeout(3000); + await page.waitForTimeout(1000); - await page.getByPlaceholder("Search options...").fill("System Message"); + await page.getByPlaceholder("Search options...").fill("System"); - await page.waitForSelector("text=System Message", { timeout: 30000 }); + await page.waitForTimeout(500); - await page.getByText("System Message").first().click(); + await page.waitForSelector("text=System", { timeout: 30000 }); + + await page.waitForTimeout(500); + + await page.getByText("System").last().click(); await page.getByPlaceholder("Search options...").fill("openAI"); - await page.waitForSelector("text=OpenAI API Base", { timeout: 30000 }); + await page.waitForSelector("text=openai", { timeout: 30000 }); - await page.getByText("OpenAI API Base").first().click(); + await page.waitForTimeout(500); - await page.getByPlaceholder("Search options...").fill("llama"); + await page.getByText("openai").last().click(); - await page.getByText("Ollama").first().click(); + await page.waitForTimeout(500); + + await page.getByPlaceholder("Search options...").fill("ollama"); + + await page.waitForSelector("text=ollama", { timeout: 30000 }); + + await page.getByText("ollama").first().click(); await page.keyboard.press("Escape"); await page.getByText("Save Variable", { exact: true }).click(); await page.getByText(randomName).last().isVisible(); + await page.getByText(randomName).last().click(); + await page.getByText(randomName).last().click(); + + await page.waitForTimeout(500); + + await page + .getByPlaceholder("Insert a name for the variable...") + .fill(randomName2); + + await page.getByText("Update Variable", { exact: true }).last().click(); + + await page.getByText(randomName2).last().isVisible(); + + await page.getByText(randomName2).last().click(); + + await page.waitForTimeout(500); + + await page + .getByPlaceholder("Insert a name for the variable...") + .fill(randomName3); + + await page.getByText("Update Variable", { exact: true }).last().click(); + + await page.getByText(randomName3).last().isVisible(); + const focusElementsOnBoard = async ({ page }) => { await page.waitForSelector( '[aria-label="Press Space to toggle all rows selection (unchecked)"]',