refactor: use factory pattern to create new flow

[kriptonian1]

User description

Description

refactor the code to use factory pattern for creating new flow

Fixes #26

Developer's checklist

• My PR follows the style guidelines of this project
• I have performed a self-check on my work

If changes are made in the code:

• I have followed the coding guidelines
• My changes in code generate no new warnings
• My changes are breaking another fix/feature of the project
• I have added test cases to show that my feature works
• I have added relevant screenshots in my PR
• There are no UI/UX issues

---

PR Type

Enhancement

---

Description

• Implement factory pattern via createFlow function for consistent flow structure

• Refactor 8 flow files to use createFlow for reduced code duplication

• Extract helper functions for locator resolution and message generation

• Rename message methods to setLoadingMessage, setSuccessMessage, setErrorMessage

---

Diagram Walkthrough

flowchart LR
  A["Individual Flow Files<br/>clickon, input, keyboard, etc."] -->|"Refactored to use"| B["createFlow Factory<br/>Function"]
  B -->|"Provides"| C["Consistent Flow<br/>Structure"]
  C -->|"Includes"| D["Spinner Management<br/>Error Handling<br/>Message Formatting"]
  E["Helper Functions<br/>clickOnLocator<br/>inputDescription<br/>etc."] -->|"Support"| B

Loading

File Walkthrough

Relevant files

Enhancement

