Skip to content

fix: show module as a section when there is only 1 module#3402

Merged
kasya merged 3 commits intoOWASP:mainfrom
HarshitVerma109:refactor/remove-nested-module-card
Jan 24, 2026
Merged

fix: show module as a section when there is only 1 module#3402
kasya merged 3 commits intoOWASP:mainfrom
HarshitVerma109:refactor/remove-nested-module-card

Conversation

@HarshitVerma109
Copy link
Contributor

Proposed change

Resolves #3083

Display single module as a section instead of a nested card.

Changes

  • Remove SecondaryCard wrapper when only 1 module exists
  • Inline contributor rendering in SingleModuleCard
  • Update styling to use section background instead of card borders/shadows
  • Update tests to match new implementation

Checklist

  • Required: I followed the contributing workflow
  • Required: I verified that my code works as intended and resolves the issue as described
  • Required: I ran make check-test locally: all warnings addressed, tests passed
  • I used AI for code, documentation, tests, or communication related to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 18, 2026

Summary by CodeRabbit

  • New Features

    • Show more/less toggle for mentor and mentee lists when contributors exceed the initial display.
  • UI/UX Improvements

    • Inline contributor avatars and unified contributor layout.
    • Single-module pages omit extra wrapper; multiple modules grouped under a "Modules" section.
    • Improved headings (module title as h2) and folder icon in headers.
    • Profile links adapt to private vs. public view contexts.
  • Bug Fixes

    • Missing description now shows a fallback; mentors section omitted when empty.
  • Tests

    • Expanded coverage for contributor rendering, toggles, URL behaviors, headings, admin context, and edge cases.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Conditional rendering in CardDetailsPage now omits the SecondaryCard when a program has exactly one module. SingleModuleCard was refactored to render mentors/mentees inline with avatar links, a show-more toggle, and unified contributor URL logic. Tests and mocks were updated accordingly.

Changes

Cohort / File(s) Summary
Tests
frontend/__tests__/unit/components/SingleModuleCard.test.tsx
Reworked mocks and assertions: added fireEvent, mocked next/image, simplified next/link, mocked getMemberUrl, replaced contributor mock with ShowMoreButton mock, added mockModuleWithManyMentors, and updated tests for inline avatars, show-more toggle, heading semantics, description fallback, and private/public member URL handling.
SingleModuleCard component
frontend/src/components/SingleModuleCard.tsx
Added local state (showAllMentors, showAllMentees) and maxInitialDisplay; introduced renderContributors helper to render inline avatar links (uses getMemberUrl/programKey logic), integrated ShowMoreButton toggle, updated markup and headings, and adjusted imports (e.g., Image, upperFirst).
CardDetailsPage conditional wrapper
frontend/src/components/CardDetailsPage.tsx
Conditional rendering: render ModuleCard directly when program has exactly one module; render ModuleCard inside a SecondaryCard with folder icon and title "Modules" when multiple modules.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Suggested reviewers

  • arkid15r
  • kasya
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: show module as a section when there is only 1 module' directly matches the main objective of the PR—to display a single module as a section without the outer SecondaryCard wrapper.
Description check ✅ Passed The description clearly explains the proposed change, references the resolved issue (#3083), and details the specific implementation changes including the removal of SecondaryCard wrapper, inline contributor rendering, and test updates.
Linked Issues check ✅ Passed The PR implementation fully addresses issue #3083 by conditionally rendering ModuleCard without the outer SecondaryCard wrapper when only one module exists, removing the double-boxing effect and making it a top-level section.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the objective: CardDetailsPage.tsx conditional wrapper logic, SingleModuleCard inline contributor rendering, and corresponding test updates. No extraneous modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 18, 2026
@HarshitVerma109
Copy link
Contributor Author

@kasya, please review it and let me know if any changes are required.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@frontend/src/components/SingleModuleCard.tsx`:
- Around line 60-109: The renderContributors function signature currently
includes an unused showAll parameter; remove showAll from the signature of
renderContributors and from its type list (Contributor[] | undefined,
displayContributors..., label, toggleShowAll, isMentee) so the function only
accepts the parameters it uses, and update any call sites that pass showAll to
stop passing that argument (ensure calls still pass toggleShowAll and isMentee
in the correct order). Also update any related type annotations or usages (e.g.,
props or local callers) to match the new parameter list to avoid TypeScript
errors.

coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 21, 2026
kasya
kasya previously approved these changes Jan 21, 2026
Copy link
Collaborator

@kasya kasya left a comment

Choose a reason for hiding this comment

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

@HarshitVerma109 this looks good 👍🏼
Pushed tiny change to update one icon.

Thanks!

@kasya kasya added this pull request to the merge queue Jan 21, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 21, 2026
@HarshitVerma109 HarshitVerma109 requested a review from kasya January 21, 2026 04:47
@kasya kasya added this pull request to the merge queue Jan 21, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Jan 21, 2026
@kasya
Copy link
Collaborator

kasya commented Jan 21, 2026

@HarshitVerma109 could you please resolve conflicts and check that tests pass so we could merge this in?

@HarshitVerma109 HarshitVerma109 force-pushed the refactor/remove-nested-module-card branch from eb378a4 to d2e4655 Compare January 21, 2026 17:27
coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 21, 2026
@HarshitVerma109
Copy link
Contributor Author

@kasya, please review it and let me know if any changes are required.

@sonarqubecloud
Copy link

@codecov
Copy link

codecov bot commented Jan 24, 2026

Codecov Report

❌ Patch coverage is 74.07407% with 7 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@8229c5c). Learn more about missing BASE report.

Files with missing lines Patch % Lines
frontend/src/components/SingleModuleCard.tsx 76.92% 1 Missing and 5 partials ⚠️
frontend/src/components/CardDetailsPage.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3402   +/-   ##
=======================================
  Coverage        ?   85.56%           
=======================================
  Files           ?      461           
  Lines           ?    14246           
  Branches        ?     1902           
=======================================
  Hits            ?    12190           
  Misses          ?     1679           
  Partials        ?      377           
Flag Coverage Δ
backend 84.47% <ø> (?)
frontend 88.60% <74.07%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
frontend/src/components/CardDetailsPage.tsx 88.00% <0.00%> (ø)
frontend/src/components/SingleModuleCard.tsx 81.63% <76.92%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8229c5c...4db1f15. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Collaborator

@kasya kasya left a comment

Choose a reason for hiding this comment

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

@HarshitVerma109 Great work! Thanks!

@kasya kasya added this pull request to the merge queue Jan 24, 2026
Merged via the queue into OWASP:main with commit 4b408c3 Jan 24, 2026
36 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix: Show module as a section when there is only 1 module

2 participants