Skip to content
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
5 changes: 1 addition & 4 deletions src/api/admin/admin.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,7 @@ export class AdminService {
`Processing payment ${payment.payment_id} (installment ${payment.installment_number}) with current status=${payment.payment_status}`,
);

if (
payment.payment_status &&
payment.payment_status === PaymentStatus.CANCELLED
) {
if (payment.payment_status && payment.payment_status === 'CANCELLED') {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Consider using the PaymentStatus.CANCELLED enum instead of the string 'CANCELLED' for consistency and to prevent potential typos or mismatches in the future.

this.logger.warn(
`Attempt to update cancelled payment ${payment.payment_id} — rejecting`,
);
Expand Down
161 changes: 99 additions & 62 deletions src/api/repository/winnings.repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,33 @@ export class WinningsRepository {

constructor(private readonly prisma: PrismaService) {}

/**
* Extracts the optional hours-worked value from winning attributes.
* @param attributes Winning attributes payload retrieved from the database.
* @returns Parsed hours-worked value, or `undefined` when absent/invalid.
*/
private getHoursWorked(
attributes: Prisma.JsonValue | null,
): number | undefined {
if (
!attributes ||
typeof attributes !== 'object' ||
Array.isArray(attributes)
) {
return undefined;
}

const value = (attributes as Record<string, unknown>).hoursWorked;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ correctness]
The type assertion (attributes as Record<string, unknown>) assumes that attributes is a plain object. Consider using a type guard to ensure this assumption is valid, which can improve type safety and prevent runtime errors.

if (value === undefined || value === null || value === '') {
return undefined;
}

const parsedValue = Number(value);
return Number.isFinite(parsedValue) && parsedValue > 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ correctness]
The check parsedValue > 0 excludes zero as a valid value. If zero is a valid input for hoursWorked, this condition should be adjusted. Otherwise, clarify why zero should be excluded.

? parsedValue
: undefined;
}

private generateFilterDate(date?: DateFilterType) {
let filterDate: object | undefined;
const currentDay = new Date(new Date().setHours(0, 0, 0, 0));
Expand Down Expand Up @@ -252,38 +279,43 @@ export class WinningsRepository {
const totalItems = includeCount ? count : winnings.length;

result.data = {
winnings: winnings.map((item) => ({
id: item.winning_id,
type: item.type,
winnerId: item.winner_id,
origin: item.origin?.origin_name,
category: (item.category ?? '') as WinningsCategory,
title: item.title as string,
description: item.description as string,
externalId: item.external_id as string,
attributes: (item.attributes ?? {}) as object,
details: item.payment?.map((paymentItem) => ({
id: paymentItem.payment_id,
netAmount: Number(paymentItem.net_amount),
grossAmount: Number(paymentItem.gross_amount),
totalAmount: Number(paymentItem.total_amount),
installmentNumber: paymentItem.installment_number as number,
datePaid: (paymentItem.date_paid ?? undefined) as Date,
status: paymentItem.payment_status as PaymentStatus,
currency: paymentItem.currency as string,
releaseDate: paymentItem.release_date as Date,
category: item.category as string,
billingAccount: paymentItem.billing_account,
})),
createdAt: item.created_at as Date,
updatedAt: (item.payment?.[0].date_paid ??
item.payment?.[0].updated_at ??
undefined) as Date,
releaseDate: item.payment?.[0]?.release_date as Date,
paymentStatus: usersPayoutStatusMap[
item.winner_id
] as WinningDto['paymentStatus'],
})),
winnings: winnings.map((item) => {
const attributes = (item.attributes ?? {}) as object;

return {
id: item.winning_id,
type: item.type,
winnerId: item.winner_id,
origin: item.origin?.origin_name,
category: (item.category ?? '') as WinningsCategory,
title: item.title as string,
description: item.description as string,
externalId: item.external_id as string,
attributes,
hoursWorked: this.getHoursWorked(item.attributes),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ correctness]
The method getHoursWorked is called with item.attributes, but it's not clear if item.attributes is always defined or if getHoursWorked can handle undefined. Ensure getHoursWorked can handle undefined or provide a default value.

details: item.payment?.map((paymentItem) => ({
id: paymentItem.payment_id,
netAmount: Number(paymentItem.net_amount),
grossAmount: Number(paymentItem.gross_amount),
totalAmount: Number(paymentItem.total_amount),
installmentNumber: paymentItem.installment_number as number,
datePaid: (paymentItem.date_paid ?? undefined) as Date,
status: paymentItem.payment_status as PaymentStatus,
currency: paymentItem.currency as string,
releaseDate: paymentItem.release_date as Date,
category: item.category as string,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ correctness]
The category field in the details object is set to item.category as string. This seems inconsistent with the category field at line 290, which uses (item.category ?? '') as WinningsCategory. Consider using a consistent approach for handling category to avoid potential type issues.

billingAccount: paymentItem.billing_account,
})),
createdAt: item.created_at as Date,
updatedAt: (item.payment?.[0].date_paid ??
item.payment?.[0].updated_at ??
undefined) as Date,
releaseDate: item.payment?.[0]?.release_date as Date,
paymentStatus: usersPayoutStatusMap[
item.winner_id
] as WinningDto['paymentStatus'],
};
}),
pagination: {
totalItems,
totalPages: Math.ceil(totalItems / limit),
Expand Down Expand Up @@ -344,36 +376,41 @@ export class WinningsRepository {
? await this.getUsersPayoutStatusForWinnings(winnings)
: ({} as { [key: string]: payment_status });

result.data = winnings.map((item) => ({
id: item.winning_id,
type: item.type,
winnerId: item.winner_id,
origin: item.origin?.origin_name,
category: (item.category ?? '') as WinningsCategory,
title: item.title as string,
description: item.description as string,
externalId: item.external_id as string,
attributes: (item.attributes ?? {}) as object,
details: item.payment?.map((paymentItem) => ({
id: paymentItem.payment_id,
netAmount: Number(paymentItem.net_amount),
grossAmount: Number(paymentItem.gross_amount),
totalAmount: Number(paymentItem.total_amount),
installmentNumber: paymentItem.installment_number as number,
datePaid: (paymentItem.date_paid ?? undefined) as Date,
status: paymentItem.payment_status as PaymentStatus,
currency: paymentItem.currency as string,
releaseDate: paymentItem.release_date as Date,
category: item.category as string,
billingAccount: paymentItem.billing_account,
})),
createdAt: item.created_at as Date,
updatedAt: (item.payment?.[0].date_paid ??
item.payment?.[0].updated_at ??
undefined) as Date,
releaseDate: item.payment?.[0]?.release_date as Date,
paymentStatus: usersPayoutStatusMap[item.winner_id],
}));
result.data = winnings.map((item) => {
const attributes = (item.attributes ?? {}) as object;

return {
id: item.winning_id,
type: item.type,
winnerId: item.winner_id,
origin: item.origin?.origin_name,
category: (item.category ?? '') as WinningsCategory,
title: item.title as string,
description: item.description as string,
externalId: item.external_id as string,
attributes,
hoursWorked: this.getHoursWorked(item.attributes),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ correctness]
The method getHoursWorked is called with item.attributes, which is cast to an object. Ensure that getHoursWorked can handle cases where attributes might not have the expected structure or properties, as this could lead to runtime errors.

details: item.payment?.map((paymentItem) => ({
id: paymentItem.payment_id,
netAmount: Number(paymentItem.net_amount),
grossAmount: Number(paymentItem.gross_amount),
totalAmount: Number(paymentItem.total_amount),
installmentNumber: paymentItem.installment_number as number,
datePaid: (paymentItem.date_paid ?? undefined) as Date,
status: paymentItem.payment_status as PaymentStatus,
currency: paymentItem.currency as string,
releaseDate: paymentItem.release_date as Date,
category: item.category as string,
billingAccount: paymentItem.billing_account,
})),
createdAt: item.created_at as Date,
updatedAt: (item.payment?.[0].date_paid ??

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ correctness]
The use of item.payment?.[0].date_paid ?? item.payment?.[0].updated_at ?? undefined for updatedAt assumes that item.payment is an array and that accessing the first element is safe. Consider checking if item.payment is non-empty before accessing the first element to avoid potential runtime errors.

item.payment?.[0].updated_at ??
undefined) as Date,
releaseDate: item.payment?.[0]?.release_date as Date,
paymentStatus: usersPayoutStatusMap[item.winner_id],
};
});
} catch (error) {
this.logger.error('Getting winnings by external ID failed', error);
const message = 'Getting winnings by external ID failed. ' + error;
Expand Down
3 changes: 2 additions & 1 deletion src/api/webhooks/trolley/handlers/tax-form.handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,9 @@ export class TaxFormHandler {
});

if (existingFormAssociation) {
const dateFiled = existingFormAssociation.date_filed.toISOString();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ correctness]
Converting date_filed to an ISO string before logging improves readability and consistency in log outputs. However, ensure that date_filed is always a valid Date object to avoid potential runtime errors. Consider adding a check or handling for cases where date_filed might be null or undefined.

this.logger.log(
`Found existing association id='${existingFormAssociation.id}' status='${existingFormAssociation.tax_form_status}' date_filed='${existingFormAssociation.date_filed}'`,
`Found existing association id='${existingFormAssociation.id}' status='${existingFormAssociation.tax_form_status}' date_filed='${dateFiled}'`,
);
} else {
this.logger.log('No existing tax form association found');
Expand Down
39 changes: 32 additions & 7 deletions src/api/winnings/winnings.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,28 @@ export class WinningsService {
private readonly tcEmailService: TopcoderEmailService,
) {}

/**
* Builds the persisted winning attributes object.
* @param attributes Arbitrary attributes supplied by the client.
* @param hoursWorked Optional engagement-payment hours to persist.
* @returns The normalized attributes object, or `null` when empty.
*/
private buildWinningAttributes(
attributes: object | undefined,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ correctness]
Consider adding a type guard to ensure attributes is not only an object but also not null, as typeof null is 'object'. This could prevent unexpected behavior if null is passed as attributes.

hoursWorked?: number,
): Record<string, unknown> | null {
const normalizedAttributes =
attributes && typeof attributes === 'object' ? { ...attributes } : {};

if (hoursWorked !== undefined) {
normalizedAttributes.hoursWorked = hoursWorked;
}

return Object.keys(normalizedAttributes).length
? normalizedAttributes
: null;
}

