fix: #4758 added EvaluatedValuePopup over PrimaryColumnsControl to show errors and handled duplicate column name (#8770)

* fix: #4758 added EvaluatedValuePopup over PrimaryColumnsControl to show errors and handled duplicate column name

* highlighted duplicate column labels, show error message only when duplicate column label focused

* updated test case

* updated duplicate label indicator logic for test case
This commit is contained in:
Yash Vibhandik 2021-11-23 11:35:01 +05:30 committed by GitHub
parent 5ec733e5a2
commit 399eabd987
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 315 additions and 21 deletions

View File

@ -0,0 +1,33 @@
const widgetsPage = require("../../../../locators/Widgets.json");
const dsl = require("../../../../fixtures/tableNewDsl.json");
const commonlocators = require("../../../../locators/commonlocators.json");
describe("prevent duplicate column name in table", function() {
before(() => {
cy.addDsl(dsl);
});
it("evaluted value popup should show when focus on duplicate column input", function() {
cy.openPropertyPane("tablewidget");
// Updating the column name ; "id" > "TestUpdated"
cy.tableColumnPopertyUpdate("id", "TestUpdated");
// Updating the column name ; "email" > "TestUpdated"
cy.tableColumnPopertyUpdate("email", "TestUpdated");
cy.wait("@updateLayout");
cy.get(commonlocators.evaluatedTypeTitle).should("exist");
// Updating the column name ; "userName" > "TestUpdated2"
// this will move focus of input to another column input and let popup close
cy.tableColumnPopertyUpdate("userName", "TestUpdated2");
// duplicate column's border should remain red
cy.get("[data-rbd-draggable-id='email'] > div > div").should(
"have.class",
"has-duplicate-label",
);
});
afterEach(() => {
// put your clean up code if any
});
});

View File

