Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add guard clause for flow[nodeId] in autoAnswerableOptions #4123

Merged
merged 4 commits into from
Jan 9, 2025
Merged

fix: add guard clause for flow[nodeId] in autoAnswerableOptions #4123

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f507fb1
add guard clause for flow[nodeId]
RODO94 Jan 9, 2025
ba2d6e8
alter guard clause for single line catch
RODO94 Jan 9, 2025
80ed6d0
revert back to single line return
RODO94 Jan 9, 2025
75d6fcd
add fix to next ref to flow[nodeId]
RODO94 Jan 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 40 additions & 29 deletions editor.planx.uk/src/pages/FlowEditor/lib/store/preview.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,19 +223,27 @@ export const previewStore: StateCreator<
if (passportValue.length > 0) {
const existingValue = acc.data?.[key] ?? [];

const combined = key === planningConstraintsFn
? passportValue.concat(existingValue) // Planning constraints uniquely store all-levels of granularity, rather than most granular only
: passportValue
.concat(existingValue)
.reduce(
(acc: string[], curr: string, _i: number, arr: string[]) => {
if (!arr.some((x) => x !== curr && x?.startsWith(curr))) {
acc.push(curr);
}
return acc;
},
[],
);
const combined =
key === planningConstraintsFn
? passportValue.concat(existingValue) // Planning constraints uniquely store all-levels of granularity, rather than most granular only
: passportValue
.concat(existingValue)
.reduce(
(
acc: string[],
curr: string,
_i: number,
arr: string[],
) => {
if (
!arr.some((x) => x !== curr && x?.startsWith(curr))
) {
acc.push(curr);
}
return acc;
},
[],
);

passportData[key] = uniq(combined);
}
Expand Down Expand Up @@ -414,8 +422,7 @@ export const previewStore: StateCreator<
}));
const hidden = !selections.some(
(selection) =>
selection.data?.flags &&
selection.data.flags.includes(flag?.value)
selection.data?.flags && selection.data.flags.includes(flag?.value),
);

return {
Expand Down Expand Up @@ -521,7 +528,7 @@ export const previewStore: StateCreator<
// Only proceed if the user has seen at least one node with this fn before
const visitedFns = Object.entries(breadcrumbs).filter(
([nodeId, _breadcrumb]) =>
flow[nodeId].data?.fn === data.fn ||
flow[nodeId]?.data?.fn === data.fn ||
// Account for nodes like FindProperty that don't have `data.fn` prop but still set passport vars like `property.region` etc
Object.keys(passport?.data || {}).includes(data.fn),
);
Expand All @@ -530,7 +537,7 @@ export const previewStore: StateCreator<
// For each visited node, get the data values of its' options (aka edges or Answer nodes)
const visitedOptionVals: string[] = [];
visitedFns.forEach(([nodeId, _breadcrumb]) => {
flow[nodeId].edges?.map((edgeId) => {
flow[nodeId]?.edges?.map((edgeId) => {
if (flow[edgeId].type === TYPES.Answer && flow[edgeId].data?.val) {
visitedOptionVals.push(flow[edgeId].data.val);
}
Expand Down Expand Up @@ -563,11 +570,12 @@ export const previewStore: StateCreator<

// Get existing passport value(s) for this node's fn, accounting for Planning Constraints special `_nots`
const passportValues = passport.data?.[data.fn];
const nots: string[] | undefined = passport.data?.["_nots"]?.[planningConstraintsFn];
const nots: string[] | undefined =
passport.data?.["_nots"]?.[planningConstraintsFn];

const foundPassportValues =
Array.isArray(passportValues) && passportValues.length > 0;

// If we have existing passport value(s) for this fn in an eligible automation format (eg not numbers or plain strings),
// then proceed through the matching option(s) or the blank option independent if other vals have been seen before
if (foundPassportValues && data.fn !== planningConstraintsFn) {
Expand Down Expand Up @@ -598,20 +606,22 @@ export const previewStore: StateCreator<
});
});
} else {
if (blankOption?.id) { optionsThatCanBeAutoAnswered.push(blankOption.id) };
if (blankOption?.id) {
optionsThatCanBeAutoAnswered.push(blankOption.id);
}
}
}

if (data.fn === planningConstraintsFn && (foundPassportValues || nots)) {
// Planning constraints queried from an external source are stored via two separate passport vars:
// Planning constraints queried from an external source are stored via two separate passport vars:
// - One for intersections aka `planningConstraintsFn`
// - Another for not-intersections aka `_nots`
const matchingIntersectingConstraints = passportValues?.filter(
(passportValue: any) =>
sortedOptions.some((option) => passportValue === option.data?.val)
sortedOptions.some((option) => passportValue === option.data?.val),
);
const matchingNots = nots?.filter(
(not) => sortedOptions.some((option => not === option.data?.val))
const matchingNots = nots?.filter((not) =>
sortedOptions.some((option) => not === option.data?.val),
);

if (matchingIntersectingConstraints?.length > 0) {
Expand All @@ -629,12 +639,13 @@ export const previewStore: StateCreator<
sortedOptions.forEach((option) => {
nots?.forEach((not) => {
if (not === option.data?.val) {
if (blankOption?.id) optionsThatCanBeAutoAnswered.push(blankOption.id);
if (blankOption?.id)
optionsThatCanBeAutoAnswered.push(blankOption.id);
}
});
});
} else {
// If this node is asking about a constraint that we have NOT queried from an external source,
// If this node is asking about a constraint that we have NOT queried from an external source,
// Then put it to the user exactly once and automate future instances of it
if (blankOption?.id && hasVisitedEveryOption)
optionsThatCanBeAutoAnswered.push(blankOption.id);
Expand Down Expand Up @@ -831,9 +842,9 @@ export const sortBreadcrumbs = (
return editingNodes?.length
? nextBreadcrumbs
: sortIdsDepthFirst(flow)(new Set(Object.keys(nextBreadcrumbs))).reduce(
(acc, id) => ({ ...acc, [id]: nextBreadcrumbs[id] }),
{} as Store.Breadcrumbs,
);
(acc, id) => ({ ...acc, [id]: nextBreadcrumbs[id] }),
{} as Store.Breadcrumbs,
);
};

function handleNodesWithPassport({
Expand Down
Loading