-
Notifications
You must be signed in to change notification settings - Fork 1
[PROD RELEASE] - Update & fixes #147
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
Changes from all commits
7ca6fd0
3827cbe
54a1e4e
81bc0d3
75f86d6
8fbc095
496298a
258d602
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| CREATE INDEX idx_winnings_type_created_at ON winnings USING btree (type, created_at DESC); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| jest.mock('src/shared/global', () => ({ | ||
| Logger: class { | ||
| debug = jest.fn(); | ||
|
|
||
| error = jest.fn(); | ||
|
|
||
| info = jest.fn(); | ||
|
|
||
| log = jest.fn(); | ||
|
|
||
| warn = jest.fn(); | ||
| }, | ||
| })); | ||
|
|
||
| jest.mock('src/config', () => ({ | ||
| ENV_CONFIG: { | ||
| TOPCODER_API_V6_BASE_URL: 'https://api.topcoder-dev.com/v6', | ||
| }, | ||
| })); | ||
|
|
||
| import { ChallengePaymentsService } from './challenge-payments.service'; | ||
|
|
||
| describe('ChallengePaymentsService', () => { | ||
| let findManyMock: jest.Mock; | ||
| let m2mFetchMock: jest.Mock; | ||
| let service: ChallengePaymentsService; | ||
|
|
||
| beforeEach(() => { | ||
| findManyMock = jest.fn().mockResolvedValue([]); | ||
| m2mFetchMock = jest.fn(); | ||
|
|
||
| service = new ChallengePaymentsService( | ||
| { | ||
| winnings: { | ||
| findMany: findManyMock, | ||
| }, | ||
| } as any, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| { | ||
| m2mFetch: m2mFetchMock, | ||
| } as any, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| ); | ||
| }); | ||
|
|
||
| it('returns all challenge payments for managers', async () => { | ||
| m2mFetchMock | ||
| .mockResolvedValueOnce([ | ||
| { | ||
| memberHandle: 'manager-user', | ||
| memberId: '123', | ||
| roleId: 'manager-role-id', | ||
| }, | ||
| ]) | ||
| .mockResolvedValueOnce([ | ||
| { | ||
| fullWriteAccess: false, | ||
| id: 'manager-role-id', | ||
| name: 'Manager', | ||
| }, | ||
| ]); | ||
|
|
||
| await service.listChallengePayments({ | ||
| auth0User: { roles: ['Topcoder User'] }, | ||
| challengeId: 'challenge-id', | ||
| isMachineToken: false, | ||
| requestUserId: '123', | ||
| winnerOnly: false, | ||
| }); | ||
|
|
||
| const where = findManyMock.mock.calls[0][0].where; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| expect(where.winner_id).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('keeps winner filtering for users without challenge-wide access', async () => { | ||
| m2mFetchMock | ||
| .mockResolvedValueOnce([ | ||
| { | ||
| memberHandle: 'reviewer-user', | ||
| memberId: '456', | ||
| roleId: 'reviewer-role-id', | ||
| }, | ||
| ]) | ||
| .mockResolvedValueOnce([ | ||
| { | ||
| fullWriteAccess: false, | ||
| id: 'reviewer-role-id', | ||
| name: 'Reviewer', | ||
| }, | ||
| ]); | ||
|
|
||
| await service.listChallengePayments({ | ||
| auth0User: { roles: ['Topcoder User'] }, | ||
| challengeId: 'challenge-id', | ||
| isMachineToken: false, | ||
| requestUserId: '456', | ||
| winnerOnly: false, | ||
| }); | ||
|
|
||
| const where = findManyMock.mock.calls[0][0].where; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| expect(where.winner_id).toBe('456'); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -101,16 +101,17 @@ export class ChallengePaymentsService { | |
| } | ||
|
|
||
| try { | ||
| const isCopilot = await this.isCopilotForChallenge( | ||
| challengeId, | ||
| requestUserId, | ||
| ); | ||
| if (isCopilot) { | ||
| const hasChallengeWideAccess = | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [💡 |
||
| await this.hasChallengeWideAccessForChallenge( | ||
| challengeId, | ||
| requestUserId, | ||
| ); | ||
| if (hasChallengeWideAccess) { | ||
| allowAllForChallenge = true; | ||
| } | ||
| } catch (error) { | ||
| this.logger.warn( | ||
| `Failed to verify copilot status for user ${requestUserId} on challenge ${challengeId}`, | ||
| `Failed to verify challenge-wide payment access for user ${requestUserId} on challenge ${challengeId}`, | ||
| error instanceof Error ? error.message : error, | ||
| ); | ||
| } | ||
|
|
@@ -150,7 +151,11 @@ export class ChallengePaymentsService { | |
| roles.push(String(role)); | ||
| } | ||
| }); | ||
| } else if (value) { | ||
| } else if ( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| typeof value === 'string' || | ||
| typeof value === 'number' || | ||
| typeof value === 'boolean' | ||
| ) { | ||
| roles.push(String(value)); | ||
| } | ||
| }); | ||
|
|
@@ -165,7 +170,7 @@ export class ChallengePaymentsService { | |
| return roles; | ||
| } | ||
|
|
||
| private async isCopilotForChallenge( | ||
| private async hasChallengeWideAccessForChallenge( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [💡 |
||
| challengeId: string, | ||
| userId: string, | ||
| ): Promise<boolean> { | ||
|
|
@@ -181,19 +186,26 @@ export class ChallengePaymentsService { | |
| return false; | ||
| } | ||
|
|
||
| const copilotRoleIds = new Set( | ||
| const challengeWideRoleIds = new Set( | ||
| resourceRoles | ||
| ?.filter((role) => | ||
| role?.name ? role.name.toLowerCase().includes('copilot') : false, | ||
| ?.filter( | ||
| (role) => | ||
| role?.fullWriteAccess === true || | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| (role?.name | ||
| ? role.name.toLowerCase().includes('copilot') || | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [💡 |
||
| role.name.toLowerCase().includes('manager') | ||
| : false), | ||
| ) | ||
| .map((role) => role.id), | ||
| ); | ||
|
|
||
| if (copilotRoleIds.size === 0) { | ||
| if (challengeWideRoleIds.size === 0) { | ||
| return false; | ||
| } | ||
|
|
||
| return resources.some((resource) => copilotRoleIds.has(resource.roleId)); | ||
| return resources.some((resource) => | ||
| challengeWideRoleIds.has(resource.roleId), | ||
| ); | ||
| } | ||
|
|
||
| private async fetchWinnings( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| import { ChallengeStatuses } from '../../dto/challenge.dto'; | ||
|
|
||
| jest.mock('src/config', () => ({ | ||
| ENV_CONFIG: { | ||
| TGBillingAccounts: [], | ||
| TOPCODER_API_V6_BASE_URL: 'https://api.topcoder-dev.com/v6', | ||
| }, | ||
| })); | ||
|
|
||
| jest.mock('src/shared/global', () => ({ | ||
| Logger: class { | ||
| debug = jest.fn(); | ||
|
|
||
| error = jest.fn(); | ||
|
|
||
| info = jest.fn(); | ||
|
|
||
| log = jest.fn(); | ||
|
|
||
| warn = jest.fn(); | ||
| }, | ||
| })); | ||
|
|
||
| import { ChallengesService } from './challenges.service'; | ||
|
|
||
| describe('ChallengesService', () => { | ||
| it('skips creating payments for fun challenges', async () => { | ||
| const prisma = { | ||
| challenge_lock: { | ||
| create: jest.fn(), | ||
| deleteMany: jest.fn(), | ||
| }, | ||
| }; | ||
| const service = new ChallengesService( | ||
| prisma as any, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| {} as any, | ||
| {} as any, | ||
| {} as any, | ||
| {} as any, | ||
| ); | ||
| const challenge = { | ||
| funChallenge: true, | ||
| id: '11111111-1111-1111-1111-111111111111', | ||
| name: 'MM 163', | ||
| status: ChallengeStatuses.Completed, | ||
| }; | ||
| const createPaymentsSpy = jest | ||
| .spyOn(service as any, 'createPayments') | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| .mockResolvedValue(undefined); | ||
|
|
||
| jest.spyOn(service, 'getChallenge').mockResolvedValue(challenge as any); | ||
|
|
||
| await service.generateChallengePayments( | ||
| '11111111-1111-1111-1111-111111111111', | ||
| 'test-user', | ||
| ); | ||
|
|
||
| expect(createPaymentsSpy).not.toHaveBeenCalled(); | ||
| expect(prisma.challenge_lock.create).not.toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,7 +41,7 @@ interface PaymentPayload { | |
| amount: number; | ||
| userId: string; | ||
| type: WinningsCategory; | ||
| currency: string; | ||
| currency: PrizeType; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [❗❗ |
||
| description?: string; | ||
| } | ||
|
|
||
|
|
@@ -566,6 +566,13 @@ export class ChallengesService { | |
| throw new Error("Challenge isn't in a payable status!"); | ||
| } | ||
|
|
||
| if (challenge.funChallenge === true) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| this.logger.log( | ||
| `Skipping payment generation for fun challenge ${challenge.id} (${challenge.name}).`, | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| // need to read for update (LOCK the rows) | ||
| this.logger.log(`Attempting to acquire lock for challenge ${challenge.id}`); | ||
| try { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ export interface Challenge { | |
| timelineTemplateId: string; | ||
| currentPhaseNames: string[]; | ||
| wiproAllowed: boolean; | ||
| funChallenge?: boolean; | ||
| tags: string[]; | ||
| groups: string[]; | ||
| submissionStartDate: string; | ||
|
|
@@ -147,6 +148,7 @@ export interface ChallengeResource { | |
| export interface ResourceRole { | ||
| id: string; | ||
| name: string; | ||
| fullWriteAccess?: boolean; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| } | ||
|
|
||
| export interface ChallengeReview { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[⚠️
correctness]Consider adding a conditional check to ensure that the
winningstable exists before attempting to create the index. This can prevent potential errors during the migration if the table is not present.