chore(landing): remove dead testimonials placeholder and document fetchCourses data shape - #124
Conversation
…ews, and reduced-motion support
- Added FeaturedCourses server section: fetches /api/courses, ranks by enrollment
then rating, surfaces top items in an embla carousel (loop-free, keyboard
+ swipe accessible).
- Added LandingCourseCard: lightweight, server-renderable card without auth
or bookmark hooks so it works for unauthenticated landing visitors.
- Refactored Testimonials into a server component that pulls real quotes
from course.reviews and renders up to 6 attributable reviews; renders
null if none exist (no fabrication).
- Implemented prefers-reduced-motion across the landing page:
- Stats now wrapped in <MotionConfig reducedMotion="user"> and the
AnimatedCounter respects the OS preference immediately.
- Added a global @media (prefers-reduced-motion: reduce) block disabling
marquee, scroll, gradient, spin, fade-in, and animate-in-out animations.
- Polish: aria-labelledby on each section pointing at visible H2, redundant
aria-labels removed from article elements, PascalCase Page component.
Closes Deen-Bridge#114
…testimonials (Deen-Bridge#114) - Add FeaturedCourses server component: fetches GET /api/courses with 1-hour revalidation, ranks by rating×log(reviewCount), renders top 8 in existing embla Carousel (keyboard/swipe navigable). Section hides cleanly when API is unreachable or returns no data. - Add PublicCourseCard: lightweight card for unauthenticated visitors — no useAuth/bookmark hooks. Shows thumbnail (lazy-loaded), title, category, star rating, review count, instructor avatar+name, USDC price. Links to /dashboard/courses/[courseId]. - Replace fabricated Testimonials: remove all 6 invented hardcoded quotes. Add lib/testimonials.js as the maintainer-managed source of real, consent-confirmed reviews. Testimonials.jsx reads from that file and renders nothing if the array is empty, so the section stays hidden until real quotes are added. - Rebuild Testimonials.jsx: initials avatar (consistent with Partners), quote attribution to course title, marquee pauses on hover and keyboard focus (onFocus/onBlur + CSS paused class). Re-enabled in app/page.jsx. - Reduced-motion across the landing page (useReducedMotion from framer-motion + CSS @media prefers-reduced-motion): * Stats.jsx: AnimatedCounter shows final value immediately; all motion.div entrance animations are skipped. * Partners.jsx: marquee replaced with static flex-wrap grid. * Testimonials.jsx: marquees replaced with static flex-wrap grid. * globals.css: @media rule stops animate-marquee, animate-marquee- reverse, animate-scroll, animate-spin-slow, animate-gradient, animate-fade-in. - Heading/landmark audit: * One h1 in Hero (inside <header>). * Every section now has aria-labelledby pointing to its h2: About (#about-heading), WhyDeenBridge (#why-heading), Stats (#stats-heading), FeaturedCourses (#featured-courses-heading), Testimonials (#testimonials-heading), Partners (#partners-heading), CTA (#cta-heading). * Decorative divs marked aria-hidden=true throughout. - Below-the-fold images use loading=lazy in PublicCourseCard.
…chCourses Follow-up cleanup to PR Deen-Bridge#121. Two scoped changes: - Delete lib/testimonials.js: an empty placeholder array added by Deen-Bridge#121 with zero importers across the codebase and full git history (verified via git grep). Pure dead code. - Add JSDoc to lib/actions/courses/fetch-courses.js: documents the GET /api/courses response shapes and the required/optional course fields consumed by FeaturedCourses.jsx and Testimonials.jsx. No behavior change to fetchCourses() itself. Verification: npm run lint and npm run build both exit 0 on this branch. Built on top of origin/feat/landing-social-proof so the Serwist PWA wrapper is also exercised.
|
@Whiznificent is attempting to deploy a commit to the Deen Bridge Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe landing page now includes cached, server-rendered featured courses and testimonials, reusable public course cards, reduced-motion handling for animated sections, improved heading relationships, and updated page composition. ChangesLanding page experience
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Page
participant FeaturedCourses
participant fetchCourses
participant FeaturedCoursesCarousel
participant LandingCourseCard
participant Testimonials
Page->>FeaturedCourses: render featured courses
FeaturedCourses->>fetchCourses: request course catalog
fetchCourses-->>FeaturedCourses: return courses
FeaturedCourses->>FeaturedCoursesCarousel: pass ranked course cards
FeaturedCoursesCarousel->>LandingCourseCard: render course slides
Page->>Testimonials: render testimonials
Testimonials->>fetchCourses: request course catalog
fetchCourses-->>Testimonials: return courses with reviews
Testimonials-->>Page: render linked testimonial cards or null
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/actions/courses/fetch-courses.js (1)
47-48: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDebug
console.logof full API response/error left in place.Logging the entire axios
response/response.data(Line 47-48) can leak course/reviewer PII (reviews[].user,createdBy) into server logs, and logging the rawerrorobject (Line 54) can leak request/auth details. These add noisy, unbounded log volume on every landing-page render.🧹 Proposed cleanup
- const response = await axiosInstance.get("/api/courses"); - console.log("API Response:", response); - console.log("API Response Data:", response.data); + const response = await axiosInstance.get("/api/courses"); if (response.data && Array.isArray(response.data.courses)) { return response.data.courses; } ... } catch (error) { - console.log("Error fetching courses:", error); + console.error("[fetchCourses] request failed:", error?.message || error); return [];Also applies to: 54-54
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/actions/courses/fetch-courses.js` around lines 47 - 48, Remove the debug console.log calls in the course-fetching flow that print the full response and response.data, and replace raw error-object logging in its catch handler with a concise, bounded message that excludes response payloads and request/auth details. Preserve the existing error handling behavior in the function containing the API request.
🧹 Nitpick comments (4)
lib/actions/courses/fetch-courses.js (1)
24-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winField documentation omits several fields the landing cards actually consume.
The "Course record fields consumed by landing-page components" section only lists
_id/id/title(required) andreviews/enrolledUsers(optional), butLandingCourseCard.jsxandPublicCourseCard.jsxalso readthumbnail,price,category,createdBy, anddescriptionfrom the same course objects. Since documenting this contract is the point of the PR, worth extending the optional-fields list to cover these too so future readers have the full picture.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/actions/courses/fetch-courses.js` around lines 24 - 39, Extend the “Course record fields consumed by landing-page components” documentation near the existing optional fields to include thumbnail, price, category, createdBy, and description, matching the fields read by LandingCourseCard and PublicCourseCard. Keep the existing required/optional classification and formatting consistent.app/(pages)/(landingPage)/FeaturedCourses.jsx (1)
39-47: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOuter try/catch is likely redundant.
fetchCourses()already swallows all errors internally and returns[](seefetch-courses.js's own catch block), so this outer try/catch can't actually be reached under the current implementation. Not harmful, just dead defensive code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/`(pages)/(landingPage)/FeaturedCourses.jsx around lines 39 - 47, Remove the redundant try/catch around fetchCourses in FeaturedCourses and assign its returned value directly to courses, relying on fetchCourses’s internal error handling and [] fallback.app/(pages)/(landingPage)/Partners.jsx (2)
14-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the dead
keyprop insidePartnerPill.
keyset on this component's own root element (line 17) has no effect —keyonly matters when the parent maps this element into an array, which happens at the call sites (lines 54, 71), not here. This leavesindexeffectively unused insidePartnerPilltoo.♻️ Suggested cleanup
-function PartnerPill({ partner, index }) { +function PartnerPill({ partner }) { return ( <div - key={`${partner.name}-${index}`} className="flex items-center gap-3 shrink-0 rounded-full border border-green-900/10 dark:border-white/10 bg-green-50/60 dark:bg-white/5 px-5 py-3 opacity-80 hover:opacity-100 transition-all duration-300" title={partner.name} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/`(pages)/(landingPage)/Partners.jsx around lines 14 - 29, Remove the unused key attribute from the root element in PartnerPill and remove the index prop from its signature, since list keys are applied at the parent call sites.
58-75: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winHide the duplicated marquee content from screen readers.
[...partners, ...partners]duplicates every name for the seamless scroll loop, but the container isn'taria-hidden, unlike the gradient overlays right above it (lines 60-67) which correctly are. Screen readers will announce each partner name twice, which is a confusing regression compared to the reduced-motion grid path.🔧 Suggested fix
- <div className="flex w-max animate-scroll hover:[animation-play-state:paused] items-center gap-4 sm:gap-6 py-2"> + <div + aria-hidden="true" + className="flex w-max animate-scroll hover:[animation-play-state:paused] items-center gap-4 sm:gap-6 py-2" + > {[...partners, ...partners].map((partner, index) => ( <PartnerPill key={`${partner.name}-${index}`} partner={partner} index={index} /> ))} </div> + <span className="sr-only"> + {partners.map((p) => p.name).join(", ")} + </span>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/`(pages)/(landingPage)/Partners.jsx around lines 58 - 75, Mark the duplicated marquee content container around the `[...partners, ...partners].map` in the Partners component with `aria-hidden="true"`, while leaving the visual animation and `PartnerPill` rendering unchanged, so screen readers announce the reduced-motion grid content only once.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/`(pages)/(landingPage)/FeaturedCourses.jsx:
- Around line 39-54: Update the landing-page rendering of the FeaturedCourses
component to provide a loading boundary, using React Suspense or a route-level
loading component with an appropriate fallback. Keep the existing
FeaturedCourses fetch, ranking, and empty-state behavior unchanged while
allowing the page shell to stream before fetchCourses completes.
In `@app/`(pages)/(landingPage)/LandingCourseCard.jsx:
- Around line 14-23: Update LandingCourseCard’s course identifier handling to
support legacy records by using course._id with course.id as the fallback, and
use that resolved identifier for the Explore Course Link href. Preserve the
existing behavior for records that provide _id.
- Around line 14-85: Support both course identifier fields in each landing card:
in app/(pages)/(landingPage)/LandingCourseCard.jsx lines 14-85, destructure id
and build the detail Link from _id ?? id; in
components/molecules/landingpage/PublicCourseCard.jsx lines 12-25, compute
courseId from course._id ?? course.id and use it in the Link href.
In `@app/`(pages)/(landingPage)/Testimonials.jsx:
- Around line 9-16: Update getInitials to use a Unicode-aware letter test
instead of the ASCII-only /^[A-Za-z]/ filter, enabling initials for Arabic,
Urdu, Cyrillic, and other scripts while preserving the existing splitting,
two-word limit, capitalization, and joining behavior.
- Around line 152-157: Update the Link in the testimonials course CTA to use the
public course route instead of the protected
`/dashboard/courses/${t.course._id}` path. Reuse the established public course
URL pattern from the landing stack while preserving the existing course
identifier and link styling.
- Around line 128-136: Update the testimonial avatar rendering around the
t.avatar Image in the testimonials component to validate the avatar URL against
the hosts allowed by next.config.mjs before passing it to next/image. Use the
avatar only for trusted hosts, and otherwise render the existing initials
placeholder without allowing an unlisted remote URL to reach Image.
In `@lib/actions/courses/fetch-courses.js`:
- Around line 13-22: Update the success-path normalization in the
course-fetching function so it returns the wrapped courses array only when
valid, the response data when it is already an array, and [] for all other
response shapes. Preserve the existing catch behavior and ensure the function’s
documented always-array contract holds before returning to consumers.
---
Outside diff comments:
In `@lib/actions/courses/fetch-courses.js`:
- Around line 47-48: Remove the debug console.log calls in the course-fetching
flow that print the full response and response.data, and replace raw
error-object logging in its catch handler with a concise, bounded message that
excludes response payloads and request/auth details. Preserve the existing error
handling behavior in the function containing the API request.
---
Nitpick comments:
In `@app/`(pages)/(landingPage)/FeaturedCourses.jsx:
- Around line 39-47: Remove the redundant try/catch around fetchCourses in
FeaturedCourses and assign its returned value directly to courses, relying on
fetchCourses’s internal error handling and [] fallback.
In `@app/`(pages)/(landingPage)/Partners.jsx:
- Around line 14-29: Remove the unused key attribute from the root element in
PartnerPill and remove the index prop from its signature, since list keys are
applied at the parent call sites.
- Around line 58-75: Mark the duplicated marquee content container around the
`[...partners, ...partners].map` in the Partners component with
`aria-hidden="true"`, while leaving the visual animation and `PartnerPill`
rendering unchanged, so screen readers announce the reduced-motion grid content
only once.
In `@lib/actions/courses/fetch-courses.js`:
- Around line 24-39: Extend the “Course record fields consumed by landing-page
components” documentation near the existing optional fields to include
thumbnail, price, category, createdBy, and description, matching the fields read
by LandingCourseCard and PublicCourseCard. Keep the existing required/optional
classification and formatting consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 58bda74d-d4a1-4761-858a-f872e449b0c5
📒 Files selected for processing (14)
app/(pages)/(landingPage)/About.jsxapp/(pages)/(landingPage)/CTA.jsxapp/(pages)/(landingPage)/FeaturedCourses.jsxapp/(pages)/(landingPage)/FeaturedCoursesCarousel.jsxapp/(pages)/(landingPage)/Hero.jsxapp/(pages)/(landingPage)/LandingCourseCard.jsxapp/(pages)/(landingPage)/Partners.jsxapp/(pages)/(landingPage)/Stats.jsxapp/(pages)/(landingPage)/Testimonials.jsxapp/(pages)/(landingPage)/WhyDeenBridge.jsxapp/page.jsxcomponents/molecules/landingpage/PublicCourseCard.jsxlib/actions/courses/fetch-courses.jsstyles/globals.css
| export default async function FeaturedCourses() { | ||
| let courses = []; | ||
| try { | ||
| courses = await fetchCourses(); | ||
| } catch (error) { | ||
| // eslint-disable-next-line no-console | ||
| console.error("[FeaturedCourses] fetch failed:", error?.message || error); | ||
| courses = []; | ||
| } | ||
|
|
||
| const ranked = rankCourses(courses).slice(0, 8); | ||
|
|
||
| if (ranked.length === 0) { | ||
| // Graceful degradation — silently skip the section. | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "FeaturedCourses" -C5 'app/page.jsx'
fd -HI 'loading.jsx' 'app/(pages)/(landingPage)'Repository: Deen-Bridge/dnb-frontend
Length of output: 940
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## loading files\n'
git ls-files 'app/**/loading.*' 'app/**/error.*' | sort
printf '\n## page file\n'
cat -n app/page.jsx
printf '\n## featured courses file (relevant slice)\n'
cat -n 'app/(pages)/(landingPage)/FeaturedCourses.jsx' | sed -n '1,140p'Repository: Deen-Bridge/dnb-frontend
Length of output: 5909
Add a loading boundary for FeaturedCourses
app/page.jsx renders <FeaturedCourses /> directly, and there’s no app/loading.jsx in this route tree. Because this server component awaits fetchCourses() before it can render, the landing page can’t stream a fallback while that request is in flight. Wrap it in <Suspense> or add a route-level loading.jsx so the section doesn’t block the page shell.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/`(pages)/(landingPage)/FeaturedCourses.jsx around lines 39 - 54, Update
the landing-page rendering of the FeaturedCourses component to provide a loading
boundary, using React Suspense or a route-level loading component with an
appropriate fallback. Keep the existing FeaturedCourses fetch, ranking, and
empty-state behavior unchanged while allowing the page shell to stream before
fetchCourses completes.
Source: Path instructions
| const { | ||
| _id, | ||
| title = "Untitled course", | ||
| description = "", | ||
| thumbnail, | ||
| price, | ||
| category, | ||
| createdBy, | ||
| reviews = [], | ||
| } = course; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
"Explore Course" link breaks for courses without _id.
fetch-courses.js documents id as an alternate identifier for legacy records, and FeaturedCourses.jsx already guards for this (course._id || course.id for the list key). This component's destructuring only pulls _id, so the Link href on Line 80 resolves to /dashboard/courses/undefined for any course that only has id.
🔗 Proposed fix
const {
_id,
+ id,
title = "Untitled course",
description = "",
thumbnail,
price,
category,
createdBy,
reviews = [],
} = course;
+ const courseId = _id ?? id;
...
- href={`/dashboard/courses/${_id}`}
+ href={`/dashboard/courses/${courseId}`}Also applies to: 79-85
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/`(pages)/(landingPage)/LandingCourseCard.jsx around lines 14 - 23, Update
LandingCourseCard’s course identifier handling to support legacy records by
using course._id with course.id as the fallback, and use that resolved
identifier for the Explore Course Link href. Preserve the existing behavior for
records that provide _id.
| const { | ||
| _id, | ||
| title = "Untitled course", | ||
| description = "", | ||
| thumbnail, | ||
| price, | ||
| category, | ||
| createdBy, | ||
| reviews = [], | ||
| } = course; | ||
|
|
||
| const rating = getAverageRating(reviews); | ||
| const reviewCount = Array.isArray(reviews) ? reviews.length : 0; | ||
| const priceLabel = | ||
| price === 0 || price === undefined || price === null ? "Free" : `$${price}`; | ||
| const instructorName = createdBy?.name || "DeenBridge Tutor"; | ||
|
|
||
| return ( | ||
| <article className="group relative flex h-full flex-col overflow-hidden rounded-2xl border border-green-200/40 bg-white/90 shadow-md backdrop-blur-xl transition-all hover:-translate-y-1 hover:shadow-xl dark:border-white/10 dark:bg-white/5"> | ||
| <div className="relative h-48 w-full overflow-hidden"> | ||
| <Image | ||
| src={thumbnail || "/images/dnb.png"} | ||
| alt={title} | ||
| fill | ||
| sizes="(max-width: 768px) 100vw, (max-width: 1200px) 50vw, 33vw" | ||
| className="object-cover transition-transform duration-500 group-hover:scale-105" | ||
| loading="lazy" | ||
| /> | ||
| <div className="absolute inset-0 bg-gradient-to-t from-black/70 via-black/10 to-transparent" /> | ||
| {category && ( | ||
| <span className="absolute left-3 top-3 z-10 rounded-full bg-white/85 px-3 py-1 text-xs font-semibold uppercase tracking-wider text-accent shadow"> | ||
| {category} | ||
| </span> | ||
| )} | ||
| </div> | ||
|
|
||
| <div className="flex flex-1 flex-col gap-3 p-5"> | ||
| <h3 className="line-clamp-1 text-lg font-bold text-accent">{title}</h3> | ||
| <p className="line-clamp-2 min-h-[2.5rem] text-sm text-muted-foreground"> | ||
| {description} | ||
| </p> | ||
|
|
||
| <div className="mt-auto flex items-center justify-between gap-3 text-sm"> | ||
| <div className="flex items-center gap-2 text-muted-foreground"> | ||
| <GraduationCap className="size-4 text-accent" aria-hidden="true" /> | ||
| <span className="line-clamp-1 font-medium text-foreground/80"> | ||
| {instructorName} | ||
| </span> | ||
| </div> | ||
| <div className="flex items-center gap-2"> | ||
| {reviewCount > 0 && ( | ||
| <span | ||
| className="flex items-center gap-1 rounded-full bg-yellow-100 px-2 py-0.5 text-xs font-semibold text-yellow-800 dark:bg-yellow-500/15 dark:text-yellow-300" | ||
| aria-label={`Rated ${rating.toFixed(1)} out of 5 from ${reviewCount} reviews`} | ||
| > | ||
| <Star className="size-3 fill-yellow-500 text-yellow-500" aria-hidden="true" /> | ||
| {rating.toFixed(1)} | ||
| </span> | ||
| )} | ||
| <span className="rounded-full bg-gradient-to-r from-highlight to-accent px-3 py-1 text-xs font-bold text-white shadow"> | ||
| {priceLabel} | ||
| </span> | ||
| </div> | ||
| </div> | ||
|
|
||
| <Link | ||
| href={`/dashboard/courses/${_id}`} | ||
| className="mt-1 inline-flex w-full items-center justify-center rounded-full bg-accent px-4 py-2 text-sm font-semibold text-white shadow transition-colors hover:bg-highlight focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-accent focus-visible:ring-offset-2" | ||
| > | ||
| Explore Course | ||
| <span className="sr-only">: {title}</span> | ||
| </Link> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Both landing course cards assume _id is always present, but fetch-courses.js documents id as a required legacy alternate. FeaturedCourses.jsx already defends against this (course._id || course.id for its list key), but neither card's own detail link does, so courses lacking _id get a dead /dashboard/courses/undefined link.
app/(pages)/(landingPage)/LandingCourseCard.jsx#L14-L85: destructureidalongside_idand build theLink hreffrom_id ?? id.components/molecules/landingpage/PublicCourseCard.jsx#L12-L25: compute acourseId = course._id ?? course.idand use it for theLink hrefon Line 25.
📍 Affects 2 files
app/(pages)/(landingPage)/LandingCourseCard.jsx#L14-L85(this comment)components/molecules/landingpage/PublicCourseCard.jsx#L12-L25
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/`(pages)/(landingPage)/LandingCourseCard.jsx around lines 14 - 85,
Support both course identifier fields in each landing card: in
app/(pages)/(landingPage)/LandingCourseCard.jsx lines 14-85, destructure id and
build the detail Link from _id ?? id; in
components/molecules/landingpage/PublicCourseCard.jsx lines 12-25, compute
courseId from course._id ?? course.id and use it in the Link href.
| function getInitials(name = "") { | ||
| return name | ||
| .split(/\s+/) | ||
| .filter((w) => /^[A-Za-z]/.test(w)) | ||
| .slice(0, 2) | ||
| .map((w) => w[0].toUpperCase()) | ||
| .join(""); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does JavaScript RegExp support the \p{L} Unicode property escape with the u flag?
💡 Result:
Yes, JavaScript regular expressions fully support the \p{L} Unicode property escape when the u (Unicode) flag is used [1][2]. The \p{L} escape is a shorthand for \p{Letter}, which matches any character with the Unicode General Category property of "Letter" [2]. Because \p{...} escapes are only available in Unicode-aware mode, attempting to use them without the u flag (or the v flag) will result in a syntax error or, in older/non-Unicode-aware contexts, lead the engine to treat the sequence as an escaped literal 'p' [1][3][4]. In summary, the u flag is required to enable Unicode property escapes like \p{L} [2][5]. When the u flag is set, the engine correctly interprets \p{L} as a request to match any Unicode letter [2].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Regular_expressions/Unicode_character_class_escape
- 2: https://javascript.info/regexp-unicode
- 3: https://github.com/tc39/proposal-regexp-unicode-property-escapes
- 4: https://exploringjs.com/js/book/ch_regexps.html
- 5: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_expressions
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file with line numbers and a bit of surrounding context.
sed -n '1,140p' 'app/(pages)/(landingPage)/Testimonials.jsx' | nl -ba
# Quick check for other initials helpers or related patterns in the repo.
rg -n 'getInitials|/\\^\\[A-Za-z\\]/|\\p\{L\}' 'app' || trueRepository: Deen-Bridge/dnb-frontend
Length of output: 202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the target file with line numbers using tools available in the sandbox.
sed -n '1,180p' 'app/(pages)/(landingPage)/Testimonials.jsx' | cat -n
# Search for the initials helper and nearby avatar logic elsewhere in the repo.
rg -n 'function getInitials|review\.user\?\.avatar|next/image|remotePatterns|dashboard/courses' 'app' || trueRepository: Deen-Bridge/dnb-frontend
Length of output: 9298
Make getInitials Unicode-aware
The /^[A-Za-z]/ filter drops names written in other scripts, so users with Arabic, Urdu, Cyrillic, and similar names always fall back to the generic U avatar. Switching to a Unicode letter class keeps initials working for everyone.
🌍 Proposed i18n-friendly fix
function getInitials(name = "") {
return name
.split(/\s+/)
- .filter((w) => /^[A-Za-z]/.test(w))
+ .filter((w) => /^\p{L}/u.test(w))
.slice(0, 2)
.map((w) => w[0].toUpperCase())
.join("");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function getInitials(name = "") { | |
| return name | |
| .split(/\s+/) | |
| .filter((w) => /^[A-Za-z]/.test(w)) | |
| .slice(0, 2) | |
| .map((w) => w[0].toUpperCase()) | |
| .join(""); | |
| } | |
| function getInitials(name = "") { | |
| return name | |
| .split(/\s+/) | |
| .filter((w) => /^\p{L}/u.test(w)) | |
| .slice(0, 2) | |
| .map((w) => w[0].toUpperCase()) | |
| .join(""); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/`(pages)/(landingPage)/Testimonials.jsx around lines 9 - 16, Update
getInitials to use a Unicode-aware letter test instead of the ASCII-only
/^[A-Za-z]/ filter, enabling initials for Arabic, Urdu, Cyrillic, and other
scripts while preserving the existing splitting, two-word limit, capitalization,
and joining behavior.
| {t.avatar ? ( | ||
| <Image | ||
| src={t.avatar} | ||
| alt={`${t.name}'s avatar`} | ||
| width={44} | ||
| height={44} | ||
| loading="lazy" | ||
| className="size-11 rounded-full object-cover ring-2 ring-white/30" | ||
| /> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the Next.js config and inspect image host allow-listing.
fd -H -t f -g 'next.config.*'
fd -H -t f -g 'next.config.*' --exec sh -c 'echo "=== $1 ==="; cat "$1"' _ {}
# Surface how remotePatterns / domains are set (if at all).
rg -nP -C3 'remotePatterns|images\s*:|domains\s*:|unoptimized' -g 'next.config.*'Repository: Deen-Bridge/dnb-frontend
Length of output: 836
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the testimonials component and trace the avatar source.
sed -n '1,240p' app/(pages)/(landingPage)/Testimonials.jsx
printf '\n=== avatar references ===\n'
rg -n "review\.user\?\.avatar|avatar" app -g '!**/*.map'
printf '\n=== next.config.mjs ===\n'
cat next.config.mjsRepository: Deen-Bridge/dnb-frontend
Length of output: 215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the testimonials component and trace the avatar source.
sed -n '1,240p' 'app/(pages)/(landingPage)/Testimonials.jsx'
printf '\n=== avatar references ===\n'
rg -n "review\.user\?\.avatar|avatar" app -g '!**/*.map'
printf '\n=== next.config.mjs ===\n'
cat next.config.mjsRepository: Deen-Bridge/dnb-frontend
Length of output: 9497
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '=== avatar / upload / profile sources ===\n'
rg -n -C3 "avatar\s*[:=]|profile\.avatar|user\?.avatar|upload.*avatar|setAvatar|AvatarImage src|avatarUrl|avatarURL" app lib components -g '!**/*.map'
printf '\n=== files that mention user avatar handling ===\n'
fd -H -t f 'avatar' app lib componentsRepository: Deen-Bridge/dnb-frontend
Length of output: 21965
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '=== fetchCourses source ===\n'
fd -H -t f 'fetch-courses' lib app components
fd -H -t f 'fetch-courses' lib app components --exec sh -c 'echo "=== $1 ==="; sed -n "1,220p" "$1"' _ {}
printf '\n=== avatar upload/storage clues ===\n'
rg -n -C3 "cloudinary|upload.*avatar|avatar.*cloudinary|images\.cloudinary|bucket|s3|presigned|remotePatterns|domains" app lib components -g '!**/*.map'Repository: Deen-Bridge/dnb-frontend
Length of output: 4057
Guard testimonial avatars against unlisted remote hosts
review.user?.avatar is passed straight to next/image, but next.config.mjs only allow-lists a fixed set of domains. Any review avatar from a different host will fail this section’s render, so normalize avatar URLs onto one of the trusted hosts or fall back to the initials placeholder when the URL isn’t allow-listed.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 128-135: A list component should have a key to prevent re-rendering
Context: <Image
src={t.avatar}
alt={${t.name}'s avatar}
width={44}
height={44}
loading="lazy"
className="size-11 rounded-full object-cover ring-2 ring-white/30"
/>
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/`(pages)/(landingPage)/Testimonials.jsx around lines 128 - 136, Update
the testimonial avatar rendering around the t.avatar Image in the testimonials
component to validate the avatar URL against the hosts allowed by
next.config.mjs before passing it to next/image. Use the avatar only for trusted
hosts, and otherwise render the existing initials placeholder without allowing
an unlisted remote URL to reach Image.
Source: Linters/SAST tools
| <Link | ||
| href={`/dashboard/courses/${t.course._id}`} | ||
| className="underline-offset-2 hover:underline" | ||
| > | ||
| {t.course.title} | ||
| </Link> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether /dashboard is behind auth (middleware / layout guards).
fd -H -t f -g 'middleware.*'
fd -H -t f -g 'middleware.*' --exec sh -c 'echo "=== $1 ==="; cat "$1"' _ {}
# Look for a public course route the card/testimonials could share.
fd -H -t d courses app
rg -nP -C2 "href=\{?[\"'\`]/(dashboard/)?courses/" app -g '*.jsx'Repository: Deen-Bridge/dnb-frontend
Length of output: 1265
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== app tree (relevant files) =="
fd -H -t f 'page\.(jsx|js|tsx|ts)$|layout\.(jsx|js|tsx|ts)$|middleware\.(js|ts)$' app | sed 's#^\./##' | sort
echo
echo "== dashboard routes =="
fd -H -t f . app/dashboard | sed 's#^\./##' | sort
echo
echo "== inspect landing course cards and testimonials =="
for f in app/'(pages)'/'(landingPage)'/LandingCourseCard.jsx app/'(pages)'/'(landingPage)'/Testimonials.jsx; do
echo "=== $f ==="
wc -l "$f"
sed -n '1,220p' "$f"
echo
done
echo "== search for public course links and public course routes =="
rg -n "PublicCourse|/courses/|/dashboard/courses/" app -g '*.jsx' -g '*.js' -g '*.tsx' -g '*.ts'Repository: Deen-Bridge/dnb-frontend
Length of output: 13450
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== dashboard layout guard =="
sed -n '1,220p' app/dashboard/layout.jsx
echo
echo "== app root layout and middleware =="
for f in app/layout.js middleware.js middleware.ts; do
if [ -f "$f" ]; then
echo "=== $f ==="
sed -n '1,220p' "$f"
echo
fi
done
echo "== course detail route page =="
sed -n '1,220p' app/dashboard/courses/[courseId]/page.jsx
echo
echo "== search for redirects/auth gating around dashboard =="
rg -n "redirect\\(|unauthorized|auth|session|cookies\\(|getServerSession|protect|middleware" app middleware.* -g '*.js' -g '*.jsx' -g '*.ts' -g '*.tsx'Repository: Deen-Bridge/dnb-frontend
Length of output: 9017
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ProtectedRoute implementation =="
fd -H -t f 'protected-route.*' . | sed 's#^\./##' | sort
for f in $(fd -H -t f 'protected-route.*' . | sed 's#^\./##' | sort); do
echo "=== $f ==="
wc -l "$f"
sed -n '1,220p' "$f"
echo
done
echo "== dashboard course page links in landing page =="
rg -n "href=\{`/dashboard/courses/\$\{.*\}`\}" app/'(pages)'/'(landingPage)' -g '*.jsx'Repository: Deen-Bridge/dnb-frontend
Length of output: 1258
Use the public course URL here. This CTA sits on the public landing page, but /dashboard/courses/${t.course._id} is behind ProtectedRoute, so logged-out visitors get sent to /login instead of the course. Point it to the public course route used elsewhere in the landing stack.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 148-157: A list component should have a key to prevent re-rendering
Context:
Student
{" — "}
<Link
href={/dashboard/courses/${t.course._id}}
className="underline-offset-2 hover:underline"
>
{t.course.title}
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/`(pages)/(landingPage)/Testimonials.jsx around lines 152 - 157, Update
the Link in the testimonials course CTA to use the public course route instead
of the protected `/dashboard/courses/${t.course._id}` path. Reuse the
established public course URL pattern from the landing stack while preserving
the existing course identifier and link styling.
| * Return shape | ||
| * ──────────── | ||
| * Always returns an `Array<Course>`. The underlying `GET /api/courses` | ||
| * response is normalised: | ||
| * 1. `{ courses: Course[] }` – the canonical wrapped envelope | ||
| * 2. `Course[]` – the unwrapped array | ||
| * 3. anything else – returned as `[]` | ||
| * | ||
| * On any error this also returns `[]`, so server-component consumers can | ||
| * safely fall back to "render nothing" rather than crashing. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Documented "anything else → []" fallback isn't implemented.
The JSDoc promises three normalized outcomes, including { courses: Course[] } – the canonical wrapped envelope, Course[] – the unwrapped array, and anything else – returned as []``. The implementation only returns [] from the `catch` block; on the success path, if `response.data` lacks a `.courses` key, it returns `response.data` as-is — which could be `undefined`, a plain object, or anything else non-array. Consumers that trust the documented "always an array" contract (e.g. `.map`/`.filter` in `FeaturedCourses.jsx`) could crash on this untyped path.
🐛 Proposed fix to align implementation with the documented contract
try {
const response = await axiosInstance.get("/api/courses");
console.log("API Response:", response);
console.log("API Response Data:", response.data);
- if (response.data && response.data.courses) {
- return response.data.courses;
- }
- return response.data;
+ if (response.data && Array.isArray(response.data.courses)) {
+ return response.data.courses;
+ }
+ if (Array.isArray(response.data)) {
+ return response.data;
+ }
+ return [];
} catch (error) {Also applies to: 44-57
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/actions/courses/fetch-courses.js` around lines 13 - 22, Update the
success-path normalization in the course-fetching function so it returns the
wrapped courses array only when valid, the response data when it is already an
array, and [] for all other response shapes. Preserve the existing catch
behavior and ensure the function’s documented always-array contract holds before
returning to consumers.
|
@Whiznificent this PR has merge conflicts with the |
2 participants
Summary
Follow-up cleanup to PR #121 (
feat(landing): real social proof, featured-courses carousel, and reduced-motion support). Two scoped changes; both reduce noise and harden the landing-page data plan without altering visual or runtime behavior.What changes
lib/testimonials.jsdeleted. This file was added by PR feat(landing): real social proof, featured-courses carousel, and reduced-motion support #121 as an empty placeholder array (export default testimonials = []). I verified viagit grep 'lib/testimonials'across the working tree and the full git history that zero files import it. It is dead code, so removing it now keeps it from being shipped todev.JSDoc added to
lib/actions/courses/fetch-courses.js. The new landing-page server components (FeaturedCourses.jsx,Testimonials.jsx— both shipped in PR feat(landing): real social proof, featured-courses carousel, and reduced-motion support #121) callfetchCourses()during ISR at build time. The JSDoc now documents:GET /api/courses(wrapped{ courses: [] }, bare[], fallback[])_id,id,title)reviews[],enrolledUsers[])FeaturedCourses.jsxandTestimonials.jsxNo behavior change to
fetchCourses()itself — only documentation added.Why
During the code review of #121, the reviewer flagged
lib/testimonials.jsas dead code and called out that the runtime shape offetchCourses()had no documented contract. Landing both as a single follow-up PR means the landing-page real-review work ondevarrives with its data-shape contract already pinned down.Verification
npm run lint: exit 0 — only the pre-existing two warnings about anonymous default exports (unrelated to this PR).npm run build: exit 0 — all routes generated, including the new FeaturedCourses and Testimonials server components.origin/feat/landing-social-proofso the build also exercises the Serwist PWA wrapper that feat(landing): real social proof, featured-courses carousel, and reduced-motion support #121 introduces.Risk
Very low. The deletion targets a file with no importers; the JSDoc is comment-only and has zero behavioral impact.
Related
devonce feat(landing): real social proof, featured-courses carousel, and reduced-motion support #121 lands.Summary by CodeRabbit
New Features
Accessibility & Experience