Conversation
Master -> dev
PM-4482 Return MM name in member skill
PM-4198 - composite index on (memberTraitId, key)
There was a problem hiding this comment.
Pull request overview
This release PR bundles several production-facing changes, primarily focused on improving member search performance by avoiding unnecessary hydration work, adding skill activity enrichment, and introducing a DB index to speed up trait personalization lookups.
Changes:
- Update
SearchService.fillMembersto conditionally hydrate stats/skills based on requested fields and a newincludeStatsquery flag, and to tailor Prisma query options for field-limited lookups. - Enrich
MemberService.getMemberSkillby resolving “marathon match” activity source IDs into challenge metadata. - Add a composite Prisma/Postgres index on
(memberTraitId, key)formemberTraitPersonalization, plus regenerated Prisma client artifacts and a CI branch filter update.
Reviewed changes
Copilot reviewed 6 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/SearchService.test.js | Adds unit coverage ensuring field-limited member lookups skip stats/skills hydration. |
| src/services/SearchService.js | Introduces conditional stats/skills hydration and field-aware Prisma query option construction. |
| src/services/MemberService.js | Adds marathon match activity hydration by fetching challenge details. |
| prisma/schema.prisma | Adds composite index on memberTraitPersonalization(memberTraitId, key). |
| prisma/migrations/20260330000000_add_member_trait_personalization_key_index/migration.sql | Creates the corresponding Postgres index. |
| prisma/generated/client/* | Regenerated Prisma client to reflect schema/index changes. |
| .circleci/config.yml | Updates dev build branch filter to PM-4482. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function buildMemberQueryOptions (fields, shouldLoadStats) { | ||
| const shouldLoadAddresses = _.includes(fields, 'addresses') | ||
| const shouldLoadMaxRating = _.includes(fields, 'maxRating') || shouldLoadStats | ||
| const shouldLoadCurrentMaxRatingStats = _.includes(fields, 'maxRating') | ||
|
|
||
| const include = {} | ||
| if (shouldLoadAddresses) { | ||
| include.addresses = true | ||
| } | ||
| if (shouldLoadMaxRating) { | ||
| include.maxRating = true | ||
| } | ||
| if (shouldLoadCurrentMaxRatingStats) { | ||
| include.memberStats = { | ||
| select: prismaHelper.currentMaxRatingStatsSelect | ||
| } | ||
| } | ||
|
|
||
| if (!_.isEmpty(include)) { | ||
| return { include } | ||
| } |
There was a problem hiding this comment.
buildMemberQueryOptions returns include whenever addresses/maxRating/memberStats are needed. With Prisma, include implicitly fetches all scalar columns for member (including large Json fields like aggregatedSkills/enteredSkills), which undercuts the goal of making field-limited lookups cheaper. Consider always building a select shape (including relations via nested selects / true) so you can still restrict scalars to the minimal set needed for convertMember + requested fields.
| // Include the stats by default, but allow them to be ignored with ?includeStats=false | ||
| // This is for performance reasons - pulling the stats is a bit of a resource hog |
There was a problem hiding this comment.
The inline comment says stats are included "by default" unless includeStats=false, but the new logic also gates stats loading on whether the requested fields include stats-derived fields. Please update the comment to reflect the actual condition (stats are only loaded when stats-dependent fields are requested and not explicitly disabled).
| // Include the stats by default, but allow them to be ignored with ?includeStats=false | |
| // This is for performance reasons - pulling the stats is a bit of a resource hog | |
| // Include stats only when stats-dependent fields are requested and they have not been | |
| // explicitly disabled with ?includeStats=false. This is for performance reasons - | |
| // pulling the stats is a bit of a resource hog. |
https://topcoder.atlassian.net/browse/PM-4482
https://topcoder.atlassian.net/browse/PM-4198
https://topcoder.atlassian.net/browse/PM-4817
https://topcoder.atlassian.net/browse/PM-3343