Skip to content

debt(PaymentMethod): surface bitcoin type #11952

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Betree wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

debt(PaymentMethod): surface bitcoin type #11952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions server/constants/paymentMethods.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ export enum PAYMENT_METHOD_TYPE {
PAYOUT = 'payout',
VIRTUAL_CARD = 'virtual_card',
SWISH = 'swish',
/** @deprecated but we still have ledger entries with this value */
BITCOIN = 'bitcoin',
}

export const PAYMENT_METHOD_TYPES = Object.values(PAYMENT_METHOD_TYPE);
10 changes: 9 additions & 1 deletion server/graphql/v2/enum/PaymentMethodType.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { GraphQLEnumType } from 'graphql';

import { PAYMENT_METHOD_TYPE } from '../../../constants/paymentMethods';

const deprecatedValues: readonly PAYMENT_METHOD_TYPE[] = [PAYMENT_METHOD_TYPE.BITCOIN] as const;

Comment on lines +5 to +6

@coderabbitai coderabbitai Bot Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm Object.keys() behavior for string enums compiled by TS, and inspect final generated schema for BITCOIN deprecation.
rg -n "PAYMENT_METHOD_TYPE" server/graphql/schemaV2.graphql -A1 -B1 | rg -i bitcoin -A2 -B2

Repository: opencollective/opencollective-api

Length of output: 171

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the target file and the related enum usage.
git ls-files server/graphql/v2/enum/PaymentMethodType.ts server/graphql/v2/enum
echo '--- outline ---'
ast-grep outline server/graphql/v2/enum/PaymentMethodType.ts --view expanded || true
echo '--- file ---'
wc -l server/graphql/v2/enum/PaymentMethodType.ts
cat -n server/graphql/v2/enum/PaymentMethodType.ts

Repository: opencollective/opencollective-api

Length of output: 5203

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for the generated enum/schema references and any tests for payment method deprecation.
rg -n "deprecatedValues|deprecationReason|PAYMENT_METHOD_TYPE|PaymentMethodType" server -g '!**/node_modules/**' -A3 -B3

Repository: opencollective/opencollective-api

Length of output: 50390

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Programmatic probe: replicate TS string-enum runtime shape and Object.keys()/includes behavior.
node - <<'JS'
const PAYMENT_METHOD_TYPE = { BITCOIN: 'bitcoin', CREDIT_CARD: 'credit_card' };
const deprecatedValues = ['bitcoin'];
for (const key of Object.keys(PAYMENT_METHOD_TYPE)) {
  console.log(key, deprecatedValues.includes(key), deprecatedValues.includes(PAYMENT_METHOD_TYPE[key]));
}
JS

Repository: opencollective/opencollective-api

Length of output: 214

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the enum definition and the surrounding GraphQL enum file with line numbers.
cat -n server/constants/paymentMethods.ts | sed -n '1,120p'
echo '---'
cat -n server/graphql/v2/enum/PaymentMethodType.ts

Repository: opencollective/opencollective-api

Length of output: 2558

BITCOIN never picks up the deprecation reason in the second reduce.
Object.keys(PAYMENT_METHOD_TYPE) yields member names ('BITCOIN'), while deprecatedValues holds enum values ('bitcoin'), so this check stays false. The lowercase bitcoin enum is already deprecated by the first reduce; compare against PAYMENT_METHOD_TYPE[key] if the uppercase alias should also be deprecated.

🤖 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 `@server/graphql/v2/enum/PaymentMethodType.ts` around lines 5 - 6, The
deprecation merge in PaymentMethodType is comparing enum member names from
Object.keys(PAYMENT_METHOD_TYPE) against enum values in deprecatedValues, so the
BITCOIN alias never matches. Update the second reduce in the PAYMENT_METHOD_TYPE
handling to compare using PAYMENT_METHOD_TYPE[key] instead of the raw key, and
ensure the deprecated reason from the bitcoin entry is applied to the uppercase
BITCOIN alias as well.

export const GraphQLPaymentMethodType = new GraphQLEnumType({
name: 'PaymentMethodType',
values: {
Expand All @@ -10,7 +12,13 @@ export const GraphQLPaymentMethodType = new GraphQLEnumType({
{},
),
...Object.keys(PAYMENT_METHOD_TYPE).reduce(
(values, key) => ({ ...values, [key]: { value: PAYMENT_METHOD_TYPE[key] } }),
(values, key: PAYMENT_METHOD_TYPE) => ({
...values,
[key]: {
deprecationReason: deprecatedValues.includes(key) ? 'This value is deprecated' : undefined,
value: PAYMENT_METHOD_TYPE[key],
},
Comment on lines +18 to +20

@sentry sentry Bot Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The deprecation check for the BITCOIN payment method fails because it incorrectly compares an enum key ('BITCOIN') with a list of enum values (['bitcoin']).
Severity: LOW

Suggested Fix

Modify the deprecation check to compare enum values instead of keys. Change the condition deprecatedValues.includes(key) to deprecatedValues.includes(PAYMENT_METHOD_TYPE[key]). This will correctly look up the value associated with the key and check for its presence in the deprecatedValues array.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: server/graphql/v2/enum/PaymentMethodType.ts#L18-L20

Potential issue: The logic to deprecate the `BITCOIN` payment method in the GraphQL enum
will not work as intended. The code creates a `deprecatedValues` array containing the
enum's string value (`'bitcoin'`). However, it then iterates through the enum's keys
(e.g., `'BITCOIN'`) and checks for inclusion in the `deprecatedValues` array. The check
`deprecatedValues.includes(key)` will always be false because it compares a key like
`'BITCOIN'` with a value like `'bitcoin'`. Consequently, the `deprecationReason` will
not be applied, and the `BITCOIN` enum value will not be marked as deprecated in the
schema.

Did we get this right? 👍 / 👎 to inform future reviews.

}),
{},
),
},
Expand Down
Loading