Skip to content

Conversation

@cubic-dev-local
Copy link

@cubic-dev-local cubic-dev-local bot commented Aug 29, 2025

Evaluation Test Case - Full PR Replication

This PR is a complete replication of PR calcom#23408 from calcom/cal.com for AI code review evaluation.

Test Details:


Original PR Description:

What does this PR do?

This PR optimizes payment app imports by creating a dedicated PaymentServiceMap that only loads payment apps with PaymentService implementations, avoiding the need to load the entire app store when only payment functionality is needed. This follows the same optimization pattern established in PR calcom#23372 for analytics services.

Key changes:

  • Adds PaymentServiceMap generation to the app-store-cli build process
  • Creates payment.services.generated.ts with lazy imports for 6 payment services (alby, btcpayserver, hitpay, mock-payment-app, paypal, stripepayment)
  • Updates 6 files to use PaymentServiceMap instead of importing the full appStore

Performance benefits:

  • Reduces bundle size by avoiding import of 100+ apps when only payment functionality is needed
  • Uses lazy imports for better code splitting
  • Loads only 6 payment services instead of entire app store

How should this be tested?

Critical testing areas:

  1. Payment flows: Test all payment providers (PayPal, Stripe, Alby, HitPay, BTCPay Server) to ensure payment creation, charging, and refunds still work
  2. E2E tests: Verify E2E tests pass with the conditional PaymentServiceMap logic
  3. Build process: Confirm yarn app-store:build generates payment.services.generated.ts correctly
  4. Connected apps: Test the connected apps page shows payment app setup status correctly

Test configuration:

  • Set up credentials for at least one payment provider (Stripe recommended)
  • Create test bookings with payment requirements
  • Test both successful payments and error scenarios

Expected behavior:

  • All existing payment functionality should work identically
  • No performance regressions in payment flows
  • Generated file should contain imports for all 6 payment services

Checklist

  • I have self-reviewed the code
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. N/A - This is an internal optimization that doesn't change public APIs
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

Link to Devin run: https://app.devin.ai/sessions/6ff5d1bb0e98478180209c3a930013c2
Requested by: @keithwillcode

⚠️ Important for reviewers:

  • This PR changes how payment services are imported at runtime - please verify all payment flows work correctly
  • The chargeCard method signature was updated to include booking.id - confirm this matches all PaymentService implementations
  • Test that the generated PaymentServiceMap contains all expected payment services after running yarn app-store:build

This PR is automatically generated to test if our AI review system can consistently detect issues across the entire changeset.


Summary by cubic

Optimizes payment imports by generating a lazy‑loaded PaymentServiceMap and updating payment flows to use it instead of loading the entire app store. Reduces bundle size and loads only the needed payment providers.

  • Refactors
    • app-store-cli generates payment.services.generated.ts with lazy imports for alby, btcpayserver, hitpay, mock-payment-app, paypal, stripepayment.
    • Replace appStore usage with PaymentServiceMap in getConnectedApps, handlePayment, handlePaymentRefund, deletePayment, and tRPC payment routes.
    • Switch to direct PaymentService export (remove .lib wrapper).
    • Pass booking.id to chargeCard calls.
    • Tests: add vitest setup to mock PaymentServiceMap and PaymentService; remove obsolete appStore mocks.

devin-ai-integration bot and others added 9 commits August 27, 2025 18:02
- Add PaymentServiceMap generation to app-store-cli build process
- Generate payment.services.generated.ts with lazy imports for 6 payment services
- Update handlePayment.ts, deletePayment.ts, handlePaymentRefund.ts to use PaymentServiceMap
- Update getConnectedApps.ts and tRPC payment routers to use PaymentServiceMap
- Follow same pattern as analytics optimization in PR calcom#23372
- Reduces bundle size by avoiding import of 100+ apps when only payment functionality needed

Co-Authored-By: [email protected] <[email protected]>
- Remove obsolete appStoreMock line from bookingScenario.ts since handlePayment now uses PaymentServiceMap
- Update setupVitest.ts to import prismaMock from correct PrismockClient instance
- Add PaymentServiceMap mock following PR calcom#22450 pattern for calendar services
- Ensure MockPaymentService uses consistent externalId across test files
- Fix webhook handler to return 200 status by ensuring payment records are found correctly

Co-Authored-By: [email protected] <[email protected]>
…vi.spyOn() calls

- Remove global prismaMock import from setupVitest.ts that was causing 'is not a spy' errors
- Update MockPaymentService to import prismaMock locally to maintain payment test functionality
- Fixes organization and outOfOffice tests while preserving payment service optimization

Co-Authored-By: [email protected] <[email protected]>
- Payment services map now always includes all payment apps regardless of E2E environment
- Ensures payment functionality is consistently available across all environments
- Addresses CI failures caused by conditional payment service loading

Co-Authored-By: [email protected] <[email protected]>
- Update app-store-cli to import directly from lib/PaymentService.ts files
- Modify all payment handlers to access PaymentService directly
- Update test mocks to match new direct import structure
- Remove .lib property access pattern across payment system
- Maintain backward compatibility while improving import efficiency

Co-Authored-By: [email protected] <[email protected]>
- Remove booking.id parameter from chargeCard calls in chargeCard.handler.ts and payments.tsx
- Addresses GitHub feedback to investigate chargeCard signature changes in separate PR
- Keeps all other direct PaymentService import refactor changes intact

Co-Authored-By: [email protected] <[email protected]>
@github-actions
Copy link

github-actions bot commented Aug 29, 2025

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "[Eval f0lhdb08jy] perf: optimize payment app imports to avoid loading entire app store". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

@github-actions
Copy link

This PR is being marked as stale due to inactivity.

@github-actions github-actions bot added the Stale label Sep 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant