Skip to content

Commit

Permalink
fix: do not autocomplete flows with incomplete steps
Browse files Browse the repository at this point in the history
  • Loading branch information
christianmat committed Oct 1, 2024
1 parent 95f66cd commit 96a7862
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 4 deletions.
6 changes: 6 additions & 0 deletions .changeset/silly-bears-teach.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@frigade/js": patch
"@frigade/react": patch
---

Fixes an issue where checklists complete when finishing the last step
1 change: 0 additions & 1 deletion apps/smithy/src/stories/Checklist/Checklist.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ export const Collapsible = {
test: TestStep,
},
width: "500px",
forceMount: true,
dismissible: true,
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { Checklist, useFlow } from "@frigade/react";
import { type Meta, type StoryObj } from "@storybook/react";
import { expect, userEvent, within } from "@storybook/test";

type ChecklistStory = StoryObj<typeof Checklist>;

function sleep(ms: number) {
return new Promise((resolve) => setTimeout(resolve, ms));
}

const StoryMeta: Meta<typeof Checklist.Collapsible> = {
title: "Components/Checklist",
component: Checklist.Collapsible,
};

export default StoryMeta;

export const InteractionTests: ChecklistStory = {
args: {
flowId: "flow_K2dmIlteW8eGbGoo",
},
decorators: [
(Story, { args }) => {
const { flow } = useFlow(args.flowId);

return (
<>
<Story {...args} />
<button
id="reset-flow"
onClick={() => {
flow?.restart();
}}
style={{ marginTop: "36px" }}
>
Reset Flow
</button>
</>
);
},
],

play: async ({ step, canvasElement }) => {
await sleep(1000);

This comment has been minimized.

Copy link
@dfltr

dfltr Oct 1, 2024

Contributor

Instead of sleeping, I think you can do:

await waitFor(() => {
  expect(checklistElement).toBeInTheDocument()
})

to wait for the initial render.

This comment has been minimized.

Copy link
@christianmat

christianmat Oct 1, 2024

Author Contributor

Much cleaner, I can do this a few places

const canvas = within(canvasElement);
const checklistElement = document.querySelector(".fr-checklist");

await step("Test interactions", async () => {
// test that checklistElement is in the document
expect(checklistElement).toBeInTheDocument();

const lastHeader = canvas.getByText("Donec id diam lectus");
let checkboxes = document.querySelectorAll(".fr-field-radio-value");

// expectd lastCheckbox to not have any children
expect(checkboxes[checkboxes.length - 1]?.childNodes.length).toBe(0);
// click last header
await userEvent.click(lastHeader);
// Click the primary button that says "Testing"
await userEvent.click(canvas.getByText("Testing"));
await sleep(100);

checkboxes = document.querySelectorAll(".fr-field-radio-value");
// expect lastCheckbox to have a child (it is checked)
expect(
checkboxes[checkboxes.length - 1]?.childNodes.length
).toBeGreaterThanOrEqual(1);
await sleep(1000);
expect(checklistElement).toBeInTheDocument();

await userEvent.click(canvas.getByText("Reset Flow"));
});
},
};
12 changes: 9 additions & 3 deletions packages/js-api/src/core/flow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,11 @@ export class Flow extends Fetchable {
}

const isLastStep = this.getCurrentStepOrder() + 1 === this.getNumberOfAvailableSteps()
const isLastIncompleteStep =
Array.from(this.steps.values()).filter(
(step) => step.$state.visible && !step.$state.completed && !step.$state.skipped
).length === 1

if (optimistic) {
const copy = clone(this.getGlobalState().flowStates[this.id])

Expand All @@ -249,15 +254,16 @@ export class Flow extends Fetchable {
copy.$state.currentStepIndex = nextStepIndex
copy.data.steps[nextStepIndex].$state.started = true
}
} else {
}
if (isLastIncompleteStep) {
copy.$state.completed = true
copy.$state.visible = false
}

this.getGlobalState().flowStates[this.id] = copy
this.resyncState()

if (isLastStep) {
if (isLastIncompleteStep) {
this.optimisticallyMarkFlowCompleted()
}
}
Expand All @@ -267,7 +273,7 @@ export class Flow extends Fetchable {
properties,
thisStep.id
)
if (isLastStep) {
if (isLastIncompleteStep) {
await this.sendFlowStateToAPI(COMPLETED_FLOW, properties)
}
}
Expand Down

0 comments on commit 96a7862

Please sign in to comment.