[PROD RELEASE] - Update & fixes by kkartunov · Pull Request #147 · topcoder-platform/tc-finance-api

kkartunov · 2026-03-05T06:41:24Z

https://topcoder.atlassian.net/browse/PM-4026
https://topcoder.atlassian.net/browse/PM-4165
https://topcoder.atlassian.net/browse/PM-2557
https://topcoder.atlassian.net/browse/PM-1839

What was broken\nFinance payment generation treated leaderboard-only marathon fun challenges like normal payable challenges and attempted to create winnings records.\n\nRoot cause\nNo payment-generation guard existed for funChallenge in challenge processing, and the challenge model did not expose the flag.\n\nWhat was changed\nAdded funChallenge to the challenge model and short-circuited generateChallengePayments when it is true. Added Jest module alias mapping and minimal compile-safe typing fixes needed to execute the new unit test in this repo.\n\nAny added/updated tests\nAdded ChallengesService unit test to verify payment generation is skipped and no challenge lock is created for fun challenges.

What was broken:\nDate filters for wallet admin payments (last 7/30 days) did not narrow results when records had future created_at dates.\n\nRoot cause:\nThe date filter in WinningsRepository only applied a lower bound (gte) and had no upper bound, so future-dated rows still matched last7days/last30days.\n\nWhat was changed:\nUpdated generateFilterDate to apply both gte and lte bounds for LAST7DAYS and LAST30DAYS, with lte set to end-of-today.\nAdded repository unit tests to verify date windows for LAST7DAYS, LAST30DAYS, and ALL.\n\nAny added/updated tests:\nAdded src/api/repository/winnings.repo.spec.ts covering date filter range behavior in searchWinnings.

What was broken: Challenge payment lookups in the review flow were forced to winner-only results for non-privileged users, so challenge managers reviewing completed challenges could see "No payments found" even when payments existed. Root cause: Challenge-wide access expansion only checked copilot roles. Manager/full-write challenge resource roles were not treated as challenge-wide readers. What was changed: Updated challenge-payments access checks to allow full challenge visibility for users with challenge-wide roles (fullWriteAccess roles plus manager/copilot role names). Kept winner-based filtering for users without challenge-wide access. Hardened role claim parsing to only stringify primitive claim values. Extended the challenge resource-role model with optional fullWriteAccess for this access check. Any added/updated tests: Added unit tests for ChallengePaymentsService to verify manager access returns all challenge payments and non-challenge-wide users remain winner-filtered.

github-actions · 2026-03-05T06:41:37Z

@@ -0,0 +1 @@
+CREATE INDEX idx_winnings_type_created_at ON winnings USING btree (type, created_at DESC);


[⚠️ correctness]
Consider adding a conditional check to ensure that the winnings table exists before attempting to create the index. This can prevent potential errors during the migration if the table is not present.

github-actions · 2026-03-05T06:41:43Z

+        winnings: {
+          findMany: findManyMock,
+        },
+      } as any,


[⚠️ maintainability]
Casting to any can obscure type errors and reduce type safety. Consider using a more specific type or interface for the winnings and m2mFetch objects to enhance maintainability and type safety.

github-actions · 2026-03-05T06:41:44Z

+      } as any,
+      {
+        m2mFetch: m2mFetchMock,
+      } as any,


[⚠️ maintainability]
Casting to any can obscure type errors and reduce type safety. Consider using a more specific type or interface for the winnings and m2mFetch objects to enhance maintainability and type safety.

github-actions · 2026-03-05T06:41:44Z

+      winnerOnly: false,
+    });
+
+    const where = findManyMock.mock.calls[0][0].where;


[⚠️ correctness]
Accessing mock.calls[0][0].where directly assumes that the mock has been called at least once and that the structure of the call arguments is as expected. Consider adding checks or using a more robust way to verify the mock call structure to prevent potential runtime errors.

github-actions · 2026-03-05T06:41:44Z

+      winnerOnly: false,
+    });
+
+    const where = findManyMock.mock.calls[0][0].where;


