Skip to content

Akshay Fix Top Skills list not reflecting active filter selection - #2370

Open
akv-iu wants to merge 3 commits into
developmentfrom
Akshay_fix_skills_overview_top_skills_filter_mismatch
Open

akv-iu wants to merge 3 commits into
developmentfrom
Akshay_fix_skills_overview_top_skills_filter_mismatch

Conversation

@akv-iu

@akv-iu akv-iu commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Description

Fixes a non-blocking issue flagged in review of frontend PR OneCommunityGlobal/HighestGoodNetworkApp#5440 (Skills Overview data mismatch fix). Jae has confirmed this is in scope to fix now rather than defer to a follow-up issue.

On /hgnhelp/skills-overview, selecting skill-tag filters under "Find Community Members" sometimes returns member cards whose "Top Skills" list does not include the skills that were actually selected/matched, showing unrelated (but higher-scoring) skills instead.

Root cause is in getRankedResponses in src/controllers/hgnFormResponseController.js. When a skills filter is active, the endpoint found the first skill matching the filter, then used its section (frontend/backend/general) to pick the top 4 skills by score for that user - not the skills that actually matched the filter. So 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).

Related PRs (if any):

This backend PR is related to the OneCommunityGlobal/HighestGoodNetworkApp#5440 frontend PR.
.

Main changes explained:

  • Update src/controllers/hgnFormResponseController.js:
    • Who matches the skills filter is unchanged: a selected skill must be in the user's top 4 for that skill's section, as on development. Users who only have a low score (for example 1/10) in the selected skill are not returned.
    • What "Top Skills" shows is new: the selected skills first (by score), then the user's other highest-scoring skills, up to 4. This is display only and no longer decides who matches.
    • Blank names: some form responses were saved with an empty name. The card now falls back to the linked profile's first and last name.
  • Update src/controllers/hgnFormResponseController.test.js: tests for the selected skill shown first, a low-score user being excluded, the internal match flag staying out of the response, and the name fallback.

How to test:

  1. Check out Akshay_fix_skills_overview_top_skills_filter_mismatch.
  2. Run npm install, npm run build, then npm start (port 4500).
  3. Run the frontend on PR #5440's branch (Akshay_fix_skills_overview_data_mismatch) against this backend.
  4. Log in as an Admin and go to /hgnhelp/skills-overview > "Find Community Members".
  5. Select one skill (for example EnvironmentSetup). The list should stay short (about 30 on the dev data, not about 180), and every card's Top Skills should start with the selected skill.
  6. Select 2 or 3 skills. The selected skills appear first, and the list stays narrow.
  7. Combine a preference filter with a skill. The results narrow further.
  8. Check that every card shows a name.
  9. Clear all filters. Browsing everyone works as before.
  10. Run npx jest src/controllers/hgnFormResponseController.test.js. All 11 tests should pass.

Screenshots or videos of changes:

image

Note:

Checked read-only against the shared dev database (184 form responses). For single skills, two skills together, a preference filter and no filter, this branch returns exactly the same users as development; only the Top Skills order changes. Blank names on member cards went from 11 to 0.

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.

@AaditTrivedi AaditTrivedi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed by Aadit Trivedi.

Checked out the PR, installed with npm ci, and ran npx jest src/controllers/hgnFormResponseController.test.js: 9/9 passing.

Verified

  • Scope matches the title: hgnFormResponseController.js and its test file.
  • The Top Skills logic now puts the skills that matched the active filter first, sorted by score, then fills the remaining slots with the user's other highest-scoring skills. Previously it took the top skills from the section of the first match only, so a user could appear in filtered results without the matching skill in their displayed Top Skills. The new behavior fixes that, and the inline comment explains the intent clearly.
  • The new test covers the filtered case.

Pointer, not blocking

  • This changes the result from "top skills within one section" to "matched skills first, then any section," which seems to be the intent; worth confirming the frontend in #5440 expects mixed sections.

Approving.

@vidiyala99 vidiyala99 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed 66318a9 against the real form data on the shared dev database (read-only). Showing the matched skills in "Top Skills" is a good idea, but it also changes who passes the skills filter, because the filter is applied to topSkills right afterwards:

filteredUsers = filteredUsers.filter((user) =>
  user.topSkills.some((skill) => skillList.includes(skill.toLowerCase())),
);

With this PR, the matched skills always go to the front of topSkills, so anyone who has a score for the selected skill passes, however low that score is. Every form response stores a number for every skill, so the filter stops narrowing anything.

I fetched the 182 dev form responses (GET /api/hgnform) and ran both versions of the topSkills logic on them. The old logic gives exactly the same counts as the live GET /api/hgnform/ranked?skills=... on a backend without this PR, so the comparison is like for like:

skills= without this PR (live endpoint) with this PR of those, score 0 to 2
EnvironmentSetup 29 178 25
AdvancedCoding 31 178 25
Database 129 178 24
MongoDB 131 178 26
React 110 175 22

So "Find Community Members" with any skill selected would list nearly everyone who filled in the form, including people who rated themselves 0 to 2 on that skill. hgnFormResponseController.test.js passes (9/9), including the new test, because that test only checks the topSkills content for a single user.

Keeping the two concerns separate would fix it. For example, decide whether a user matches using the old rule (or a minimum score on the matched skill), and only then build the displayed topSkills with the matched skills first. A test with a user who has the selected skill at a low score (who should not appear) would cover it.

Requesting changes.

@sai-velagala-swe sai-velagala-swe left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed by Sai Manojna Velagala
How I verified
Checked out Akshay_fix_skills_overview_top_skills_filter_mismatch and tested it locally with frontend PR #5440. I tested multiple active skill filters, preference filtering, returned member cards, Top Skills output, low-score matches, and dark mode. I also ran npx jest src/controllers/hgnFormResponseController.test.js.
Verified working

  • Backend tests pass.

npx jest src/controllers/hgnFormResponseController.test.js passed all 9/9 tests.

  • Matched skills appear in Top Skills.
    Returned member cards include filter-relevant skills inside their Top Skills list.

  • Filtering UI continues to return member results.
    Skill filters and preference filters continue to return member cards, and the UI remains usable.

  • Dark mode works with filtered results.

Filter controls and returned member cards remain readable in dark mode.

Issues found

  • Very low-score users are returned by skill filtering.
    While skill filters were active, I observed returned member cards with scores as low as 1/10. This supports the concern that the new filtering logic may treat users with very low proficiency as valid matches simply because the selected skill exists in their survey response.
Image
  • Some member cards have no displayed name.
    Multiple returned cards show an avatar, score, and Top Skills, but no member name.

  • The same missing-name/low-score behavior is still visible with additional filtering.
    This was also observed while preference filtering was active.

Image
  • Required action

Please review the skill matching logic so that users with very low scores are not treated as meaningful skill matches, and confirm how records with missing member names should be handled.

akv-iu added 2 commits October 8, 2026 18:00
- 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.
@akv-iu

akv-iu commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

@vidiyala99 @sai-velagala-swe @AaditTrivedi Thanks for the reviews, especially the comparison against the real form data. The latest push (3622734) addresses both points.

1. Low-score users passing the skills filter
You were right: the skills filter ran on topSkills, and the previous commit always put the selected skills at the front of that list, so anyone with any score passed. I separated the two concerns as suggested:

  • Who matches uses the original rule from development: a selected skill must be in the user's top 4 for that skill's section.
  • What "Top Skills" shows still lists the selected skills first, then the user's other highest-scoring skills. It is display only now.

I checked this read-only against the shared dev database (184 responses today, so the counts are slightly higher than the 182 in the earlier table). For EnvironmentSetup, AdvancedCoding, Database, MongoDB, React, React,MongoDB, preferences=backend and no filter, this branch returns exactly the same users as development (for example 30 for EnvironmentSetup, not 178). Every returned card has the selected skill first in Top Skills.

2. Member cards with no name
11 of the 184 form responses were saved with an empty userInfo.name (email, GitHub and Slack are empty too). All 11 are linked to a profile that has a name, so the card now falls back to the profile's first and last name. Blank names went from 11 to 0. I did not fill in missing emails, since those users left their contact details blank.

Tests

  • Added a test where a user has the selected skill at 1/10 and must not be returned. It fails on the previous commit and passes now.
  • Added a test for the name fallback, and a check that the internal match flag is not in the API response.
  • hgnFormResponseController.test.js: 11/11 passing. ESLint: no errors, and one fewer warning than development.

I also merged the latest development (no conflicts) and updated the description with the new test steps. Could you take another look when you have a moment? Thank you!

@sonarqubecloud

sonarqubecloud Bot commented Oct 9, 2026

Copy link
Copy Markdown

@vidiyala99 vidiyala99 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Re-tested the new head 3622734 on a test server against the shared dev data (read-only), with the same comparison as my last review: an in-browser copy of the development rule run over all form responses (GET /api/hgnform, 184 today), next to the live GET /api/hgnform/ranked?skills=... from this branch.

skills= development rule this branch (live) selected skill listed first
MongoDB 133 133 133 / 133
EnvironmentSetup 30 30 30 / 30
AdvancedCoding 31 31 31 / 31
Database 130 130 130 / 130
React 110 110 110 / 110

So the filter is strict again (it matches development exactly), and "Top Skills" still leads with the selected skill on every returned card, which was the goal of the PR.

  • Blank names: with no filter, all 184 cards have a name (0 blank), matching your note about the 11 responses saved with an empty userInfo.name.
  • Tests: hgnFormResponseController.test.js and communityController.test.js pass on the box (2 suites, 12/12).

Approving. Thanks for separating the two concerns.

@Niket07pathak Niket07pathak left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed by Niket Pathak

How I verified

Reviewed the backend changes in hgnFormResponseController.js and hgnFormResponseController.test.js alongside the related frontend PR #5440. Both frontend and backend applications were set up and running locally. Examined the skill-filtering logic, Top Skills ordering, member eligibility conditions, name fallback, and regression test coverage.

Verified working

  • The updated logic preserves the existing skill-filter eligibility criteria, requiring selected skills to qualify within the top four skills of the relevant section.
  • Selected skills are prioritized by score in the Top Skills display, followed by other highest-scoring skills, with a maximum of four skills per card.
  • Low-scoring users who do not meet the filtering criteria are excluded.
  • The name fallback correctly uses the linked user profile when the form response name is empty.
  • The internal matchesSkills flag is excluded from the API response.
  • The regression tests cover selected skill visibility, low-score exclusion, response structure, and name fallback.
  • The existing unfiltered behavior is preserved by the updated logic.

Issues found

No major or blocking issues were identified during the code review.

Evidence

Image Image Image

Required action

No changes required. Approved. The implementation addresses the reported Skills Overview mismatch while preserving the existing member-filtering criteria. The added regression tests provide coverage for the updated behavior.

@RichaSapre RichaSapre left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I reviewed the skills-filter logic and confirmed that selected skills now appear first in each member’s Top Skills list while low-score users remain excluded. The profile-name fallback also prevents blank member cards, and the response does not expose the internal matching flag.

I ran the targeted Jest suite: 11/11 tests passed, and the backend build compiled 773 files successfully. No blocking issues were found.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants