Skip to content

Conversation

@kkartunov
Copy link
Contributor

No description provided.

@kkartunov kkartunov requested review from jmgasper and vas3a October 29, 2025 08:09
jobs:
trivy-scan:
name: Use Trivy
runs-on: ubuntu-24.04

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Consider using ubuntu-latest instead of a specific version like ubuntu-24.04 to ensure the workflow uses the most up-to-date and supported version of the runner. This can help avoid issues with deprecated or unsupported versions in the future.

output: "trivy-results.sarif"
severity: "CRITICAL,HIGH,UNKNOWN"
scanners: vuln,secret,misconfig,license
github-pat: ${{ secrets.GITHUB_TOKEN }}

Choose a reason for hiding this comment

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

[⚠️ security]
Ensure that the GITHUB_TOKEN secret has the necessary permissions for the actions being performed. While the default permissions should suffice, it's important to verify this to avoid unexpected permission issues.

// Database schemas for direct counts (shared DB)
RESOURCES_DB_SCHEMA: process.env.RESOURCES_DB_SCHEMA || "resources",
REVIEW_DB_SCHEMA: process.env.REVIEW_DB_SCHEMA || "reviews",
CHALLENGE_SERVICE_PRISMA_TIMEOUT: process.env.CHALLENGE_SERVICE_PRISMA_TIMEOUT ? parseInt(process.env.CHALLENGE_SERVICE_PRISMA_TIMEOUT, 10) : 10000,

Choose a reason for hiding this comment

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

[⚠️ correctness]
Consider using Number() instead of parseInt() for parsing CHALLENGE_SERVICE_PRISMA_TIMEOUT. Number() will convert the entire string to a number, while parseInt() stops at the first non-numeric character, which might lead to unexpected results if the environment variable contains non-numeric characters.

@@ -0,0 +1,29 @@
-- CreateIndex
CREATE INDEX "Challenge_status_startDate_idx" ON "Challenge"("status", "startDate");

Choose a reason for hiding this comment

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

[⚠️ performance]
Consider the selectivity of the status and startDate columns when creating this index. If status has low cardinality, this index might not be as effective as expected.

CREATE INDEX "Challenge_status_startDate_idx" ON "Challenge"("status", "startDate");

-- CreateIndex
CREATE INDEX "Challenge_trackId_typeId_status_idx" ON "Challenge"("trackId", "typeId", "status");

Choose a reason for hiding this comment

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

[⚠️ performance]
Ensure that the order of columns in this index (trackId, typeId, status) aligns with the most common query patterns. The order can significantly impact the index's effectiveness.

CREATE INDEX "Challenge_legacyId_idx" ON "Challenge"("legacyId");

-- CreateIndex
CREATE INDEX "Challenge_projectId_status_idx" ON "Challenge"("projectId", "status");

Choose a reason for hiding this comment

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

[⚠️ performance]
Verify that the projectId and status columns are frequently queried together. Otherwise, this index might not provide the expected performance benefits.

CREATE INDEX "Challenge_projectId_status_idx" ON "Challenge"("projectId", "status");

-- CreateIndex
CREATE INDEX "ChallengePhase_challengeId_isOpen_idx" ON "ChallengePhase"("challengeId", "isOpen");

Choose a reason for hiding this comment

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

[⚠️ performance]
Consider the impact of indexing the isOpen column if it has low cardinality. This might not be beneficial if the column has few distinct values.

CREATE INDEX "ChallengeReviewer_challengeId_phaseId_idx" ON "ChallengeReviewer"("challengeId", "phaseId");

-- CreateIndex
CREATE INDEX "ChallengeWinner_challengeId_type_placement_idx" ON "ChallengeWinner"("challengeId", "type", "placement");

Choose a reason for hiding this comment

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

[⚠️ performance]
Check if the placement column is used in range queries or sorting operations. If not, its inclusion in the index might not be necessary.