@ -12,6 +12,7 @@ type RenderComponentProps = {
updateOption: (index: number, value: string) => void;
toggleVisibility?: (index: number) => void;
onEdit?: (index: number) => void;
updateFocus?: (index: number, isFocused: boolean) => void;
};
interface DroppableComponentProps {
@ -23,6 +24,7 @@ interface DroppableComponentProps {
toggleVisibility?: (index: number) => void;
updateItems: (items: Array<Record<string, unknown>>) => void;
onEdit?: (index: number) => void;
updateFocus?: (index: number, isFocused: boolean) => void;
}
export class DroppableComponent extends React.Component<
@ -46,6 +48,7 @@ export class DroppableComponent extends React.Component<
id: item.id,
label: item.label,
isVisible: item.isVisible,
isDuplicateLabel: item.isDuplicateLabel,
};
}
@ -60,11 +63,13 @@ export class DroppableComponent extends React.Component<
onEdit,
renderComponent,
toggleVisibility,
updateFocus,
updateOption,
} = this.props;
return renderComponent({
deleteOption,
updateFocus,
updateOption,
toggleVisibility,
onEdit,

View File

@ -1,4 +1,9 @@
import React, { useCallback, useEffect, useState } from "react";
import React, { useCallback, useEffect, useState, Component } from "react";
import { AppState } from "reducers";
import { connect } from "react-redux";
import { Placement } from "popper.js";
import * as Sentry from "@sentry/react";
import _ from "lodash";
import BaseControl, { ControlProps } from "./BaseControl";
import {
StyledDragIcon,
@ -10,22 +15,38 @@ import {
StyledOptionControlInputGroup,
} from "./StyledControls";
import styled from "constants/DefaultTheme";
import { Indices } from "constants/Layers";
import { DroppableComponent } from "components/ads/DraggableListComponent";
import { ColumnProperties } from "widgets/TableWidget/component/Constants";
import { Size, Category } from "components/ads/Button";
import EmptyDataState from "components/utils/EmptyDataState";
import { getNextEntityName } from "utils/AppsmithUtils";
import EvaluatedValuePopup from "components/editorComponents/CodeEditor/EvaluatedValuePopup";
import { EditorTheme } from "components/editorComponents/CodeEditor/EditorConfig";
import { CodeEditorExpected } from "components/editorComponents/CodeEditor";
import { ColumnProperties } from "widgets/TableWidget/component/Constants";
import {
getDefaultColumnProperties,
getTableStyles,
} from "widgets/TableWidget/component/TableUtilities";
import { debounce } from "lodash";
import { Size, Category } from "components/ads/Button";
import { reorderColumns } from "widgets/TableWidget/component/TableHelpers";
import { DataTree } from "entities/DataTree/dataTreeFactory";
import { getDataTreeForAutocomplete } from "selectors/dataTreeSelectors";
import {
EvaluationError,
getEvalErrorPath,
getEvalValuePath,
PropertyEvaluationErrorType,
} from "utils/DynamicBindingUtils";
import { getNextEntityName } from "utils/AppsmithUtils";
import { Colors } from "constants/Colors";
import { noop } from "utils/AppsmithUtils";
const ItemWrapper = styled.div`
display: flex;
justify-content: flex-start;
align-items: center;
&.has-duplicate-label > div:nth-child(2) {
border: 1px solid ${Colors.DANGER_SOLID};
}
`;
const TabsWrapper = styled.div`
@ -44,13 +65,33 @@ const AddColumnButton = styled(StyledPropertyPaneButton)`
}
`;
interface ReduxStateProps {
dynamicData: DataTree;
datasources: any;
}
type EvaluatedValuePopupWrapperProps = ReduxStateProps & {
isFocused: boolean;
theme: EditorTheme;
popperPlacement?: Placement;
popperZIndex?: Indices;
dataTreePath?: string;
evaluatedValue?: any;
expected?: CodeEditorExpected;
hideEvaluatedValue?: boolean;
useValidationMessage?: boolean;
children: JSX.Element;
};
type RenderComponentProps = {
index: number;
item: {
label: string;
isDerived?: boolean;
isVisible?: boolean;
isDuplicateLabel?: boolean;
};
updateFocus?: (index: number, isFocused: boolean) => void;
updateOption: (index: number, value: string) => void;
onEdit?: (index: number) => void;
deleteOption: (index: number) => void;
@ -84,10 +125,12 @@ function ColumnControlComponent(props: RenderComponentProps) {
item,
onEdit,
toggleVisibility,
updateFocus,
updateOption,
} = props;
const [visibility, setVisibility] = useState(item.isVisible);
const debouncedUpdate = debounce(updateOption, 1000);
const debouncedUpdate = _.debounce(updateOption, 1000);
const debouncedFocus = updateFocus ? _.debounce(updateFocus, 400) : noop;
const onChange = useCallback(
(index: number, value: string) => {
setValue(value);
@ -96,11 +139,19 @@ function ColumnControlComponent(props: RenderComponentProps) {
[updateOption],
);
const onFocus = () => setEditing(true);
const onBlur = () => setEditing(false);
const onFocus = () => {
setEditing(true);
debouncedFocus(index, true);
};
const onBlur = () => {
setEditing(false);
debouncedFocus(index, false);
};
return (
<ItemWrapper>
<ItemWrapper
className={props.item.isDuplicateLabel ? "has-duplicate-label" : ""}
>
<StyledDragIcon height={20} width={20} />
<StyledOptionControlInputGroup
dataType="text"
@ -154,7 +205,36 @@ function ColumnControlComponent(props: RenderComponentProps) {
);
}
class PrimaryColumnsControl extends BaseControl<ControlProps> {
type State = {
focusedIndex: number | null;
duplicateColumnIds: string[];
};
class PrimaryColumnsControl extends BaseControl<ControlProps, State> {
constructor(props: ControlProps) {
super(props);
const columns: Record<string, ColumnProperties> = props.propertyValue || {};
const columnOrder = Object.keys(columns);
const reorderedColumns = reorderColumns(columns, columnOrder);
const tableColumnLabels = _.map(reorderedColumns, "label");
const duplicateColumnIds = [];
for (let index = 0; index < tableColumnLabels.length; index++) {
const currLabel = tableColumnLabels[index] as string;
const duplicateValueIndex = tableColumnLabels.indexOf(currLabel);
if (duplicateValueIndex !== index) {
// get column id from columnOrder index
duplicateColumnIds.push(reorderedColumns[columnOrder[index]].id);
}
}
this.state = {
focusedIndex: null,
duplicateColumnIds,
};
}
render() {
// Get columns from widget properties
const columns: Record<string, ColumnProperties> =
@ -183,22 +263,38 @@ class PrimaryColumnsControl extends BaseControl<ControlProps> {
isVisible: column.isVisible,
isDerived: column.isDerived,
index: column.index,
isDuplicateLabel: _.includes(
this.state.duplicateColumnIds,
column.id,
),
};
},
);
const column: ColumnProperties | undefined = Object.values(
reorderedColumns,
).find(
(column: ColumnProperties) => column.index === this.state.focusedIndex,
);
// show popup on duplicate column label input focused
const isFocused =
!_.isNull(this.state.focusedIndex) &&
_.includes(this.state.duplicateColumnIds, column?.id);
return (
<TabsWrapper>
<DroppableComponent
deleteOption={this.deleteOption}
itemHeight={45}
items={draggableComponentColumns}
onEdit={this.onEdit}
renderComponent={ColumnControlComponent}
toggleVisibility={this.toggleVisibility}
updateItems={this.updateItems}
updateOption={this.updateOption}
/>
<EvaluatedValuePopupWrapper {...this.props} isFocused={isFocused}>
<DroppableComponent
deleteOption={this.deleteOption}
itemHeight={45}
items={draggableComponentColumns}
onEdit={this.onEdit}
renderComponent={ColumnControlComponent}
toggleVisibility={this.toggleVisibility}
updateFocus={this.updateFocus}
updateItems={this.updateItems}
updateOption={this.updateOption}
/>
</EvaluatedValuePopupWrapper>
<AddColumnButton
category={Category.tertiary}
@ -300,6 +396,12 @@ class PrimaryColumnsControl extends BaseControl<ControlProps> {
propertiesToDelete.push(`columnOrder[${columnOrderIndex}]`);
this.deleteProperties(propertiesToDelete);
// if column deleted, clean up duplicateIndexes
let duplicateColumnIds = [...this.state.duplicateColumnIds];
duplicateColumnIds = duplicateColumnIds.filter(
(id) => id !== originalColumn.id,
);
this.setState({ duplicateColumnIds });
}
};
@ -317,12 +419,118 @@ class PrimaryColumnsControl extends BaseControl<ControlProps> {
`${this.props.propertyName}.${originalColumn.id}.label`,
updatedLabel,
);
// check entered label is unique or duplicate
const tableColumnLabels = _.map(columns, "label");
let duplicateColumnIds = [...this.state.duplicateColumnIds];
// if duplicate, add into array
if (_.includes(tableColumnLabels, updatedLabel)) {
duplicateColumnIds.push(originalColumn.id);
this.setState({ duplicateColumnIds });
} else {
duplicateColumnIds = duplicateColumnIds.filter(
(id) => id !== originalColumn.id,
);
this.setState({ duplicateColumnIds });
}
}
};
updateFocus = (index: number, isFocused: boolean) => {
this.setState({ focusedIndex: isFocused ? index : null });
};
static getControlType() {
return "PRIMARY_COLUMNS";
}
}
export default PrimaryColumnsControl;
/**
* wrapper component on dragable primary columns
* render popup if primary column labels are not unique
* show unique name error in PRIMARY_COLUMNS
*/
class EvaluatedValuePopupWrapperClass extends Component<
EvaluatedValuePopupWrapperProps
> {
getPropertyValidation = (
dataTree: DataTree,
dataTreePath?: string,
): {
isInvalid: boolean;
errors: EvaluationError[];
pathEvaluatedValue: unknown;
} => {
if (!dataTreePath) {
return {
isInvalid: false,
errors: [],
pathEvaluatedValue: undefined,
};
}
const errors = _.get(
dataTree,
getEvalErrorPath(dataTreePath),
[],
) as EvaluationError[];
const filteredLintErrors = errors.filter(
(error) => error.errorType !== PropertyEvaluationErrorType.LINT,
);
const pathEvaluatedValue = _.get(dataTree, getEvalValuePath(dataTreePath));
return {
isInvalid: filteredLintErrors.length > 0,
errors: filteredLintErrors,
pathEvaluatedValue,
};
};
render = () => {
const {
dataTreePath,
dynamicData,
evaluatedValue,
expected,
hideEvaluatedValue,
useValidationMessage,
} = this.props;
const {
errors,
isInvalid,
pathEvaluatedValue,
} = this.getPropertyValidation(dynamicData, dataTreePath);
let evaluated = evaluatedValue;
if (dataTreePath) {
evaluated = pathEvaluatedValue;
}
return (
<EvaluatedValuePopup
errors={errors}
evaluatedValue={evaluated}
expected={expected}
hasError={isInvalid}
hideEvaluatedValue={hideEvaluatedValue}
isOpen={this.props.isFocused && isInvalid}
popperPlacement={this.props.popperPlacement}
popperZIndex={this.props.popperZIndex}
theme={this.props.theme || EditorTheme.LIGHT}
useValidationMessage={useValidationMessage}
>
{this.props.children}
</EvaluatedValuePopup>
);
};
}
const mapStateToProps = (state: AppState): ReduxStateProps => ({
dynamicData: getDataTreeForAutocomplete(state),
datasources: state.entities.datasources,
});
const EvaluatedValuePopupWrapper = Sentry.withProfiler(
connect(mapStateToProps)(EvaluatedValuePopupWrapperClass),
);

View File

@ -123,6 +123,7 @@ describe("getAllPathsFromPropertyConfig", () => {
defaultSearchText: EvaluationSubstitutionType.TEMPLATE,
defaultSelectedRow: EvaluationSubstitutionType.TEMPLATE,
isVisible: EvaluationSubstitutionType.TEMPLATE,
primaryColumns: "TEMPLATE",
isSortable: EvaluationSubstitutionType.TEMPLATE,
compactMode: EvaluationSubstitutionType.TEMPLATE,
delimiter: EvaluationSubstitutionType.TEMPLATE,
@ -199,6 +200,16 @@ describe("getAllPathsFromPropertyConfig", () => {
isVisible: {
type: "BOOLEAN",
},
primaryColumns: {
params: {
expected: {
autocompleteDataType: AutocompleteDataType.STRING,
example: "abc",
type: "Unique Column Names",
},
},
type: "FUNCTION",
},
isSortable: {
type: "BOOLEAN",
params: {
@ -222,6 +233,7 @@ describe("getAllPathsFromPropertyConfig", () => {
// Note: Removing until we figure out how functions are represented here.
delete result.validationPaths.defaultSelectedRow.params?.fn;
delete result.validationPaths.primaryColumns.params?.fn;
expect(result).toStrictEqual(expected);
});

View File

@ -18,6 +18,7 @@ import {
updateIconAlignment,
getBasePropertyPath,
hideByColumnType,
uniqueColumnNameValidation,
} from "./propertyUtils";
export default [
@ -49,8 +50,19 @@ export default [
label: "Columns",
updateHook: updateDerivedColumnsHook,
dependencies: ["derivedColumns", "columnOrder"],
isBindProperty: false,
isBindProperty: true,
isTriggerProperty: false,
validation: {
type: ValidationTypes.FUNCTION,
params: {
fn: uniqueColumnNameValidation,
expected: {
type: "Unique Column Names",
example: "abc",
autocompleteDataType: AutocompleteDataType.STRING,
},
},
},
panelConfig: {
editableTitle: true,
titlePropertyName: "label",

View File

@ -125,6 +125,30 @@ export function totalRecordsCountValidation(
};
}
export function uniqueColumnNameValidation(
value: unknown,
props: TableWidgetProps,
_?: any,
) {
const tableColumns = _.map(value, "label");
const duplicates = tableColumns.filter(
(val: string, index: number, arr: string[]) => arr.indexOf(val) !== index,
);
const hasError = !!duplicates.length;
if (value && hasError) {
return {
isValid: false,
parsed: value,
messages: ["Column names should be unique."],
};
}
return {
isValid: true,
parsed: value,
messages: [],
};
}
// A hook to update all column styles when global table styles are updated
export const updateColumnStyles = (
props: TableWidgetProps,