9 files create-flow.ts New factory function for creating standardized flows          +60/-0    clickon-flow.ts Refactor to use createFlow factory pattern                              +45/-24  input-flow.ts Refactor with createFlow and extract helper functions        +102/-61 is-disabled-flow.ts Refactor to use createFlow factory pattern                              +43/-25  istitle-flow.ts Refactor to use createFlow factory pattern                              +25/-27  isurl-flow.ts Refactor to use createFlow factory pattern                              +29/-22  isvisible-flow.ts Refactor to use createFlow factory pattern                              +42/-26  keyboard-flow.ts Refactor to use createFlow factory pattern                              +15/-14  waitfor-flow.ts Refactor to use createFlow factory pattern                              +15/-14

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟢	🎫 #26 🟢 Reduce repeated flow code (e.g., spinner logic and error handling) across individual flows. Introduce a factory-pattern function createFlow to standardize and speed up flow creation. Ensure the factory handles flow execution plus customizable success and error behaviors/messages.
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🔴	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Misspelled identifiers: The helper function name isVisisbleDescription is misspelled, reducing readability and making the code less self-documenting. Referred Code function isVisisbleDescription(step: IsVisibleAction): string { let targetDescription: `text: ${string}` | `selector: ${string}`; if (typeof step.isVisible === "string") { targetDescription = `text: ${step.isVisible}`; } else { targetDescription = `selector: ${step.isVisible.selector}`; } return targetDescription; } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Error handler contract: createFlow declares onError as returning an Error but multiple flows implement onError by throwing, which breaks the intended contract and can lead to inconsistent/undefined rethrow behavior. Referred Code /** Custom error handler */ onError?: (error: unknown, step: T) => Error; /** Custom error message for stop spinner (defaults to getMessage) */ setErrorMessage?: (step: T) => string; /** Custom success message for stop spinner (defaults to getMessage) */ setSuccessMessage?: (step: T) => string; } export function createFlow<T extends FlowStep>({ action, execute, setLoadingMessage: getMessage, setSuccessMessage: getSuccessMessage, onError, setErrorMessage: getErrorMessage, }: FlowConfig<T>) { return async ({ page, step }: BaseFlowParam<T>): Promise<void> => { const flowSpinner = spinner(); const message = getMessage(step); ... (clipped 20 lines) Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No audit context: The new createFlow factory emits CLI spinner messages but does not record user ID/timestamp/outcome in a persistent audit log, so it is unclear whether critical actions are audit-trailed elsewhere. Referred Code return async ({ page, step }: BaseFlowParam<T>): Promise<void> => { const flowSpinner = spinner(); const message = getMessage(step); flowSpinner.start(message); try { await execute({ step, page }); const successMsg = getSuccessMessage ? getSuccessMessage(step) : message; flowSpinner.stop(`${chalk.green("✓")} ${successMsg}`); } catch (error) { const errorMsg = getErrorMessage ? getErrorMessage(step) : `${action}: ${message}`; flowSpinner.stop(`${chalk.red("✖")} ${errorMsg}`); if (onError) { throw onError(error, step); } ... (clipped 3 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: User-facing error detail: The spinner stop message prints setErrorMessage/message content (which can include selectors/text) directly to the user-facing CLI output, and it is unclear whether this could expose sensitive/internal details in some workflows. Referred Code const errorMsg = getErrorMessage ? getErrorMessage(step) : `${action}: ${message}`; flowSpinner.stop(`${chalk.red("✖")} ${errorMsg}`); if (onError) { throw onError(error, step); } throw error; Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Unstructured console output: The new factory uses spinner/chalk console messages rather than structured logging, and without knowing what values can appear in selectors/text it is unclear whether sensitive data could be printed. Referred Code const flowSpinner = spinner(); const message = getMessage(step); flowSpinner.start(message); try { await execute({ step, page }); const successMsg = getSuccessMessage ? getSuccessMessage(step) : message; flowSpinner.stop(`${chalk.green("✓")} ${successMsg}`); } catch (error) { const errorMsg = getErrorMessage ? getErrorMessage(step) : `${action}: ${message}`; flowSpinner.stop(`${chalk.red("✖")} ${errorMsg}`); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Selector parsing assumptions: The new selector parsing logic (getActionType/inputValue) assumes well-formed step.input.selector formats and does not validate or handle malformed/hostile values beyond defaulting to selector, which may be acceptable but cannot be confirmed from the diff alone. Referred Code function inputValue({ action, step }: InputValueProp): InputValue { const inputSelector = step.input.selector; switch (action) { case "label": { const labelText = inputSelector.replace("label:", "").trim(); return { action: "label", value: labelText }; } case "placeholder": { const placeholderText = inputSelector.replace("placeholder:", "").trim(); return { action: "placeholder", value: placeholderText }; } case "role": { const roleAndName = inputSelector.replace("role:", "").trim(); const [role, ...nameParts] = roleAndName.split(" "); const name = nameParts.join(" ").replace('name="', "").replace('"', ""); return { action: "role", value: { name, role } }; } case "selector": { return { action: "selector", value: inputSelector }; } ... (clipped 16 lines) Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Simplify the factory by reducing message-related boilerplate Refactor the createFlow factory to accept a single getDescription function instead of separate message-setting functions. This will allow the factory to generate loading, success, and error messages from a single description source, reducing boilerplate in each flow. Examples: src/execution-flow/flows/clickon-flow.ts [29-51] const clickOnFlow = createFlow<ClickAction>({ action: "clickOn", setLoadingMessage: (step) => { const targetDescription = clickOnDescription(step); return `Clicking on element with ${targetDescription}`; }, execute: async (params) => { const targetLocator = clickOnLocator(params); ... (clipped 13 lines) src/execution-flow/flows/is-disabled-flow.ts [34-51] const isDisabledFlow = createFlow<IsDisabledAction>({ action: "isDisabled", setLoadingMessage(step) { const targetDescription = isDisableDescription(step); return `Checking visibility of element with ${targetDescription}`; }, async execute(params) { const targetLocator = isDisabledLocator(params); await expect(targetLocator).toBeDisabled(); }, ... (clipped 8 lines) Solution Walkthrough: Before: // In clickon-flow.ts const clickOnFlow = createFlow<ClickAction>({ action: "clickOn", setLoadingMessage: (step) => { const targetDescription = clickOnDescription(step); // Redundant call return `Clicking on element with ${targetDescription}`; }, execute: async (params) => { ... }, setSuccessMessage: (step) => { const targetDescription = clickOnDescription(step); // Redundant call return `Clicked on element with ${targetDescription}`; }, setErrorMessage: (step) => { const targetDescription = clickOnDescription(step); // Redundant call return `clickOn: Unable to find element with ${targetDescription}`; }, ... }); After: // In a modified create-flow.ts export function createFlow<T>({ getDescription, ... }) { // ... const description = getDescription(step); const loadingMsg = `Action: ${description}`; const successMsg = `Success: ${description}`; const errorMsg = `Error: ${action}: ${description}`; // ... } // In clickon-flow.ts const clickOnFlow = createFlow<ClickAction>({ action: "clickOn", getDescription: (step) => { // Logic to get description is now in one place return clickOnDescription(step); }, execute: async (params) => { ... }, ... }); Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies significant code duplication across the new flow definitions and proposes a valid architectural improvement to the createFlow factory, enhancing maintainability.	Medium
General	Wrap input errors in specific exception Add setErrorMessage and onError handlers to the inputFlow to provide specific error messages and throw a domain-specific ElementNotFoundError on failure. src/execution-flow/flows/input-flow.ts [97-107] export const inputFlow = createFlow<InputAction>({ action: "input", async execute({ page, step }) { const locator = inputLocator({ page, step }); await locator.fill(step.input.value); }, setLoadingMessage(step) { - const target = inputDescription(step); - return `Filling input for ${target}`; + return `Filling input for ${inputDescription(step)}`; + }, + setErrorMessage(step) { + return `input: Unable to fill input for ${inputDescription(step)}`; + }, + onError(_, step) { + throw new ElementNotFoundError(`Input missing: ${inputDescription(step)}`); }, }); Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that the inputFlow is missing explicit error handling, and adding it makes the flow more robust and consistent with other flows in the PR.	Medium
	✅ Improve clarity of URL assertion error Suggestion Impact: Updated the isURL flow error message to say "Page URL does not match" instead of "Page URL is not visible", matching the suggested wording. code diff: setErrorMessage(step) { const { isRegex, value: titleOrRegex } = regexOrStringMaker(step.isURL); const LocatorDescription = isRegex ? "Regex:" : "URL:"; - return `isURL: Page URL is not visible with ${LocatorDescription} "${titleOrRegex}"`; + return `isURL: Page URL does not match ${LocatorDescription} "${titleOrRegex}"`; }, Rephrase the isURL error message from "Page URL is not visible" to "Page URL does not match" for better clarity. src/execution-flow/flows/isurl-flow.ts [27-31] setErrorMessage(step) { const { isRegex, value: titleOrRegex } = regexOrStringMaker(step.isURL); const LocatorDescription = isRegex ? "Regex:" : "URL:"; - return `isURL: Page URL is not visible with ${LocatorDescription} "${titleOrRegex}"`; + return `isURL: Page URL does not match ${LocatorDescription} "${titleOrRegex}"`; }, [Suggestion processed] Suggestion importance[1-10]: 5 __ Why: The suggestion improves the clarity of a user-facing error message by changing confusing wording ("not visible") to a more accurate description ("does not match"), enhancing user experience.	Low
	✅ Correct typo in description function Suggestion Impact: The commit renames the helper function to isVisibleDescription and updates all call sites accordingly. code diff: -function isVisisbleDescription(step: IsVisibleAction): string { +function isVisibleDescription(step: IsVisibleAction): string { let targetDescription: `text: ${string}` | `selector: ${string}`; if (typeof step.isVisible === "string") { targetDescription = `text: ${step.isVisible}`; @@ -33,7 +33,7 @@ const isVisibleFlow = createFlow<IsVisibleAction>({ action: "isVisible", setLoadingMessage(step) { - const targetDescription = isVisisbleDescription(step); + const targetDescription = isVisibleDescription(step); return `Checking visibility of element with ${targetDescription}`; }, async execute(params) { @@ -41,16 +41,16 @@ await expect(targetLocator).toBeVisible(); }, setSuccessMessage(step) { - const targetDescription = isVisisbleDescription(step); + const targetDescription = isVisibleDescription(step); return ` Element is visible with ${targetDescription}`; }, setErrorMessage(step) { - const targetDescription = isVisisbleDescription(step); + const targetDescription = isVisibleDescription(step); return `isVisible: Element is not visible with ${targetDescription}`; Correct the typo in the function name from isVisisbleDescription to isVisibleDescription. src/execution-flow/flows/isvisible-flow.ts [8-16] -function isVisisbleDescription(step: IsVisibleAction): string { +function isVisibleDescription(step: IsVisibleAction): string { let targetDescription: `text: ${string}` | `selector: ${string}`; if (typeof step.isVisible === "string") { targetDescription = `text: ${step.isVisible}`; } else { targetDescription = `selector: ${step.isVisible.selector}`; } return targetDescription; } [Suggestion processed] Suggestion importance[1-10]: 4 __ Why: The suggestion correctly identifies a typo in a function name and proposes a fix, which improves code readability and maintainability.	Low
	Remove extraneous leading space from message Remove the leading space from the success message in the isvisible-flow to prevent a double space in the final output. src/execution-flow/flows/isvisible-flow.ts [43-46] setSuccessMessage(step) { const targetDescription = isVisisbleDescription(step); - return ` Element is visible with ${targetDescription}`; + return `Element is visible with ${targetDescription}`; }, Apply / Chat Suggestion importance[1-10]: 3 __ Why: The suggestion correctly identifies and fixes a minor formatting issue that would cause a double space in the output, improving the UI consistency.	Low
Possible issue	Use regex for robust role parsing Improve role selector parsing by using a regular expression to robustly extract the name attribute's value, handling complex cases correctly. src/execution-flow/flows/input-flow.ts [33-38] case "role": { const roleAndName = inputSelector.replace("role:", "").trim(); - const [role, ...nameParts] = roleAndName.split(" "); - const name = nameParts.join(" ").replace('name="', "").replace('"', ""); + const [role] = roleAndName.split(" "); + const nameMatch = roleAndName.match(/name="([^"]*)"/); + const name = nameMatch ? nameMatch[1] : ""; return { action: "role", value: { name, role } }; } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a fragile string manipulation for parsing role selectors and proposes a more robust regex-based solution, preventing potential bugs with complex attribute values.	Low
Update

[kriptonian1]

/review

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 26 - Partially compliant Compliant requirements: Reduce repeated logic across flow implementations (notably spinner management and error handling). Implement a factory pattern via a createFlow function to create flows faster with no duplicate logic. createFlow should handle execution, custom error handling, and custom success handling/messages. Non-compliant requirements: Requires further human verification: Confirm all refactored flows behave identically to previous behavior in real runs (spinner output, thrown error types, and messages). Confirm all flows intended by the ticket were migrated (diff shows multiple migrations, but not all flows are visible here).

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue The catch block now returns an error object instead of throwing it. This likely changes control flow (the caller may treat it as success), and also risks TypeScript not flagging failures as exceptions. await expect(targetLocator).toBeHidden(); isNotVisibleSpiner.stop( `${chalk.green("✓")} Element is not visible with ${targetDescription}`, ); } catch { isNotVisibleSpiner.stop( `${chalk.red("✖")} isNotVisible: Element is visible with ${targetDescription}`, ); return new FailedAssertionError(`Assertion failed: ${targetDescription}`); } Error Handling When onError wraps the original exception into a new Error, the original error context/stack may be lost. Consider preserving the original error (e.g., via cause) or including details from error in the wrapped error to aid debugging. export function createFlow<T extends FlowStep>({ action, execute, setLoadingMessage: getMessage, setSuccessMessage: getSuccessMessage, onError, setErrorMessage: getErrorMessage, }: FlowConfig<T>) { return async ({ page, step }: BaseFlowParam<T>): Promise<void> => { const flowSpinner = spinner(); const message = getMessage(step); flowSpinner.start(message); try { await execute({ step, page }); const successMsg = getSuccessMessage ? getSuccessMessage(step) : message; flowSpinner.stop(`${chalk.green("✓")} ${successMsg}`); } catch (error) { const errorMsg = getErrorMessage ? getErrorMessage(step) : `${action}: ${message}`; flowSpinner.stop(`${chalk.red("✖")} ${errorMsg}`); if (onError) { throw onError(error, step); } throw error; } Message Accuracy The loading message says "Checking visibility" while the assertion is "toBeDisabled". This can confuse users and makes logs misleading; consider aligning wording with the actual check (disabled state). const isDisabledFlow = createFlow<IsDisabledAction>({ action: "isDisabled", setLoadingMessage(step) { const targetDescription = isDisableDescription(step); return `Checking visibility of element with ${targetDescription}`; }, async execute(params) { const targetLocator = isDisabledLocator(params); await expect(targetLocator).toBeDisabled(); },

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud