Skip to content

Commit

Permalink
fix: Change multiselect back to supporting numbers and boolean. (apps…
Browse files Browse the repository at this point in the history 
…mithorg#7895)

* Revert "fix: multiselect validation (appsmithorg#7698)"

This reverts commit 728a255.

* - Convert the multiselect options value and labels to string before filtering as the values can be numbers.

* - Discourage users from using string in multiselect default value

Co-authored-by: Satish Gandham <satish@appsmith.com>
(cherry picked from commit c3d5b10)
  • Loading branch information
@SatishGandham @riodeuno
SatishGandham authored and riodeuno committed Sep 28, 2021
1 parent af484dd commit 2d7b9a4
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 233 deletions.
10 changes: 8 additions & 2 deletions app/client/src/widgets/MultiSelectWidget/component/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,16 @@ function MultiSelectComponent({
[isSelectAll, options, loading],
);

// Convert the values to string before searching.
// input is always a string.
const filterOption = useCallback(
(input, option) =>
option?.props.label.toLowerCase().indexOf(input.toLowerCase()) >= 0 ||
option?.props.value.toLowerCase().indexOf(input.toLowerCase()) >= 0,
String(option?.props.label)
.toLowerCase()
.indexOf(input.toLowerCase()) >= 0 ||
String(option?.props.value)
.toLowerCase()
.indexOf(input.toLowerCase()) >= 0,
[],
);

Expand Down
8 changes: 5 additions & 3 deletions app/client/src/widgets/MultiSelectWidget/widget/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,15 @@ class MultiSelectWidget extends BaseWidget<
name: "label",
type: ValidationTypes.TEXT,
params: {
default: "",
required: true,
},
},
{
name: "value",
type: ValidationTypes.TEXT,
params: {
default: "",
required: true,
},
},
Expand All @@ -94,16 +96,16 @@ class MultiSelectWidget extends BaseWidget<
propertyName: "defaultOptionValue",
label: "Default Value",
controlType: "INPUT_TEXT",
placeholderText: "GREEN",
placeholderText: "[GREEN]",
isBindProperty: true,
isTriggerProperty: false,
validation: {
type: ValidationTypes.FUNCTION,
params: {
fn: defaultOptionValueValidation,
expected: {
type: "value or Array of values",
example: `option1 | ['option1', 'option2']`,
type: "Array of values",
example: `['option1', 'option2']`,
autocompleteDataType: AutocompleteDataType.ARRAY,
},
},
Expand Down
223 changes: 0 additions & 223 deletions app/client/src/workers/validations.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -962,229 +962,6 @@ describe("Validate Validators", () => {
expect(result).toStrictEqual(expected);
});
});
it("correctly validates objects", () => {
const inputs = [
{
label: true,
value: "true",
},
{
label: true,
value: "true",
},
{
paletteColors1: "#ffffff",
palettecolors2: "#ffffff",
},
];
const config = [
{
type: ValidationTypes.OBJECT,
params: {
required: true,
allowedKeys: [
{
name: "label",
type: ValidationTypes.TEXT,
params: {
required: true,
},
},
{
name: "value",
type: ValidationTypes.TEXT,
params: {
required: true,
unique: true,
},
},
],
},
},
{
type: ValidationTypes.OBJECT,
params: {
required: true,
allowedKeys: [
{
name: "label",
type: ValidationTypes.BOOLEAN,
params: {
required: true,
},
},
{
name: "value",
type: ValidationTypes.TEXT,
params: {
required: true,
unique: true,
},
},
],
},
},
{
type: ValidationTypes.OBJECT,
params: {
allowedKeys: [
{
name: "paletteColors1",
type: ValidationTypes.TEXT,
params: {
strict: true,
},
},
{
name: "paletteColors2",
type: ValidationTypes.TEXT,
params: {
strict: true,
ignoreCase: true,
},
},
],
},
},
];
const expected = [
{
isValid: true,
parsed: {
label: "true",
value: "true",
},
},
{
isValid: true,
parsed: { label: true, value: "true" },
},
{
isValid: true,
parsed: {
paletteColors1: "#ffffff",
palettecolors2: "#ffffff",
},
},
];
inputs.forEach((input, index) => {
const result = validate(config[index], input, DUMMY_WIDGET);
expect(result).toStrictEqual(expected[index]);
});
});
it("correctly validates objects to fail", () => {
const inputs = [
{
labels: true,
values: "true",
},
{
label: true,
},
{
paletteColors1: "#ffffff",
palettecolors2: 3,
},
];
const config = [
{
type: ValidationTypes.OBJECT,
params: {
required: true,
allowedKeys: [
{
name: "label",
type: ValidationTypes.TEXT,
params: {
required: true,
},
},
{
name: "value",
type: ValidationTypes.TEXT,
params: {
required: true,
unique: true,
},
},
],
},
},
{
type: ValidationTypes.OBJECT,
params: {
required: true,
allowedKeys: [
{
name: "label",
type: ValidationTypes.BOOLEAN,
params: {
required: true,
},
},
{
name: "value",
type: ValidationTypes.TEXT,
params: {
required: true,
unique: true,
},
},
],
},
},
{
type: ValidationTypes.OBJECT,
params: {
allowedKeys: [
{
name: "paletteColors1",
type: ValidationTypes.TEXT,
params: {
strict: true,
},
},
{
name: "paletteColors2",
type: ValidationTypes.TEXT,
params: {
strict: true,
ignoreCase: true,
},
},
],
},
},
];
const expected = [
{
isValid: false,
parsed: {
labels: true,
values: "true",
},
message: "Missing required key: label Missing required key: value",
},
{
isValid: false,
message: "Missing required key: value",
parsed: { label: true },
},
{
isValid: false,
message:
"Value of key: palettecolors2 is invalid: This value does not evaluate to type string",
parsed: {
paletteColors1: "#ffffff",
palettecolors2: "",
},
},
];
inputs.forEach((input, index) => {
const result = validate(config[index], input, DUMMY_WIDGET);
expect(result).toStrictEqual(expected[index]);
});
});
});