private async sendSetupEmailNotification(userId: string, amount: number) {
this.logger.debug(`Fetching member info for user handle: ${userId}`);
const member = await this.tcMembersService.getMemberInfoByUserId(userId, {
Expand Down Expand Up @@ -175,17 +197,17 @@ export class WinningsService {
// if so, and the others are not matching the expectation, throw error
if (
(body.type === WinningsType.POINTS ||
body.category === winnings_category.POINTS_AWARD ||
body.details.some((p) => p.currency === PrizeType.POINT)) &&
body.category === WinningsCategory.POINTS_AWARD ||
body.details.some((p) => String(p.currency) === 'POINT')) &&

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The conversion of p.currency to a string using String(p.currency) is repeated multiple times. Consider extracting this logic into a helper function or variable to improve maintainability and reduce the risk of errors if the conversion logic needs to change.

(body.type !== WinningsType.POINTS ||
body.category !== winnings_category.POINTS_AWARD ||
!body.details.some((p) => p.currency === PrizeType.POINT))
body.category !== WinningsCategory.POINTS_AWARD ||
!body.details.some((p) => String(p.currency) === 'POINT'))
) {
const isTypePoints = body.type === WinningsType.POINTS;
const isCategoryPoints =
body.category === winnings_category.POINTS_AWARD;
body.category === WinningsCategory.POINTS_AWARD;
const hasPointsCurrency = body.details.some(
(p) => p.currency === PrizeType.POINT,
(p) => String(p.currency) === 'POINT',
);

const mismatches: string[] = [];
Expand Down Expand Up @@ -234,7 +256,10 @@ export class WinningsService {
title: body.title,
description: body.description,
external_id: body.externalId,
attributes: body.attributes,
attributes: this.buildWinningAttributes(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[❗❗ correctness]
Ensure that this.buildWinningAttributes handles cases where body.attributes or body.hoursWorked might be undefined or null. This could lead to unexpected behavior if not properly handled.

body.attributes,
body.hoursWorked,
),
created_by: userId,
payment: {
create: [] as Pick<
Expand Down
5 changes: 3 additions & 2 deletions src/api/withdrawal/withdrawal.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import { TopcoderMembersService } from 'src/shared/topcoder/members.service';
import { BasicMemberInfo, BASIC_MEMBER_FIELDS } from 'src/shared/topcoder';
import { Logger } from 'src/shared/global';
import { OtpService } from 'src/shared/global/otp.service';
import { PrizeType } from '../challenges/models';

const TROLLEY_MINIMUM_PAYMENT_AMOUNT =
ENV_CONFIG.TROLLEY_MINIMUM_PAYMENT_AMOUNT;
Expand Down Expand Up @@ -78,7 +77,9 @@ export class WithdrawalService {
// only USD payments can be withdrawn
if (
winnings.some(
(w) => w.type !== winnings_type.PAYMENT || w.currency !== PrizeType.USD,
(w) =>
String(w.type) !== winnings_type.PAYMENT ||

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ correctness]
Converting w.type and w.currency to strings using String() may mask potential issues if these values are not strings to begin with. Consider ensuring that w.type and w.currency are of the expected type before this comparison to avoid unexpected behavior.

String(w.currency) !== 'USD',
)
) {
throw new BadRequestException('Withdrawal supports USD payments only.');
Expand Down
1 change: 0 additions & 1 deletion src/dto/payment.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
IsInt,
IsString,
IsNotEmpty,
isEnum,
IsEnum,
} from 'class-validator';
import { PrizeType } from 'src/api/challenges/models';
Expand Down
14 changes: 14 additions & 0 deletions src/dto/winning.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
IsEnum,
IsObject,
ValidateNested,
IsNumber,
Min,
} from 'class-validator';
import { PaginationInfo, SortPagination } from './sort-pagination.dto';
import { DateFilterType } from './date-filter.type';
Expand Down Expand Up @@ -115,6 +117,7 @@ export class WinningDto {
description: string;
externalId: string;
attributes: object;
hoursWorked?: number;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ correctness]
Consider specifying a more detailed type for hoursWorked instead of number. If hoursWorked should only be a positive number or within a certain range, using a more specific type or validation could prevent incorrect data from being assigned.

details: PaymentDetailDto[];
paymentStatus: PayoutStatus;
createdAt: Date;
Expand Down Expand Up @@ -268,6 +271,17 @@ export class WinningCreateRequestDto {
@IsObject()
attributes: object;

@ApiProperty({
description:
'Optional engagement-payment hours worked value. When provided, it is stored with the winning attributes and returned in payment history.',
example: 37.5,
required: false,
})
@IsOptional()
@IsNumber()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Consider using @IsPositive() instead of @Min(0.01) for hoursWorked. This will ensure that any positive number is valid, which might be more intuitive and flexible for future use cases.

@Min(0.01)
hoursWorked?: number;

@ApiProperty({
description: 'Optional payment status to apply to created payments',
enum: PaymentStatus,
Expand Down
2 changes: 1 addition & 1 deletion src/shared/global/ba-prisma.client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const clientOptions = {
};

let baPrismaClient: BaPrismaClient;
export const getBaClient = () => {
export const getBaClient = (): BaPrismaClient => {
if (!baPrismaClient) {
if (!ENV_CONFIG.BILLING_ACCOUNTS_DB_URL) {
throw new Error(
Expand Down
Loading