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
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ All reports will return JSON data with the expected fields for the individual re

Currently, an M2M token is required to pull any report, and each report has its own scope associated with it that must be applied to the M2M token client ID

The report directory (list of endpoints and parameters) is available at `GET /v6/reports/directory` and uses the same authorization rules as other endpoints. The service accepts bearer tokens from the standard `Authorization` header, and also from proxies that forward the token in `X-Authorization`/`X-Forwarded-Authorization`.

## Layout

Each report will be a separate SQL query, potentially with a few parameters (like a start and an end date, for example). The individual SQL queries can be found in the `sql` folder and should be able to be run against the `topcoder-services` RDS database in dev or prod, with minimal changes to replace the parameters.
Expand Down
86 changes: 0 additions & 86 deletions sql/reports/topcoder/30-day-payments.sql

This file was deleted.

87 changes: 87 additions & 0 deletions sql/reports/topcoder/member-payment-accrual.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
WITH provided_dates AS (
SELECT
NULLIF($1, '')::timestamptz AS start_date,

Choose a reason for hiding this comment

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

[❗❗ correctness]
Casting user input directly to timestamptz without validation can lead to runtime errors if the input is not a valid timestamp. Consider adding input validation or error handling to ensure robustness.

NULLIF($2, '')::timestamptz AS end_date
),
params AS (
SELECT
COALESCE(
pd.start_date,
CASE
WHEN pd.end_date IS NOT NULL THEN pd.end_date - INTERVAL '3 months'
ELSE CURRENT_DATE - INTERVAL '3 months'
END
) AS start_date,
COALESCE(pd.end_date, CURRENT_DATE) AS end_date
FROM provided_dates pd
),
latest_payment AS (
SELECT
p.winnings_id,
MAX(p.version) AS max_version
FROM finance.payment p
GROUP BY p.winnings_id
),
recent_payments AS (
SELECT
w.winning_id,
w.winner_id,
w.type,
w.description,
w.category,
w.external_id AS challenge_id,
w.created_at AS winning_created_at,
p.payment_id,
p.payment_status,
p.payment_method_id,
p.installment_number,
p.billing_account,
p.total_amount,
p.gross_amount,
p.challenge_fee,
p.challenge_markup,
p.date_paid,
p.created_at AS payment_created_at
FROM finance.winnings w
JOIN finance.payment p
ON p.winnings_id = w.winning_id
JOIN latest_payment lp
ON lp.winnings_id = p.winnings_id
AND lp.max_version = p.version
JOIN params pr ON TRUE
WHERE w.type = 'PAYMENT'
AND p.created_at >= pr.start_date
AND p.created_at <= pr.end_date
)
SELECT
rp.payment_created_at AS payment_created_at,
rp.payment_id,
rp.description AS payment_description,
rp.challenge_id,
rp.payment_status,
rp.type AS payment_type,
mem.handle AS payee_handle,
pm.name AS payment_method,
ba."name" AS billing_account_name,
cl."name" AS customer_name,
ba."subcontractingEndCustomer" AS reporting_account_name,
rp.winner_id AS member_id,
to_char(c."createdAt", 'YYYY-MM-DD') AS challenge_created_date,
rp.gross_amount AS user_payment_gross_amount
FROM recent_payments rp
LEFT JOIN challenges."Challenge" c
ON c."id" = rp.challenge_id
LEFT JOIN challenges."ChallengeBilling" cb
ON cb."challengeId" = c."id"
LEFT JOIN "billing-accounts"."BillingAccount" ba
ON ba."id" = COALESCE(
NULLIF(rp.billing_account, '')::int,

Choose a reason for hiding this comment

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

[⚠️ correctness]
Using NULLIF and COALESCE to handle potential null or empty values is good, but ensure that the ::int cast will not fail if cb."billingAccountId" is not a valid integer. Consider adding validation or error handling for this case.

NULLIF(cb."billingAccountId", '')::int
)
LEFT JOIN "billing-accounts"."Client" cl
ON cl."id" = ba."clientId"
LEFT JOIN finance.payment_method pm
ON pm.payment_method_id = rp.payment_method_id
LEFT JOIN members.member mem
ON mem."userId"::text = rp.winner_id