// describe("Color Picker Text validator", () => {
Expand Down
9 changes: 4 additions & 5 deletions app/client/src/workers/validations.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ function validatePlainObject(
if (config.params?.allowedKeys) {
let _valid = true;
const _messages: string[] = [];
const parsedValue: Record<string, unknown> = value;
config.params.allowedKeys.forEach((entry) => {
const ignoreCase = !!entry.params?.ignoreCase;
const entryName = getPropertyEntry(value, entry.name, ignoreCase);
Expand All @@ -65,7 +64,6 @@ function validatePlainObject(
value[entryName],
props,
);
parsedValue[entryName] = parsed;
if (!isValid) {
value[entryName] = parsed;
_valid = isValid;
Expand All @@ -80,7 +78,7 @@ function validatePlainObject(
if (_valid) {
return {
isValid: true,
parsed: parsedValue,
parsed: value,
};
}
return {
Expand Down Expand Up @@ -119,7 +117,7 @@ function validateArray(
config.params?.children?.type === ValidationTypes.OBJECT &&
(config.params.children.params?.allowedKeys || []).length > 0
) {
const allowedKeysConfigArray =
const allowedKeysCofigArray =
config.params.children.params?.allowedKeys || [];

const allowedKeys = (config.params.children.params?.allowedKeys || []).map(
Expand All @@ -131,7 +129,7 @@ function validateArray(
};

const valueWithType = value as ItemType[];
allowedKeysConfigArray.forEach((allowedKeyConfig) => {
allowedKeysCofigArray.forEach((allowedKeyConfig) => {
if (allowedKeyConfig.params?.unique) {
const allowedKeyValues = valueWithType.map(
(item) => item[allowedKeyConfig.name],
Expand Down Expand Up @@ -354,6 +352,7 @@ export const VALIDATORS: Record<ValidationTypes, Validator> = {
isValid: false,
};
}

return {
isValid: true,
parsed,
Expand Down

0 comments on commit 2d7b9a4

Please sign in to comment.