transactionOptions: {
maxWait: Number(process.env.PRISMA_TRANSACTION_MAX_WAIT_MS || 10000), // wait up to 10s to start
timeout: Number(process.env.PRISMA_TRANSACTION_TIMEOUT_MS || 10000), // allow up to 30s per transaction
timeout: config.CHALLENGE_SERVICE_PRISMA_TIMEOUT, // allow up to 30s per transaction

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The change from an environment variable to a config value for timeout means that the timeout is now controlled by the config module rather than environment variables. Ensure that config.CHALLENGE_SERVICE_PRISMA_TIMEOUT is correctly set and documented, as this could impact transaction behavior if not properly configured.

transactionOptions: {
maxWait: Number(process.env.PRISMA_TRANSACTION_MAX_WAIT_MS || 10000),
timeout: Number(process.env.PRISMA_TRANSACTION_TIMEOUT_MS || 10000),
timeout: config.CHALLENGE_SERVICE_PRISMA_TIMEOUT,

Choose a reason for hiding this comment

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

[❗❗ correctness]
The timeout value is now being set from config.CHALLENGE_SERVICE_PRISMA_TIMEOUT. Ensure that this configuration value is validated and defaults are handled appropriately elsewhere in the codebase. This change assumes that config.CHALLENGE_SERVICE_PRISMA_TIMEOUT is always set and valid, which could lead to runtime errors if not properly managed.

* @param {Object} res the response
*/
async function searchDefaultChallengeReviewers(req, res) {
const result = await service.searchDefaultChallengeReviewers(req.query);

Choose a reason for hiding this comment

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

[❗❗ correctness]
Consider adding error handling for the service.searchDefaultChallengeReviewers call to manage potential exceptions and provide meaningful error responses.

* @param {Object} res the response
*/
async function createDefaultChallengeReviewer(req, res) {
const result = await service.createDefaultChallengeReviewer(req.authUser, req.body);

Choose a reason for hiding this comment

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

[❗❗ correctness]
Consider adding error handling for the service.createDefaultChallengeReviewer call to manage potential exceptions and provide meaningful error responses.

* @param {Object} res the response
*/
async function getDefaultChallengeReviewer(req, res) {
const result = await service.getDefaultChallengeReviewer(

Choose a reason for hiding this comment

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

[❗❗ correctness]
Consider adding error handling for the service.getDefaultChallengeReviewer call to manage potential exceptions and provide meaningful error responses.

* @param {Object} res the response
*/
async function fullyUpdateDefaultChallengeReviewer(req, res) {
const result = await service.fullyUpdateDefaultChallengeReviewer(

Choose a reason for hiding this comment

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

[❗❗ correctness]
Consider adding error handling for the service.fullyUpdateDefaultChallengeReviewer call to manage potential exceptions and provide meaningful error responses.

* @param {Object} res the response
*/
async function partiallyUpdateDefaultChallengeReviewer(req, res) {
const result = await service.partiallyUpdateDefaultChallengeReviewer(

Choose a reason for hiding this comment

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

[❗❗ correctness]
Consider adding error handling for the service.partiallyUpdateDefaultChallengeReviewer call to manage potential exceptions and provide meaningful error responses.

* @param {Object} res the response
*/
async function deleteDefaultChallengeReviewer(req, res) {
const result = await service.deleteDefaultChallengeReviewer(

Choose a reason for hiding this comment

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

[❗❗ correctness]
Consider adding error handling for the service.deleteDefaultChallengeReviewer call to manage potential exceptions and provide meaningful error responses.

controller: "DefaultChallengeReviewerController",
method: "searchDefaultChallengeReviewers",
auth: "jwt",
access: [constants.UserRoles.Admin],

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Consider adding more roles to the access list for the /default-challenge-reviewers endpoint if other roles besides Admin should have access. This will enhance flexibility and maintainability if access requirements change in the future.

controller: "DefaultChallengeReviewerController",
method: "getDefaultChallengeReviewer",
auth: "jwt",
access: [constants.UserRoles.Admin],

Choose a reason for hiding this comment

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

[⚠️ security]
The access list for the /default-challenge-reviewers/:defaultChallengeReviewerId endpoint is limited to Admin. Ensure this aligns with the intended access control policy, as it might restrict legitimate users from accessing this resource.

let rows;
try {
rows = await reviewPrisma.$queryRaw(
Prisma.sql`

Choose a reason for hiding this comment

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

[⚠️ correctness]
The SQL query uses LEFT JOIN on appealResponseTable but then checks ar."id" IS NULL. This pattern is correct for finding appeals without responses, but ensure that this behavior is intentional and aligns with business logic.


async function ensureRequiredResourcesBeforeOpeningPhase(challengeId, phaseName) {
const normalizedPhaseName = _.toLower(_.trim(phaseName || ""));
const requiredRoleName = PHASE_RESOURCE_ROLE_REQUIREMENTS[normalizedPhaseName];

Choose a reason for hiding this comment

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

[⚠️ correctness]
The PHASE_RESOURCE_ROLE_REQUIREMENTS object is accessed using a normalized phase name. Ensure that all possible phase names are covered in this object to avoid potential undefined values.

);
}
if (
String(closingPhaseName || "").toLowerCase() === "appeals response" &&

Choose a reason for hiding this comment

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

[⚠️ correctness]
The check for closingPhaseName being 'appeals response' is case-sensitive. Consider normalizing closingPhaseName to avoid potential issues with case variations.

const perPage = criteria.perPage || 50;

const [total, rows] = await Promise.all([
prisma.defaultChallengeReviewer.count({ where: searchFilter }),

Choose a reason for hiding this comment

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

[⚠️ correctness]
Consider adding error handling for the prisma.defaultChallengeReviewer.count and prisma.defaultChallengeReviewer.findMany calls to handle potential database errors gracefully.

payload.createdBy = userId;
payload.updatedBy = userId;

const duplicate = await prisma.defaultChallengeReviewer.findFirst({

Choose a reason for hiding this comment

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

[⚠️ correctness]
The findFirst method is used here to check for duplicates. Ensure that the combination of fields used in the where clause is sufficient to uniquely identify a duplicate. Otherwise, consider using a more specific query or adding a unique constraint at the database level.

* @returns {Promise<Object>} updated record
*/
async function fullyUpdateDefaultChallengeReviewer(authUser, id, data) {
await getDefaultChallengeReviewer(id);

Choose a reason for hiding this comment

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

[⚠️ correctness]
Consider handling the case where getDefaultChallengeReviewer(id) might throw a NotFoundError. This could be done by catching the error and returning a more user-friendly message or status code.

* @returns {Promise<Object>} deleted record
*/
async function deleteDefaultChallengeReviewer(id) {
const existing = await prisma.defaultChallengeReviewer.findUnique({

Choose a reason for hiding this comment

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

[⚠️ correctness]
The deleteDefaultChallengeReviewer function does not handle potential errors from the prisma.defaultChallengeReviewer.delete call. Consider adding error handling to manage database errors gracefully.

"reviewItemCommentId" varchar(14) NOT NULL
)
`)
await reviewClient.$executeRawUnsafe(`

Choose a reason for hiding this comment

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

[❗❗ security]
Using $executeRawUnsafe can be risky as it may expose the application to SQL injection vulnerabilities. Consider using parameterized queries or $executeRaw with proper parameter binding to mitigate this risk.


after(async () => {
if (reviewClient) {
await reviewClient.$executeRawUnsafe(`TRUNCATE TABLE "${reviewSchema}"."appealResponse"`)

Choose a reason for hiding this comment

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

[❗❗ security]
Using $executeRawUnsafe for truncating tables can be risky if the table names are derived from user input or untrusted sources. Ensure that reviewSchema is sanitized and validated to prevent SQL injection.

}
})

try {

Choose a reason for hiding this comment

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

[⚠️ security]
Using $executeRaw with Prisma.sql for inserting data is safer than $executeRawUnsafe, but ensure that all variables used in the query are properly sanitized and validated to prevent SQL injection.

"Appeals Response phase can't be closed because there are still appeals that haven't been responded to"
)
} finally {
await reviewClient.$executeRaw(

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Ensure that the cleanup operations in the finally block are robust and handle any potential errors that might occur during the deletion of records. Consider wrapping each delete operation in a try-catch block to prevent partial cleanup in case of an error.

{ id: 'reviewer-role-id', name: 'Reviewer' }
])

try {

Choose a reason for hiding this comment

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

[⚠️ design]
The test case modifies global helper functions (getChallengeResources and getResourceRoles) which could lead to side effects in other tests. Consider using a mocking library to temporarily override these functions within the test scope.

{ id: 'reviewer-role-id', name: 'Reviewer' }
])

try {

Choose a reason for hiding this comment

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

[⚠️ design]
The test case modifies global helper functions (getChallengeResources and getResourceRoles) which could lead to side effects in other tests. Consider using a mocking library to temporarily override these functions within the test scope.


it('preserves existing terms when update payload omits the terms field', async () => {
const challengeData = _.cloneDeep(testChallengeData)
challengeData.name = `${challengeData.name} Terms ${Date.now()}`

Choose a reason for hiding this comment

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

[⚠️ correctness]
Using Date.now() for generating a unique name is not reliable for uniqueness due to potential race conditions. Consider using a UUID or a more robust unique identifier.

updatedBy: 'unit-test'
}
]
await prisma.challengeTerm.createMany({ data: termRecords })

Choose a reason for hiding this comment

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

[⚠️ correctness]
Ensure that prisma.challengeTerm.createMany handles potential errors, such as database constraints or connectivity issues, to prevent unhandled promise rejections.

)
sortedTerms.should.deep.equal(expectedTerms)
} finally {
await prisma.challenge.delete({ where: { id: challengeWithTerms.id } })

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Consider using a transaction to ensure that both the creation of terms and the deletion of the challenge are atomic. This will prevent partial updates if an error occurs.

return knownFields;
};

const validateIncrementalFieldConfiguration = (config) => {

Choose a reason for hiding this comment

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

[⚠️ design]
The function validateIncrementalFieldConfiguration directly logs warnings and errors to the console. Consider refactoring to allow the caller to handle logging, which would improve testability and flexibility in handling different logging mechanisms.

* Works in tandem with INCREMENTAL_SINCE_DATE to scope the incremental window.
*/
INCREMENTAL_FIELDS: parseListEnv(process.env.INCREMENTAL_FIELDS),
INCREMENTAL_DATE_FIELDS: parseListEnv(process.env.INCREMENTAL_DATE_FIELDS) || ['updatedAt', 'updated'],

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 INCREMENTAL_DATE_FIELDS is set to ['updatedAt', 'updated'] if the environment variable is not provided. Ensure that these default fields are appropriate for all use cases, as hardcoding defaults can lead to unexpected behavior if the schema changes.

},
};

config.incrementalFieldValidation = validateIncrementalFieldConfiguration(config);

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Assigning the result of validateIncrementalFieldConfiguration to config.incrementalFieldValidation might lead to unexpected behavior if the configuration is modified after this assignment. Consider making this validation a function that can be called to get the latest validation results.

} else {
const now = new Date();
if (sinceDateObj > now) {
this.manager.logger.error(`${this.modelName} incremental run date ${sinceDateObj.toISOString()} is in the future; this may yield zero records.`);

Choose a reason for hiding this comment

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

[⚠️ correctness]
The check for sinceDateObj > now could lead to unexpected behavior if the system clock is incorrect. Consider logging a warning if the system time is significantly different from a reliable time source.

if (sinceDateObj > now) {
this.manager.logger.error(`${this.modelName} incremental run date ${sinceDateObj.toISOString()} is in the future; this may yield zero records.`);
}
const oneYearMs = 365 * 24 * 60 * 60 * 1000;

Choose a reason for hiding this comment

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

[💡 correctness]
The calculation for oneYearMs assumes a year has 365 days, which is not accurate for leap years. Consider using a more precise method to calculate the difference in years.

}
}

async _executeUpsertWithStats(prisma, upsertData) {

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The _executeUpsertWithStats method includes a try-catch block for findUnique but not for upsert. Consider adding error handling for the upsert operation to ensure any issues are logged and managed appropriately.

const start = Date.now();
let existing = null;
if (this.collectUpsertStats) {
try {

Choose a reason for hiding this comment

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

[💡 readability]
The collectUpsertStats flag is checked multiple times within _executeUpsertWithStats. Consider consolidating these checks to improve readability and reduce redundancy.

} catch (error) {
const resolution = await this.handleUpsertError(error, upsertData);

if (resolution?.retryData) {

Choose a reason for hiding this comment

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

[⚠️ correctness]
In performUpsert, the retry logic does not handle the case where resolution.retryData is malformed or causes another type of error. Consider adding validation or additional error handling for retryData.

const config = require('../config');
const isDebugLog = (config.LOG_VERBOSITY || '').toLowerCase() === 'debug';
const summaryLimit = Number.isFinite(config.SUMMARY_LOG_LIMIT)
? Math.max(0, config.SUMMARY_LOG_LIMIT)

Choose a reason for hiding this comment

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

[💡 maintainability]
Using Math.max(0, config.SUMMARY_LOG_LIMIT) ensures that summaryLimit is non-negative, but it might be more intuitive to handle non-numeric values explicitly before this check. Consider validating config.SUMMARY_LOG_LIMIT separately to ensure it's a number before applying Math.max.

};

const parseBehavior = (behavior) => {
const normalized = (behavior || '').toLowerCase().trim();

Choose a reason for hiding this comment

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

[💡 maintainability]
The parseBehavior function uses a default strategy of 'skip' for unrecognized behavior strings. Consider logging a warning when an unrecognized behavior is encountered to aid in debugging and configuration validation.

outcome.usedField = usedField;
const recordDateValue = record[usedField];
const parsedDate = new Date(recordDateValue);
if (Number.isNaN(parsedDate.getTime())) {

Choose a reason for hiding this comment

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

[⚠️ correctness]
The new Date(recordDateValue) conversion does not handle invalid date formats robustly. Consider using a library like date-fns or moment for more reliable date parsing and validation, especially if date formats can vary.

const nowTimestamp = Date.now();
let suspiciousReason = null;

if (parsedDate.getTime() > nowTimestamp) {

Choose a reason for hiding this comment

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

[💡 design]
The check for future dates (parsedDate.getTime() > nowTimestamp) assumes that any future date is suspicious. If future dates are valid in some contexts, consider making this behavior configurable.

}
}

if (parsedDate.getFullYear() < 2000) {

Choose a reason for hiding this comment

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

[💡 design]
The check for 'ancient' dates (parsedDate.getFullYear() < 2000) is hardcoded. Consider making the threshold year configurable to accommodate different use cases or datasets.

// For Elasticsearch format (line-delimited JSON)
const results = [];
const fileStream = fs.createReadStream(filePath, {
encoding: 'utf8',

Choose a reason for hiding this comment

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

[💡 performance]
The highWaterMark option for fs.createReadStream is set to 1MB. Ensure this buffer size is appropriate for the expected file sizes and system memory constraints to avoid potential performance issues.


if (suspiciousReason) {
if (invalidDateBehavior.warn) {
console.warn(`${fileName}: record ${recordIdentifier} has ${suspiciousReason.replace('|', ' & ')} (${parsedDate.toISOString()}); strategy=${invalidDateBehavior.strategy}`);

Check failure

Code scanning / CodeQL

Incomplete string escaping or encoding High

This replaces only the first occurrence of '|'.

Copilot Autofix

AI 11 days ago

The best way to fix this problem is to ensure that all occurrences of the pipe character (|) in the suspiciousReason string are replaced with " & ". This can be achieved by using a regular expression with the global flag (/g), specifically .replace(/\|/g, ' & '), so that every pipe in the string is replaced—not just the first. Only line 178 needs to be changed, and no additional imports or method definitions are required.


Suggested changeset 1
data-migration/src/utils/dataLoader.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/data-migration/src/utils/dataLoader.js b/data-migration/src/utils/dataLoader.js
--- a/data-migration/src/utils/dataLoader.js
+++ b/data-migration/src/utils/dataLoader.js
@@ -175,7 +175,7 @@
 
     if (suspiciousReason) {
       if (invalidDateBehavior.warn) {
-        console.warn(`${fileName}: record ${recordIdentifier} has ${suspiciousReason.replace('|', ' & ')} (${parsedDate.toISOString()}); strategy=${invalidDateBehavior.strategy}`);
+        console.warn(`${fileName}: record ${recordIdentifier} has ${suspiciousReason.replace(/\|/g, ' & ')} (${parsedDate.toISOString()}); strategy=${invalidDateBehavior.strategy}`);
       }
       if (!invalidDateBehavior.include) {
         invalidDateSkippedCount += 1;
EOF
@@ -175,7 +175,7 @@

if (suspiciousReason) {
if (invalidDateBehavior.warn) {
console.warn(`${fileName}: record ${recordIdentifier} has ${suspiciousReason.replace('|', ' & ')} (${parsedDate.toISOString()}); strategy=${invalidDateBehavior.strategy}`);
console.warn(`${fileName}: record ${recordIdentifier} has ${suspiciousReason.replace(/\|/g, ' & ')} (${parsedDate.toISOString()}); strategy=${invalidDateBehavior.strategy}`);
}
if (!invalidDateBehavior.include) {
invalidDateSkippedCount += 1;
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
@@ -0,0 +1,13 @@
-- Add indexes to support faster `/v6/my-reviews` queries.

CREATE EXTENSION IF NOT EXISTS pg_trgm;

Choose a reason for hiding this comment

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

[❗❗ correctness]
Ensure that the pg_trgm extension is supported and enabled in all environments where this migration will be applied. This extension is not enabled by default in all PostgreSQL installations and could lead to deployment issues.

ON "Challenge" ("status", "typeId", "trackId", "createdAt" DESC);

CREATE INDEX IF NOT EXISTS "challenge_phase_challenge_open_end_idx"
ON "ChallengePhase" ("challengeId", "isOpen", "scheduledEndDate", "actualEndDate");

Choose a reason for hiding this comment

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

[⚠️ performance]
Consider the order of columns in the challenge_phase_challenge_open_end_idx index. If queries often filter by isOpen first, it might be beneficial to place isOpen as the first column in the index for better performance.

@@index([endDate])
@@index([status, startDate])
@@index([trackId, typeId, status])
@@index([status, typeId, trackId, createdAt(sort: Desc)], map: "challenge_status_type_track_created_at_idx")

Choose a reason for hiding this comment

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

[⚠️ performance]
The index challenge_status_type_track_created_at_idx includes createdAt with a descending sort order. Ensure that this aligns with the query patterns you expect to optimize, as descending order can impact index usage and performance.

@@index([status, startDate])
@@index([trackId, typeId, status])
@@index([status, typeId, trackId, createdAt(sort: Desc)], map: "challenge_status_type_track_created_at_idx")
@@index([name], type: Gin, ops: [gin_trgm_ops], map: "challenge_name_trgm_idx")

Choose a reason for hiding this comment

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

[⚠️ performance]
The use of a Gin index with gin_trgm_ops on the name field is appropriate for text search. However, consider the potential impact on write performance due to the overhead of maintaining this index, especially if the name field is frequently updated.

@@index([challengeId])
@@index([challengeId, isOpen])
@@index([challengeId, name])
@@index([challengeId, isOpen, scheduledEndDate, actualEndDate], map: "challenge_phase_challenge_open_end_idx")

Choose a reason for hiding this comment

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

[⚠️ performance]
The index challenge_phase_challenge_open_end_idx includes both scheduledEndDate and actualEndDate. Ensure that queries requiring both fields are common enough to justify the composite index, as maintaining it could affect performance.

generator client {
provider = "prisma-client-js"
previewFeatures = ["fullTextSearchPostgres"]
previewFeatures = ["fullTextSearchPostgres", "postgresqlExtensions"]

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Adding postgresqlExtensions to previewFeatures could introduce instability if the feature is not yet fully supported or stable. Ensure that this feature is necessary and that any potential issues are documented and tested.

@@index([status, startDate])
@@index([trackId, typeId, status])
@@index([status, typeId, trackId, createdAt(sort: Desc)], map: "challenge_status_type_track_created_at_idx")
@@index([name(ops: raw("pg_catalog.gin_trgm_ops"))], type: Gin, map: "challenge_name_trgm_idx")

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The use of raw("pg_catalog.gin_trgm_ops") in the index definition is technically correct, but it introduces raw SQL into the Prisma schema. Ensure that this is well-documented and understood by the team, as it may affect portability and maintainability of the schema.

@jmgasper jmgasper merged commit c57383e into master Nov 1, 2025
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants