From 66318a90b5a941954d30def808c12b7d653cb123 Mon Sep 17 00:00:00 2001 From: Akshay Date: Sat, 26 Sep 2026 07:56:14 -0600 Subject: [PATCH 1/2] fix: Top Skills list not reflecting the active filter selection getRankedResponses picked the top 4 scores from a guessed section (frontend/backend/general) rather than the skills that actually matched the requested `skills` filter. A user's genuinely matched skill (e.g. EnvironmentSetup) could be silently dropped from topSkills if it wasn't among their top 4 highest-scoring skills in that section, and replaced by an unrelated higher-scoring skill from the same section (e.g. MongoDB). topSkills is now built from the matched filter skills first (ranked by their own score), then filled up to 4 with the user's other highest-scoring skills. Unfiltered browsing is unchanged. Adds a regression test reproducing the bug; confirmed it fails on the old logic and passes with the fix. --- src/controllers/hgnFormResponseController.js | 18 ++++++----- .../hgnFormResponseController.test.js | 32 +++++++++++++++++++ 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/src/controllers/hgnFormResponseController.js b/src/controllers/hgnFormResponseController.js index 0838b05b2a..35960ec4f6 100644 --- a/src/controllers/hgnFormResponseController.js +++ b/src/controllers/hgnFormResponseController.js @@ -85,18 +85,20 @@ const hgnFormController = () => { ? allSkills.reduce((a, b) => a + b.score, 0) / allSkills.length : 0; - // Decide which section to use for topSkills - let sectionToUse = null; + // When a skills filter is active, the displayed "Top Skills" must include + // the skills that actually matched the filter (that's why this user is in + // the results), ranked first; remaining slots fill with the user's other + // highest-scoring skills so the list still shows up to 4. + let matchedSkills = []; if (skills) { const skillList = skills.split(',').map((s) => s.trim().toLowerCase()); - const match = allSkills.find((s) => skillList.includes(s.skill.toLowerCase())); - if (match) sectionToUse = match.section; + matchedSkills = allSkills.filter((s) => skillList.includes(s.skill.toLowerCase())); } + const remainingSkills = allSkills + .filter((s) => !matchedSkills.includes(s)) + .sort((a, b) => b.score - a.score); - // Pick top 4 from chosen section, or global top 4 - const topSkills = allSkills - .filter((s) => (sectionToUse ? s.section === sectionToUse : true)) - .sort((a, b) => b.score - a.score) + const topSkills = [...matchedSkills.sort((a, b) => b.score - a.score), ...remainingSkills] .slice(0, 4) .map((s) => s.skill); diff --git a/src/controllers/hgnFormResponseController.test.js b/src/controllers/hgnFormResponseController.test.js index 5647504339..09d45863bd 100644 --- a/src/controllers/hgnFormResponseController.test.js +++ b/src/controllers/hgnFormResponseController.test.js @@ -175,6 +175,38 @@ describe('HgnFormResponseController', () => { expect(result[1]._id).toBe('1'); }); + it('topSkills should include every selected skill the matched user has, even if it is not their highest-scoring skill', async () => { + // EnvironmentSetup is the user's lowest-scoring backend skill, so the old + // "top 4 of the matched section" logic dropped it from topSkills even + // though it was one of the filters that matched this user. + mockReq.query = { skills: 'EnvironmentSetup,AdvancedCoding,AgileDevelopment' }; + UserProfile.find = jest.fn().mockResolvedValue([{ _id: '123', isActive: true }]); + + FormResponse.find = jest.fn().mockResolvedValue([ + { + _id: '1', + user_id: '123', + userInfo: { name: 'John' }, + frontend: {}, + backend: { + EnvironmentSetup: '3', + AdvancedCoding: '9', + AgileDevelopment: '8', + MongoDB: '9', + Database: '9', + }, + general: {}, + }, + ]); + + await controller.getRankedResponses(mockReq, mockRes); + + const result = mockRes.json.mock.calls[0][0]; + expect(result[0].topSkills).toEqual( + expect.arrayContaining(['EnvironmentSetup', 'AdvancedCoding', 'AgileDevelopment']), + ); + }); + it('should return all users if no query params are provided', async () => { UserProfile.find = jest.fn().mockResolvedValue([ { _id: '123', isActive: true }, From 3622734c19ace60353e485dbb679b761b71844ae Mon Sep 17 00:00:00 2001 From: Akshay Date: Thu, 8 Oct 2026 18:10:24 -0600 Subject: [PATCH 2/2] fix: keep skills filter strict and fill blank member names (#2370) - Decide who passes the skills filter with the original rule (a selected skill in the user's top 4 for that section). The previous commit filtered on the display list, which now always starts with the selected skills, so anyone with a 0-2/10 score passed. Top Skills still shows the selected skills first; it no longer decides who matches. - Some form responses were saved with an empty userInfo.name. Use the linked profile's first and last name so member cards are not blank. - Tests: a low-score user is excluded, the internal flag stays out of the response, and the name falls back to the profile. --- src/controllers/hgnFormResponseController.js | 64 +++++++++++------- .../hgnFormResponseController.test.js | 65 +++++++++++++++++++ 2 files changed, 106 insertions(+), 23 deletions(-) diff --git a/src/controllers/hgnFormResponseController.js b/src/controllers/hgnFormResponseController.js index ddc569d551..b82801dc91 100644 --- a/src/controllers/hgnFormResponseController.js +++ b/src/controllers/hgnFormResponseController.js @@ -6,6 +6,9 @@ const FormResponse = require('../models/hgnFormResponse'); const { hasPermission } = require('../utilities/permissions'); +// How many skills a member card shows, and how deep the skills filter looks. +const TOP_SKILLS_COUNT = 4; + const hgnFormController = () => { const submitFormResponse = async (req, res) => { const { userInfo, general, frontend, backend, followUp, user_id } = req.body; @@ -44,12 +47,17 @@ const hgnFormController = () => { // FIX ISSUE #8: Manually fetch user profiles to get isActive const UserProfile = require('../models/userProfile'); const userIds = responses.map((r) => r.user_id).filter(Boolean); - const users = await UserProfile.find({ _id: { $in: userIds } }, 'isActive'); + const users = await UserProfile.find( + { _id: { $in: userIds } }, + 'isActive firstName lastName', + ); // Create a map for quick lookup const userMap = {}; + const profileNameMap = {}; users.forEach((u) => { userMap[u._id.toString()] = u.isActive; + profileNameMap[u._id.toString()] = [u.firstName, u.lastName].filter(Boolean).join(' '); }); const scoredUsers = responses.map((user) => { @@ -85,21 +93,30 @@ const hgnFormController = () => { ? allSkills.reduce((a, b) => a + b.score, 0) / allSkills.length : 0; - // When a skills filter is active, the displayed "Top Skills" must include - // the skills that actually matched the filter (that's why this user is in - // the results), ranked first; remaining slots fill with the user's other - // highest-scoring skills so the list still shows up to 4. - let matchedSkills = []; - if (skills) { - const skillList = skills.split(',').map((s) => s.trim().toLowerCase()); - matchedSkills = allSkills.filter((s) => skillList.includes(s.skill.toLowerCase())); - } - const remainingSkills = allSkills - .filter((s) => !matchedSkills.includes(s)) - .sort((a, b) => b.score - a.score); - - const topSkills = [...matchedSkills.sort((a, b) => b.score - a.score), ...remainingSkills] - .slice(0, 4) + const byScore = (a, b) => b.score - a.score; + const skillList = skills ? skills.split(',').map((s) => s.trim().toLowerCase()) : []; + const isSelected = (s) => skillList.includes(s.skill.toLowerCase()); + + // Who matches the skills filter: unchanged rule. A selected skill must be + // among the user's top 4 in the section of their first selected skill. + // Every form response stores a score for every skill, so matching on + // "has the skill" alone would let 0-2/10 scores through. + const firstMatch = allSkills.find(isSelected); + const matchesSkills = + !skills || + allSkills + .filter((s) => (firstMatch ? s.section === firstMatch.section : true)) + .sort(byScore) + .slice(0, TOP_SKILLS_COUNT) + .some(isSelected); + + // What "Top Skills" displays: the selected skills first (by score), then + // the user's other highest-scoring skills, up to 4. Display only; it no + // longer decides who passes the filter. + const matchedSkills = allSkills.filter(isSelected).sort(byScore); + const remainingSkills = allSkills.filter((s) => !isSelected(s)).sort(byScore); + const topSkills = [...matchedSkills, ...remainingSkills] + .slice(0, TOP_SKILLS_COUNT) .map((s) => s.skill); // FIX ISSUE #8: Get isActive from userMap @@ -109,13 +126,16 @@ const hgnFormController = () => { return { _id: user._id, userId: user.user_id, - name: user.userInfo?.name, + // Some responses were saved with an empty userInfo.name; fall back to the + // linked profile's name so the member card is not blank. + name: user.userInfo?.name?.trim() || profileNameMap[userId] || user.userInfo?.name, email: user.userInfo?.email, slack: user.userInfo?.slack, score: Number(avgScore.toFixed(1)), topSkills, preferences: user.general?.preferences || [], isActive, + matchesSkills, }; }); @@ -129,18 +149,16 @@ const hgnFormController = () => { ); } - // Filter by skills + // Filter by skills (decided per user above, independent of the display list) if (skills) { - const skillList = skills.split(',').map((s) => s.trim().toLowerCase()); - filteredUsers = filteredUsers.filter((user) => - user.topSkills.some((skill) => skillList.includes(skill.toLowerCase())), - ); + filteredUsers = filteredUsers.filter((user) => user.matchesSkills); } // Sort by avg score filteredUsers.sort((a, b) => b.score - a.score); - res.json(filteredUsers); + // matchesSkills is internal; keep the response shape unchanged + res.json(filteredUsers.map(({ matchesSkills, ...user }) => user)); } catch (err) { console.error('Error in getRankedResponses:', err); res.status(500).json({ error: 'Failed to rank users' }); diff --git a/src/controllers/hgnFormResponseController.test.js b/src/controllers/hgnFormResponseController.test.js index ceb67135e6..d81c6e3675 100644 --- a/src/controllers/hgnFormResponseController.test.js +++ b/src/controllers/hgnFormResponseController.test.js @@ -208,6 +208,71 @@ describe('HgnFormResponseController', () => { ); }); + it('skills filter excludes users who only have the selected skill at a low score', async () => { + // Every form response stores a score for every skill, so "has the skill" + // must not be enough to match. Low ranks EnvironmentSetup last in backend + // (1/10), High ranks it in their top 4, so only High should be returned. + mockReq.query = { skills: 'EnvironmentSetup' }; + UserProfile.find = jest.fn().mockResolvedValue([ + { _id: 'u1', isActive: true }, + { _id: 'u2', isActive: true }, + ]); + const backend = (env) => ({ + EnvironmentSetup: env, + AdvancedCoding: '8', + MongoDB: '8', + Database: '8', + AgileDevelopment: '8', + }); + FormResponse.find = jest.fn().mockResolvedValue([ + { + _id: 'low', + user_id: 'u1', + userInfo: { name: 'Low' }, + frontend: {}, + backend: backend('1'), + general: {}, + }, + { + _id: 'high', + user_id: 'u2', + userInfo: { name: 'High' }, + frontend: {}, + backend: backend('9'), + general: {}, + }, + ]); + + await controller.getRankedResponses(mockReq, mockRes); + + const result = mockRes.json.mock.calls[0][0]; + expect(result.map((u) => u._id)).toEqual(['high']); + expect(result[0].topSkills[0]).toBe('EnvironmentSetup'); + // internal flag must not leak into the API response + expect(result[0]).not.toHaveProperty('matchesSkills'); + }); + + it('uses the linked profile name when the form response name is empty', async () => { + mockReq.query = {}; + UserProfile.find = jest + .fn() + .mockResolvedValue([{ _id: 'u1', isActive: true, firstName: 'Ada', lastName: 'Lovelace' }]); + FormResponse.find = jest.fn().mockResolvedValue([ + { + _id: 'r1', + user_id: 'u1', + userInfo: { name: '' }, + frontend: {}, + backend: {}, + general: {}, + }, + ]); + + await controller.getRankedResponses(mockReq, mockRes); + + expect(mockRes.json.mock.calls[0][0][0].name).toBe('Ada Lovelace'); + }); + it('should return all users if no query params are provided', async () => { UserProfile.find = jest.fn().mockResolvedValue([ { _id: '123', isActive: true },