refactor(condo): DOMA-13058 refactor usage of acquiring context#7383
Merged
vovaaxeapolla merged 18 commits intomainfrom Apr 13, 2026
Merged
Conversation
Contributor
|
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:
📝 WalkthroughWalkthroughMigrates acquiring context usage from a single object to arrays, tightens AcquiringIntegrationContext queries to require online-processing and non-deleted integrations, restructures registerMultiPayment into loaders/validators/builders/helpers with new error/domain constants, removes some lodash usages, and marks ServiceConsumer.acquiringIntegrationContext as deprecated. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as registerMultiPayment
participant Validators
participant Loaders
participant Builders
participant Helpers
participant DB
Client->>API: submit registerMultiPayment request
API->>Validators: detectRequestMode -> run validations
API->>Loaders: load receipts/invoices, consumers, billing, acquiring contexts
Loaders->>DB: query entities (AcquiringIntegrationContext with type=OnlineProcessing)
DB-->>Loaders: return entities
API->>Builders: build paymentCreateInputs (receipts or invoices)
Builders->>DB: freeze receipts/invoices (updates)
DB-->>Builders: confirm freezes
API->>Helpers: calculateTotals, assert limits, buildOutputUrls
API-->>Client: return multiPayment result (id, urls, totals, errors)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
vovaaxeapolla
added
the
🔬 WIP
![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: cdab221a60
ℹ️ 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".
| } | ||
|
|
||
| if (billingContexts.length && acquiringCtx && get(acquiringCtx, 'status') === CONTEXT_FINISHED_STATUS) { | ||
| if (billingContexts.length && acquiringContexts.length) { |
There was a problem hiding this comment.
This condition now renders BillingPageContent whenever any acquiring context exists, but the query above includes CONTEXT_VERIFICATION_STATUS and does not filter by integration type. Organizations that only have a verification (or non-online-processing) context can therefore bypass onboarding and land on the billing UI in a not-ready state, where payment flows use invalid contexts. Please gate this branch on an ONLINE_PROCESSING context in CONTEXT_FINISHED_STATUS instead of acquiringContexts.length.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
It sounds weird. Why should we have acquiring in finished status to show billing page?
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/condo/domains/billing/schema/BillingReceipt.js (1)
245-250:⚠️ Potential issue | 🟠 MajorThe
isPayableresolver should filter byACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPEfor consistency.The
isPayableresolver queriesAcquiringIntegrationContextwithout theintegration.typefilter, while thefindAcquiringContexthelper in the same file (lines 44–50) includes this filter. This causes inconsistency: other resolvers in the same file that need acquiring context (lines 279, 294, 309) usefindAcquiringContextwith the type filter, and the codebase pattern across multiple files (findOrganizationsByAddress.js,SuggestServiceProviderService.js, etc.) consistently filters for online-processing integration types.Without the filter, the check
activeAcquiringContexts.length !== 1could incorrectly return true if a non-online-processing context exists. Update line 245 to matchfindAcquiringContext:integration: { type: ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE, deletedAt: null }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/billing/schema/BillingReceipt.js` around lines 245 - 250, The isPayable resolver is querying AcquiringIntegrationContext without filtering by integration.type which is inconsistent with findAcquiringContext; update the find call that populates activeAcquiringContexts to include integration: { type: ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE, deletedAt: null } so it matches findAcquiringContext's behavior and prevents non-online contexts from affecting the activeAcquiringContexts.length check; reference the isPayable resolver, the activeAcquiringContexts variable, the existing find('AcquiringIntegrationContext', ...) call, and the ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE constant when making the change.apps/condo/domains/marketplace/schema/Invoice.js (1)
366-374:⚠️ Potential issue | 🟡 MinorAdd integration type filter to virtual field resolvers for consistency.
The
validateInputhook at line 501 explicitly queries for an acquiring context withintegration: { type: ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE, deletedAt: null }, but the virtual field resolvers (recipient,acquiringIntegrationId,acquiringHostUrl,canGroupReceipts) at lines 369, 383, 398, and 416 query only withinvoiceStatus: CONTEXT_FINISHED_STATUSwithout the integration type filter. This inconsistency could result in the virtual fields resolving from a different acquiring context than the one validated byvalidateInput.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/marketplace/schema/Invoice.js` around lines 366 - 374, The virtual field resolvers (the async resolver functions for recipient, acquiringIntegrationId, acquiringHostUrl, canGroupReceipts) currently call getByCondition('AcquiringIntegrationContext', { organization: { id: invoice.organization }, deletedAt: null, invoiceStatus: CONTEXT_FINISHED_STATUS }) but omit the integration type filter used in validateInput; update those resolver queries to include integration: { type: ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE, deletedAt: null } so they resolve the same acquiring context as validateInput (keep the getByCondition call and return logic unchanged).apps/condo/domains/acquiring/schema/RegisterMultiPaymentService.js (1)
103-123:⚠️ Potential issue | 🟠 MajorMake the payment creation step atomic.
This stage fires multiple
Payment.createcalls and only then createsMultiPayment. If one payment insert succeeds and another insert orMultiPayment.createfails, the mutation returns an error after persisting orphan payment rows, and a retry can register duplicates. Please wrap the whole write phase in one transaction or add compensating cleanup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/acquiring/schema/RegisterMultiPaymentService.js` around lines 103 - 123, The Payment.create calls and MultiPayment.create must be executed atomically to avoid orphaned Payment rows: wrap the write phase (all Payment.create and the final MultiPayment.create) in a single database transaction so either all records are committed or none are; locate the block using Payment.create and MultiPayment.create and use your project's transactional API (e.g., context.transaction / context.db.$transaction / equivalent) to run Promise.all(paymentCreateInputs.map(...)) plus the MultiPayment.create inside one transaction, or if transactions are not available implement compensating cleanup by catching errors and deleting any created payments (using Payment.delete or the low-level client) before rethrowing. Ensure you preserve connecting the payments to MultiPayment (payments: { connect: paymentIds }) inside the same transactional scope.apps/condo/pages/billing/index.tsx (1)
33-46:⚠️ Potential issue | 🟠 MajorFilter the provided acquiring contexts down to online-processing ones.
acquiringContexts.lengthcurrently treatsEXTERNAL_IMPORTas enough to enterBillingPageContent, and the provider then passes those non-online context ids into downstream payments queries. For orgs that only have external-import acquiring, this skips onboarding and renders the billing flow against the wrong context set.Suggested fix
- const onlineProcessingAcquiringContext = useMemo(() => { - return acquiringContexts.find(({ integration: { type } }) => type === ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE) - }, [acquiringContexts]) + const onlineProcessingAcquiringContexts = useMemo(() => { + return acquiringContexts.filter(({ integration }) => integration?.type === ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE) + }, [acquiringContexts]) + + const onlineProcessingAcquiringContext = onlineProcessingAcquiringContexts[0] ?? null ... - if (billingContexts.length && acquiringContexts.length) { + if (billingContexts.length && onlineProcessingAcquiringContexts.length) { return ( - <BillingAndAcquiringContext.Provider value={{ billingContexts: billingContexts, acquiringContexts: acquiringContexts, refetchBilling }}> + <BillingAndAcquiringContext.Provider value={{ billingContexts, acquiringContexts: onlineProcessingAcquiringContexts, refetchBilling }}> <BillingPageContent/> </BillingAndAcquiringContext.Provider> ) }Also applies to: 58-67
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/pages/billing/index.tsx` around lines 33 - 46, The acquiringContexts list currently includes non-online types (like EXTERNAL_IMPORT) and that leaks into billing logic; update the code to first derive a filtered list of only online-processing contexts (e.g., const onlineAcquiringContexts = acquiringContexts.filter(ctx => ctx.integration?.type === ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE)) and then use onlineAcquiringContexts for the downstream checks and for computing onlineProcessingAcquiringContext (use onlineAcquiringContexts.find(...)). Ensure you reference acquiringContexts, onlineProcessingAcquiringContext, and ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE when making these changes so all length checks, provider props, and refetch logic operate on the online-only set.
🧹 Nitpick comments (5)
apps/condo/pages/settings/index.tsx (2)
1-1: Import order violation:@app/condo/schemashould follow external imports.Per coding guidelines, import order should be:
builtin → external →@open-condo→ internal. The@app/condo/schemaimport should be placed after external imports likeantd,dayjs,next/*, andreact.♻️ Suggested import reordering
-import { AcquiringIntegrationTypeType } from '@app/condo/schema' import { notification } from 'antd' import dayjs from 'dayjs' import getConfig from 'next/config' import Head from 'next/head' import { useRouter } from 'next/router' import React, { useMemo, useEffect } from 'react' import { useFeatureFlags } from '@open-condo/featureflags/FeatureFlagsContext' import { useIntl } from '@open-condo/next/intl' import { useOrganization } from '@open-condo/next/organization' import { TabItem, Typography } from '@open-condo/ui' +import { AcquiringIntegrationTypeType } from '@app/condo/schema' import { AcquiringIntegrationContext } from '@condo/domains/acquiring/utils/clientSchema'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/pages/settings/index.tsx` at line 1, The import of AcquiringIntegrationTypeType from '@app/condo/schema' is placed before external imports; move the "import { AcquiringIntegrationTypeType } from '@app/condo/schema'" statement so it appears after all external imports (e.g., 'react', 'next/*', 'antd', 'dayjs') to comply with the required import order (builtin → external → `@open-condo` → internal).
79-86: Consider addingdeletedAt: nullto integration filter for consistency.Other files in this PR (e.g.,
SetupAcquiring.tsx,Verification.tsx,BillingReceipt.js) includeintegration: { type: ..., deletedAt: null }to explicitly exclude deleted integrations. This query only filters bytype, which may unintentionally include contexts linked to soft-deleted integrations.♻️ Suggested fix for consistency
const { objs: [acquiringIntegrationContext], loading: acquiringIntegrationContextLoading } = AcquiringIntegrationContext.useObjects({ where: { organization: { id: userOrganizationId }, invoiceStatus: CONTEXT_FINISHED_STATUS, - integration: { type: AcquiringIntegrationTypeType.OnlineProcessing }, + integration: { type: AcquiringIntegrationTypeType.OnlineProcessing, deletedAt: null }, }, first: 1, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/pages/settings/index.tsx` around lines 79 - 86, The query fetching acquiringIntegrationContext should explicitly exclude soft-deleted integrations: update the where.integration clause in AcquiringIntegrationContext.useObjects (the call that populates acquiringIntegrationContext / acquiringIntegrationContextLoading) to include deletedAt: null alongside type: AcquiringIntegrationTypeType.OnlineProcessing so only non-deleted integrations are returned.apps/condo/domains/acquiring/utils/serverSchema/getOnlineProcessingContext.js (1)
5-5: Unusedcontextparameter.The
contextparameter is declared but never used in the function body. If this is intended for future use or follows a Keystone convention, consider adding a comment. Otherwise, remove it to avoid confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/acquiring/utils/serverSchema/getOnlineProcessingContext.js` at line 5, The getOnlineProcessingContext function declares a context parameter that is never used; either remove the unused parameter from the function signature (i.e., change async function getOnlineProcessingContext(organizationIds)) or, if it's intentionally reserved for Keystone conventions or future use, add an inline comment above the function (or next to the parameter) explaining why context is present so readers won't be confused; update any call sites or tests that invoke getOnlineProcessingContext to match the new signature if you remove the parameter.apps/condo/domains/billing/schema/BillingReceipt.js (1)
33-33: Import order violation flagged by ESLint.The
@condo/domains/acquiring/constants/integrationimport should appear before@condo/domains/acquiring/schema/fields/paymentChangeWebhookto maintain alphabetical ordering within the@condo/domainsimport group.♻️ Suggested fix
const { PAYMENT_STATUS_CHANGE_WEBHOOK_URL_FIELD, PAYMENT_STATUS_CHANGE_WEBHOOK_SECRET_FIELD, applyWebhookSecretGeneration, isWebhookUrlInWhitelist, } = require('@condo/domains/acquiring/schema/fields/paymentChangeWebhook') +const { ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE } = require('@condo/domains/acquiring/constants/integration') const access = require('@condo/domains/billing/access/BillingReceipt') ... -const { ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE } = require('@condo/domains/acquiring/constants/integration')Or better yet, place it with the other
@condo/domains/acquiringimports near line 15-20.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/billing/schema/BillingReceipt.js` at line 33, Move the import of ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE so it appears before the import of paymentChangeWebhook to satisfy ESLint alphabetical ordering within the `@condo/domains` group; ideally group this import with the other `@condo/domains/acquiring` imports (the nearby acquiring-related imports) so ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE is adjacent to the other acquiring symbols.apps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/loader.js (1)
11-12: Consider deduplicating consumersIds for consistency.
invoiceIdsis deduplicated usinguniq(), butconsumersIdsis not. While the errorDUPLICATED_SERVICE_CONSUMERin the constants file suggests duplicates are validated elsewhere, deduplicating here would be more defensive and consistent.♻️ Proposed fix
- const consumersIds = groupedReceipts.map(group => group.serviceConsumer.id) + const consumersIds = uniq(groupedReceipts.map(group => group.serviceConsumer.id))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/loader.js` around lines 11 - 12, consumersIds is not deduplicated while invoiceIds is, so make consumersIds defensive and consistent by applying the same uniq deduplication (e.g., replace the current groupedReceipts.map(...) assignment for consumersIds with a uniq(...) call over the mapped serviceConsumer.id values) — update the variable where consumersIds is defined (referencing groupedReceipts and consumersIds) to use uniq so duplicates are removed before downstream validation (DUPLICATED_SERVICE_CONSUMER).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/condo/domains/acquiring/constants/registerMultiPayemnt.js`:
- Around line 97-103: In the INVOICES_FOR_THIRD_USER constant object, add a
space after the colon in the object opening (change "INVOICES_FOR_THIRD_USER:{”
to "INVOICES_FOR_THIRD_USER: {") to fix the formatting; locate the
INVOICES_FOR_THIRD_USER declaration and update the object literal spacing
accordingly.
- Around line 188-195: The object constant
RECEIPT_DOES_NOT_HAVE_COMMON_BILLING_ACCOUNT_WITH_SERVICE_CONSUMER has an
awkward line break before the comma after the type property; fix it by moving
the trailing comma to the end of the type line (i.e., ensure "type:
BILLING_RECEIPT_DOES_NOT_HAVE_COMMON_BILLING_ACCOUNT_WITH_SERVICE_CONSUMER,"
appears on one line) so the object properties (mutation, variable, code, type,
message) are correctly formatted and the comma is not on its own line.
- Line 1: Rename the module file currently named "registerMultiPayemnt.js" to
"registerMultiPayment.js" (fix the Payemnt -> Payment typo) and update any
require/import statements that reference "registerMultiPayemnt" inside the
modules named "processor.js" and "validation.js" so they point to
"registerMultiPayment" instead; ensure exported constant GQLErrorCode usage
remains unchanged (e.g., BAD_USER_INPUT) and run tests/build to verify no import
failures remain.
In `@apps/condo/domains/acquiring/schema/RegisterMultiPaymentService.js`:
- Around line 55-56: Remove the raw console.log of the full payment payload in
RegisterMultiPaymentService and stop printing the full `data` object in
`validateInput`; instead use the existing Pino logger (e.g.,
`logger.info`/`logger.warn`/`logger.error`) to emit a structured message with
only non-sensitive standard fields and a redacted payload summary (omit or mask
sender details and receipt/invoice identifiers). Locate the
`console.log('[DEBUG] RegisterMultiPaymentService: validateInput start', { data
})` and any similar logs inside `validateInput` and replace them with a Pino
call that includes a `msg` and explicit fields (e.g., `operation:
'RegisterMultiPaymentService.validateInput'`, `status`, and `payloadSummary`)
where `payloadSummary` is built by whitelisting or masking sensitive keys.
In
`@apps/condo/domains/acquiring/utils/serverSchema/getOnlineProcessingContext.js`:
- Around line 5-12: Import CONTEXT_FINISHED_STATUS from
'@condo/domains/acquiring/constants/context' and update
getOnlineProcessingContext so it returns the query result: keep the existing
find call assigning to acquiringIntegrationContexts (used to filter by
CONTEXT_FINISHED_STATUS and ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE) and
add an explicit return acquiringIntegrationContexts so the function returns the
fetched contexts.
In
`@apps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/loader.js`:
- Line 8: Remove the debug console.log calls in loader.fetchInitialEntities (and
the similar calls around lines 13, 19, 25) and replace them with structured Pino
logging (e.g., logger.info/warn/error) passing an object with a clear msg field
and any relevant context fields (like data, params, or correlationId); locate
the console.log invocations inside the fetchInitialEntities function in
registerMultiPayment/loader.js and swap them to use the module's pino logger
instance so logs follow the logger.info({ msg: '...', data, ... }) pattern.
In
`@apps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/processor.js`:
- Around line 223-245: The code incorrectly collapses all org contexts into a
single acqContext by using acquiringContexts[0]; instead, for each invoice in
foundInvoices use the invoice.acquiringIntegrationContext (e.g.,
invoice.acquiringIntegrationContext) to find the matching context from
acquiringContexts (match by id or organization) and throw the same GQLError if a
specific invoice's context is missing or not finished; then load the
AcquiringIntegration per that invoice's context (use AcquiringIntegration.getAll
with acqContext.integration or cache by integration id) rather than reusing the
first context for every generated payment — apply the same per-invoice context
selection and validation in the later block referenced (around lines 257-295).
- Around line 69-73: The AcquiringIntegrationContext query that populates
acquiringContexts currently omits the status field and does not enforce that
contexts are finished, allowing contexts in Verification/Error to be used;
update the find('AcquiringIntegrationContext', ...) call to request the status
field and then filter the resulting acquiringContexts (or add a query condition)
to only include contexts whose status === CONTEXT_FINISHED_STATUS before you
build paymentCreateInputs (also apply the same fix for the other occurrence
around the block referenced at 167-174).
In
`@apps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/validation.js`:
- Around line 39-48: The current check in groupedReceipts processing only
compares counts and uniqueness (using distributionReceiptsIds vs receiptsIds)
which allows wrong or foreign ids; replace it with a strict set equality check:
build Sets from distributionReceipts.map(r => r?.receipt?.id) and receiptsIds,
ensure every id in receiptsIds is present in distributionReceiptsIds and vice
versa (no extras or missing, e.g. compare sizes and that forEach id in oneSet
has otherSet.has(id)), and if not throw new
GQLError(ERRORS.BAD_AMOUNT_DISTRIBUTION_FOR_RECEIPTS, context); keep using the
existing symbols groupedReceipts, distributionReceipts, distributionReceiptsIds,
receiptsIds, GQLError, ERRORS.BAD_AMOUNT_DISTRIBUTION_FOR_RECEIPTS and context.
---
Outside diff comments:
In `@apps/condo/domains/acquiring/schema/RegisterMultiPaymentService.js`:
- Around line 103-123: The Payment.create calls and MultiPayment.create must be
executed atomically to avoid orphaned Payment rows: wrap the write phase (all
Payment.create and the final MultiPayment.create) in a single database
transaction so either all records are committed or none are; locate the block
using Payment.create and MultiPayment.create and use your project's
transactional API (e.g., context.transaction / context.db.$transaction /
equivalent) to run Promise.all(paymentCreateInputs.map(...)) plus the
MultiPayment.create inside one transaction, or if transactions are not available
implement compensating cleanup by catching errors and deleting any created
payments (using Payment.delete or the low-level client) before rethrowing.
Ensure you preserve connecting the payments to MultiPayment (payments: {
connect: paymentIds }) inside the same transactional scope.
In `@apps/condo/domains/billing/schema/BillingReceipt.js`:
- Around line 245-250: The isPayable resolver is querying
AcquiringIntegrationContext without filtering by integration.type which is
inconsistent with findAcquiringContext; update the find call that populates
activeAcquiringContexts to include integration: { type:
ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE, deletedAt: null } so it matches
findAcquiringContext's behavior and prevents non-online contexts from affecting
the activeAcquiringContexts.length check; reference the isPayable resolver, the
activeAcquiringContexts variable, the existing
find('AcquiringIntegrationContext', ...) call, and the
ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE constant when making the change.
In `@apps/condo/domains/marketplace/schema/Invoice.js`:
- Around line 366-374: The virtual field resolvers (the async resolver functions
for recipient, acquiringIntegrationId, acquiringHostUrl, canGroupReceipts)
currently call getByCondition('AcquiringIntegrationContext', { organization: {
id: invoice.organization }, deletedAt: null, invoiceStatus:
CONTEXT_FINISHED_STATUS }) but omit the integration type filter used in
validateInput; update those resolver queries to include integration: { type:
ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE, deletedAt: null } so they resolve
the same acquiring context as validateInput (keep the getByCondition call and
return logic unchanged).
In `@apps/condo/pages/billing/index.tsx`:
- Around line 33-46: The acquiringContexts list currently includes non-online
types (like EXTERNAL_IMPORT) and that leaks into billing logic; update the code
to first derive a filtered list of only online-processing contexts (e.g., const
onlineAcquiringContexts = acquiringContexts.filter(ctx => ctx.integration?.type
=== ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE)) and then use
onlineAcquiringContexts for the downstream checks and for computing
onlineProcessingAcquiringContext (use onlineAcquiringContexts.find(...)). Ensure
you reference acquiringContexts, onlineProcessingAcquiringContext, and
ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE when making these changes so all
length checks, provider props, and refetch logic operate on the online-only set.
---
Nitpick comments:
In
`@apps/condo/domains/acquiring/utils/serverSchema/getOnlineProcessingContext.js`:
- Line 5: The getOnlineProcessingContext function declares a context parameter
that is never used; either remove the unused parameter from the function
signature (i.e., change async function
getOnlineProcessingContext(organizationIds)) or, if it's intentionally reserved
for Keystone conventions or future use, add an inline comment above the function
(or next to the parameter) explaining why context is present so readers won't be
confused; update any call sites or tests that invoke getOnlineProcessingContext
to match the new signature if you remove the parameter.
In
`@apps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/loader.js`:
- Around line 11-12: consumersIds is not deduplicated while invoiceIds is, so
make consumersIds defensive and consistent by applying the same uniq
deduplication (e.g., replace the current groupedReceipts.map(...) assignment for
consumersIds with a uniq(...) call over the mapped serviceConsumer.id values) —
update the variable where consumersIds is defined (referencing groupedReceipts
and consumersIds) to use uniq so duplicates are removed before downstream
validation (DUPLICATED_SERVICE_CONSUMER).
In `@apps/condo/domains/billing/schema/BillingReceipt.js`:
- Line 33: Move the import of ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE so it
appears before the import of paymentChangeWebhook to satisfy ESLint alphabetical
ordering within the `@condo/domains` group; ideally group this import with the
other `@condo/domains/acquiring` imports (the nearby acquiring-related imports) so
ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE is adjacent to the other acquiring
symbols.
In `@apps/condo/pages/settings/index.tsx`:
- Line 1: The import of AcquiringIntegrationTypeType from '@app/condo/schema' is
placed before external imports; move the "import { AcquiringIntegrationTypeType
} from '@app/condo/schema'" statement so it appears after all external imports
(e.g., 'react', 'next/*', 'antd', 'dayjs') to comply with the required import
order (builtin → external → `@open-condo` → internal).
- Around line 79-86: The query fetching acquiringIntegrationContext should
explicitly exclude soft-deleted integrations: update the where.integration
clause in AcquiringIntegrationContext.useObjects (the call that populates
acquiringIntegrationContext / acquiringIntegrationContextLoading) to include
deletedAt: null alongside type: AcquiringIntegrationTypeType.OnlineProcessing so
only non-deleted integrations are returned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4fe683c7-3137-4c2c-a698-c942b7009c07
📒 Files selected for processing (22)
apps/condo/domains/acquiring/components/payments/PaymentFilesTable.tsxapps/condo/domains/acquiring/constants/registerMultiPayemnt.jsapps/condo/domains/acquiring/gql.jsapps/condo/domains/acquiring/schema/RegisterMultiPaymentService.jsapps/condo/domains/acquiring/utils/serverSchema/getOnlineProcessingContext.jsapps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/loader.jsapps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/processor.jsapps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/validation.jsapps/condo/domains/billing/components/BillingPageContent/ContextProvider.tsapps/condo/domains/billing/components/BillingPageContent/PaymentsTab/PaymentsFiles.tsxapps/condo/domains/billing/components/OnBoarding/SetupAcquiring.tsxapps/condo/domains/billing/components/OnBoarding/Verification.tsxapps/condo/domains/billing/schema/BillingReceipt.jsapps/condo/domains/marketplace/schema/Invoice.jsapps/condo/domains/resident/schema/DiscoverServiceConsumersService.jsapps/condo/domains/resident/schema/RegisterResidentInvoiceService.jsapps/condo/domains/resident/schema/RegisterServiceConsumerService.jsapps/condo/domains/resident/schema/ServiceConsumer.jsapps/condo/domains/resident/schema/SuggestServiceProviderService.jsapps/condo/domains/resident/utils/serverSchema/findOrganizationsByAddress.jsapps/condo/pages/billing/index.tsxapps/condo/pages/settings/index.tsx
dkoviazin
reviewed
Mar 24, 2026
vovaaxeapolla
force-pushed
the
refactor/condo/DOMA-13058/refactor-usage-of-acquiring-context
branch
from
cdab221 to
4e449e7
Compare
dkoviazin
reviewed
Mar 25, 2026
vovaaxeapolla
added
✋🙂 Review please
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c398cf098
ℹ️ 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".
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/condo/domains/acquiring/schema/CalculateFeeForReceiptService.js (2)
92-104:⚠️ Potential issue | 🔴 CriticalMissing null check for
acquiringContextbefore accessing its properties.After adding the
ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPEfilter,acquiringContextmay beundefinedif no matching context exists. AccessingacquiringContext.integrationon line 102 andacquiringContext.idon line 104 will throw if no context is found.🐛 Proposed fix: Add null check with appropriate error
const [acquiringContext] = await find('AcquiringIntegrationContext', { organization: { id: billingContext.organization }, deletedAt: null, status: CONTEXT_FINISHED_STATUS, integration: { type: ACQUIRING_INTEGRATION_ONLINE_PROCESSING_TYPE, supportedBillingIntegrationsGroup: billingIntegration.group, deletedAt: null, }, }) + + if (!acquiringContext) { + throw new GQLError({ + ...ERRORS.CANNOT_FIND_BILLING_RECEIPT, + message: 'Cannot find AcquiringIntegrationContext for the billing receipt', + }, context) + } + const acquiringIntegration = await getById('AcquiringIntegration', acquiringContext.integration)Note: You may need to add a more specific error constant for this case.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/acquiring/schema/CalculateFeeForReceiptService.js` around lines 92 - 104, The code calls find('AcquiringIntegrationContext') and then immediately uses acquiringContext.integration and acquiringContext.id (passed to getById and getAcquiringIntegrationContextFormula) without verifying acquiringContext exists; add a null/undefined check after the find result (the acquiringContext variable) and if missing throw or return a clear error (use an existing error constant or add a new one) before calling getById('AcquiringIntegration', acquiringContext.integration) and getAcquiringIntegrationContextFormula(context, acquiringContext.id) so the function fails safely when no context is found.
102-102:⚠️ Potential issue | 🟡 MinorFix double space typo.
There's a double space between
awaitandgetById.🔧 Proposed fix
- const acquiringIntegration = await getById('AcquiringIntegration', acquiringContext.integration) + const acquiringIntegration = await getById('AcquiringIntegration', acquiringContext.integration)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/acquiring/schema/CalculateFeeForReceiptService.js` at line 102, There's a stray double space between the await keyword and the getById call in the statement that assigns acquiringIntegration; remove the extra space so the expression reads "await getById(...)" to keep consistent formatting and avoid linting issues (affecting the const acquiringIntegration = await getById('AcquiringIntegration', acquiringContext.integration) line).apps/condo/pages/marketplace/setup.tsx (1)
49-59:⚠️ Potential issue | 🟠 MajorQuery the completed acquiring context explicitly.
useObjectstill fetches any online-processing context for the org and only checksinvoiceStatusin the effect. With multiple contexts around, a draft or old record can be returned first, so users may stay on onboarding even when a finished context exists, or get redirected based on the wrong record.Suggested fix
} = AcquiringIntegrationContext.useObject({ where: { organization: { id: orgId }, + invoiceStatus: CONTEXT_FINISHED_STATUS, integration: { type: AcquiringIntegrationTypeType.OnlineProcessing }, }, })Also applies to: 66-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/pages/marketplace/setup.tsx` around lines 49 - 59, The query using AcquiringIntegrationContext.useObject is too broad and may return draft/old OnlineProcessing contexts; modify the query filter to explicitly include the completed invoiceStatus (or the appropriate finished status enum) so it only returns the finished acquiring context (update the useObject call that assigns acquiringContext/acquiringContextLoading/acquiringContextError/refetchAcquiringContext and the similar second useObject at lines ~66-70) and remove reliance on effect-only checks of invoiceStatus; ensure you reference the invoiceStatus field and the AcquiringIntegrationTypeType.OnlineProcessing filter so the database query itself returns only completed records.
🧹 Nitpick comments (6)
apps/condo/domains/acquiring/constants/registerMultiPaymentErrors.js (2)
276-281: Inconsistent error structure: missingmutationandvariablefields.Same issue as above -
FULL_PAYMENT_AMOUNT_MISMATCHis missing themutationandvariablefields.♻️ Proposed fix
FULL_PAYMENT_AMOUNT_MISMATCH: { + mutation: 'registerMultiPayment', + variable: ['data', 'groupedReceipts', '[]', 'amountDistribution'], code: BAD_USER_INPUT, type: FULL_PAYMENT_AMOUNT_MISMATCH, message: 'Different amount and toPay from receipt are forbidden for receipt with Billing Category that has requiresFullPayment set to true', messageForUser: 'api.acquiring.multiPayment.FULL_PAYMENT_AMOUNT_MISMATCH', },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/acquiring/constants/registerMultiPaymentErrors.js` around lines 276 - 281, The FULL_PAYMENT_AMOUNT_MISMATCH error entry in registerMultiPaymentErrors.js is missing the required mutation and variable fields; update the FULL_PAYMENT_AMOUNT_MISMATCH object to include the same `mutation` and `variable` properties pattern used by the other error entries (match keys/naming used elsewhere in registerMultiPaymentErrors.js) so it conforms to the consistent error structure, ensuring both fields are present and populated appropriately.
270-275: Inconsistent error structure: missingmutationandvariablefields.Unlike other error definitions in this file,
RECEIPTS_WITH_INVOICES_FORBIDDENis missing themutationandvariablefields. Consider adding them for consistency with the rest of the error catalog.♻️ Proposed fix
RECEIPTS_WITH_INVOICES_FORBIDDEN: { + mutation: 'registerMultiPayment', + variable: ['data'], code: BAD_USER_INPUT, type: MULTIPAYMENT_RECEIPTS_WITH_INVOICES_FORBIDDEN, message: 'Receipts and invoices are forbidden to be together', messageForUser: 'api.acquiring.multiPayment.RECEIPTS_WITH_INVOICES_FORBIDDEN', },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/acquiring/constants/registerMultiPaymentErrors.js` around lines 270 - 275, The RECEIPTS_WITH_INVOICES_FORBIDDEN error object is missing the mutation and variable fields; update the RECEIPTS_WITH_INVOICES_FORBIDDEN constant to include the same mutation and variable keys used by the other multi-payment errors in this file (i.e., copy the mutation and variable values from a nearby MULTIPAYMENT_* error entry so the structure matches the rest of the catalog).apps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/builders.js (1)
127-127: Simplify string conversion.
String(Big(invoice.toPay))is equivalent toString(invoice.toPay)iftoPayis already a numeric string. TheBigwrapper is unnecessary here unless you need to normalize the format.♻️ Proposed fix (if toPay is already a valid numeric string)
- const amount = String(Big(invoice.toPay)) + const amount = String(invoice.toPay)However, if
toPaymight be a number or needs format normalization, keepingBigis acceptable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/builders.js` at line 127, Replace the unnecessary Big wrapper when converting invoice.toPay to a string: change the assignment to use String(invoice.toPay) instead of String(Big(invoice.toPay)) if invoice.toPay is already a numeric string; if invoice.toPay may be a number or untrusted input, validate/normalize it first (e.g., ensure it's a numeric string) before assigning to the amount variable so the const amount = String(...) line uses the minimal conversion required.apps/condo/pages/marketplace/setup.tsx (1)
1-12: Move@app/condo/schemainto the internal import group.The new schema import is internal, but it now sits above the external and
@open-condogroups.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 the current code and only fix it if needed. In `@apps/condo/pages/marketplace/setup.tsx` around lines 1 - 12, The import for AcquiringIntegrationTypeType from '@app/condo/schema' is in the wrong group; move the AcquiringIntegrationTypeType import so it sits in the internal import group (after all `@open-condo` imports like useIntl, useOrganization, StepItem, Typography, Steps) and ensure there's a blank line separating the `@open-condo` group and the internal group to follow the enforced order (builtin → external → `@open-condo` → internal → sibling → parent).apps/condo/pages/marketplace/index.tsx (1)
1-8: Move@app/condo/schemainto the internal import group.The new schema import is internal, but it now sits above the external and
@open-condogroups.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 the current code and only fix it if needed. In `@apps/condo/pages/marketplace/index.tsx` around lines 1 - 8, Import ordering is wrong: move the import for AcquiringIntegrationTypeType from '@app/condo/schema' into the internal import group (i.e., below the `@open-condo` imports and separated by a blank line) so imports follow builtin → external → `@open-condo` → internal → sibling → parent; locate the line that imports AcquiringIntegrationTypeType and place it after the useIntl/useOrganization imports (the `@open-condo` group) with a newline separating groups.apps/condo/domains/acquiring/schema/RegisterMultiPaymentService.js (1)
170-192: Consider merging consecutive mode checks.Lines 170-172 and 174-192 both check
mode === REQUEST_MODE.RECEIPTS. These could be consolidated into a single conditional block for cleaner code structure.♻️ Optional refactor to merge mode checks
- assertAcquiringIntegrationIsActive(acquiringIntegration, context) - if (mode === REQUEST_MODE.RECEIPTS) { - assertServiceConsumersBelongToCurrentUser(consumers, residentsById, context.authedItem.id, context) - } - - if (mode === REQUEST_MODE.RECEIPTS) { + assertAcquiringIntegrationIsActive(acquiringIntegration, context) + if (mode === REQUEST_MODE.RECEIPTS) { + assertServiceConsumersBelongToCurrentUser(consumers, residentsById, context.authedItem.id, context) assertCanGroupReceiptsIfNeeded(receipts.length, acquiringIntegration, context) assertBillingContextsNotDeleted(billingContexts, receipts, context) assertBillingIntegrationsSupportedByAcquiring(billingIntegrations, acquiringIntegration.supportedBillingIntegrationsGroup, context)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/acquiring/schema/RegisterMultiPaymentService.js` around lines 170 - 192, There are two consecutive checks for mode === REQUEST_MODE.RECEIPTS; merge them into a single conditional to avoid duplication: replace the separate if blocks with one if (mode === REQUEST_MODE.RECEIPTS) { ... } that calls assertServiceConsumersBelongToCurrentUser(...) first and then runs assertCanGroupReceiptsIfNeeded(...), assertBillingContextsNotDeleted(...), assertBillingIntegrationsSupportedByAcquiring(...), assertBillingIntegrationsNotDeleted(...), assertCurrencyConsistency(...), and assertReceiptBelongsToServiceConsumer(...) in the same order so all receipt-specific validations remain intact and else still handles invoices with assertInvoicesArePublished(...), assertInvoicesBelongToCurrentUser(...), and assertInvoiceAcquiringContextsFinished(...).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/loaders.js`:
- Around line 150-159: The GQLError payload is consumer-specific and breaks
invoice flow; update the error to use an entity-neutral field (e.g.
data.failedEntities) instead of data.failedConsumers and map each
deletedEntities item to a neutral shape like { id: entityId, acquiringContextId
} (optionally include a type/key if available) in the throw for
ERRORS.ACQUIRING_INTEGRATION_CONTEXT_IS_DELETED inside
buildResolvedAcquiringContextMaps so both consumer and invoice callers can map
failures; apply the same neutral payload change to the other similar branch
around lines 213-217.
- Around line 63-66: The loader loadInvoicesByIds currently returns whatever
invoices are found and silently ignores unknown IDs; change it to validate that
every requested invoice ID was retrieved: after calling find('Invoice', { id_in:
[...] }) compare the unique requested IDs (from invoiceIds) to the IDs in
foundInvoices, compute any missingIds, and if any are missing throw a clear
error (or reject) listing the missing invoice IDs so callers cannot proceed with
a partial set; keep the returned structure ({ byId, list: foundInvoices }) when
all IDs are present.
In
`@apps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/validators.js`:
- Around line 34-60: The current assertValidAmountDistribution performs global
checks across all groups allowing receipt IDs to be swapped between groups;
update assertValidAmountDistribution to validate each group individually: for
each group in groupedReceipts (use group.receipts and group.amountDistribution),
build groupReceiptIds = group.receipts.map(r => r.id) and groupDistribution =
(group.amountDistribution || []).filter(Boolean), then ensure
groupDistribution.length === groupReceiptIds.length, every distribution item has
a receipt.id that exists in groupReceiptIds, distribution receipt IDs are
unique, and distribution amounts (map to Big via distributionReceiptsAmounts)
are all > 0; throw the same
GQLError(ERRORS.BAD_AMOUNT_DISTRIBUTION_FOR_RECEIPTS, context) on any failure to
preserve existing error handling.
---
Outside diff comments:
In `@apps/condo/domains/acquiring/schema/CalculateFeeForReceiptService.js`:
- Around line 92-104: The code calls find('AcquiringIntegrationContext') and
then immediately uses acquiringContext.integration and acquiringContext.id
(passed to getById and getAcquiringIntegrationContextFormula) without verifying
acquiringContext exists; add a null/undefined check after the find result (the
acquiringContext variable) and if missing throw or return a clear error (use an
existing error constant or add a new one) before calling
getById('AcquiringIntegration', acquiringContext.integration) and
getAcquiringIntegrationContextFormula(context, acquiringContext.id) so the
function fails safely when no context is found.
- Line 102: There's a stray double space between the await keyword and the
getById call in the statement that assigns acquiringIntegration; remove the
extra space so the expression reads "await getById(...)" to keep consistent
formatting and avoid linting issues (affecting the const acquiringIntegration =
await getById('AcquiringIntegration', acquiringContext.integration) line).
In `@apps/condo/pages/marketplace/setup.tsx`:
- Around line 49-59: The query using AcquiringIntegrationContext.useObject is
too broad and may return draft/old OnlineProcessing contexts; modify the query
filter to explicitly include the completed invoiceStatus (or the appropriate
finished status enum) so it only returns the finished acquiring context (update
the useObject call that assigns
acquiringContext/acquiringContextLoading/acquiringContextError/refetchAcquiringContext
and the similar second useObject at lines ~66-70) and remove reliance on
effect-only checks of invoiceStatus; ensure you reference the invoiceStatus
field and the AcquiringIntegrationTypeType.OnlineProcessing filter so the
database query itself returns only completed records.
---
Nitpick comments:
In `@apps/condo/domains/acquiring/constants/registerMultiPaymentErrors.js`:
- Around line 276-281: The FULL_PAYMENT_AMOUNT_MISMATCH error entry in
registerMultiPaymentErrors.js is missing the required mutation and variable
fields; update the FULL_PAYMENT_AMOUNT_MISMATCH object to include the same
`mutation` and `variable` properties pattern used by the other error entries
(match keys/naming used elsewhere in registerMultiPaymentErrors.js) so it
conforms to the consistent error structure, ensuring both fields are present and
populated appropriately.
- Around line 270-275: The RECEIPTS_WITH_INVOICES_FORBIDDEN error object is
missing the mutation and variable fields; update the
RECEIPTS_WITH_INVOICES_FORBIDDEN constant to include the same mutation and
variable keys used by the other multi-payment errors in this file (i.e., copy
the mutation and variable values from a nearby MULTIPAYMENT_* error entry so the
structure matches the rest of the catalog).
In `@apps/condo/domains/acquiring/schema/RegisterMultiPaymentService.js`:
- Around line 170-192: There are two consecutive checks for mode ===
REQUEST_MODE.RECEIPTS; merge them into a single conditional to avoid
duplication: replace the separate if blocks with one if (mode ===
REQUEST_MODE.RECEIPTS) { ... } that calls
assertServiceConsumersBelongToCurrentUser(...) first and then runs
assertCanGroupReceiptsIfNeeded(...), assertBillingContextsNotDeleted(...),
assertBillingIntegrationsSupportedByAcquiring(...),
assertBillingIntegrationsNotDeleted(...), assertCurrencyConsistency(...), and
assertReceiptBelongsToServiceConsumer(...) in the same order so all
receipt-specific validations remain intact and else still handles invoices with
assertInvoicesArePublished(...), assertInvoicesBelongToCurrentUser(...), and
assertInvoiceAcquiringContextsFinished(...).
In
`@apps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/builders.js`:
- Line 127: Replace the unnecessary Big wrapper when converting invoice.toPay to
a string: change the assignment to use String(invoice.toPay) instead of
String(Big(invoice.toPay)) if invoice.toPay is already a numeric string; if
invoice.toPay may be a number or untrusted input, validate/normalize it first
(e.g., ensure it's a numeric string) before assigning to the amount variable so
the const amount = String(...) line uses the minimal conversion required.
In `@apps/condo/pages/marketplace/index.tsx`:
- Around line 1-8: Import ordering is wrong: move the import for
AcquiringIntegrationTypeType from '@app/condo/schema' into the internal import
group (i.e., below the `@open-condo` imports and separated by a blank line) so
imports follow builtin → external → `@open-condo` → internal → sibling → parent;
locate the line that imports AcquiringIntegrationTypeType and place it after the
useIntl/useOrganization imports (the `@open-condo` group) with a newline
separating groups.
In `@apps/condo/pages/marketplace/setup.tsx`:
- Around line 1-12: The import for AcquiringIntegrationTypeType from
'@app/condo/schema' is in the wrong group; move the AcquiringIntegrationTypeType
import so it sits in the internal import group (after all `@open-condo` imports
like useIntl, useOrganization, StepItem, Typography, Steps) and ensure there's a
blank line separating the `@open-condo` group and the internal group to follow the
enforced order (builtin → external → `@open-condo` → internal → sibling → parent).
🪄 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: b67f3f77-aadf-4652-9046-f59b2cc0e817
📒 Files selected for processing (35)
apps/condo/domains/acquiring/components/payments/PaymentFilesTable.tsxapps/condo/domains/acquiring/components/payments/PaymentsTable.tsxapps/condo/domains/acquiring/constants/registerMultiPaymentDomain.jsapps/condo/domains/acquiring/constants/registerMultiPaymentErrors.jsapps/condo/domains/acquiring/gql.jsapps/condo/domains/acquiring/schema/CalculateFeeForReceiptService.jsapps/condo/domains/acquiring/schema/CreatePaymentByLinkService.jsapps/condo/domains/acquiring/schema/RegisterMultiPaymentForInvoicesService.jsapps/condo/domains/acquiring/schema/RegisterMultiPaymentService.jsapps/condo/domains/acquiring/schema/RegisterMultiPaymentService.test.jsapps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/builders.jsapps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/helpers.jsapps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/loaders.jsapps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/validators.jsapps/condo/domains/acquiring/utils/taskSchema/index.spec.jsapps/condo/domains/billing/components/BillingPageContent/ContextProvider.tsapps/condo/domains/billing/components/BillingPageContent/PaymentsTab/PaymentsFiles.tsxapps/condo/domains/billing/components/OnBoarding/SetupAcquiring.tsxapps/condo/domains/billing/components/OnBoarding/Verification.tsxapps/condo/domains/billing/schema/BillingReceipt.jsapps/condo/domains/billing/schema/ValidateQRCodeService.jsapps/condo/domains/marketplace/schema/Invoice.jsapps/condo/domains/resident/schema/DiscoverServiceConsumersService.jsapps/condo/domains/resident/schema/RegisterResidentInvoiceService.jsapps/condo/domains/resident/schema/RegisterServiceConsumerService.jsapps/condo/domains/resident/schema/ServiceConsumer.jsapps/condo/domains/resident/schema/SuggestServiceProviderService.jsapps/condo/domains/resident/utils/serverSchema/findOrganizationsByAddress.jsapps/condo/domains/ticket/components/BaseTicketForm/index.tsxapps/condo/pages/billing/index.tsxapps/condo/pages/marketplace/index.tsxapps/condo/pages/marketplace/setup.tsxapps/condo/pages/settings/index.tsxapps/condo/schema.graphqlapps/condo/schema.ts
💤 Files with no reviewable changes (1)
- apps/condo/domains/acquiring/components/payments/PaymentsTable.tsx
✅ Files skipped from review due to trivial changes (5)
- apps/condo/schema.ts
- apps/condo/schema.graphql
- apps/condo/domains/resident/schema/ServiceConsumer.js
- apps/condo/domains/billing/components/BillingPageContent/PaymentsTab/PaymentsFiles.tsx
- apps/condo/domains/acquiring/constants/registerMultiPaymentDomain.js
🚧 Files skipped from review as they are similar to previous changes (9)
- apps/condo/domains/acquiring/gql.js
- apps/condo/pages/settings/index.tsx
- apps/condo/domains/billing/schema/BillingReceipt.js
- apps/condo/domains/resident/utils/serverSchema/findOrganizationsByAddress.js
- apps/condo/domains/billing/components/OnBoarding/Verification.tsx
- apps/condo/domains/resident/schema/SuggestServiceProviderService.js
- apps/condo/domains/acquiring/components/payments/PaymentFilesTable.tsx
- apps/condo/domains/resident/schema/RegisterServiceConsumerService.js
- apps/condo/pages/billing/index.tsx
dkoviazin
reviewed
Mar 31, 2026
dkoviazin
approved these changes
Apr 1, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/condo/domains/acquiring/constants/registerMultiPaymentErrors.js (1)
153-154: Polish a few user-facing error messages for grammar clarity.These strings are understandable, but small grammar fixes will make API errors cleaner.
✍️ Suggested text-only patch
- message: 'ServiceConsumers with ids {ids} is not linked to AcquiringIntegrationContext', + message: 'ServiceConsumers with ids {ids} are not linked to AcquiringIntegrationContext', ... - message: 'Some of ServiceConsumer\'s AcquiringIntegration does not supports following BillingReceipt\'s BillingIntegrations: {unsupportedBillingIntegrations}', + message: 'Some ServiceConsumer acquiring integrations do not support the following BillingReceipt billing integrations: {unsupportedBillingIntegrations}', ... - message: 'BillingReceipts has multiple currencies', + message: 'BillingReceipts have multiple currencies',Also applies to: 216-216, 230-230
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/acquiring/constants/registerMultiPaymentErrors.js` around lines 153 - 154, The user-facing error strings in registerMultiPaymentErrors.js are grammatically incorrect; update the message "ServiceConsumers with ids {ids} is not linked to AcquiringIntegrationContext" and the similar occurrences referenced (other messages at the same file) to use correct plural/grammar and clearer wording—for example replace with "ServiceConsumers with ids {ids} are not linked to AcquiringIntegrationContext." Apply the same grammatical fixes to the other occurrences flagged (the strings at the other two locations) so all messages are consistent and use "are" for plural subjects and clear punctuation/capitalization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/condo/domains/acquiring/constants/registerMultiPaymentErrors.js`:
- Around line 153-154: The user-facing error strings in
registerMultiPaymentErrors.js are grammatically incorrect; update the message
"ServiceConsumers with ids {ids} is not linked to AcquiringIntegrationContext"
and the similar occurrences referenced (other messages at the same file) to use
correct plural/grammar and clearer wording—for example replace with
"ServiceConsumers with ids {ids} are not linked to AcquiringIntegrationContext."
Apply the same grammatical fixes to the other occurrences flagged (the strings
at the other two locations) so all messages are consistent and use "are" for
plural subjects and clear punctuation/capitalization.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e8a6dcd3-aab9-493e-84d3-37502ce667ee
📒 Files selected for processing (5)
apps/condo/domains/acquiring/constants/errors.jsapps/condo/domains/acquiring/constants/registerMultiPaymentErrors.jsapps/condo/domains/acquiring/schema/RegisterMultiPaymentService.test.jsapps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/loaders.jsapps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/validators.js
✅ Files skipped from review due to trivial changes (1)
- apps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/loaders.js
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/condo/domains/acquiring/schema/RegisterMultiPaymentService.test.js
- apps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/validators.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/condo/domains/acquiring/schema/RegisterMultiPaymentForInvoicesService.js (1)
139-152:⚠️ Potential issue | 🔴 CriticalValidate one acquiring context per invoice organization before continuing.
Line 141 now filters out non-
ONLINE_PROCESSINGcontexts, but this resolver still never checks that every invoice organization resolved to a context. If one organization drops out,acquiringIntegrations.sizecan still be1, and the lateracquiringContexts[0]path will either crash on an empty list or wire some invoices to the wrong context. Please reuseresolveAcquiringContextsForInvoices()from the new loaders module here, or add the same per-organization completeness check before proceeding.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/acquiring/schema/RegisterMultiPaymentForInvoicesService.js` around lines 139 - 152, The current logic populates acquiringContexts and acquiringIntegrations but does not ensure every invoice organization has a corresponding acquiring context; reuse resolveAcquiringContextsForInvoices() from the loaders module (or implement the same per-organization completeness check) to get a map of organizationId → context, validate that every organization present in foundInvoices has a context, and if any are missing throw the existing GQLError(ERRORS.MULTIPLE_ACQUIRING_INTEGRATION, context) (or a clearer missing-context error) before using acquiringContexts[0] or building acquiringIntegrations; update the code paths that call find('AcquiringIntegrationContext')/acquiringContexts/acquiringIntegrations to use the resolved-per-organization results so you never proceed with an empty or incomplete context set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/condo/domains/acquiring/schema/RegisterMultiPaymentForInvoicesService.js`:
- Around line 65-68: The error definition MULTIPLE_ACQUIRING_INTEGRATION in
RegisterMultiPaymentForInvoicesService.js uses a misleading message referencing
"serviceConsumers"; change the message text to reference invoices (e.g., "Listed
invoices are linked to different acquiring integrations" or similar
invoice-specific wording) so the BAD_USER_INPUT/MULTIPLE_ACQUIRING_INTEGRATION
error accurately reflects the invoice-only schema; update only the message
string for MULTIPLE_ACQUIRING_INTEGRATION to be invoice-specific.
In `@apps/condo/domains/acquiring/schema/RegisterMultiPaymentService.js`:
- Around line 103-105: The invoice-mode branch in RegisterMultiPaymentService.js
lost the NOT_UNIQUE_CLIENTS validation: after the existing
assertNoDuplicateInvoices(...) and
assertNoRecurrentPaymentContextForInvoiceMode(...) calls, collect invoice.client
values into a Set (treat null/undefined as a distinct "anonymous" marker) and if
the resulting set size > 1 (i.e., mixed anonymous and user clients or multiple
different clients) throw/reject with the existing NOT_UNIQUE_CLIENTS validation
error used elsewhere; this enforces that all invoices are either the same client
or all anonymous while keeping assertInvoicesBelongToCurrentUser() intact.
---
Outside diff comments:
In
`@apps/condo/domains/acquiring/schema/RegisterMultiPaymentForInvoicesService.js`:
- Around line 139-152: The current logic populates acquiringContexts and
acquiringIntegrations but does not ensure every invoice organization has a
corresponding acquiring context; reuse resolveAcquiringContextsForInvoices()
from the loaders module (or implement the same per-organization completeness
check) to get a map of organizationId → context, validate that every
organization present in foundInvoices has a context, and if any are missing
throw the existing GQLError(ERRORS.MULTIPLE_ACQUIRING_INTEGRATION, context) (or
a clearer missing-context error) before using acquiringContexts[0] or building
acquiringIntegrations; update the code paths that call
find('AcquiringIntegrationContext')/acquiringContexts/acquiringIntegrations to
use the resolved-per-organization results so you never proceed with an empty or
incomplete context set.
🪄 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: 5bd8a4e6-2ce9-4071-bc38-4a4d75cdc327
📒 Files selected for processing (4)
apps/condo/domains/acquiring/schema/RegisterMultiPaymentForInvoicesService.jsapps/condo/domains/acquiring/schema/RegisterMultiPaymentForInvoicesService.test.jsapps/condo/domains/acquiring/schema/RegisterMultiPaymentService.jsapps/condo/domains/acquiring/utils/serverSchema/registerMultiPayment/loaders.js
vovaaxeapolla
force-pushed
the
refactor/condo/DOMA-13058/refactor-usage-of-acquiring-context
branch
from
2e4f3d7 to
cf72ee9
Compare
Comment on lines
+17
to
+23
| }, { | ||
| amountWithoutExplicitFee: Big('0.0'), | ||
| explicitFee: Big('0.0'), | ||
| explicitServiceCharge: Big('0.0'), | ||
| serviceFee: Big('0.0'), | ||
| implicitFee: Big('0.0'), | ||
| }) |
Member
There was a problem hiding this comment.
why total fields not used to build initial object?
| const { filters, sorters, offset } = parseQuery(router.query) | ||
| const currentTab = router.query.tab | ||
|
|
||
| // TODO(dkovyazin): DOMA-11394 find out why acquiring uses currency from billing integration |
vovaaxeapolla
force-pushed
the
refactor/condo/DOMA-13058/refactor-usage-of-acquiring-context
branch
from
9fb0a71 to
e97d76c
Compare
|
vovaaxeapolla
deleted the
refactor/condo/DOMA-13058/refactor-usage-of-acquiring-context
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Deprecations