[⚠️ correctness]
Accessing mock.calls[0][0].where directly assumes that the mock has been called at least once and that the structure of the call arguments is as expected. Consider adding checks or using a more robust way to verify the mock call structure to prevent potential runtime errors.

github-actions · 2026-03-05T06:41:46Z

-          requestUserId,
-        );
-        if (isCopilot) {
+        const hasChallengeWideAccess =


[💡 readability]
The method name hasChallengeWideAccessForChallenge is somewhat redundant and could be simplified to hasChallengeWideAccess. This would improve readability by reducing verbosity without losing clarity.

github-actions · 2026-03-05T06:41:49Z

          }
        });
-      } else if (value) {
+      } else if (


[⚠️ correctness]
The condition checks for typeof value being string, number, or boolean. Consider whether other types (e.g., object, function) should be explicitly handled or if they should trigger a warning or error. This could prevent unexpected behavior if value is of an unhandled type.

github-actions · 2026-03-05T06:41:52Z

  }

-  private async isCopilotForChallenge(
+  private async hasChallengeWideAccessForChallenge(


[💡 readability]
The method name hasChallengeWideAccessForChallenge is somewhat redundant and could be misleading. Consider renaming it to hasChallengeWideAccess for clarity and maintainability.

github-actions · 2026-03-05T06:41:55Z

-          role?.name ? role.name.toLowerCase().includes('copilot') : false,
+        ?.filter(
+          (role) =>
+            role?.fullWriteAccess === true ||


[⚠️ correctness]
The condition role?.fullWriteAccess === true is checked before checking the role name. If fullWriteAccess is a boolean, this is fine, but if it can be null or undefined, consider using a more robust check like role?.fullWriteAccess to ensure it handles all falsy values correctly.

github-actions · 2026-03-05T06:41:55Z

+          (role) =>
+            role?.fullWriteAccess === true ||
+            (role?.name
+              ? role.name.toLowerCase().includes('copilot') ||


[💡 readability]
The use of toLowerCase() on role.name is repeated multiple times. Consider extracting this to a variable to improve readability and avoid redundant operations.

github-actions · 2026-03-05T06:41:58Z

+      },
+    };
+    const service = new ChallengesService(
+      prisma as any,


[⚠️ maintainability]
Using as any for type casting can lead to potential type safety issues. Consider defining proper interfaces or types for the dependencies to ensure type safety.

github-actions · 2026-03-05T06:41:58Z

+      status: ChallengeStatuses.Completed,
+    };
+    const createPaymentsSpy = jest
+      .spyOn(service as any, 'createPayments')


[⚠️ design]
Spying on private methods (using service as any) can make tests brittle and tightly coupled to the implementation details. Consider testing the public API of the service instead.

github-actions · 2026-03-05T06:42:01Z

  userId: string;
  type: WinningsCategory;
-  currency: string;
+  currency: PrizeType;


[❗❗ correctness]
The change from string to PrizeType for the currency field suggests a type change. Ensure that PrizeType is an appropriate type for representing currency values and that all usages of PaymentPayload are updated accordingly. If PrizeType is an enum or a complex type, verify that it aligns with expected currency formats and values.

github-actions · 2026-03-05T06:42:04Z

      throw new Error("Challenge isn't in a payable status!");
    }

+    if (challenge.funChallenge === true) {


[⚠️ correctness]
Consider using === instead of == for strict equality check. While funChallenge is likely a boolean, using strict equality ensures type safety and avoids potential issues if the type changes in the future.

github-actions · 2026-03-05T06:42:08Z

 export interface ResourceRole {
  id: string;
  name: string;
+  fullWriteAccess?: boolean;


[⚠️ correctness]
The addition of the fullWriteAccess property as an optional boolean in the ResourceRole interface could lead to potential issues if not handled properly elsewhere in the codebase. Ensure that all logic that checks or uses this property accounts for the possibility of it being undefined. Consider providing a default value or handling it explicitly in the code that uses this interface.

github-actions · 2026-03-05T06:42:11Z

+      },
+    };
+
+    winningsRepo = new WinningsRepository(prismaMock as any);


[⚠️ maintainability]
Casting prismaMock as any can hide potential type errors. Consider defining a proper type for prismaMock to ensure type safety and maintainability.

github-actions · 2026-03-05T06:42:11Z

+    jest.useRealTimers();
+  });
+
+  async function getCreatedAtFilter(date: DateFilterType): Promise<any> {


[⚠️ maintainability]
The function getCreatedAtFilter returns any, which can lead to type safety issues. Consider specifying a more precise return type to improve maintainability and readability.

github-actions · 2026-03-05T06:42:14Z

  private generateFilterDate(date?: DateFilterType) {
    let filterDate: object | undefined;
    const currentDay = new Date(new Date().setHours(0, 0, 0, 0));
+    const currentDayEnd = new Date(currentDay.getTime() + ONE_DAY - 1);


[❗❗ correctness]
The calculation of currentDayEnd using ONE_DAY - 1 assumes ONE_DAY is defined as the number of milliseconds in a day. Ensure ONE_DAY is correctly defined to avoid potential off-by-one errors or incorrect date calculations.

github-actions · 2026-03-05T06:42:18Z

  private getOrderByWithWinnerId(
    sortBy: string | undefined,
-    sortOrder: 'asc' | 'desc',
+    sortOrder: 'asc' | 'desc' | undefined,


[❗❗ correctness]
Allowing sortOrder to be undefined without handling it in the method logic could lead to unexpected behavior. Consider providing a default value or handling the undefined case explicitly within the method.

github-actions · 2026-03-05T06:42:21Z

    const result = new ResponseDto<SearchWinningResult>();
    const includeCount = options.includeCount ?? true;
    const includePayoutStatus = options.includePayoutStatus ?? true;
+    const latestPaymentOnly = options.latestPaymentOnly ?? false;


[⚠️ correctness]
The newly introduced latestPaymentOnly variable is set to false by default. Ensure that this default behavior aligns with the intended functionality, as it may affect the logic where this variable is used.

…credited-payments PM-4165 allow editing credited payments

github-actions · 2026-03-05T06:42:24Z

                created_at: 'desc',
              },
            ],
+            take: latestPaymentOnly ? 1 : undefined,


[💡 readability]
The use of take: latestPaymentOnly ? 1 : undefined is technically correct, but consider explicitly setting take to null instead of undefined when latestPaymentOnly is false. This can improve clarity by making it explicit that no limit is intended, as undefined might imply an unintentional omission.

github-actions · 2026-03-05T06:42:26Z

 import { isNumber, includes } from 'lodash';
 import { ENV_CONFIG } from 'src/config';
 import { ChallengeStatuses } from 'src/dto/challenge.dto';
+import { Logger } from 'src/shared/global';


[❗❗ correctness]
The import path for Logger has been changed to src/shared/global. Ensure that this new path is correct and that the Logger implementation meets the requirements of the service. If the Logger is a custom implementation, verify that it provides the necessary functionality and is properly tested.

jmgasper and others added 7 commits February 20, 2026 12:19

Additional index and optimization for CSV export on wallet-admin

7ca6fd0

PM-4165 - allow editing credited payments

75f86d6

deploy

8fbc095

PM-4165 - allow editing credited payments

496298a

github-actions Bot reviewed Mar 5, 2026

View reviewed changes

Merge pull request #145 from topcoder-platform/PM-4165_allow-editing-…

258d602

…credited-payments PM-4165 allow editing credited payments

github-actions Bot reviewed Mar 5, 2026

View reviewed changes

kkartunov changed the title ~~[PROD RELEASE]~~ [PROD RELEASE] - Update & fixes Mar 5, 2026

kkartunov merged commit 3e1bf7a into master Mar 5, 2026
4 of 7 checks passed

		@@ -0,0 +1 @@
		CREATE INDEX idx_winnings_type_created_at ON winnings USING btree (type, created_at DESC);

Uh oh!

Conversation

kkartunov commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kkartunov commented Mar 5, 2026 •

edited

Loading