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
44 changes: 39 additions & 5 deletions src/api/admin/admin.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
HttpCode,
HttpStatus,
BadRequestException,
InternalServerErrorException,
} from '@nestjs/common';
import {
ApiOperation,
Expand Down Expand Up @@ -37,6 +38,8 @@ import { AccessControlService } from 'src/shared/access-control';
@Controller('/admin')
@ApiBearerAuth()
export class AdminController {
private static readonly EXPORT_BATCH_SIZE = 1000;

constructor(
private readonly adminService: AdminService,
private readonly winningsRepo: WinningsRepository,
Expand Down Expand Up @@ -113,22 +116,53 @@ export class AdminController {
@Header('Content-Type', 'text/csv')
@Header('Content-Disposition', 'attachment; filename="winnings.csv"')
async exportWinnings(@Body() body: WinningRequestDto, @User() user: any) {
const filters =
const baseFilters =
await this.accessControlService.applyFilters<WinningRequestDto>(
user.id,
user.roles,
{
...body,
limit: 999,
limit: undefined,
offset: undefined,
},
);
const result = await this.winningsRepo.searchWinnings(filters);

const winnings: SearchWinningResult['winnings'] = [];
let offset = 0;

while (true) {
const result = await this.winningsRepo.searchWinnings(
{
...baseFilters,
limit: AdminController.EXPORT_BATCH_SIZE,
offset,
},
{
includeCount: false,
includePayoutStatus: false,
},
);

if (result.error || !result.data) {

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 logging the error details before throwing the InternalServerErrorException. This can help with debugging and monitoring by providing more context about the failure.

throw new InternalServerErrorException(
result.error?.message ?? 'Export winnings failed',
);
}

winnings.push(...result.data.winnings);

if (result.data.winnings.length < AdminController.EXPORT_BATCH_SIZE) {

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 condition result.data.winnings.length < AdminController.EXPORT_BATCH_SIZE assumes that the API will always return a number of winnings equal to or less than the batch size. Ensure that this logic aligns with the API's behavior, especially if the API could return more winnings than requested due to pagination issues.

break;
}

offset += AdminController.EXPORT_BATCH_SIZE;
}

const handles = await this.tcMembersService.getHandlesByUserIds(
result.data.winnings.map((d) => d.winnerId),
winnings.map((d) => d.winnerId),
);

const csvRes = result.data.winnings.map((item) => {
const csvRes = winnings.map((item) => {
const payment =
item.details && item.details.length > 0 ? item.details[0] : null;

Expand Down
43 changes: 31 additions & 12 deletions src/api/repository/winnings.repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ import { Logger } from 'src/shared/global';

const ONE_DAY = 24 * 60 * 60 * 1000;

interface SearchWinningsOptions {
includeCount?: boolean;
includePayoutStatus?: boolean;
}

@Injectable()
export class WinningsRepository {
private readonly logger = new Logger(WinningsRepository.name);
Expand Down Expand Up @@ -162,8 +167,11 @@ export class WinningsRepository {
*/
async searchWinnings(
searchProps: WinningRequestDto,
options: SearchWinningsOptions = {},

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 options parameter is introduced without any documentation or type definition. Consider defining a TypeScript interface for SearchWinningsOptions to ensure type safety and improve maintainability.

): Promise<ResponseDto<SearchWinningResult>> {
const result = new ResponseDto<SearchWinningResult>();
const includeCount = options.includeCount ?? true;

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 default value for includeCount is set to true. Ensure that this default behavior is intentional and documented, as it may affect the logic where this function is called.

const includePayoutStatus = options.includePayoutStatus ?? true;

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 default value for includePayoutStatus is set to true. Verify that this default aligns with the expected behavior of the function and is documented accordingly.


try {
let winnerIds: string[] | undefined;
Expand Down Expand Up @@ -202,7 +210,9 @@ export class WinningsRepository {
!winnerIds && !!externalIds?.length,
);

const winnings = await this.prisma.winnings.findMany({
const limit = searchProps.limit ?? 10;

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 validating searchProps.limit and searchProps.offset to ensure they are non-negative integers. This will prevent potential issues with pagination logic.

const offset = searchProps.offset ?? 0;
const winningsPromise = this.prisma.winnings.findMany({
where: queryWhere,
include: {
payment: {
Expand All @@ -218,15 +228,22 @@ export class WinningsRepository {
origin: true,
},
orderBy,
skip: searchProps.offset,
take: searchProps.limit,
skip: offset,
take: limit,
});

const count = await this.prisma.winnings.count({ where: queryWhere });
const [winnings, count] = includeCount

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 Promise.all here is appropriate for parallelizing the fetching of winnings and count. However, ensure that both operations are independent and that the count operation does not depend on any side effects of the winningsPromise. If there is any dependency, this could lead to incorrect results.

? await Promise.all([
winningsPromise,
this.prisma.winnings.count({ where: queryWhere }),
])
: [await winningsPromise, 0];

const usersPayoutStatusMap = winnings?.length
? await this.getUsersPayoutStatusForWinnings(winnings)
: ({} as { [key: string]: payment_status });
const usersPayoutStatusMap: Record<string, unknown> =

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 type Record<string, unknown> for usersPayoutStatusMap is quite generic. If possible, specify a more precise type to improve type safety and maintainability.

includePayoutStatus && winnings?.length
? await this.getUsersPayoutStatusForWinnings(winnings)
: {};
const totalItems = includeCount ? count : winnings.length;

result.data = {
winnings: winnings.map((item) => ({
Expand Down Expand Up @@ -257,13 +274,15 @@ export class WinningsRepository {
item.payment?.[0].updated_at ??
undefined) as Date,
releaseDate: item.payment?.[0]?.release_date as Date,
paymentStatus: usersPayoutStatusMap[item.winner_id],
paymentStatus: usersPayoutStatusMap[
item.winner_id
] as WinningDto['paymentStatus'],

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]
Casting item.winner_id to WinningDto['paymentStatus'] may hide potential type mismatches. Ensure that usersPayoutStatusMap always returns a value compatible with WinningDto['paymentStatus'] to avoid runtime errors.

})),
pagination: {
totalItems: count,
totalPages: Math.ceil(count / searchProps.limit),
pageSize: searchProps.limit,
currentPage: Math.ceil(searchProps.offset / searchProps.limit) + 1,
totalItems,
totalPages: Math.ceil(totalItems / limit),

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 limit is never zero to prevent a division by zero error when calculating totalPages.

pageSize: limit,
currentPage: Math.ceil(offset / limit) + 1,
},
};
// response.data = winnings as any
Expand Down
2 changes: 1 addition & 1 deletion src/shared/access-control/engagement-pa.provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export class EngagementPaymentApproverProvider implements RoleAccessProvider {
);
if (unauthorized.length > 0) {
throw new Error(
`${Role.EngagementPaymentApprover} user is trying to access winning with category='${unauthorized.map(w => w.category).join(', ')}'`,
`${Role.EngagementPaymentApprover} user is trying to access winning with category='${unauthorized.map((w) => w.category).join(', ')}'`,
);
}
}
Expand Down
Loading