Conversation
…emplateId and attach default ai config when creating a challenge
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
prisma/migrations/20260225100000_rename_default_ai_workflow_id/migration.sql
Show resolved
Hide resolved
| { isMachine: true, sub: 'sub', userId: 'testuser' }, | ||
| challengeData, | ||
| config.M2M_FULL_ACCESS_TOKEN | ||
| config.M2M_FULL_ACCESS_TOKEN || 'test-token' |
There was a problem hiding this comment.
[❗❗ security]
Using a default token 'test-token' when config.M2M_FULL_ACCESS_TOKEN is not set could lead to unexpected behavior in production environments. Consider ensuring that this fallback is only used in test environments to prevent security issues.
| const result = await service.createChallenge( | ||
| { isMachine: true, sub: 'sub', userId: 'testuser' }, | ||
| challengeData, | ||
| config.M2M_FULL_ACCESS_TOKEN || 'test-token' |
There was a problem hiding this comment.
[❗❗ security]
Using a default token 'test-token' when config.M2M_FULL_ACCESS_TOKEN is not set could lead to unexpected behavior in production environments. Consider ensuring that this fallback is only used in test environments to prevent security issues.
| const result = await service.createChallenge( | ||
| { isMachine: true, sub: 'sub', userId: 'testuser' }, | ||
| challengeData, | ||
| config.M2M_FULL_ACCESS_TOKEN || 'test-token' |
There was a problem hiding this comment.
[❗❗ security]
Using a default token 'test-token' when config.M2M_FULL_ACCESS_TOKEN is not set could lead to unexpected behavior in production environments. Consider ensuring that this fallback is only used in test environments to prevent security issues.
kkartunov
changed the title
…v6 into PM-4062_ai-config
| numOfCheckpointSubmissions: 'numOfCheckpointSubmissions', | ||
| currentPhaseNames: 'currentPhaseNames', | ||
| wiproAllowed: 'wiproAllowed', | ||
| funChallenge: 'funChallenge', |
There was a problem hiding this comment.
[❗❗ correctness]
The addition of funChallenge to ChallengeScalarFieldEnum should be verified against the database schema to ensure consistency and correctness. Ensure that this field exists in the database and is correctly mapped.
| incrementalCoefficient: 'incrementalCoefficient', | ||
| opportunityType: 'opportunityType', | ||
| aiWorkflowId: 'aiWorkflowId', | ||
| aiConfigTemplateId: 'aiConfigTemplateId', |
There was a problem hiding this comment.
[❗❗ correctness]
The renaming of aiWorkflowId to aiConfigTemplateId in DefaultChallengeReviewerScalarFieldEnum should be checked for consistency across all related database schemas and application logic to prevent potential mismatches or errors.
| phaseName: 'phaseName', | ||
| phaseId: 'phaseId', | ||
| aiWorkflowId: 'aiWorkflowId', | ||
| aiConfigTemplateId: 'aiConfigTemplateId', |
There was a problem hiding this comment.
[❗❗ correctness]
Ensure that the renaming of aiWorkflowId to aiConfigTemplateId in DefaultChallengeReviewerOrderByRelevanceFieldEnum is consistently applied across all relevant parts of the codebase and database schema to avoid any discrepancies.
| currentPhaseNames String[] // current phase names | ||
|
|
||
| wiproAllowed Boolean @default(false) | ||
| funChallenge Boolean @default(false) |
There was a problem hiding this comment.
[maintainability]
The addition of the funChallenge field with a default value of false seems straightforward, but ensure that this change is reflected in any related business logic or API endpoints that might need to handle this new field.
| updatedAt DateTime @updatedAt | ||
| updatedBy String | ||
|
|
||
| @@index([challengeId]) |
There was a problem hiding this comment.
[💡 performance]
Adding an index on challengeId in the ChallengeTerm model is a good practice for performance, especially if queries frequently filter by this field. Ensure that this index is necessary and beneficial for the expected query patterns.
| incrementalCoefficient Float? | ||
| opportunityType ReviewOpportunityTypeEnum? | ||
| aiWorkflowId String? @db.VarChar(14) | ||
| aiConfigTemplateId String? @db.VarChar(14) |
There was a problem hiding this comment.
[❗❗ correctness]
Renaming aiWorkflowId to aiConfigTemplateId requires careful consideration of backward compatibility. Ensure that all parts of the system that interact with this field are updated accordingly, including any external integrations or documentation.
2 participants
This pull request introduces significant improvements to how default reviewers and AI review configurations are handled during challenge creation. It refactors the process to separate member reviewer and AI reviewer logic, introduces new helper methods for applying and creating AI review configs, and replaces the
aiWorkflowIdfield withaiConfigTemplateIdthroughout the codebase and database schema. Additionally, it adds configuration for the reviews API and makes minor test fixture corrections.Refactored challenge creation to use new helper methods:
applyDefaultMemberReviewersForChallengeCreationandapplyDefaultAIConfigForChallengeCreation, which separately handle member and AI reviewers, improving clarity and extensibility. AI review configurations are created via the reviews API only when reviewers are not provided explicitly.Renamed the
aiWorkflowIdfield toaiConfigTemplateIdin theDefaultChallengeReviewermodel and throughout the codebase, including validation schemas, normalization logic, and API payloads. This aligns the naming with the new AI review template-based approach.Added
REVIEWS_API_URLto configuration files and environment variable templates to support dynamic endpoint configuration for the reviews API. (config/default.js,docs/dev.env,docs/prod.env). NOTE we need to make sure this exists in prod.Test and Fixture Corrections:
'DEVELOP'to'DEVELOPMENT'to match expected values. Also included a missing import in a unit test file. (test/testHelper.js,test/unit/ChallengeService.test.js)NOTE: Auth0 scopes need to be updated for
v6 Challenge APIin prod