Skip to content

Conversation

@shuhuiluo
Copy link
Collaborator

@shuhuiluo shuhuiluo commented Nov 27, 2025

  • Add DEFAULT_EVENT_TYPES_ARRAY and ALLOWED_EVENT_TYPES_SET constants
  • Change SubscribeParams.eventTypes from string to EventType[]
  • Update parseEventTypes() to return EventType[] directly
  • Update formatEventTypes() to accept EventType[]
  • Update isEventTypeMatch() to accept EventType[]
  • Extract requiresInstallationFailure() helper to reduce duplication
  • Update oauth types to use z.enum(ALLOWED_EVENT_TYPES)
  • Convert to/from string only at database boundaries

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor

    • Improved type safety and data handling for event subscriptions, yielding more consistent validation and processing.
    • Optimized event-type structures for faster, more reliable filtering.
  • Bug Fixes

    • Fixed subscription matching so empty selections behave as match-all and validation is more efficient.
  • UI

    • Consistent, properly escaped display of selected event types on OAuth success pages.
  • Tests

    • Updated tests to reflect array-based event-type handling.

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

@coderabbitai
Copy link

coderabbitai bot commented Nov 27, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This PR converts event-type handling from comma-separated strings to a typed EventType[] across the codebase. It adds DEFAULT_EVENT_TYPES_ARRAY and ALLOWED_EVENT_TYPES_SET to constants, updates parsing/formatting and validation to use typed arrays and Set lookups, and changes function signatures in handlers and services to accept/return EventType[]. Persistence still stores event types as comma-delimited strings with conversions at storage/retrieval boundaries. Tests and OAuth redirect schema/pages are adjusted to accept and render arrays.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Pay attention to conversion points where EventType[] ↔ comma-delimited string for DB persistence (subscription-service).
  • Verify signature/API changes and exports in github-subscription-handler, polling-service, and related callers.
  • Confirm isEventTypeMatch behavior and EVENT_TYPE_MAP lookups now iterate arrays correctly.
  • Validate OAuth schema and redirect handling (types/oauth.ts, routes/oauth-callback.ts) and UI rendering changes in views/oauth-pages.ts.
  • Run/update unit tests that were modified to ensure no behavioral regressions.

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main refactoring change: introducing EventType[] for type-safe event handling in application code while maintaining string representation only at database boundaries.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 673df4d and 6cc3f68.

📒 Files selected for processing (8)
  • src/constants.ts (2 hunks)
  • src/handlers/github-subscription-handler.ts (6 hunks)
  • src/routes/oauth-callback.ts (2 hunks)
  • src/services/polling-service.ts (1 hunks)
  • src/services/subscription-service.ts (15 hunks)
  • src/types/oauth.ts (1 hunks)
  • src/views/oauth-pages.ts (4 hunks)
  • tests/unit/handlers/github-subscription-handler.test.ts (6 hunks)

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

- Add DEFAULT_EVENT_TYPES_ARRAY and ALLOWED_EVENT_TYPES_SET constants
- Change SubscribeParams.eventTypes from string to EventType[]
- Update parseEventTypes() to return EventType[] directly
- Update formatEventTypes() to accept EventType[]
- Update isEventTypeMatch() to accept EventType[]
- Extract requiresInstallationFailure() helper to reduce duplication
- Update oauth types to use z.enum(ALLOWED_EVENT_TYPES)
- Convert to/from string only at database boundaries

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@shuhuiluo shuhuiluo force-pushed the refactor/event-types-array branch from 673df4d to 6cc3f68 Compare November 27, 2025 07:31
@shuhuiluo shuhuiluo merged commit 5c28373 into main Nov 27, 2025
1 of 2 checks passed
@shuhuiluo shuhuiluo deleted the refactor/event-types-array branch November 27, 2025 07:32
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.

2 participants