fix: handle undefined steps field for legacy features

[AhamSammich]

Fix TypeError when toggling 'Enable automated testing' on existing features. Add backward compatibility migration for steps field in use-board-features.ts and defensive checks in testing-tab-content.tsx.

Fixes #198

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of verification steps to prevent errors when data is unavailable or undefined.

• Refactor

  • Enhanced type safety throughout the board features system.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[gemini-code-assist]

Summary of Changes

Hello @AhamSammich, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request provides a critical fix for a TypeError encountered when interacting with the automated testing functionality for features that predate the introduction of the 'steps' field. By implementing backward compatibility during feature loading and adding robust defensive checks in the UI, the changes ensure a more stable and resilient user experience, preventing crashes and improving the handling of legacy data.

Highlights

• Bug Fix: Resolves a TypeError that occurred when enabling automated testing on existing features, specifically when the 'steps' field was undefined.
• Backward Compatibility: Introduces a migration in the feature loading logic to ensure that legacy features without a defined 'steps' field are initialized with an empty array, preventing runtime errors.
• Defensive Programming: Adds defensive checks in the UI component responsible for displaying testing steps, ensuring that operations on the 'steps' array gracefully handle cases where it might be null or undefined.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[coderabbitai]

Walkthrough

This PR fixes a bug where toggling the "Enable automated testing" checkbox crashes the app due to missing steps field in older persisted features. It adds data migration at load time to provide default empty arrays and includes defensive null-checks in the component to handle undefined steps gracefully.

Changes

Cohort / File(s)	Change Summary
Data Migration & Type Safety apps/ui/src/components/views/board-view/hooks/use-board-features.ts	Changed Feature import from value to type; updated callback parameter typing from any to Feature; added steps: f.steps || [] to ensure backward compatibility for old persisted features lacking the steps field
Defensive Component Updates apps/ui/src/components/views/board-view/shared/testing-tab-content.tsx	Added null-safety guards throughout: changed [...steps] to [...(steps || [])] in handleStepChange and handleAddStep; replaced steps.map() with Array.isArray(steps) && steps.map() in render logic to prevent crashes when steps is undefined

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Changes are straightforward defensive coding and data normalization patterns applied consistently across two files
• Type import conversion is mechanical and low-risk
• Null-safety checks follow a predictable pattern with no complex branching logic

Possibly related PRs

• Refactor kanban cards #194: Adds typed Feature mapping with default steps initialization and guards steps usage in kanban-card rendering—directly complements this PR's data migration and steps field handling

Poem

🐰 A hop, a skip, a bug we'd found,
Where steps were lost without a sound,
Now safely guarded, defaults set,
No crashes here—just features yet!
The testing tab hops on with glee, ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and accurately summarizes the main purpose of the PR: handling undefined steps field for legacy features.
Linked Issues check	✅ Passed	The PR fully implements both proposed solutions from issue #198: adds steps migration in use-board-features.ts and adds defensive checks in testing-tab-content.tsx.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the TypeError by addressing the missing steps field in legacy features, with no extraneous modifications.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[gemini-code-assist]

Code Review

This pull request effectively addresses a TypeError for legacy features that are missing the steps field. It introduces a migration in use-board-features.ts to ensure steps is always an array when features are loaded, which is a solid approach to fix the root cause. Additionally, defensive checks have been added to testing-tab-content.tsx to make the component more robust. The changes are well-targeted and correct. I've added a couple of minor suggestions to improve consistency and type correctness.

[gemini-code-assist]

While I appreciate the move towards stronger types by changing f: any to f: Feature, it introduces a slight logical inconsistency. The Feature type defines steps as a required string[], but the new logic steps: f.steps || [] correctly assumes steps might be missing on legacy feature objects. To be more explicit about this data transformation from a potentially incomplete object to a valid Feature object, it would be more accurate to type f as any or a partial type.

Suggested change

	(f: Feature, index: number) => ({
	(f: any, index: number) => ({

[gemini-code-assist]

For consistency with the other defensive checks in this file (which use || []), you could use (steps || []).map(...) here as well. It's slightly more concise and achieves the same goal.

Suggested change

	{Array.isArray(steps) && steps.map((step, index) => (
	{(steps || []).map((step, index) => (

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

apps/ui/src/components/views/board-view/shared/testing-tab-content.tsx (1)

8-14: Fix the type definition to reflect runtime handling of undefined steps.

The interface declares steps: string[] (line 11) as a non-optional, always-present array. However, the implementation throughout this component guards against undefined (lines 26, 32, 63), indicating that steps can actually be undefined at runtime.

This creates a type safety gap:

• TypeScript won't warn callers that passing undefined is handled
• The declared API contract doesn't match the implementation
• Type checking won't catch potential bugs

Update the interface to accurately reflect the implementation:

 interface TestingTabContentProps {
   skipTests: boolean;
   onSkipTestsChange: (skipTests: boolean) => void;
-  steps: string[];
+  steps?: string[];
   onStepsChange: (steps: string[]) => void;
   testIdPrefix?: string;
 }

This makes the type system aware of the defensive handling and improves type safety across call sites.

🧹 Nitpick comments (1)

apps/ui/src/components/views/board-view/hooks/use-board-features.ts (1)

2-2: Good TypeScript best practice with type-only import.

Using type Feature for the import correctly marks it as a type-only import, which helps TypeScript and bundlers optimize the build by ensuring this import is stripped at runtime.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 823e42e and ce13870.

📒 Files selected for processing (2)

• apps/ui/src/components/views/board-view/hooks/use-board-features.ts (2 hunks)
• apps/ui/src/components/views/board-view/shared/testing-tab-content.tsx (2 hunks)

🔇 Additional comments (4)

apps/ui/src/components/views/board-view/shared/testing-tab-content.tsx (3)

25-29: Good defensive check for backward compatibility.

The guard steps || [] correctly handles legacy features where steps might be undefined, preventing the TypeError mentioned in issue #198.

---

31-33: Good defensive check when adding new steps.

The guard steps || [] ensures that adding a step works even when steps is undefined, maintaining consistency with the defensive approach.

---

63-71: Excellent defense-in-depth with Array.isArray check.

Using Array.isArray(steps) && steps.map(...) is more robust than just steps?.map(...) because it handles edge cases where steps might be a non-array value (e.g., if corrupted data is loaded). This provides strong runtime safety for the rendering logic.

apps/ui/src/components/views/board-view/hooks/use-board-features.ts (1)

62-73: Excellent backward compatibility migration for legacy features.

The changes here address the root cause identified in issue #198:

1. Line 63: Proper typing (Feature instead of any) improves type safety
2. Line 71: The steps: f.steps || [] migration ensures legacy features that lack the steps field receive an empty array, preventing the TypeError when steps.map() is called downstream

This is the primary fix mentioned in the PR objectives and correctly handles the data migration at load time, ensuring all features have a valid steps array before being used by the UI components.

[webdevcody]

the steps was actually removed completely from the application, so I'm thinking maybe some stuff was left around after removing

[webdevcody]

I will close for now, reopen if this is still an issue on main, thank you for the initial pr