From ef1dfbc2e7fdc84a3eb6a4d1c33e59d002a31d6c Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Tue, 7 Feb 2023 21:32:27 +0200
Subject: [PATCH] Retain field and card values when switching to custom list
(#28117)
---
.../ValuesSourceModal.unit.spec.tsx | 83 +++++++++++++
.../ValuesSourceTypeModal.tsx | 111 ++++++++++++------
2 files changed, 159 insertions(+), 35 deletions(-)
diff --git a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.unit.spec.tsx b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.unit.spec.tsx
index b3bc021fd21..d6468b4e693 100644
--- a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.unit.spec.tsx
+++ b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceModal.unit.spec.tsx
@@ -93,6 +93,52 @@ describe("ValuesSourceModal", () => {
expect(screen.getByRole("radio", { name: "Custom list" })).toBeChecked();
});
+
+ it("should copy field values when switching to custom list", async () => {
+ setup({
+ parameter: createMockUiParameter({
+ fields: [
+ new Field(createMockField({ id: 1 })),
+ new Field(createMockField({ id: 2 })),
+ ],
+ values_source_config: {
+ values: ["A", "B"],
+ },
+ }),
+ parameterValues: createMockParameterValues({
+ values: [["C"], ["D"]],
+ }),
+ });
+
+ await waitFor(() => {
+ expect(screen.getByRole("textbox")).toHaveValue("C\nD");
+ });
+
+ userEvent.click(screen.getByRole("radio", { name: "Custom list" }));
+ expect(screen.getByRole("radio", { name: "Custom list" })).toBeChecked();
+ expect(screen.getByRole("textbox")).toHaveValue("C\nD");
+ });
+
+ it("should not overwrite custom list values when field values are empty", async () => {
+ setup({
+ parameter: createMockUiParameter({
+ fields: [
+ new Field(createMockField({ id: 1 })),
+ new Field(createMockField({ id: 2 })),
+ ],
+ values_source_config: {
+ values: ["A", "B"],
+ },
+ }),
+ parameterValues: createMockParameterValues({
+ values: [],
+ }),
+ });
+
+ userEvent.click(screen.getByRole("radio", { name: "Custom list" }));
+ expect(screen.getByRole("radio", { name: "Custom list" })).toBeChecked();
+ expect(screen.getByRole("textbox")).toHaveValue("A\nB");
+ });
});
describe("card source", () => {
@@ -216,6 +262,43 @@ describe("ValuesSourceModal", () => {
await screen.findByText("You don't have permissions to do that."),
).toBeInTheDocument();
});
+
+ it("should copy card values when switching to custom list", async () => {
+ setup({
+ parameter: createMockUiParameter({
+ values_source_type: "card",
+ values_source_config: {
+ card_id: 1,
+ value_field: ["field", 2, null],
+ },
+ }),
+ parameterValues: createMockParameterValues({
+ values: [["A"], ["B"], ["C"]],
+ }),
+ cards: [
+ createMockCard({
+ id: 1,
+ name: "Products",
+ result_metadata: [
+ createMockField({
+ id: 2,
+ display_name: "Category",
+ base_type: "type/Text",
+ semantic_type: "type/Category",
+ }),
+ ],
+ }),
+ ],
+ });
+
+ await waitFor(() => {
+ expect(screen.getByRole("textbox")).toHaveValue("A\nB\nC");
+ });
+
+ userEvent.click(screen.getByRole("radio", { name: "Custom list" }));
+ expect(screen.getByRole("radio", { name: "Custom list" })).toBeChecked();
+ expect(screen.getByRole("textbox")).toHaveValue("A\nB\nC");
+ });
});
describe("list source", () => {
diff --git a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceTypeModal.tsx b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceTypeModal.tsx
index 56660258cef..b1dfbe5c445 100644
--- a/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceTypeModal.tsx
+++ b/frontend/src/metabase/parameters/components/ValuesSourceModal/ValuesSourceTypeModal.tsx
@@ -93,10 +93,6 @@ const ValuesSourceTypeModal = ({
onSubmit,
onClose,
}: ModalProps): JSX.Element => {
- const sourceTypeOptions = useMemo(() => {
- return getSourceTypeOptions(parameter);
- }, [parameter]);
-
return (
<ModalContent
title={t`Selectable values for ${parameter.name}`}
@@ -114,17 +110,16 @@ const ValuesSourceTypeModal = ({
<FieldSourceModal
parameter={parameter}
sourceType={sourceType}
- sourceTypeOptions={sourceTypeOptions}
sourceConfig={sourceConfig}
onFetchParameterValues={onFetchParameterValues}
onChangeSourceType={onChangeSourceType}
+ onChangeSourceConfig={onChangeSourceConfig}
/>
) : sourceType === "card" ? (
<CardSourceModal
parameter={parameter}
question={question}
sourceType={sourceType}
- sourceTypeOptions={sourceTypeOptions}
sourceConfig={sourceConfig}
onFetchParameterValues={onFetchParameterValues}
onChangeCard={onChangeCard}
@@ -133,8 +128,8 @@ const ValuesSourceTypeModal = ({
/>
) : sourceType === "static-list" ? (
<ListSourceModal
+ parameter={parameter}
sourceType={sourceType}
- sourceTypeOptions={sourceTypeOptions}
sourceConfig={sourceConfig}
onChangeSourceType={onChangeSourceType}
onChangeSourceConfig={onChangeSourceConfig}
@@ -144,24 +139,67 @@ const ValuesSourceTypeModal = ({
);
};
+interface SourceTypeOptionsProps {
+ parameter: Parameter;
+ parameterValues?: ParameterValue[];
+ sourceType: ValuesSourceType;
+ sourceConfig: ValuesSourceConfig;
+ onChangeSourceType: (sourceType: ValuesSourceType) => void;
+ onChangeSourceConfig: (sourceConfig: ValuesSourceConfig) => void;
+}
+
+const SourceTypeOptions = ({
+ parameter,
+ parameterValues = [],
+ sourceType,
+ sourceConfig,
+ onChangeSourceType,
+ onChangeSourceConfig,
+}: SourceTypeOptionsProps) => {
+ const sourceTypeOptions = useMemo(() => {
+ return getSourceTypeOptions(parameter);
+ }, [parameter]);
+
+ const handleSourceTypeChange = useCallback(
+ (sourceType: ValuesSourceType) => {
+ onChangeSourceType(sourceType);
+
+ if (parameterValues.length > 0) {
+ const values = getSourceValues(parameterValues);
+ onChangeSourceConfig({ ...sourceConfig, values });
+ }
+ },
+ [sourceConfig, parameterValues, onChangeSourceType, onChangeSourceConfig],
+ );
+
+ return (
+ <Radio
+ value={sourceType}
+ options={sourceTypeOptions}
+ vertical
+ onChange={handleSourceTypeChange}
+ />
+ );
+};
+
interface FieldSourceModalProps {
parameter: Parameter;
sourceType: ValuesSourceType;
- sourceTypeOptions: RadioOption<ValuesSourceType>[];
sourceConfig: ValuesSourceConfig;
onFetchParameterValues: (
opts: FetchParameterValuesOpts,
) => Promise<ParameterValues>;
onChangeSourceType: (sourceType: ValuesSourceType) => void;
+ onChangeSourceConfig: (sourceConfig: ValuesSourceConfig) => void;
}
const FieldSourceModal = ({
parameter,
sourceType,
- sourceTypeOptions,
sourceConfig,
onFetchParameterValues,
onChangeSourceType,
+ onChangeSourceConfig,
}: FieldSourceModalProps) => {
const parameterValues = useParameterValues({
parameter,
@@ -171,7 +209,7 @@ const FieldSourceModal = ({
});
const parameterValuesText = useMemo(
- () => getParameterValuesText(parameterValues),
+ () => getValuesText(getSourceValues(parameterValues)),
[parameterValues],
);
@@ -183,11 +221,13 @@ const FieldSourceModal = ({
<ModalPane>
<ModalSection>
<ModalLabel>{t`Where values should come from`}</ModalLabel>
- <Radio
- value={sourceType}
- options={sourceTypeOptions}
- vertical
- onChange={onChangeSourceType}
+ <SourceTypeOptions
+ parameter={parameter}
+ parameterValues={parameterValues}
+ sourceType={sourceType}
+ sourceConfig={sourceConfig}
+ onChangeSourceType={onChangeSourceType}
+ onChangeSourceConfig={onChangeSourceConfig}
/>
</ModalSection>
</ModalPane>
@@ -212,7 +252,6 @@ interface CardSourceModalProps {
parameter: Parameter;
question: Question | undefined;
sourceType: ValuesSourceType;
- sourceTypeOptions: RadioOption<ValuesSourceType>[];
sourceConfig: ValuesSourceConfig;
onFetchParameterValues: (
opts: FetchParameterValuesOpts,
@@ -226,7 +265,6 @@ const CardSourceModal = ({
parameter,
question,
sourceType,
- sourceTypeOptions,
sourceConfig,
onFetchParameterValues,
onChangeCard,
@@ -249,7 +287,7 @@ const CardSourceModal = ({
});
const parameterValuesText = useMemo(
- () => getParameterValuesText(parameterValues),
+ () => getValuesText(getSourceValues(parameterValues)),
[parameterValues],
);
@@ -268,11 +306,13 @@ const CardSourceModal = ({
<ModalPane>
<ModalSection>
<ModalLabel>{t`Where values should come from`}</ModalLabel>
- <Radio
- value={sourceType}
- options={sourceTypeOptions}
- vertical
- onChange={onChangeSourceType}
+ <SourceTypeOptions
+ parameter={parameter}
+ parameterValues={parameterValues}
+ sourceType={sourceType}
+ sourceConfig={sourceConfig}
+ onChangeSourceType={onChangeSourceType}
+ onChangeSourceConfig={onChangeSourceConfig}
/>
</ModalSection>
<ModalSection>
@@ -323,16 +363,16 @@ const CardSourceModal = ({
};
interface ListSourceModalProps {
+ parameter: Parameter;
sourceType: ValuesSourceType;
- sourceTypeOptions: RadioOption<ValuesSourceType>[];
sourceConfig: ValuesSourceConfig;
onChangeSourceType: (sourceType: ValuesSourceType) => void;
onChangeSourceConfig: (sourceConfig: ValuesSourceConfig) => void;
}
const ListSourceModal = ({
+ parameter,
sourceType,
- sourceTypeOptions,
sourceConfig,
onChangeSourceType,
onChangeSourceConfig,
@@ -349,18 +389,19 @@ const ListSourceModal = ({
<ModalPane>
<ModalSection>
<ModalLabel>{t`Where values should come from`}</ModalLabel>
- <Radio
- value={sourceType}
- options={sourceTypeOptions}
- vertical
- onChange={onChangeSourceType}
+ <SourceTypeOptions
+ parameter={parameter}
+ sourceType={sourceType}
+ sourceConfig={sourceConfig}
+ onChangeSourceType={onChangeSourceType}
+ onChangeSourceConfig={onChangeSourceConfig}
/>
<ModalHelpMessage>{t`Enter one value per line.`}</ModalHelpMessage>
</ModalSection>
</ModalPane>
<ModalMain>
<ModalTextArea
- defaultValue={getStaticValuesText(sourceConfig.values)}
+ defaultValue={getValuesText(sourceConfig.values)}
fullWidth
onChange={handleValuesChange}
/>
@@ -369,12 +410,12 @@ const ListSourceModal = ({
);
};
-const getParameterValuesText = (values: ParameterValue[] = []) => {
- return values.map(([value]) => value).join(NEW_LINE);
+const getValuesText = (values: string[] = []) => {
+ return values.join(NEW_LINE);
};
-const getStaticValuesText = (values: string[] = []) => {
- return values.join(NEW_LINE);
+const getSourceValues = (values: ParameterValue[] = []) => {
+ return values.map(([value]) => String(value));
};
const getStaticValues = (value: string) => {
--
GitLab