Skip to content

PM-1279 - paypal fee collection #66

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.

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

Merged
merged 3 commits into from
May 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/api/webhooks/trolley/handlers/payment.handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ export class PaymentHandler {
const winningIds = (
await this.prisma.$queryRaw<{ id: string }[]>`
SELECT winnings_id as id
FROM public.payment p
INNER JOIN public.payment_release_associations pra
FROM payment p
INNER JOIN payment_release_associations pra
ON pra.payment_id = p.payment_id
WHERE pra.payment_release_id::text = ${paymentId}
FOR UPDATE
Expand Down
78 changes: 68 additions & 10 deletions src/api/withdrawal/withdrawal.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export class WithdrawalService {
private readonly tcMembersService: TopcoderMembersService,
) {}

getTrolleyRecipientByUserId(userId: string) {
getDbTrolleyRecipientByUserId(userId: string) {
return this.prisma.trolley_recipient.findUnique({
where: { user_id: userId },
});
Expand Down Expand Up @@ -118,6 +118,7 @@ export class WithdrawalService {
paymentMethodId: number,
recipientId: string,
winnings: ReleasableWinningRow[],
metadata: any,
) {
try {
const paymentRelease = await tx.payment_releases.create({
Expand All @@ -127,6 +128,7 @@ export class WithdrawalService {
status: payment_status.PROCESSING,
payment_method_id: paymentMethodId,
payee_id: recipientId,
metadata,
payment_release_associations: {
createMany: {
data: winnings.map((w) => ({
Expand Down Expand Up @@ -176,7 +178,9 @@ export class WithdrawalService {
winningsIds: string[],
paymentMemo?: string,
) {
this.logger.log('Processing withdrawal request');
this.logger.log(
`Processing withdrawal request for user ${userHandle}(${userId}), winnings: ${winningsIds.join(', ')}`,
);
const hasActiveTaxForm = await this.taxFormRepo.hasActiveTaxForm(userId);

if (!hasActiveTaxForm) {
Expand Down Expand Up @@ -206,6 +210,9 @@ export class WithdrawalService {
}

if (userInfo.email.toLowerCase().indexOf('wipro.com') > -1) {
this.logger.error(
`User ${userHandle}(${userId}) attempted withdrawal but is restricted due to email domain '${userInfo.email}'.`,
);
throw new Error(
'Please contact Topgear support to process your withdrawal.',
);
Expand All @@ -219,33 +226,84 @@ export class WithdrawalService {
tx,
);

this.logger.log(
`Begin processing payments for user ${userHandle}(${userId})`,
winnings,
);

const dbTrolleyRecipient =
await this.getDbTrolleyRecipientByUserId(userId);

if (!dbTrolleyRecipient) {
throw new Error(
`Trolley recipient not found for user ${userHandle}(${userId})!`,
);
}

const totalAmount = this.checkTotalAmount(winnings);
let paymentAmount = totalAmount;
let feeAmount = 0;
const trolleyRecipientPayoutDetails =
await this.trolleyService.getRecipientPayoutDetails(
dbTrolleyRecipient.trolley_id,
);

if (!trolleyRecipientPayoutDetails) {
throw new Error(
`Recipient payout details not found for Trolley Recipient ID '${dbTrolleyRecipient.trolley_id}', for user ${userHandle}(${userId}).`,
);
}

this.logger.log('Begin processing payments', winnings);
if (
trolleyRecipientPayoutDetails.payoutMethod === 'paypal' &&
ENV_CONFIG.TROLLEY_PAYPAL_FEE_PERCENT
) {
const feePercent =
Number(ENV_CONFIG.TROLLEY_PAYPAL_FEE_PERCENT) / 100;

const recipient = await this.getTrolleyRecipientByUserId(userId);
feeAmount = +Math.min(
ENV_CONFIG.TROLLEY_PAYPAL_FEE_MAX_AMOUNT,

Choose a reason for hiding this comment

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

high
correctness
The calculation for feeAmount seems incorrect. The Math.max function should ensure that the fee does not exceed TROLLEY_PAYPAL_FEE_MAX_AMOUNT, but currently, it sets the fee to at least TROLLEY_PAYPAL_FEE_MAX_AMOUNT. Consider using Math.min instead to ensure the fee does not exceed the maximum allowed amount.

Copy link
Contributor

Choose a reason for hiding this comment

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

@vas3a I also think this needs correction and kind of struggle to understand it. Math.max picks the larger of them all.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, good catch. wanted to test in dev, as I had some issues locally. restarting PC resolved local so I'll run through it before merging.

feePercent * paymentAmount,
).toFixed(2);

if (!recipient) {
throw new Error(`Trolley recipient not found for user '${userId}'!`);
paymentAmount -= feeAmount;
Copy link
Contributor

Choose a reason for hiding this comment

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

We do round up paymentAmount to 2nd digit after the point, right? Or shold we do it here?

}

this.logger.log(
`
Total amount won: $${totalAmount.toFixed(2)} USD, to be paid: $${paymentAmount.toFixed(2)} USD.
Fee applied: $${feeAmount.toFixed(2)} USD (${Number(ENV_CONFIG.TROLLEY_PAYPAL_FEE_PERCENT)}%, max ${ENV_CONFIG.TROLLEY_PAYPAL_FEE_MAX_AMOUNT}).
Payout method type: ${trolleyRecipientPayoutDetails.payoutMethod}.
`,
);

const paymentRelease = await this.createDbPaymentRelease(
tx,
userId,
totalAmount,
paymentAmount,
connectedPaymentMethod.payment_method_id,
recipient.trolley_id,
dbTrolleyRecipient.trolley_id,
winnings,
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's also include the TROLLEY_PAYPAL_FEE_PERCENT and TROLLEY_PAYPAL_FEE_MAX_AMOUNT in the meta please.

netAmount: paymentAmount,
feeAmount,
totalAmount: totalAmount,
payoutMethod: trolleyRecipientPayoutDetails.payoutMethod,
env_trolley_paypal_fee_percent:
ENV_CONFIG.TROLLEY_PAYPAL_FEE_PERCENT,
env_trolley_paypal_fee_max_amount:

Choose a reason for hiding this comment

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

medium
correctness
Consider validating ENV_CONFIG.TROLLEY_PAYPAL_FEE_MAX_AMOUNT to ensure it is a non-negative number. This can prevent potential issues if the environment variable is misconfigured.

ENV_CONFIG.TROLLEY_PAYPAL_FEE_MAX_AMOUNT,
},
);

const paymentBatch = await this.trolleyService.startBatchPayment(
`${userId}_${userHandle}`,
);

const trolleyPayment = await this.trolleyService.createPayment(
recipient.trolley_id,
dbTrolleyRecipient.trolley_id,
paymentBatch.id,
totalAmount,
paymentAmount,
paymentRelease.payment_release_id,
paymentMemo,
);
Expand Down
13 changes: 13 additions & 0 deletions src/config/config.env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@ import {
IsBoolean,
IsInt,
IsNotEmpty,
IsNumber,
IsOptional,
IsString,
Max,
Min,
} from 'class-validator';

export class ConfigEnv {
Expand Down Expand Up @@ -90,4 +93,14 @@ export class ConfigEnv {

@IsString()
TOPCODER_WALLET_URL = 'https://wallet.topcoder.com';

@IsInt()
@Min(0)
@Max(99)
@IsOptional()
TROLLEY_PAYPAL_FEE_PERCENT: number = 0;

@IsNumber()
@IsOptional()
TROLLEY_PAYPAL_FEE_MAX_AMOUNT: number = 0;
}
6 changes: 3 additions & 3 deletions src/shared/global/trolley.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ export class TrolleyService {
recipient: {
id: recipientId,
},
sourceAmount: totalAmount.toString(),
sourceCurrency: 'USD',
amount: totalAmount.toFixed(2),
currency: 'USD',
memo: paymentMemo ?? 'Topcoder payment',
externalId: transactionId,
};
Expand Down Expand Up @@ -191,7 +191,7 @@ export class TrolleyService {
]) as RecipientTaxDetails;
} catch (error) {
this.logger.error(
'Failed to load recipient tax details from trolley!',
'Failed to load recipient tax & payout details from trolley!',
error,
);
return {} as RecipientTaxDetails;
Expand Down