Skip to content

Pm 2206 master #38

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 11, 2025
Merged

Pm 2206 master #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2d74910
Performance for past challenges call
jmgasper Nov 11, 2025
292aa4d
Performance updates for PM-2206
jmgasper Nov 11, 2025
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
61 changes: 57 additions & 4 deletions prisma/migrations/20251107090000_add_my_reviews_indexes/migration.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,66 @@
-- Add indexes to support faster `/v6/my-reviews` queries.

CREATE EXTENSION IF NOT EXISTS pg_trgm;
CREATE SCHEMA IF NOT EXISTS skills;
CREATE SCHEMA IF NOT EXISTS reviews;

CREATE EXTENSION IF NOT EXISTS fuzzystrmatch WITH SCHEMA skills;
Copy link

@github-actions github-actions bot Nov 11, 2025

Choose a reason for hiding this comment

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

[💡 design]
The fuzzystrmatch extension is being created in the skills schema. Ensure that this schema is the intended location for this extension, as it might be more conventional to place extensions in the public or pg_catalog schemas unless there's a specific reason for this choice.

CREATE EXTENSION IF NOT EXISTS pg_trgm WITH SCHEMA pg_catalog;
CREATE EXTENSION IF NOT EXISTS pgcrypto WITH SCHEMA reviews;
Copy link

@github-actions github-actions bot Nov 11, 2025

Choose a reason for hiding this comment

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

[💡 design]
The pgcrypto extension is being created in the reviews schema. Verify that this schema is the intended location for this extension, as extensions are typically placed in the public or pg_catalog schemas unless there's a specific reason for this choice.

CREATE EXTENSION IF NOT EXISTS "uuid-ossp" WITH SCHEMA skills;
Copy link

@github-actions github-actions bot Nov 11, 2025

Choose a reason for hiding this comment

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

[💡 design]
The uuid-ossp extension is being created in the skills schema. Confirm that this schema is the intended location for this extension, as it is more common to place extensions in the public or pg_catalog schemas unless there's a specific reason for this choice.


CREATE INDEX IF NOT EXISTS "challenge_status_type_track_created_at_idx"
ON "Challenge" ("status", "typeId", "trackId", "createdAt" DESC);

DROP INDEX IF EXISTS "challenge_name_idx";

CREATE INDEX IF NOT EXISTS "challenge_name_trgm_idx"
ON "Challenge" USING gin ("name" pg_catalog.gin_trgm_ops);

DO
$$
DECLARE
challenge_phase_schema TEXT;
BEGIN
SELECT n.nspname
INTO challenge_phase_schema
FROM pg_class c
JOIN pg_namespace n ON n.oid = c.relnamespace
WHERE c.relname = 'ChallengePhase'
AND c.relkind = 'r'
LIMIT 1;

IF challenge_phase_schema IS NULL THEN
RETURN;
END IF;

IF NOT EXISTS (
SELECT 1
FROM pg_class idx
JOIN pg_namespace ns ON ns.oid = idx.relnamespace
WHERE idx.relname = 'challenge_phase_order_idx'
AND ns.nspname = challenge_phase_schema
)
AND EXISTS (
SELECT 1
FROM pg_class idx
JOIN pg_namespace ns ON ns.oid = idx.relnamespace
WHERE idx.relname = 'challenge_phase_challenge_open_end_idx'
AND ns.nspname = challenge_phase_schema
AND pg_get_indexdef(idx.oid) LIKE '%("challengeId", "isOpen", "scheduledEndDate", "actualEndDate", name)%'
Copy link

@github-actions github-actions bot Nov 11, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The pg_get_indexdef function is used with a LIKE clause to check index definitions. This approach may be fragile if the index definition changes in the future. Consider using a more robust method to verify index columns, such as querying pg_index and pg_attribute tables directly.

)
THEN
EXECUTE format(
'ALTER INDEX %I.%I RENAME TO %I',
challenge_phase_schema,
'challenge_phase_challenge_open_end_idx',
'challenge_phase_order_idx'
);
END IF;
END
$$ LANGUAGE plpgsql;

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

CREATE INDEX IF NOT EXISTS "challenge_name_trgm_idx"
ON "Challenge"
USING gin ("name" pg_catalog.gin_trgm_ops);
CREATE INDEX IF NOT EXISTS "challenge_phase_order_idx"
ON "ChallengePhase" ("challengeId", "isOpen", "scheduledEndDate", "actualEndDate", "name");
30 changes: 30 additions & 0 deletions prisma/migrations/20251121140000_member_access_view/migration.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
-- View to use in performance updates (PM-2206)
DROP VIEW IF EXISTS "challenges"."MemberChallengeAccess";

DO $$
BEGIN
IF EXISTS (
SELECT 1
FROM information_schema.tables
WHERE table_schema = 'resources'
AND table_name = 'Resource'
) THEN
EXECUTE format(
'CREATE VIEW %I.%I AS
Copy link

@github-actions github-actions bot Nov 11, 2025

Choose a reason for hiding this comment

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

[⚠️ performance]
Using SELECT DISTINCT can be costly in terms of performance, especially on large datasets. Consider whether the distinctness is necessary or if it can be ensured by other means, such as unique constraints on the underlying table.

SELECT DISTINCT r."challengeId", r."memberId"
FROM resources."Resource" r
WHERE r."challengeId" IS NOT NULL
AND r."memberId" IS NOT NULL',
current_schema(), 'MemberChallengeAccess'
);
ELSE
EXECUTE format(
'CREATE VIEW %I.%I AS
SELECT CAST(NULL AS TEXT) AS "challengeId",
Copy link

@github-actions github-actions bot Nov 11, 2025

Choose a reason for hiding this comment

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

[⚠️ correctness]
The use of CAST(NULL AS TEXT) in the ELSE branch might lead to confusion or unexpected behavior when querying the view. Consider documenting the intended use case for this scenario or ensuring that downstream consumers of this view handle these NULL values appropriately.

CAST(NULL AS TEXT) AS "memberId"
WHERE FALSE',
current_schema(), 'MemberChallengeAccess'
);
END IF;
END;
$$;
15 changes: 15 additions & 0 deletions prisma/schema.prisma
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ model Challenge {
terms ChallengeTerm[]
skills ChallengeSkill[]
auditLogs AuditLog[]
memberAccesses MemberChallengeAccess[]
Copy link

@github-actions github-actions bot Nov 11, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The addition of memberAccesses as a relation in the Challenge model seems correct. However, ensure that the MemberChallengeAccess model is properly populated and maintained to avoid potential issues with orphaned records or incorrect data associations.


// Relation to ChallengeType (FK: typeId)
type ChallengeType @relation(fields: [typeId], references: [id])
Expand Down Expand Up @@ -161,6 +162,20 @@ model Challenge {
@@index([projectId, status])
}

//////////////////////////////////////////
// MemberChallengeAccess view – member/challenge pairs from resources schema
//////////////////////////////////////////

model MemberChallengeAccess {
Copy link

@github-actions github-actions bot Nov 11, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The MemberChallengeAccess model lacks auditing fields such as createdAt, createdBy, updatedAt, and updatedBy. Adding these fields would improve traceability and consistency with other models in the schema.

challengeId String
memberId String

challenge Challenge @relation(fields: [challengeId], references: [id])

@@id([challengeId, memberId])
@@map("MemberChallengeAccess")
}

//////////////////////////////////////////
// ChallengeType model
//////////////////////////////////////////
Expand Down
27 changes: 0 additions & 27 deletions src/common/helper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -976,32 +976,6 @@ function calculateChallengeEndDate(challenge, data) {
// return result.toDate()
}

/**
* Lists challenge ids that given member has access to.
* @param {Number} memberId the member id
* @returns {Promise<Array>} an array of challenge ids represents challenges that given member has access to.
*/
async function listChallengesByMember(memberId) {
const token = await m2mHelper.getM2MToken();
let allIds = [];

try {
const result = await axios.get(`${config.RESOURCES_API_URL}/${memberId}/challenges`, {
headers: { Authorization: `Bearer ${token}` },
params: {
useScroll: true,
},
});

allIds = result.data || [];
} catch (e) {
// only log the error but don't throw it, so the following logic can still be executed.
logger.debug(`Failed to get challenges that accessible to the memberId ${memberId}`, e);
}

return allIds;
}

/**
* Lists resources that given member has in the given challenge.
* @param {Number} memberId the member id
Expand Down Expand Up @@ -1544,7 +1518,6 @@ module.exports = {
dedupeChallengeTerms,
postBusEvent,
calculateChallengeEndDate,
listChallengesByMember,
listResourcesByMemberAndChallenge,
getProjectDefaultTerms,
validateChallengeTerms,
Expand Down
56 changes: 33 additions & 23 deletions src/services/ChallengeService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -938,23 +938,28 @@ async function searchChallenges(currentUser, criteria) {
});
}

let memberChallengeIds;
let currentUserChallengeIds;
let currentUserChallengeIdSet;
const requestedMemberId = !_.isNil(criteria.memberId)
? _.toString(criteria.memberId)
: null;
const currentUserMemberId =
currentUser && !_hasAdminRole && !_.get(currentUser, "isMachine", false)
? _.toString(currentUser.userId)
: null;
const memberIdForTaskFilter = requestedMemberId || currentUserMemberId;

// FIXME: This is wrong!
// if (!_.isUndefined(currentUser) && currentUser.handle) {
// accessQuery.push({ match_phrase: { createdBy: currentUser.handle } })
// }

if (criteria.memberId) {
memberChallengeIds = await helper.listChallengesByMember(criteria.memberId);
if (requestedMemberId) {
prismaFilter.where.AND.push({
id: { in: memberChallengeIds },
memberAccesses: {
some: {
memberId: requestedMemberId,
},
},
});
} else if (currentUser && !_hasAdminRole && !_.get(currentUser, "isMachine", false)) {
currentUserChallengeIds = await helper.listChallengesByMember(currentUser.userId);
memberChallengeIds = currentUserChallengeIds;
}

// FIXME: Tech Debt
Expand Down Expand Up @@ -991,9 +996,11 @@ async function searchChallenges(currentUser, criteria) {
}
} else if (excludeTasks) {
const taskFilter = [];
if (_.get(memberChallengeIds, "length", 0) > 0) {
if (memberIdForTaskFilter) {
taskFilter.push({
id: { in: memberChallengeIds },
memberAccesses: {
some: { memberId: memberIdForTaskFilter },
},
});
}
taskFilter.push({
Expand All @@ -1016,12 +1023,22 @@ async function searchChallenges(currentUser, criteria) {
const sortFilter = {};
sortFilter[sortByProp] = sortOrderProp;

const challengeInclude = currentUserMemberId
? {
...includeReturnFields,
memberAccesses: {
where: { memberId: currentUserMemberId },
select: { memberId: true },
},
}
: includeReturnFields;

const prismaQuery = {
...prismaFilter,
take: perPage,
skip: (page - 1) * perPage,
orderBy: [sortFilter],
include: includeReturnFields,
include: challengeInclude,
};

try {
Expand Down Expand Up @@ -1079,20 +1096,13 @@ async function searchChallenges(currentUser, criteria) {
if (!currentUser.isMachine && !_hasAdminRole) {
result.forEach((challenge) => {
_.unset(challenge, "billing");
});

if (!currentUserChallengeIds) {
currentUserChallengeIds = await helper.listChallengesByMember(currentUser.userId);
}

const accessibleIds = currentUserChallengeIds || [];
currentUserChallengeIdSet = currentUserChallengeIdSet || new Set(accessibleIds);

result.forEach((challenge) => {
if (!currentUserChallengeIdSet.has(challenge.id)) {
if (!_.get(challenge, "memberAccesses.length")) {
_.unset(challenge, "privateDescription");
}
_.unset(challenge, "memberAccesses");
});
} else {
result.forEach((challenge) => _.unset(challenge, "memberAccesses"));
}
} else {
result.forEach((challenge) => {
Expand Down