Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
62 changes: 41 additions & 21 deletions src/controllers/hgnFormResponseController.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -85,19 +93,30 @@ const hgnFormController = () => {
? allSkills.reduce((a, b) => a + b.score, 0) / allSkills.length
: 0;

// Decide which section to use for topSkills
let sectionToUse = null;
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;
}

// 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)
.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
Expand All @@ -107,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,
};
});

Expand All @@ -127,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' });
Expand Down
97 changes: 97 additions & 0 deletions src/controllers/hgnFormResponseController.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,103 @@ 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('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 },
Expand Down
Loading