Skip to content

Conversation

@cabljac
Copy link
Collaborator

@cabljac cabljac commented Jul 7, 2025

Should fix #699

TODO:

  1. create RC
  2. manual test

@cabljac cabljac requested a review from Copilot July 7, 2025 13:16
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR defers creation of the EventArc channel until after the Firebase Admin app is initialized by replacing the static eventChannel export with a getEventChannel() factory and updating all usages accordingly.

  • Replace eventChannel import with getEventChannel() in index.ts and webhook-events.ts
  • Update config.ts to provide a getEventChannel() function instead of a static export
  • Adjust unit tests and environment setup to reflect renamed functions and variables

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
functions/src/index.ts Import and invoke getEventChannel() after initializeApp()
functions/src/handlers/webhook-events.ts Import and invoke getEventChannel() inside HTTP handler
functions/src/config.ts Replace eventChannel constant with getEventChannel()
functions/tests/tests/unit/handlers/webhook-events.test.ts Mock getEventChannel() instead of eventChannel
functions/tests/helpers/setupEnvironment.ts Rename environment-path variables to consistent camelCase
Files not reviewed (1)
  • firestore-stripe-payments/functions/package-lock.json: Language not supported
Comments suppressed due to low confidence (2)

firestore-stripe-payments/functions/src/config.ts:55

  • [nitpick] Add a JSDoc comment to getEventChannel describing its return type (Channel | undefined) and when it returns undefined for improved clarity.
export const getEventChannel = () => {

firestore-stripe-payments/functions/src/index.ts:21

  • The default config import isn't used in this file—consider removing it to keep imports focused.
import config from './config';

import Stripe from 'stripe';
import * as logs from '../logs';
import config, { eventChannel, stripe } from '../config';
import config, { getEventChannel, stripe } from '../config';
Copy link

Copilot AI Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default config import appears unused here; you can remove it to reduce unnecessary imports.

Suggested change
import config, { getEventChannel, stripe } from '../config';
import { getEventChannel, stripe, stripeWebhookSecret } from '../config';

Copilot uses AI. Check for mistakes.
},
},
eventChannel: null,
getEventChannel: () => null,
Copy link

Copilot AI Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The real getEventChannel() returns undefined when EVENTARC_CHANNEL is not set; mock it to return undefined instead of null to better mirror production behavior.

Suggested change
getEventChannel: () => null,
getEventChannel: () => undefined,

Copilot uses AI. Check for mistakes.
@cabljac cabljac force-pushed the fix-event-channel branch from 0b275ea to 3814699 Compare July 7, 2025 13:57
@cabljac cabljac force-pushed the fix-event-channel branch from ae317b2 to dc0ee62 Compare July 7, 2025 14:57
@cabljac cabljac merged commit 96df628 into next Jul 7, 2025
3 checks passed
@cabljac cabljac deleted the fix-event-channel branch July 7, 2025 16:14
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.

Function deployment failed

3 participants