feat(condo): DOMA-13327 webhook after subscription activation#7699
feat(condo): DOMA-13327 webhook after subscription activation#7699nomerdvadcatpyat wants to merge 4 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds a subscription activation webhook capability: when a subscription context is marked ChangesSubscription Activation Webhook
Helm Submodule Update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f7b0758c0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| await queueWebhookPayload(context, { | ||
| url: conf['SUBSCRIPTION_ACTIVATED_WEBHOOK_URL'], | ||
| secret: conf['SUBSCRIPTION_ACTIVATED_WEBHOOK_SECRET'], | ||
| eventType: 'subscription.activated', |
There was a problem hiding this comment.
When these env vars are configured, this payload still cannot be created because WebhookPayload.eventType validation only accepts custom events passed through WEBHOOK_EVENTS, and Condo currently registers only payment.status.updated in apps/condo/domains/common/constants/webhooks.js. As a result queueWebhookPayload throws INVALID_EVENT_TYPE for subscription.activated, the catch block logs it, and no activation webhook is ever queued.
Useful? React with 👍 / 👎.
| try { | ||
| const payerOrg = await getById('Organization', invoice.payerOrganization) | ||
| const createdByUser = await getById('User', invoice.createdBy) | ||
| await queueWebhookPayload(context, { |
There was a problem hiding this comment.
This passes the caller's GraphQL context into queueWebhookPayload, but activateSubscriptionContext explicitly allows support users while WebhookPayload.create is restricted to admins only (canManageWebhookPayloads). In the support-user activation path, creating the payload fails with an access error and is swallowed by the catch block, so the subscription is marked DONE without sending the configured webhook.
Useful? React with 👍 / 👎.
| frozenPaymentInfo, | ||
| }) | ||
|
|
||
| if (conf['SUBSCRIPTION_ACTIVATED_WEBHOOK_URL'] && conf['SUBSCRIPTION_ACTIVATED_WEBHOOK_SECRET']) { |
| if (conf['SUBSCRIPTION_ACTIVATED_WEBHOOK_URL'] && conf['SUBSCRIPTION_ACTIVATED_WEBHOOK_SECRET']) { | ||
| try { | ||
| const payerOrg = await getById('Organization', invoice.payerOrganization) | ||
| const createdByUser = await getById('User', invoice.createdBy) |
There was a problem hiding this comment.
add checks on empty User and Organization
|
Maybe add a test: subscription activated -> webhook model created? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/condo/domains/subscription/schema/ActivateSubscriptionContextService.test.js (1)
174-174: ⚡ Quick winAvoid hardcoded expired card date in test fixtures
Line 174 uses
'12/2025', which is already in the past as of June 4, 2026. This makes tests fragile if expiration validation is added or tightened.Proposed fix
const PAYMENT_METHOD = { bindingId: faker.datatype.uuid(), paymentSystem: 'MasterCard', cardNumber: '******4242', - expiration: '12/2025', + expiration: dayjs().add(2, 'year').format('MM/YYYY'), bankName: 'Test Bank', bankCountryCode: 'RU', }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/condo/domains/subscription/schema/ActivateSubscriptionContextService.test.js` at line 174, The test fixture in ActivateSubscriptionContextService.test.js sets expiration: '12/2025' which is now past; update the fixture used in the test (the object that contains the expiration property) to produce a valid future card expiration dynamically (e.g., compute a month/year at least several months/years ahead from current date) instead of a hardcoded literal so the test stays valid over time; adjust any helper used to build the payment/card fixture or the specific test case that references expiration to use this computed value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/condo/domains/subscription/schema/ActivateSubscriptionContextService.test.js`:
- Around line 244-247: The polling condition for "new context created" currently
includes SUBSCRIPTION_CONTEXT_STATUS.ERROR; remove ERROR from the status_in
array passed to SubscriptionContext.getAll so it only checks
SUBSCRIPTION_CONTEXT_STATUS.CREATED and SUBSCRIPTION_CONTEXT_STATUS.PENDING, and
instead add an explicit check in the wait loop to fail fast if any returned
context has status === SUBSCRIPTION_CONTEXT_STATUS.ERROR (so the test fails
immediately rather than passing and crashing later). Update any related
assertions in ActivateSubscriptionContextService.test to expect this early
failure path if applicable.
- Around line 160-168: In the beforeAll/afterAll hooks around the test suite,
preserve and restore any existing environment values instead of unconditionally
deleting them: save original values of
process.env.SUBSCRIPTION_ACTIVATED_WEBHOOK_URL and
SUBSCRIPTION_ACTIVATED_WEBHOOK_SECRET at the start of beforeAll, set the test
values there, and in afterAll restore the originals (or delete only if the
original was undefined). Update the beforeAll/afterAll blocks referenced in the
test file to use these saved variables so other suites keep their prior env
state.
---
Nitpick comments:
In
`@apps/condo/domains/subscription/schema/ActivateSubscriptionContextService.test.js`:
- Line 174: The test fixture in ActivateSubscriptionContextService.test.js sets
expiration: '12/2025' which is now past; update the fixture used in the test
(the object that contains the expiration property) to produce a valid future
card expiration dynamically (e.g., compute a month/year at least several
months/years ahead from current date) instead of a hardcoded literal so the test
stays valid over time; adjust any helper used to build the payment/card fixture
or the specific test case that references expiration to use this computed value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: df1ffbf8-3acc-42d2-9546-ad1157c1ffcb
📒 Files selected for processing (4)
apps/condo/domains/subscription/schema/ActivateSubscriptionContextService.jsapps/condo/domains/subscription/schema/ActivateSubscriptionContextService.test.jsapps/condo/schema.graphqlapps/condo/schema.ts
✅ Files skipped from review due to trivial changes (2)
- apps/condo/schema.graphql
- apps/condo/schema.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/condo/domains/subscription/schema/ActivateSubscriptionContextService.js
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/condo/domains/subscription/schema/ActivateSubscriptionContextService.spec.js (1)
1-28: ⚡ Quick winReorder imports to comply with project guidelines.
The import at Line 1 (
@app/condo/index) is an internal import and should appear after the@open-condopackages, not before the external packages. As per coding guidelines, import groups must follow: builtin → external →@open-condo→ internal.📦 Proposed fix: reorder imports
-const index = require('`@app/condo/index`') const { faker } = require('`@faker-js/faker`') const dayjs = require('dayjs') const { EncryptionManager } = require('`@open-condo/keystone/crypto/EncryptionManager`') const { setFakeClientMode, makeLoggedInAdminClient, waitFor } = require('`@open-condo/keystone/test.utils`') const { WebhookPayload } = require('`@open-condo/webhooks/schema/utils/testSchema`') +const index = require('`@app/condo/index`') const { PAYMENT_DONE_STATUS,As per coding guidelines: "Enforce import order groups: builtin → external →
@open-condo→ internal → sibling → parent with newlines between groups"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/condo/domains/subscription/schema/ActivateSubscriptionContextService.spec.js` around lines 1 - 28, Move the internal import of '`@app/condo/index`' out of the top and reorder imports to follow project groups: builtin → external → `@open-condo` → internal; specifically, keep external imports like 'faker' and 'dayjs' first, then all '`@open-condo/`...' imports (e.g. EncryptionManager, setFakeClientMode, makeLoggedInAdminClient, waitFor, WebhookPayload), and then place the '`@app/condo/index`' import after the `@open-condo` group; ensure blank lines separate these import groups and verify related domain imports (SubscriptionContext, processRecurrentSubscriptionPayments, etc.) remain in the correct project/internal group.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@apps/condo/domains/subscription/schema/ActivateSubscriptionContextService.spec.js`:
- Around line 1-28: Move the internal import of '`@app/condo/index`' out of the
top and reorder imports to follow project groups: builtin → external →
`@open-condo` → internal; specifically, keep external imports like 'faker' and
'dayjs' first, then all '`@open-condo/`...' imports (e.g. EncryptionManager,
setFakeClientMode, makeLoggedInAdminClient, waitFor, WebhookPayload), and then
place the '`@app/condo/index`' import after the `@open-condo` group; ensure blank
lines separate these import groups and verify related domain imports
(SubscriptionContext, processRecurrentSubscriptionPayments, etc.) remain in the
correct project/internal group.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69840d61-a0c5-411a-9702-13bcb141b55b
📒 Files selected for processing (1)
apps/condo/domains/subscription/schema/ActivateSubscriptionContextService.spec.js
feat(condo): DOMA-13327 added tests feat(condo): DOMA-13327 added tests feat(condo): DOMA-13327 added tests
nomerdvadcatpyat
force-pushed
the
feat/condo/DOMA-13327/webhook-after-activate-subscription
branch
from
9cd8178 to
e5561ce
Compare
|
3 participants
Summary by CodeRabbit
New Features
Chores