Skip to content

Commit

Permalink
#8636: fix search/insert for variable popover using optional chaining
Browse files Browse the repository at this point in the history
  • Loading branch information
twschiller committed Jun 16, 2024
1 parent 6672d13 commit 0b64c91
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ const VarPopup: React.FunctionComponent<VarPopupProps> = ({
(selectedPath: string[]) => {
reportEvent(Events.VAR_POPOVER_SELECT);

// FIXME: if the user is editing with optional chaining, those will be removed
const fullVariableName = getPathFromArray(selectedPath);

switch (inputMode) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,15 @@ describe("filterVarMapByVariable", () => {
}),
);

// Exact match with chaining
expect(filterVarMapByVariable(inputMap, "@input?.foo")).toEqual(
expect.objectContaining({
"@input": expect.objectContaining({
foo: expect.toBeObject(),
}),
}),
);

// Empty because trailing period indicates final variable name
expect(filterVarMapByVariable(inputMap, "@input.fo.")).toEqual(
expect.objectContaining({
Expand Down
46 changes: 23 additions & 23 deletions src/components/fields/schemaFields/widgets/varPopup/menuFilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,20 @@ import { getIn } from "formik";
*/
export type MenuOptions = Array<[string, UnknownRecord]>;

/**
* Returns true if the value is null or is likely plain text/not a variable.
*/
function isTextOrNull(value: string | null): boolean {
return value == null || value === "@" || !value.startsWith("@");
}

/**
* Convert a variable to a normalized path, with support for optional chaining.
*/
function toNormalizedPath(value: string): string[] {
return toPath(value.replaceAll("?.", "."));
}

/**
* Filter top-level variables by source type. Currently, excludes integration variables because they're managed
* automatically by the Page Editor.
Expand All @@ -54,15 +68,11 @@ export function filterOptionsByVariable(
options: MenuOptions,
likelyVariable: string,
): MenuOptions {
if (
likelyVariable == null ||
likelyVariable === "@" ||
!likelyVariable.startsWith("@")
) {
if (isTextOrNull(likelyVariable)) {
return options;
}

const [base, ...rest] = toPath(likelyVariable);
const [base, ...rest] = toNormalizedPath(likelyVariable);
return options.filter(([source, vars]) => {
if (rest.length === 0) {
return Object.keys(vars).some((x) => x.startsWith(base));
Expand Down Expand Up @@ -114,15 +124,11 @@ export function filterVarMapByVariable(
varMap: UnknownRecord,
likelyVariable: string,
): UnknownRecord {
if (
likelyVariable == null ||
likelyVariable === "@" ||
!likelyVariable.startsWith("@")
) {
if (isTextOrNull(likelyVariable)) {
return varMap;
}

return filterVarMapByPath(varMap, toPath(likelyVariable));
return filterVarMapByPath(varMap, toNormalizedPath(likelyVariable));
}

/**
Expand All @@ -134,17 +140,13 @@ export function expandCurrentVariableLevel(
varMap: UnknownRecord,
likelyVariable: string,
): ShouldExpandNodeInitially {
if (
likelyVariable == null ||
likelyVariable === "@" ||
!likelyVariable.startsWith("@")
) {
if (isTextOrNull(likelyVariable)) {
return () => false;
}

// If likelyVariable ends with ".", there's a part for the empty string at the end of the path. So can just use
// as normal without logic for trailing "."
const parts = toPath(likelyVariable);
const parts = toNormalizedPath(likelyVariable);
return (keyPath, data, level) => {
// Key path from JSONTree is in reverse order
const reverseKeyPath = [...keyPath].reverse();
Expand Down Expand Up @@ -242,10 +244,8 @@ export function defaultMenuOption(
}

if (
likelyVariable == null ||
likelyVariable === "@" ||
!likelyVariable.startsWith("@") ||
toPath(likelyVariable).length === 0
isTextOrNull(likelyVariable) ||
toNormalizedPath(likelyVariable).length === 0
) {
// Must always have at least one option (e.g., the `@input`)
// Prefer the last option, because that's the latest output
Expand All @@ -255,7 +255,7 @@ export function defaultMenuOption(
return [first];
}

const parts = toPath(likelyVariable);
const parts = toNormalizedPath(likelyVariable);

const [head, ...rest] = parts;

Expand Down

0 comments on commit 0b64c91

Please sign in to comment.