bug: Add validation for active contest type store (#3005) #3006
Conversation
WalkthroughFixes initialization faults in ActiveContestTypeStore by validating stored contest-type keys against available contestTableProviderGroups; invalid or missing values are reset to a default. TaskTable is made defensive against undefined providerGroups. Unit tests expanded to cover invalid/null/sequence cases and cleanup. Changes
Sequence Diagram(s)sequenceDiagram
participant TaskTable as TaskTable component
participant Store as ActiveContestTypeStore
participant LS as localStorage
participant Providers as contestTableProviderGroups
Note over LS,Store: App load / store initialization
LS->>Store: read("activeContestType")
Store->>Providers: query available contest types
alt stored value is valid
Providers-->>Store: contains(key)
Store-->>TaskTable: expose current contest type
else stored value invalid or missing
Providers-->>Store: does not contain(key)
Store->>LS: write(defaultContestType)
Store-->>TaskTable: expose default contest type
end
Note over TaskTable,Providers: rendering providers list
TaskTable->>Providers: access providerGroups (may be undefined)
alt providerGroups defined
Providers-->>TaskTable: getAllProviders() -> list
else providerGroups undefined
Providers-->>TaskTable: undefined -> fallback []
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)src/test/lib/stores/active_contest_type.svelte.test.ts (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/dev-notes/2025-12-30/improve-validation-for-active-contest-type-store/plan.md (1)
7-7: Address markdown formatting issues.The static analysis tool flags minor markdown formatting inconsistencies:
- Line 7: Code block missing language specifier
- Lines 52, 75: Using bold text instead of proper heading tags
These don't affect functionality but improve markdown consistency.
🔎 Proposed fixes
-``` +```text Cannot read properties of undefined (reading 'getAllProviders') at TaskTable.svelte:52:42```diff -**修正内容** +### 修正内容-**修正内容** +### 修正内容Also applies to: 52-52, 75-75
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (4)
docs/dev-notes/2025-12-30/improve-validation-for-active-contest-type-store/plan.mdsrc/lib/components/TaskTables/TaskTable.sveltesrc/lib/stores/active_contest_type.svelte.tssrc/test/lib/stores/active_contest_type.svelte.test.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/lib/stores/active_contest_type.svelte.ts (1)
src/lib/utils/contest_table_provider.ts (2)
ContestTableProviderGroups(1263-1263)contestTableProviderGroups(1242-1261)
src/test/lib/stores/active_contest_type.svelte.test.ts (2)
src/lib/stores/active_contest_type.svelte.ts (1)
ActiveContestTypeStore(18-84)src/lib/utils/contest_table_provider.ts (1)
ContestTableProviderGroups(1263-1263)
🪛 markdownlint-cli2 (0.18.1)
docs/dev-notes/2025-12-30/improve-validation-for-active-contest-type-store/plan.md
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
52-52: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
75-75: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: preview
🔇 Additional comments (7)
docs/dev-notes/2025-12-30/improve-validation-for-active-contest-type-store/plan.md (1)
1-148: Excellent documentation quality.The plan document provides comprehensive coverage of the bug, root cause analysis, implementation approach, and verification steps. The Q&A section adds valuable context about why this issue appeared after JOI additions.
src/lib/components/TaskTables/TaskTable.svelte (1)
49-52: Good defensive programming approach.The type widening to
ContestTableProviderGroup | undefinedand the optional chaining with fallback (providerGroups?.getAllProviders() ?? []) provide robust handling of edge cases. This works well in conjunction with the validation logic added toActiveContestTypeStore, creating a defense-in-depth strategy.src/lib/stores/active_contest_type.svelte.ts (3)
2-5: LGTM! Import enables validation logic.The expanded import correctly includes
contestTableProviderGroupsto support the new validation method.
42-48: Validation logic correctly addresses root cause.The
isValidContestTypemethod properly validates against null, undefined, and invalid keys by checking existence incontestTableProviderGroups. This prevents the undefined reference error that occurred when localStorage contained obsolete contest type keys.
30-34: Constructor properly validates and resets invalid values.The simplified constructor logic delegates to
isValidContestTypeand correctly resets todefaultContestTypewhen localStorage contains invalid values. This ensures the store always has a valid state.src/test/lib/stores/active_contest_type.svelte.test.ts (2)
28-31: Good test isolation improvements.Clearing
mockStorageinbeforeEachensures proper test isolation and prevents state leakage between tests. TheafterEachcleanup is also appropriate.Also applies to: 39-41
96-109: Excellent coverage for validation edge cases.The new tests properly verify that the store resets to default when localStorage contains invalid or null values. These tests directly validate the bug fix.
close #3005
Summary by CodeRabbit
Bug Fixes
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.