Skip to content

Conversation

@marcodejongh
Copy link
Owner

…search

Extend the hold filter dropdown to include specific hold states beyond Include/Exclude. Users can now filter climbs that have specific holds marked as Starting, Hand, Foot, or Finish holds.

  • Add STARTING, HAND, FOOT, FINISH options to filter dropdown
  • Add color configuration for each hold state filter
  • Update tag display to show counts for all selected hold states
  • Add holdStateFilters support to backend package to match web package

…search

Extend the hold filter dropdown to include specific hold states beyond
Include/Exclude. Users can now filter climbs that have specific holds
marked as Starting, Hand, Foot, or Finish holds.

- Add STARTING, HAND, FOOT, FINISH options to filter dropdown
- Add color configuration for each hold state filter
- Update tag display to show counts for all selected hold states
- Add holdStateFilters support to backend package to match web package
@vercel
Copy link

vercel bot commented Dec 31, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
boardsesh Error Error Jan 1, 2026 4:42am

@claude
Copy link

claude bot commented Dec 31, 2025

Claude Review

packages/web/app/components/search-drawer/climb-hold-search-form.tsx

  • Lines 10-17: Hardcoded color values violate CLAUDE.md guidelines - should use design tokens from theme-config.ts instead of #06B6D4, #EF4444, #00FF00, etc.
  • Line 52: Uses emoji characters (✋, 👣) for icons - consider using consistent Ant Design icons for uniformity with Include/Exclude/Starting/Finish

packages/backend/src/db/queries/climbs/create-climb-filters.ts

  • Line 5: Duplicates HoldState type definition - this type already exists in packages/web/app/lib/types.ts:61 and should be shared from packages/shared-schema
  • Line 24: The holdsFilter type accepts both HoldState and { state: HoldState } - this dual format adds complexity; consider normalizing during URL parsing instead of handling both forms in the filter logic

- Use themeTokens for UI colors (ANY/NOT) and HOLD_STATE_MAP for
  board-specific hold state colors
- Replace emoji icons with consistent Ant Design icons
- Import HoldState from @boardsesh/shared-schema instead of duplicating
- Update ClimbSearchInput in shared-schema to support all hold states
@claude
Copy link

claude bot commented Dec 31, 2025

Claude Review

packages/backend/src/db/queries/climbs/create-climb-filters.ts:141-147

  • Potential SQL injection risk: The state parameter is inserted directly into the SQL query via ${state}. While it's filtered to only STARTING/HAND/FOOT/FINISH strings at line 74, consider using parameterized queries for the state value or validating against an explicit enum.

packages/backend/src/db/queries/climbs/create-climb-filters.ts:23

  • Type inconsistency: ClimbSearchParams.holdsFilter type is Record<string, HoldState | { state: HoldState }> but ClimbSearchInput.holdsFilter in shared-schema is Record<string, HoldState>. This mismatch could cause runtime errors when data flows between packages.

packages/web/app/components/search-drawer/climb-hold-search-form.tsx:15-22

  • Hardcoded dependency on 'kilter' board: HOLD_STATE_COLORS always uses HOLD_STATE_MAP.kilter[...] colors regardless of which board the user is on. This could show incorrect colors for Tension boards.

packages/shared-schema/src/types.ts:7

  • 'OFF' state missing from filter options: HoldState includes 'OFF' but it's not handled in the filter dropdown or backend processing - may cause unexpected behavior if 'OFF' is ever used.

- Add type guard validation for hold states before SQL query to prevent
  injection (VALID_HOLD_STATE_FILTERS const with isValidHoldStateFilter)
- Simplify ClimbSearchParams.holdsFilter type to Record<string, HoldState>
- Use board-specific colors via getHoldStateColors(boardName) instead of
  hardcoded kilter colors
- Document that OFF state is intentionally excluded from filter options
@claude
Copy link

claude bot commented Dec 31, 2025

Claude Review

Type Safety Issue - create-climb-filters.ts:31: The holdsFilter type changed to Record<string, HoldState> which includes OFF, but OFF values are silently filtered out at line 66. Consider narrowing the type at the API boundary to exclude OFF rather than filtering it at runtime.

Color Inconsistency - climb-hold-search-form.tsx:21-28: The getHoldStateColors function uses colors from HOLD_STATE_MAP for state-specific holds but uses themeTokens.colors.primary/error for ANY/NOT. This creates visual inconsistency - the board-rendered holds will show different colors than what the filter dropdown implies.

Missing Tests - packages/backend/src/__tests__/climb-queries.test.ts: No tests for the new holdStateFilters functionality. The existing tests cover basic filters but not the new STARTING/HAND/FOOT/FINISH state filters or the holdsFilter parameter at all.

- Explicitly filter out 'OFF' state in backend (it means "hold not used")
- Skip 'OFF' state when counting holds for tag display in UI
- Add comments explaining SQL parameterization safety via type guard
- Add defensive null check with comment for unknown states in tag render
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants