Use markdown-it for mentorship portal summaries #2762
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThe PR migrates description rendering from plain text to Markdown-based components across multiple card components. Module imports are updated from 'markdown-it/index.mjs' to 'markdown-it', CSS styling is added for markdown-wrapped content, and tests are adjusted to verify wrapper elements. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
frontend/src/components/MarkdownWrapper.tsx (1)
2-2: Import path update is good; consider hoisting the markdown-it instance and defaultingclassName.Using the package entrypoint for
markdown-itis appropriate and keeps the component aligned with how tests mock it. You can also avoid re-instantiating the parser on every render and eliminate the"undefined"class when noclassNameis passed by hoistingmdand givingclassNamea default:-export default function Markdown({ content, className }: { content: string; className?: string }) { - const md = markdownit({ - html: true, - linkify: true, - typographer: true, - breaks: true, - }).use(taskLists) +const md = markdownit({ + html: true, + linkify: true, + typographer: true, + breaks: true, +}).use(taskLists) + +export default function Markdown({ + content, + className = '', +}: { + content: string + className?: string +}) { return ( <div dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(md.render(content)) }} className={`md-wrapper ${className}`} /> ) }This preserves the existing DOMPurify-based sanitization pattern while reducing per-render work. Based on learnings, this keeps the established XSS protection in place.
frontend/src/components/CardDetailsPage.tsx (1)
81-86: Map-basedtype→style lookup is fine; an object literal would be a bit leaner.The
typeStylesMapworks, but for this small, static mapping a plain object is simpler and avoids constructing aMapinstance each render:- const typeStylesMap = new Map([ - ['program', 'gap-2 md:col-span-7'], - ['module', 'gap-2 md:col-span-7'], - ['chapter', 'gap-2 md:col-span-3'], - ]) - const secondaryCardStyles = typeStylesMap.get(type) ?? 'gap-2 md:col-span-5' + const typeStyles: Record<string, string> = { + program: 'gap-2 md:col-span-7', + module: 'gap-2 md:col-span-7', + chapter: 'gap-2 md:col-span-3', + } + const secondaryCardStyles = typeStyles[type] ?? 'gap-2 md:col-span-5'Purely a readability/ergonomics tweak; behavior stays identical.
frontend/src/app/globals.css (1)
272-288: List styling is good; consider deduping the.md-wrapper oldefinition.The new
.md-wrapper ul/ol/lirules give Markdown lists sane spacing and bullets. There is now both:.md-wrapper ol { list-style: decimal; }and later:
.md-wrapper ol { list-style-type: decimal; }These are functionally equivalent; you could drop the earlier
list-style: decimal;block and keep the newer one for a single source of truth.frontend/__tests__/unit/components/ProgramCard.test.tsx (1)
279-281: Tests correctly assert on the Markdown wrapper around descriptions.Updating the expectations to locate the closest
.md-wrapperand check forline-clamp-8matches the newMarkdownstructure and keeps the layout contract tested. If you want to go a bit further, you could mirror this check in the fallback description tests ('No description available.') to ensure all description paths still use the same wrapper.Also applies to: 298-300
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
frontend/__tests__/unit/components/MarkdownWrapper.test.tsx(1 hunks)frontend/__tests__/unit/components/ProgramCard.test.tsx(2 hunks)frontend/src/app/globals.css(2 hunks)frontend/src/components/CardDetailsPage.tsx(3 hunks)frontend/src/components/MarkdownWrapper.tsx(1 hunks)frontend/src/components/ProgramCard.tsx(2 hunks)frontend/src/components/SingleModuleCard.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-04-04T05:23:43.562Z
Learnt from: M-ayank2005
Repo: OWASP/Nest PR: 1282
File: frontend/src/pages/About.tsx:94-98
Timestamp: 2025-04-04T05:23:43.562Z
Learning: In the OWASP/Nest frontend project, DOMPurify is used to sanitize HTML content before rendering it with dangerouslySetInnerHTML, following security best practices to prevent XSS vulnerabilities.
Applied to files:
frontend/src/components/MarkdownWrapper.tsx
📚 Learning: 2025-07-12T17:14:28.536Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/app/mentorship/programs/[programKey]/edit/page.tsx:90-128
Timestamp: 2025-07-12T17:14:28.536Z
Learning: Both ProgramForm (programCard.tsx) and ModuleForm (mainmoduleCard.tsx) components already implement HTML validation using the `required` attribute on form fields. The browser's native validation prevents form submission and displays error messages when required fields are empty, eliminating the need for additional JavaScript validation before GraphQL mutations.
Applied to files:
frontend/src/components/SingleModuleCard.tsxfrontend/src/components/ProgramCard.tsx
📚 Learning: 2025-06-20T16:12:59.256Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Applied to files:
frontend/src/components/CardDetailsPage.tsx
📚 Learning: 2025-06-20T16:12:59.256Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
Applied to files:
frontend/src/components/CardDetailsPage.tsx
📚 Learning: 2025-09-17T02:42:41.928Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 2288
File: frontend/src/components/ActionButton.tsx:0-0
Timestamp: 2025-09-17T02:42:41.928Z
Learning: In frontend/src/components/ActionButton.tsx, the user Rajgupta36 intentionally changed text-blue-600 to text-[#1D7BD7] to align the text color with the border color (#1D7BD7) for visual consistency, prioritizing design alignment over theme tokens.
Applied to files:
frontend/src/components/CardDetailsPage.tsx
📚 Learning: 2025-07-13T11:34:31.823Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/app/mentorship/programs/page.tsx:14-14
Timestamp: 2025-07-13T11:34:31.823Z
Learning: In the Next.js frontend mentorship application, there are two distinct types for authentication-related data: ExtendedSession for useSession hook (containing accessToken and user.login properties) and UserRolesData for useUserRoles hook (containing currentUserRoles.roles array). The correct access pattern for GitHub username is `(session as ExtendedSession)?.user?.login`.
Applied to files:
frontend/src/components/CardDetailsPage.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/src/components/ProgramCard.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
frontend/src/components/ProgramCard.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/components/ProgramCard.tsx
🧬 Code graph analysis (3)
frontend/src/components/SingleModuleCard.tsx (1)
frontend/src/components/MarkdownWrapper.tsx (1)
Markdown(5-19)
frontend/src/components/CardDetailsPage.tsx (2)
frontend/src/components/MarkdownWrapper.tsx (1)
Markdown(5-19)backend/apps/github/models/release.py (1)
summary(59-61)
frontend/src/components/ProgramCard.tsx (1)
frontend/src/components/MarkdownWrapper.tsx (1)
Markdown(5-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run backend tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
🔇 Additional comments (4)
frontend/src/components/CardDetailsPage.tsx (1)
25-25: Markdown summary rendering aligns with the new pattern and looks safe.Wiring the
summarythrough the sharedMarkdownwrapper keeps formatting consistent with other mentorship views and continues to rely on DOMPurify sanitization fromMarkdownWrapper.tsx, so enabling rich text here is a solid move.Also applies to: 123-124
frontend/src/app/globals.css (1)
134-148: Link color overrides for Markdown inside links are well targeted.The
a .md-wrapper/.dark a .md-wrapperrules solve the “entire card turns link-blue” issue while still letting actual links inside Markdown use the primary link color via.md-wrapper a. This is a good balance for the Program/Module cards.frontend/src/components/ProgramCard.tsx (1)
8-8: Program description Markdown integration is consistent and safe.Using the shared
Markdowncomponent fordescriptionwith a"line-clamp-8"class on the wrapper keeps the existing truncation behavior while enabling rich text. Thedescriptionfallback ensuresMarkdownalways receives a string, which avoids runtime surprises.Also applies to: 100-100
frontend/__tests__/unit/components/MarkdownWrapper.test.tsx (1)
9-19: markdown-it mock path now matches the production import.Switching the mock to
'markdown-it'keeps the tests in sync with the component’s import while preserving the existing mock behavior (customrender+usechaining), so coverage should remain representative.
| import type { Module } from 'types/mentorship' | ||
| import { formatDate } from 'utils/dateFormatter' | ||
| import EntityActions from 'components/EntityActions' | ||
| import Markdown from 'components/MarkdownWrapper' |
There was a problem hiding this comment.
Guard module.description before passing it to Markdown to avoid non-string input.
Right now Markdown is called with module.description directly. If that field ever comes back null/undefined (or is missing), the wrapper will receive a non-string and can error when rendering. Coalescing to a string first makes this robust:
-const SingleModuleCard: React.FC<SingleModuleCardProps> = ({ module, accessLevel, admins }) => {
+const SingleModuleCard: React.FC<SingleModuleCardProps> = ({ module, accessLevel, admins }) => {
const { data } = useSession()
const pathname = usePathname()
+
+ const description = module.description ?? ''
@@
- {/* Description */}
- <div>
- <Markdown content={module.description} />
- </div>
+ {/* Description */}
+ <div>
+ <Markdown content={description} />
+ </div>If you want to mirror the ProgramCard UX, you could instead set const description = module.description || 'No description available.' so modules also get a friendly fallback.
Also applies to: 71-72
🤖 Prompt for AI Agents
In frontend/src/components/SingleModuleCard.tsx around lines 14 and also apply
same change at lines 71-72, guard module.description before passing to Markdown
by coalescing to a string (e.g., use module.description || '' or provide a
friendly fallback like 'No description available.') so Markdown always receives
a string; replace direct uses of module.description with the coalesced
description variable and use that variable when rendering the Markdown
component.
Update to use markdown text for Program and Module summary/description.
Checklist
make check-testlocally; all checks and tests passed.