feat(mentorship): add mentorship module with admin components - #2178
Closed
Sameh16 wants to merge 8 commits into
Closed
feat(mentorship): add mentorship module with admin components#2178Sameh16 wants to merge 8 commits into
Sameh16 wants to merge 8 commits into
Conversation
- Introduced a new mentorship module with routes for mentorship administration. - Created mentorship routes and integrated them into the main application routing. - Developed the admin component for managing mentorship programs, including a list view with search and filter capabilities. - Implemented program card and list components to display mentorship programs with relevant statistics. - Added a service to handle API interactions for fetching mentorship program data. - Included mock data for initial development and testing purposes. - Updated sidebar navigation to include mentorship section. This commit establishes the foundational structure for the mentorship feature, enabling future enhancements and integrations. Signed-off-by: Sameh16 <sameh_mohamed16@hotmail.com>
- Added a new route for program enrollment in the mentorship module. - Developed the EnrollProgramComponent with a multi-step enrollment wizard, including details, setup, and prerequisites steps. - Created custom prerequisite components to allow users to define specific requirements for their programs. - Integrated form validation and error handling for user inputs. - Enhanced the admin component to navigate to the enrollment page. - Updated the HTML templates for various components to support the new enrollment flow. This commit enhances the mentorship module by providing a comprehensive program enrollment feature, facilitating better management of mentorship programs. Signed-off-by: Sameh16 <sameh_mohamed16@hotmail.com>
Sameh16
requested review from
lewisojile and
mlehotskylf
and
a balanced review from Copilot
September 3, 2026 21:31
PR SummaryMedium Risk Overview The admin list loads programs from the BFF with debounced search and status filters, card navigation to detail, and an enroll CTA. The enrollment wizard is a three-step flow (details, setup, prerequisites) with shared validation, async name/CII checks, LF project lazy select, terms dialog, and Server-side, mounts Reviewed by Cursor Bugbot for commit 03626a2. Bugbot is set up for automated code reviews on this repo. Configure here. |
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Essentials Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Sameh16
added
do-not-merge
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+115
to
+119
| const slug = name | ||
| .trim() | ||
| .toLowerCase() | ||
| .replace(/[^a-z0-9]+/g, '-') | ||
| .replace(/^-+|-+$/g, ''); |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| this.form.controls.status.setValue(nextStatus, { emitEvent: false }); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
effect() mutates form controls
Medium Severity
effect() writes into form controls with setValue/patchValue in the programs list and custom-prerequisite card. In zoneless OnPush that can throw ExpressionChangedAfterItHasBeenCheckedError, and the prerequisite card re-patches on every parent object identity change while the user is typing.
Additional Locations (1)
Triggered by learned rule: Do not use effect() for side effects — use toObservable() + subscribe instead
Reviewed by Cursor Bugbot for commit 51f3331. Configure here.
| if (typeof val !== 'string') return undefined; | ||
| const trimmed = val.trim(); | ||
| return trimmed.length > 0 ? trimmed : undefined; | ||
| }; |
There was a problem hiding this comment.
Query repeats silently drop filters
Medium Severity
Optional search and status query params are parsed with typeof val !== 'string'. Express repeated keys arrive as arrays, so they become undefined and the list returns unfiltered results instead of rejecting "at most once".
Additional Locations (1)
Triggered by learned rule: Body validation must reject arrays — typeof [] === 'object' bypasses object checks
Reviewed by Cursor Bugbot for commit 51f3331. Configure here.
| } | ||
|
|
||
| function parseEnrollBody(body: unknown): MentorshipEnrollForm { | ||
| const raw = body && typeof body === 'object' ? (body as Record<string, unknown>) : {}; |
There was a problem hiding this comment.
Enroll body accepts arrays
Medium Severity
parseEnrollBody treats any typeof body === 'object' value as a payload and never refuses arrays. A JSON array body is accepted and coerced into an empty-looking form instead of a 400.
Additional Locations (1)
Triggered by learned rule: Body validation must reject arrays — typeof [] === 'object' bypasses object checks
Reviewed by Cursor Bugbot for commit 51f3331. Configure here.
| return; | ||
| } | ||
| this.form().controls['terms'].setValue([...this.terms(), result]); | ||
| }); |
There was a problem hiding this comment.
Dialog close subscribe outlives step
Medium Severity
dialogRef.onClose is subscribed with only take(1) and no takeUntilDestroyed. The setup step is destroyed by the wizard @switch or by leaving the page, so a late close can still write into the parent terms control.
Triggered by learned rule: Imperative .subscribe() in long-lived components must use takeUntilDestroyed
Reviewed by Cursor Bugbot for commit 51f3331. Configure here.
| size="small" | ||
| styleClass="!text-blue-600 !px-0 self-start" | ||
| routerLink="/mentorship/admin" | ||
| data-testid="mentorship-enroll-back-to-programs" /> |
There was a problem hiding this comment.
Back link leaks logo blob
Low Severity
The "My Programs" control navigates with routerLink and never calls revokeLogoPreview(). Cancel and successful submit revoke the object URL; this back link leaves the blob URL allocated after the user leaves the wizard.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 51f3331. Configure here.
🚀 Deployment StatusYour branch has been deployed to: https://ui-pr-2178.dev.v2.cluster.linuxfound.info Deployment Details:
The deployment will be automatically removed when this PR is closed. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a mentorship administration module with program listing, enrollment workflow, shared contracts, and mock BFF persistence.
Changes:
- Adds mentorship types, constants, validation utilities, and mocks.
- Adds authenticated list/enrollment BFF endpoints.
- Adds admin navigation, filtering, cards, and a three-step enrollment wizard.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
packages/shared/src/utils/mentorship.utils.ts |
Enrollment validation and date helpers. |
packages/shared/src/utils/mentorship.utils.spec.ts |
Shared utility tests. |
packages/shared/src/utils/index.ts |
Exports mentorship utilities. |
packages/shared/src/interfaces/mentorship.interface.ts |
Program and enrollment contracts. |
packages/shared/src/interfaces/index.ts |
Exports mentorship interfaces. |
packages/shared/src/constants/mentorship.constants.ts |
Statuses, styles, and mock programs. |
packages/shared/src/constants/mentorship-enroll.constants.ts |
Wizard options, defaults, and fixtures. |
packages/shared/src/constants/index.ts |
Exports mentorship constants. |
apps/lfx-one/src/server/services/mentorship.service.ts |
Filters and stores mock programs. |
apps/lfx-one/src/server/server.ts |
Mounts the mentorship API. |
apps/lfx-one/src/server/routes/mentorship.route.ts |
Defines list and enrollment routes. |
apps/lfx-one/src/server/controllers/mentorship.controller.ts |
Parses and validates requests. |
apps/lfx-one/src/app/shared/services/sidebar-nav.service.ts |
Adds mentorship navigation. |
apps/lfx-one/src/app/shared/services/mentorship.service.ts |
Provides frontend API access. |
apps/lfx-one/src/app/modules/mentorship/mentorship.routes.ts |
Defines mentorship UI routes. |
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/enroll-program.component.ts |
Orchestrates the enrollment wizard. |
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/enroll-program.component.html |
Renders wizard navigation and steps. |
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/components/enroll-term-dialog/enroll-term-dialog.component.ts |
Manages term editing. |
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/components/enroll-term-dialog/enroll-term-dialog.component.html |
Renders the term form. |
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/components/enroll-stepper/enroll-stepper.component.ts |
Models wizard progress. |
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/components/enroll-stepper/enroll-stepper.component.html |
Displays wizard progress. |
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/components/enroll-setup-step/enroll-setup-step.component.ts |
Manages skills and terms. |
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/components/enroll-setup-step/enroll-setup-step.component.html |
Renders setup controls. |
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/components/enroll-prerequisites-step/enroll-prerequisites-step.component.ts |
Manages prerequisite state. |
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/components/enroll-prerequisites-step/enroll-prerequisites-step.component.html |
Renders prerequisites and consent. |
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/components/enroll-details-step/enroll-details-step.component.ts |
Manages program details and logo selection. |
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/components/enroll-details-step/enroll-details-step.component.html |
Renders program detail fields. |
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/components/enroll-custom-prerequisite/enroll-custom-prerequisite.component.ts |
Manages custom prerequisite forms. |
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/components/enroll-custom-prerequisite/enroll-custom-prerequisite.component.html |
Renders custom prerequisite fields. |
apps/lfx-one/src/app/modules/mentorship/admin/components/programs-list/programs-list.component.ts |
Manages list filters. |
apps/lfx-one/src/app/modules/mentorship/admin/components/programs-list/programs-list.component.html |
Renders filters and program cards. |
apps/lfx-one/src/app/modules/mentorship/admin/components/program-card/program-card.component.ts |
Derives card presentation state. |
apps/lfx-one/src/app/modules/mentorship/admin/components/program-card/program-card.component.html |
Renders interactive program cards. |
apps/lfx-one/src/app/modules/mentorship/admin/admin.component.ts |
Loads programs and handles admin actions. |
apps/lfx-one/src/app/modules/mentorship/admin/admin.component.html |
Renders the admin landing page. |
apps/lfx-one/src/app/app.routes.ts |
Registers the mentorship module. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const mentorshipController = new MentorshipController(); | ||
|
|
||
| router.get('/programs', (req, res, next) => mentorshipController.getPrograms(req, res, next)); | ||
| router.post('/programs', (req, res, next) => mentorshipController.enrollProgram(req, res, next)); |
Comment on lines
+11
to
+15
| /** | ||
| * In-memory store so POST enrollments show up on the admin list in this | ||
| * process. Replaced when the upstream mentorship-service is wired up. | ||
| */ | ||
| const programsStore: MentorshipProgram[] = MOCK_MENTORSHIP_PROGRAMS.map((program) => ({ ...program })); |
Comment on lines
+45
to
+46
| if (!form.skills.length) errors.skills = 'Add at least one skill.'; | ||
| if (!form.terms.length) errors.terms = 'Add at least one program term.'; |
Comment on lines
+47
to
+50
| protected onProgramClick(): void { | ||
| // TODO: navigate to `/mentorship/admin/${slug}` once ProgramDetailComponent is re-introduced. | ||
| // The card still emits its slug on `(cardClick)` — restore the router.navigate call here | ||
| // and reinstate the `/mentorship/admin/:programId` route in `mentorship.routes.ts` when detail lands. |
Comment on lines
+11
to
+15
| <div class="flex flex-col gap-1"> | ||
| <label class="text-sm font-medium text-gray-700" for="importProgramId">Import from existing program</label> | ||
| <lfx-select | ||
| [form]="form()" | ||
| control="importProgramId" |
Comment on lines
+26
to
+28
| return this.http | ||
| .get<MentorshipProgramsResponse>('/api/mentorship/programs', { params: httpParams }) | ||
| .pipe(catchError(this.handleError(EMPTY_MENTORSHIP_PROGRAMS_RESPONSE, 'getPrograms'))); |
Comment on lines
+83
to
+86
| /** | ||
| * Payload collected by the enroll wizard and POSTed to `/api/mentorship/programs`. | ||
| * Logo file bytes stay client-side; only `logoFileName` is sent to the BFF. | ||
| */ |
Comment on lines
+17
to
+19
| export class MentorshipService { | ||
| public async getPrograms(req: Request, options: { search?: string; status?: MentorshipProgramStatus } = {}): Promise<MentorshipProgramsResponse> { | ||
| const startTime = logger.startOperation(req, 'mentorship_get_programs', options); |
Comment on lines
+30
to
+34
| /** Deterministic avatar-tile palette cycled by (title.charCodeAt(0) % length). */ | ||
| export const MENTORSHIP_PROGRAM_AVATAR_PALETTE: string[] = [ | ||
| 'rounded-xl bg-blue-100 !text-blue-700', | ||
| 'rounded-xl bg-violet-100 !text-violet-700', | ||
| 'rounded-xl bg-emerald-100 !text-emerald-700', |
- Added CII project ID input handling in the enrollment details step, including real-time validation and user feedback for loading and invalid states. - Integrated a new service method to fetch CII badge information based on the project ID. - Updated the EnrollProgramComponent to handle CII lookup status changes and display appropriate messages. - Enhanced the UI to show CII badge information and application links dynamically based on user input. - Improved error handling for invalid CII project IDs in the enrollment form. This commit improves the user experience by providing immediate feedback on CII project ID validity and integrating CII badge information into the mentorship enrollment process. Signed-off-by: Sameh16 <sameh_mohamed16@hotmail.com>
| throw ServiceValidationError.forField('projectId', 'CII Project ID must be numeric', { operation: 'mentorship_get_cii_badge' }); | ||
| } | ||
|
|
||
| const response = await fetch(mentorshipCiiBadgeJsonUrl(projectId)); |
| const detail = this.ciiLookupStatus() === 'loading' ? MENTORSHIP_CII_CHECKING : MENTORSHIP_CII_INVALID_ID; | ||
| this.messageService.add({ severity: 'warn', summary: 'Check this step', detail, life: 4000 }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
CII status resets on step remount
Medium Severity
Going back from a later wizard step remounts EnrollDetailsStepComponent via @switch, which restarts the CII lookup and overwrites the parent's ciiLookupStatus with idle or loading. onNext then rejects a CII ID that already passed, toasting Checking CII Project ID... or Invalid CII Project ID until the request finishes again.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit b505afe. Configure here.
|
|
||
| public constructor() { | ||
| effect(() => this.ciiLookupStatusChange.emit(this.ciiLookup().status)); | ||
| } |
There was a problem hiding this comment.
Effect emits CII lookup status
Medium Severity
effect() emits ciiLookupStatusChange whenever ciiLookup() changes. Repo convention is to bridge the signal with toObservable(), takeUntilDestroyed(), and a subscribe — not to perform output emits or other imperative writes inside effect().
Triggered by learned rule: Do not use effect() for side effects — use toObservable() + subscribe instead
Reviewed by Cursor Bugbot for commit b505afe. Configure here.
| id="ciiProjectId" | ||
| placeholder="Project ID" | ||
| styleClass="!border-0 !shadow-none !rounded-none w-full" | ||
| dataTest="mentorship-enroll-cii" /> |
There was a problem hiding this comment.
Static id on CII input
Medium Severity
lfx-input-text uses a static id="ciiProjectId" while the label's for targets that same id. A static id lands on the host and the native input, so the label focuses the host instead of the field.
Triggered by learned rule: Flag [link]="true" on lfx-button — wrapper does not forward it to p-button
Reviewed by Cursor Bugbot for commit b505afe. Configure here.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 3 comments.
Suppressed comments (12)
Previously missed (2) — in code that hasn't changed since the last review.
packages/shared/src/constants/mentorship-enroll.constants.ts:270
- Every new enrollment starts with an application window ending on 2026-08-31. As of 2026-09-07 that window is already closed, yet the current validation permits submission, so newly created programs can immediately carry stale dates. Generate defaults relative to the current date or start with no default term and require the user to add one.
apps/lfx-one/src/app/shared/services/sidebar-nav.service.ts:237 - This changes Mentorship from the documented external Me-lens link to an internal Admin section, but
docs/architecture/frontend/persona-content-matrix.md:46-58still records “Mentorships — (external link).” Update the navigation matrix in this PR so the route and audience/visibility contract do not drift from the sidebar implementation.
apps/lfx-one/src/server/routes/mentorship.route.ts:12
- These admin read/write routes require authentication but never authorize the caller for the target project or program.
getProgramsreturns the process-global list, whileenrollProgramtrustsbody.projectId, so any authenticated Contributor can see pending programs and submit one on behalf of any listed project. Add server-side permission checks and per-user/project scoping, with a matching frontend route/navigation gate; authentication alone is not action authority.
router.get('/programs', (req, res, next) => mentorshipController.getPrograms(req, res, next));
router.post('/programs', (req, res, next) => mentorshipController.enrollProgram(req, res, next));
apps/lfx-one/src/server/services/mentorship.service.ts:17
- This mutable module-level store makes the new write endpoint process-local. Production is configured with three replicas (
charts/lfx-self-serve/values.yaml:4), so a successful POST is visible only on one pod, may disappear on the next GET, and is lost on restart. Wire this to the owning mentorship service, or explicitly disable/gate the mock POST outside development instead of returning a durable-looking 201.
/**
* In-memory store so POST enrollments show up on the admin list in this
* process. Replaced when the upstream mentorship-service is wired up.
*/
const programsStore: MentorshipProgram[] = MOCK_MENTORSHIP_PROGRAMS.map((program) => ({ ...program }));
packages/shared/src/interfaces/mentorship.interface.ts:86
- The wizard requires a logo, but it retains only a filename and browser-only blob preview.
HttpClientposts no file bytes, the controller discardslogoPreviewUrl, and the service performs no upload, so a successful enrollment silently loses the selected logo. Add the repository's supported upload/presigned-storage flow before submission, or remove the required upload until that flow exists.
packages/shared/src/utils/mentorship.utils.ts:56 - Checking only
terms.lengthdoes not validate the term objects accepted by the POST endpoint. A direct request containing one term with empty names/dates passes this step, andenrollProgramthen creates a program with a blankterm. Validate every term's required fields, date formats, and ordering at this shared/server boundary rather than relying on the dialog UI.
packages/shared/src/constants/mentorship-enroll.constants.ts:24 - The documented range is “last year through 10 years ahead,” but this expression produces the current year through 11 years ahead. That also prevents editing an ongoing term that began last year. Offset the first generated year by
-1.
/** Year choices for the term dialog — last year through 10 years ahead. */
export const MENTORSHIP_TERM_YEAR_OPTIONS: ReadonlyArray<{ label: string; value: string }> = Array.from({ length: 12 }, (_, index) => {
const year = (new Date().getFullYear() + index).toString();
return { label: year, value: year };
packages/shared/src/constants/mentorship.constants.ts:34
- This runtime palette lives under
packages/shared, outside Tailwind's configured./src/**/*content scan (apps/lfx-one/tailwind.config.js:25), and it is not safelisted. At least!text-indigo-700has no scanned occurrence, so that avatar variant will lose its intended text color in production. Import this palette into the Tailwind config and safelist each whitespace-delimited token, following theAVATAR_COLORSpattern attailwind.config.js:31-33.
apps/lfx-one/src/app/shared/services/mentorship.service.ts:28 - All list failures, including 401 and 5xx responses, are replaced with an empty result.
AdminComponentthen stops loading and renders “No programs yet,” making an auth failure or outage indistinguishable from a genuinely empty account. Let non-not-found failures propagate or return a discriminated state so the page can render an error and retry action.
return this.http
.get<MentorshipProgramsResponse>('/api/mentorship/programs', { params: httpParams })
.pipe(catchError(this.handleError(EMPTY_MENTORSHIP_PROGRAMS_RESPONSE, 'getPrograms')));
apps/lfx-one/src/app/modules/mentorship/admin/admin.component.ts:51
- The program cards are rendered as keyboard-focusable buttons with click affordances, but every activation reaches this no-op handler. Users receive no navigation or action. Until the detail route exists, remove the card's button semantics and emitted click; otherwise add the route and navigate using the emitted slug.
protected onProgramClick(): void {
// TODO: navigate to `/mentorship/admin/${slug}` once ProgramDetailComponent is re-introduced.
// The card still emits its slug on `(cardClick)` — restore the router.navigate call here
// and reinstate the `/mentorship/admin/:programId` route in `mentorship.routes.ts` when detail lands.
}
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/components/enroll-prerequisites-step/enroll-prerequisites-step.component.html:83
- The required terms checkbox has no accessible name.
lfx-checkboxonly creates a label when itslabelinput is provided, and the adjacent paragraph is not associated with the generatedtermsAcceptedinput. Associate this agreement text with the checkbox so screen-reader users know what they are accepting.
<div class="flex items-start gap-3">
<lfx-checkbox [form]="form()" control="termsAccepted" />
<p class="min-w-0 flex-1 text-sm text-gray-800 leading-5">
apps/lfx-one/src/server/services/mentorship.service.ts:21
- These service methods start and complete a second request lifecycle even though the controller already owns the HTTP operation log. The repository logging standard assigns one
startOperation/successpair to the controller and step-leveldebug/infologs to services (.claude/rules/logging-patterns.md:47-67); the duplicate lifecycle also occurs inenrollProgramandgetCiiBadge. Remove the service-level lifecycle pairs or replace them with appropriate step logs.
public async getPrograms(req: Request, options: { search?: string; status?: MentorshipProgramStatus } = {}): Promise<MentorshipProgramsResponse> {
const startTime = logger.startOperation(req, 'mentorship_get_programs', options);
Comment on lines
+70
to
+73
| if (this.form.invalid) { | ||
| this.showErrors.set(true); | ||
| this.form.markAllAsTouched(); | ||
| return; |
Comment on lines
+71
to
+74
| const response = await fetch(mentorshipCiiBadgeJsonUrl(projectId)); | ||
| if (!response.ok) { | ||
| throw new ResourceNotFoundError('CII project', projectId, { operation: 'mentorship_get_cii_badge' }); | ||
| } |
Comment on lines
+35
to
+39
| public getCiiBadge(projectId: string): Observable<MentorshipCiiBadge | null> { | ||
| return this.http.get<MentorshipCiiBadge>(`/api/mentorship/cii/${encodeURIComponent(projectId)}`).pipe( | ||
| take(1), | ||
| catchError(() => of(null)) | ||
| ); |
… feedback - Added confirmation dialog for canceling enrollment in the EnrollProgramComponent. - Implemented real-time validation for program name availability, providing user feedback during the enrollment process. - Updated the EnrollDetailsStepComponent to include loading states and error messages for program name checks. - Enhanced the EnrollCustomPrerequisiteComponent with due date validation and improved error messaging. - Improved the EnrollSetupStepComponent to manage terms with a confirmation dialog for term deletion and added a limit on the number of terms. This commit significantly improves the user experience by providing immediate feedback and validation during the mentorship program enrollment process. Signed-off-by: Sameh16 <sameh_mohamed16@hotmail.com>
- Modified the isMentorshipHttpUrl function to utilize normalizeToUrl for improved URL validation. - Added a test case to allow 'google.com' as a valid input for mentorship URLs. This change enhances the flexibility of URL inputs in the mentorship module, ensuring better user experience during enrollment. Signed-off-by: Sameh16 <sameh_mohamed16@hotmail.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 7 potential issues.
There are 15 total unresolved issues (including 8 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0efaa72. Configure here.
| scrollHeight="300px" | ||
| styleClass="w-full" | ||
| (onFilter)="onLfFilter($event)" | ||
| (onLazyLoad)="onLfLazyLoad()" |
There was a problem hiding this comment.
Lazy-load handler drops scroll event
Medium Severity
(onLazyLoad) calls onLfLazyLoad() without $event, so the event.last near-end guard never runs. PrimeNG fires this when the overlay opens, which immediately fetches the next page instead of waiting for the user to scroll.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 0efaa72. Configure here.
| <label class="text-sm font-medium text-gray-700" for="name">Program Name <span class="text-red-500">*</span></label> | ||
| <span class="text-xs text-gray-400">{{ nameLength() }} / {{ nameMax }}</span> | ||
| </div> | ||
| <lfx-input-text [form]="form()" control="name" id="name" placeholder="Enter program name" [maxlength]="nameMax" dataTest="mentorship-enroll-name" /> |
There was a problem hiding this comment.
Static ids land on input-text hosts
Medium Severity
Static id="name", id="repositoryUrl", id="websiteUrl", and id="codeOfConductUrl" on lfx-input-text also become the host element id. Matching label for attributes then target the wrapper instead of the native input, so the labels do not focus the fields.
Additional Locations (2)
Triggered by learned rule: Flag [link]="true" on lfx-button — wrapper does not forward it to p-button
Reviewed by Cursor Bugbot for commit 0efaa72. Configure here.
| }, | ||
| error: () => this.lfProjectsLoading.set(false), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Project fetches race and can clobber
Medium Severity
loadLfProjects starts a new HTTP call without cancelling the previous one. A slower filter or page response can overwrite a newer list, or append an old page onto a just-replaced search result, so the picker shows the wrong projects.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 0efaa72. Configure here.
| options.unshift({ id: selectedId, name: selectedId, value: selectedId, label: selectedId }); | ||
| } | ||
| return options; | ||
| }); |
There was a problem hiding this comment.
Filtered picker loses selected project name
Medium Severity
When the selected project is missing from the current lazy/filter page, projectOptions reinserts it using the raw projectId as the label. Searching the dropdown then replaces the chosen name (for example GridFlow) with an id like proj-gridflow.
Reviewed by Cursor Bugbot for commit 0efaa72. Configure here.
| this.importLoading.set(false); | ||
| }, | ||
| error: () => this.importLoading.set(false), | ||
| }); |
There was a problem hiding this comment.
Details-step HTTP subscriptions outlive the view
Medium Severity
getPrograms() and getLfProjects() are subscribed in the details step without takeUntilDestroyed. Leaving the step (the parent @switch destroys this component) can still write importOptions or lfProjects on a dead instance.
Additional Locations (1)
Triggered by learned rule: Imperative .subscribe() in long-lived components must use takeUntilDestroyed
Reviewed by Cursor Bugbot for commit 0efaa72. Configure here.
| return this.http.get<MentorshipNameAvailability>('/api/mentorship/programs/name-available', { params: new HttpParams().set('name', name) }).pipe( | ||
| take(1), | ||
| catchError(() => of({ available: true })) | ||
| ); |
There was a problem hiding this comment.
Name availability check fails open
Medium Severity
isProgramNameAvailable treats any HTTP failure as { available: true }. The details-step gate only blocks when status is not available, so a down or failing uniqueness endpoint lets a taken name through with no error.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 0efaa72. Configure here.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 40 out of 40 changed files in this pull request and generated 9 comments.
Suppressed comments (8)
apps/lfx-one/src/server/routes/mentorship.route.ts:13
- These admin routes have only the general authentication check in the controller. Any signed-in Contributor can list pending programs and submit an enrollment for an arbitrary
projectId; the project picker likewise returns all projects. Scope reads to programs the caller may administer and require a strict server-side permission check (for example, the chosen project'swriterrelation or the mentorship-specific relation) before accepting the POST; a UI/nav guard should only mirror that server boundary.
router.get('/programs', (req, res, next) => mentorshipController.getPrograms(req, res, next));
router.post('/programs', (req, res, next) => mentorshipController.enrollProgram(req, res, next));
apps/lfx-one/src/server/services/mentorship.service.ts:32
- This production-mounted API uses process-local memory as the authoritative program store. A successful enrollment disappears on restart and can be absent immediately when the next GET hits another replica; the ever-growing array also makes every list response unbounded. The BFF should proxy an owning mentorship service, or this mock implementation and its write route must be explicitly restricted to local/mock mode.
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/components/enroll-term-dialog/enroll-term-dialog.component.ts:82 - After a date-order failure, manual
ordererrors are placed on the end controls. If the user fixes only the corresponding start value, those end controls retain the manual error, and this earlyform.invalidreturn prevents the cross-field validator from running again to clear it. Recompute/clear the order errors whenever either side changes, or run cross-field validation before this early return.
if (this.form.invalid) {
this.showErrors.set(true);
this.form.markAllAsTouched();
return;
}
apps/lfx-one/src/app/modules/mentorship/admin/admin.component.ts:50
- Program cards are rendered as keyboard-focusable buttons and emit their slug, but this handler intentionally does nothing. Clicking or pressing Enter/Space therefore gives users a dead control. Implement the detail navigation/route now, or remove the card's interactive semantics until that destination exists.
protected onProgramClick(): void {
// TODO: navigate to `/mentorship/admin/${slug}` once ProgramDetailComponent is re-introduced.
// The card still emits its slug on `(cardClick)` — restore the router.navigate call here
// and reinstate the `/mentorship/admin/:programId` route in `mentorship.routes.ts` when detail lands.
packages/shared/src/constants/mentorship.constants.ts:34
- This runtime-selected palette lives under
packages/shared, outside Tailwind's configuredapps/lfx-one/srccontent scan. The established pattern is to safelist shared class constants intailwind.config.js; without that, tokens not used elsewhere (notably!text-indigo-700) are omitted from production CSS. Import this palette into the Tailwind config and safelist each whitespace-separated class token.
packages/shared/src/interfaces/mentorship.interface.ts:86 - The required logo file is never uploaded or persisted: the browser creates only a temporary blob URL, while this API contract sends just the filename and the server discards the preview URL. The user can receive a successful enrollment even though the selected logo bytes are irretrievably lost. Upload the asset and submit a durable object key/URL, or remove the required upload UI until storage exists.
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/components/enroll-prerequisites-step/enroll-prerequisites-step.component.html:100 - The terms checkbox is unnamed to assistive technology: the adjacent paragraph is not a
<label>, and nolabelis passed tolfx-checkbox. Associate the agreement text with the checkbox (while preserving usable links), for example by addingaria-labelledbysupport to the wrapper and referencing a stable ID here.
<div class="flex items-start gap-3">
<lfx-checkbox [form]="form()" control="termsAccepted" />
<p class="min-w-0 flex-1 text-sm text-gray-800 leading-5">
<span class="text-red-500">*</span>
I agree to the
<a [href]="platformUseHref" target="_blank" rel="noopener noreferrer" class="text-blue-600 hover:text-blue-700 font-medium"
apps/lfx-one/src/app/shared/services/mentorship.service.ts:36
- This converts every programs-list failure into the legitimate empty response, so an outage or authorization failure renders “No programs yet” and hides existing programs from the admin. Preserve an explicit error state (while optionally retaining stale data) so the page can distinguish an empty list from a failed request.
return this.http
.get<MentorshipProgramsResponse>('/api/mentorship/programs', { params: httpParams })
.pipe(catchError(this.handleError(EMPTY_MENTORSHIP_PROGRAMS_RESPONSE, 'getPrograms')));
Comment on lines
+43
to
+47
| public isProgramNameAvailable(name: string): Observable<MentorshipNameAvailability> { | ||
| return this.http.get<MentorshipNameAvailability>('/api/mentorship/programs/name-available', { params: new HttpParams().set('name', name) }).pipe( | ||
| take(1), | ||
| catchError(() => of({ available: true })) | ||
| ); |
Comment on lines
+67
to
+70
| if (term.startDate < currentMonthStart) { | ||
| errors.startDate = 'Start month should be greater than or equal to current month.'; | ||
| } | ||
| if (term.endDate < term.startDate) { |
Comment on lines
+143
to
+146
| if (isBlank(item.name) || item.name.trim().length > MENTORSHIP_CUSTOM_PREREQ_NAME_MAX) return true; | ||
| if (isBlank(item.dueDate ?? '')) return true; | ||
| if ((item.dueDate ?? '') <= todayIso) return true; | ||
| return isBlank(item.description) || item.description.trim().length > MENTORSHIP_CUSTOM_PREREQ_DESCRIPTION_MAX; |
Comment on lines
+41
to
+47
| /** Two-letter initials pulled from the first two whitespace-delimited tokens of the title (e.g. "GridFlow: Time" → "GT"). */ | ||
| protected readonly initials = computed(() => { | ||
| const tokens = this.program().name.trim().split(/\s+/); | ||
| if (tokens.length === 0 || tokens[0].length === 0) return '?'; | ||
| const first = tokens[0][0]; | ||
| const second = tokens[1]?.[0] ?? tokens[0][1] ?? ''; | ||
| return (first + second).toUpperCase(); |
Comment on lines
+65
to
+67
| <div class="flex items-start gap-3"> | ||
| <lfx-checkbox [form]="form" control="requireFile" /> | ||
| <p class="min-w-0 flex-1 text-sm text-gray-700 leading-5">{{ fileLabel }}</p> |
Comment on lines
+53
to
+58
| [lazy]="true" | ||
| [loading]="lfProjectsLoading()" | ||
| scrollHeight="300px" | ||
| styleClass="w-full" | ||
| (onFilter)="onLfFilter($event)" | ||
| (onLazyLoad)="onLfLazyLoad()" |
Comment on lines
+488
to
+490
| const source = MENTORSHIP_IMPORT_PROGRAM_DETAILS[importProgramId]; | ||
| if (!source) { | ||
| return { ...createEmptyMentorshipEnrollForm(), importProgramId }; |
Comment on lines
+128
to
+134
| if (!form.terms.length) { | ||
| errors.terms = 'Add at least one program term.'; | ||
| } else if (form.terms.length > MENTORSHIP_MAX_OPEN_TERMS) { | ||
| errors.terms = MENTORSHIP_MAX_OPEN_TERMS_MESSAGE; | ||
| } else { | ||
| const firstTermError = form.terms.map((term) => Object.values(getMentorshipTermDateErrors(term))[0]).find(Boolean); | ||
| if (firstTermError) errors.terms = firstTermError; |
Comment on lines
+11
to
+15
| router.get('/programs/name-available', (req, res, next) => mentorshipController.isProgramNameAvailable(req, res, next)); | ||
| router.get('/programs', (req, res, next) => mentorshipController.getPrograms(req, res, next)); | ||
| router.post('/programs', (req, res, next) => mentorshipController.enrollProgram(req, res, next)); | ||
| router.get('/lf-projects', (req, res, next) => mentorshipController.getLfProjects(req, res, next)); | ||
| router.get('/cii/:projectId', (req, res, next) => mentorshipController.getCiiBadge(req, res, next)); |
…lity - Introduced a new route for program details in the mentorship module, allowing navigation to specific program information. - Implemented the ProgramDetailComponent with tabs for mentees, applicants, mentors, and terms, enhancing the admin interface. - Updated the AdminComponent to handle program clicks, navigating to the new program detail view. - Enhanced the ProgramCardComponent to emit program IDs instead of slugs for better routing. - Added new components for managing applicants, current mentees, mentors, and terms, each with search and filter capabilities. - Improved the EnrollTermDialogComponent to include date validation and error handling. This commit significantly enhances the mentorship admin experience by providing detailed program management features and improved navigation. Signed-off-by: Sameh16 <sameh_mohamed16@hotmail.com>
…etailsStepComponent - Updated the type definition for importOptions to use a more concise syntax, enhancing code clarity. - Removed redundant type declaration in EnrollSetupStepComponent, ensuring consistency in computed properties. These changes improve the maintainability and readability of the mentorship enrollment components. Signed-off-by: Sameh16 <sameh_mohamed16@hotmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 53 out of 53 changed files in this pull request and generated 3 comments.
Suppressed comments (7)
apps/lfx-one/src/server/services/mentorship.service.ts:35
- This mutable module-global store makes enrollment state process-local and shared across users: one user's POST becomes visible to every user hitting that worker, disappears on restart, and is absent on other replicas. Because the BFF must not own domain state, keep mocks read-only/feature-gated and persist enrollments through the owning upstream service before exposing this write.
apps/lfx-one/src/app/shared/services/mentorship.service.ts:55 - A failed availability request is converted to
{ available: true }. The wizard then treats the name as verified, while the POST endpoint does not independently enforce uniqueness, so an outage or 401 permits duplicate program names. Propagate an explicit lookup error and enforce uniqueness atomically on the server.
apps/lfx-one/src/app/shared/services/mentorship.service.ts:44 - All errors, including 401s and 5xx responses, are collapsed to
null, so the detail page renders “Program not found” during authentication failures or service outages. Only map an actual 404 to the not-found state; propagate other failures to a distinct error state.
packages/shared/src/constants/mentorship.constants.ts:34 - These status classes (and the new person/term badge maps and avatar palette below) are selected dynamically from
packages/shared, but Tailwind scans onlyapps/lfx-one/src(tailwind.config.js:25). UnlikeAVATAR_COLORS, none are safelisted, so production builds purge their color utilities and the badges/avatars lose styling. Add all new runtime class values to the Tailwind safelist.
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/components/enroll-details-step/enroll-details-step.component.html:58 - Select lazy loading is driven by PrimeNG's virtual scroller, but this enables
lazywhile leaving the wrapper'svirtualScrollat its defaultfalse. Consequently scrolling cannot request the second page, so only the first 10 LF projects are listed unless users search. Configure virtual scrolling/item size and pass$eventto the lazy-load handler.
[filter]="true"
[lazy]="true"
[loading]="lfProjectsLoading()"
scrollHeight="300px"
styleClass="w-full"
(onFilter)="onLfFilter($event)"
(onLazyLoad)="onLfLazyLoad()"
packages/shared/src/utils/mentorship.utils.ts:142
- The server-side setup validation checks only term count and date ordering; it never validates each term's required
nameor its 50-character limit. A direct POST can therefore create a program whose displayed term is empty or oversized despite the dialog validators. Add term-field validation to the shared validator so the BFF rejects forged payloads.
packages/shared/src/interfaces/mentorship.interface.ts:90 - The wizard requires a logo and creates a blob preview, but this contract deliberately drops the file bytes and the server does not retain even
logoFileNameor producelogoUrl. Submission therefore reports success while the selected logo is permanently lost. Upload the file through a real storage/API contract (or make the field optional/disabled until that path exists).
Comment on lines
+208
to
+210
| private setTerms(next: MentorshipProgramTermRow[]): void { | ||
| this.draftTerms.set(next); | ||
| this.termsChange.emit(next); |
Comment on lines
+183
to
+185
| const form = parseEnrollBody(req.body); | ||
|
|
||
| // Run every wizard step so a client that skipped validation cannot persist a partial enrollment. |
Comment on lines
+322
to
+329
| export const MENTORSHIP_DEFAULT_TERM: MentorshipProgramTerm = { | ||
| id: 'term-1-2027', | ||
| name: 'Term 1 - 2027', | ||
| startDate: '2027-03-01', | ||
| endDate: '2027-05-01', | ||
| applicationStartDate: '2026-12-01', | ||
| applicationEndDate: '2027-02-28', | ||
| }; |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 53 out of 53 changed files in this pull request and generated 2 comments.
Suppressed comments (9)
Previously missed (2) — in code that hasn't changed since the last review.
apps/lfx-one/src/server/controllers/mentorship.controller.ts:190
getMentorshipEnrollStepErrorsis UI step validation, not a complete request schema. In particular, setup validation checks only term count and date relationships, so a direct POST with an empty term name (or non-ISO date strings that compare lexicographically) passes and is stored with an invalid term. Validate every nested term/prerequisite field and date format at this server boundary before calling the service.
apps/lfx-one/src/app/modules/mentorship/admin/program-detail/components/program-detail-header/program-detail-header.component.html:38- This is visually a tab interface but exposes only unrelated buttons: there is no
tablist/tabrole, selected state, or relationship to the active panel. Screen-reader users cannot determine which section is active. Match the existing settings-tab pattern withrole="tablist",role="tab",aria-selected,aria-controls, and correspondingtabpanelidentifiers.
apps/lfx-one/src/server/services/mentorship.service.ts:35
- This mutable process-local store makes a successful enrollment non-durable: it disappears on restart and different replicas return different program lists. It also makes the BFF the owner of mentorship domain state, contrary to this repository's thin-proxy boundary. Persist through the owning mentorship service, or keep the mock surface read-only and gated until that integration exists.
apps/lfx-one/src/server/routes/mentorship.route.ts:15 - These admin routes have authentication but no authorization gate. Any signed-in contributor can call the detail endpoint and receive mentee/applicant/mentor names and emails, and can POST an enrollment for an arbitrary
projectId. Add a server-side OpenFGA permission check against each program's owning project (and filter the list to authorized projects); a client route or hidden nav item is not sufficient.
apps/lfx-one/src/app/modules/mentorship/admin/program-detail/components/terms-tab/terms-tab.component.ts:210 - Every create/edit/close/re-open/delete action ends by updating only this component signal and its parent's override;
MentorshipServiceexposes no term mutation API. The UI therefore confirms an administrative change that is lost as soon as the page reloads. Persist each operation through the BFF/upstream service, or present these actions as unavailable until persistence is wired.
apps/lfx-one/src/app/shared/services/mentorship.service.ts:55 - This fails open: any network or server error is reported as
available: true, so the wizard marks the name available and permits submission. The POST path does not repeat the uniqueness check, so duplicates are then accepted. Propagate an indeterminate/error state to the wizard and enforce uniqueness atomically on the server when creating the program.
packages/shared/src/constants/mentorship.constants.ts:44 - These classes are selected at runtime from
packages/shared, which is outside the Tailwind content glob, but the new palette is not added totailwind.config.js's safelist. At least!text-indigo-700has no literal occurrence in scanned app sources, so indigo avatars lose their intended foreground style in production. Import and spread this palette into the safelist, as the config already does forAVATAR_COLORS.
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/components/enroll-prerequisites-step/enroll-prerequisites-step.component.html:100 - The consent checkbox has no accessible name:
lfx-checkboxonly renders a<label>when itslabelinput is set, and this adjacent paragraph is not associated with the generatedtermsAcceptedinput. Screen-reader users therefore cannot tell what they are accepting. Extend the wrapper to supportaria-labelledby/projected label content and associate this terms text with the checkbox.
<div class="flex items-start gap-3">
<lfx-checkbox [form]="form()" control="termsAccepted" />
<p class="min-w-0 flex-1 text-sm text-gray-800 leading-5">
<span class="text-red-500">*</span>
I agree to the
<a [href]="platformUseHref" target="_blank" rel="noopener noreferrer" class="text-blue-600 hover:text-blue-700 font-medium"
apps/lfx-one/src/app/modules/mentorship/admin/enroll-program/components/enroll-custom-prerequisite/enroll-custom-prerequisite.component.html:67
- Each custom-prerequisite card renders this checkbox without a label, and the wrapper derives its DOM id solely from the control name, so multiple cards also create duplicate
id="requireFile"elements. Supply a per-card input id and associate the explanatory text as its accessible label.
<div class="flex items-start gap-3">
<lfx-checkbox [form]="form" control="requireFile" />
<p class="min-w-0 flex-1 text-sm text-gray-700 leading-5">{{ fileLabel }}</p>
| logoFileName: new FormControl('', { nonNullable: true }), | ||
| logoPreviewUrl: new FormControl('', { nonNullable: true }), | ||
| skills: new FormControl<string[]>([], { nonNullable: true }), | ||
| terms: new FormControl<MentorshipProgramTerm[]>(createEmptyMentorshipEnrollForm().terms, { nonNullable: true }), |
Comment on lines
+47
to
+48
| public enrollProgram(form: MentorshipEnrollForm): Observable<MentorshipProgram> { | ||
| return this.http.post<MentorshipProgram>('/api/mentorship/programs', form).pipe(take(1)); |
🧹 Deployment RemovedThe deployment for PR #2178 has been removed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
feat(mentorship): add mentorship module with admin components
This commit establishes the foundational structure for the mentorship feature, enabling future enhancements and integrations.
Summary
This pull request introduces the new Mentorship Admin module and related components to the application, enabling mentorship program administration features. The main changes include adding a new route for mentorship, implementing the admin landing page, and building out reusable, signal-driven components for listing, filtering, and managing mentorship programs and their prerequisites.
Mentorship Admin Module Implementation:
mentorshiproute under the "me" section, loading the mentorship module for authenticated users.AdminComponentas the landing page for mentorship admins, using Angular signals for state management and delegating program list rendering and filtering to child components. [1] [2]Mentorship Program Listing & Filtering:
ProgramsListComponentto handle program search, status filtering, and rendering the list of programs with empty state handling. This component manages its own reactive form and communicates changes via outputs. [1] [2]ProgramCardComponentfor displaying individual mentorship programs in a compact, accessible card format, including program metrics and deterministic avatar coloring. [1] [2]Mentorship Program Enrollment (Custom Prerequisites):
EnrollCustomPrerequisiteComponentfor managing custom prerequisites during program enrollment, supporting validation, field length limits, and emitting changes to the parent form. [1] [2]