Choose a reason for hiding this comment

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

[⚠️ correctness]
Casting mem."userId" to text for comparison with rp.winner_id assumes that rp.winner_id is always a valid string representation of a user ID. Ensure that this assumption holds or add validation to prevent potential mismatches.

ORDER BY payment_created_at DESC;
7 changes: 4 additions & 3 deletions sql/reports/topgear/hourly.sql
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ WITH base_challenges AS (
WHERE cb."challengeId" = c.id
AND cb."billingAccountId" = '80000062'
) ba ON TRUE
WHERE c."createdAt" >= now() - interval '4 months'
WHERE c."updatedAt" >= now() - interval '100 days'

Choose a reason for hiding this comment

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

[❗❗ correctness]
Changing the interval from '4 months' to '100 days' alters the time window for selecting challenges. Ensure this change aligns with the business requirements, as it could affect the data included in the report.

AND ba.billing_account_id IS NOT NULL
),
project_details AS (
Expand Down Expand Up @@ -288,5 +288,6 @@ LEFT JOIN LATERAL (
AND bc."createdAt" > '2025-01-01T00:00:00Z'
) cp ON TRUE
WHERE bc.billing_account_id = '80000062'
AND bc."createdAt" >= now() - interval '4 months'
ORDER BY bc."createdAt" DESC;
AND (pd.latest_actual_end_date >= now() - interval '100 days'

Choose a reason for hiding this comment

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

[⚠️ correctness]
The condition (pd.latest_actual_end_date >= now() - interval '100 days' OR bc.status='ACTIVE') may include challenges that are not relevant if pd.latest_actual_end_date is NULL. Consider verifying if this logic meets the intended criteria for filtering challenges.

OR bc.status='ACTIVE')
ORDER BY bc."updatedAt" DESC;

Choose a reason for hiding this comment

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

[⚠️ correctness]
The order by clause was changed from bc."createdAt" DESC to bc."updatedAt" DESC. This change will affect the sorting of the results. Ensure this change is intentional and aligns with the report's requirements.

31 changes: 30 additions & 1 deletion src/auth/auth.middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,32 @@ const { jwtAuthenticator: authenticator } = middleware;

const logger = new Logger("AuthMiddleware");

function resolveAuthorizationHeader(headers: Record<string, unknown>): string {

Choose a reason for hiding this comment

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

[⚠️ correctness]
The function resolveAuthorizationHeader returns an empty string if no valid authorization header is found. Consider returning null instead to clearly indicate the absence of a valid header, which can help prevent potential misuse of an empty string as a valid token.

const headerCandidates = [
headers["authorization"],
headers["x-authorization"],
headers["x-forwarded-authorization"],
headers["x-original-authorization"],
];

for (const value of headerCandidates) {
if (!value) {
continue;
}

if (Array.isArray(value)) {
const first = value.find(Boolean);
if (typeof first === "string") {
return first;
}
} else if (typeof value === "string") {
return value;
}
}

return "";
}

function decodeTokenPayload(token: string): Record<string, unknown> | null {
try {
const parts = token.split(".");
Expand Down Expand Up @@ -55,7 +81,10 @@ export class AuthMiddleware implements NestMiddleware {
}

use(req: any, res: Response, next: NextFunction) {
if (req.headers.authorization) {
const authorizationHeader = resolveAuthorizationHeader(req.headers ?? {});

Choose a reason for hiding this comment

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

[💡 style]
The use of req.headers ?? {} is unnecessary because req.headers is always defined as an object in Express. Consider simplifying this to req.headers.


if (authorizationHeader) {
req.headers.authorization = authorizationHeader;
this.jwtAuthenticator(req, res, (err) => {
if (err) {
const token = req.headers.authorization?.replace(/^Bearer\s+/i, "");
Expand Down
7 changes: 4 additions & 3 deletions src/reports/report-directory.data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -449,9 +449,10 @@ export const REPORTS_DIRECTORY: ReportsDirectory = {
"Weekly distinct registrants and submitters for the last five weeks",
),
report(
"30 Day Payments",
"/topcoder/30-day-payments",
"Member payments for the last 30 days",
"Member Payment Accrual",
"/topcoder/member-payment-accrual",
"Member payment accruals for the provided date range (defaults to last 3 months)",
[paymentsStartDateParam, paymentsEndDateParam],

Choose a reason for hiding this comment

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

[⚠️ correctness]
Consider adding validation to ensure that paymentsStartDateParam and paymentsEndDateParam are provided in a valid date format and that the start date is not after the end date. This will prevent potential errors when querying the report.

),
report(
"90 Day Member Spend",
Expand Down
12 changes: 12 additions & 0 deletions src/reports/reports.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,16 @@ export class ReportsController {
getReports(): ReportsDirectory {
return REPORTS_DIRECTORY;
}

@Get("/directory")

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The new endpoint /directory is an alias for the existing / endpoint. Consider whether this duplication is necessary, as it could lead to maintenance challenges if the logic for these endpoints diverges in the future.

@UseGuards(PermissionsGuard)
@Scopes(AppScopes.AllReports)
@ApiBearerAuth()
@ApiOperation({
summary:
"List available report endpoints grouped by sub-path (alias for /v6/reports)",
})
getReportsDirectory(): ReportsDirectory {
return REPORTS_DIRECTORY;
}
}
22 changes: 22 additions & 0 deletions src/reports/topcoder/dto/member-payment-accrual.dto.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { ApiPropertyOptional } from "@nestjs/swagger";
import { IsDateString, IsOptional } from "class-validator";

export class MemberPaymentAccrualQueryDto {
@ApiPropertyOptional({
description:
"Start date (inclusive) for filtering payment creation date in ISO 8601 format",
example: "2024-01-01T00:00:00.000Z",
})
@IsOptional()
@IsDateString()
startDate?: string;

@ApiPropertyOptional({
description:
"End date (inclusive) for filtering payment creation date in ISO 8601 format",
example: "2024-01-31T23:59:59.000Z",
})
@IsOptional()
@IsDateString()
endDate?: string;
}
11 changes: 7 additions & 4 deletions src/reports/topcoder/topcoder-reports.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
import { ApiBearerAuth, ApiOperation, ApiTags } from "@nestjs/swagger";
import { TopcoderReportsService } from "./topcoder-reports.service";
import { RegistrantCountriesQueryDto } from "./dto/registrant-countries.dto";
import { MemberPaymentAccrualQueryDto } from "./dto/member-payment-accrual.dto";
import { TopcoderReportsGuard } from "../../auth/guards/topcoder-reports.guard";
import { CsvResponseInterceptor } from "../../common/interceptors/csv-response.interceptor";

Expand Down Expand Up @@ -67,12 +68,14 @@ export class TopcoderReportsController {
return this.reports.getWeeklyMemberParticipation();
}

@Get("/30-day-payments")
@Get("/member-payment-accrual")

Choose a reason for hiding this comment

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

[❗❗ design]
The endpoint path /member-payment-accrual is a breaking change from the previous /30-day-payments. Ensure that any clients consuming this API are updated accordingly, or consider maintaining backward compatibility if needed.

@ApiOperation({
summary: "Member payments for the last 30 days",
summary:
"Member payment accruals for the provided date range (defaults to last 3 months)",
})
get30DayPayments() {
return this.reports.get30DayPayments();
getMemberPaymentAccrual(@Query() query: MemberPaymentAccrualQueryDto) {

Choose a reason for hiding this comment

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

[⚠️ correctness]
Consider adding validation for startDate and endDate in MemberPaymentAccrualQueryDto to ensure they are valid dates and startDate is not after endDate. This will prevent potential errors or unexpected behavior in the report generation.

const { startDate, endDate } = query;
return this.reports.getMemberPaymentAccrual(startDate, endDate);
}

@Get("/90-day-member-spend")
Expand Down
Loading
Loading