Skip to content

feat: show related testimonials on projects page#3

Merged
jhb-dev merged 4 commits into
mainfrom
claude/issue-2-20250802-2324
Dec 20, 2025
Merged

feat: show related testimonials on projects page#3
jhb-dev merged 4 commits into
mainfrom
claude/issue-2-20250802-2324

Conversation

@jhb-dev
Copy link
Copy Markdown
Contributor

@jhb-dev jhb-dev commented Aug 2, 2025

Implements feature to display related testimonials on project pages.

Changes

  • Added testimonials section to ProjectLayout that displays testimonials related to the current project
  • Utilized existing join field from Projects collection that connects to testimonials
  • Added testimonials label group to CMS for translation support
  • Only displays testimonials section when testimonials exist for the project
  • Uses same column layout as existing TestimonialsBlock for consistency

Closes #2

Generated with Claude Code


Note

Show related testimonials on project pages, fetching them with depth=2 and adding CMS labels for the testimonials section.

  • Frontend:
    • Project page: Render TestimonialsBlock in web/src/layout/collections/ProjectLayout.astro when project.testimonials exist.
    • TestimonialsBlock: Query testimonials by IDs with depth: 2 via payloadSDK.find; add responsive row layout when ≤2 items; keep masonry layout and “show more” control for larger sets.
    • TestimonialCard: Use populated project.customer for website/name; remove unused types and add guard if customer is missing.
  • CMS/Types:
    • Labels: Add testimonials group with testimonials-title in cms/src/globals/Labels.ts.
    • Types: Update cms/src/payload-types.ts to include labels.testimonials and corresponding LabelsSelect entries.

Written by Cursor Bugbot for commit 801ad44. This will update automatically on new commits. Configure here.

- Add testimonials section to ProjectLayout that displays testimonials related to the current project
- Utilize existing join field from Projects collection that connects to testimonials
- Add testimonials label group to CMS for translation support
- Only display testimonials section when testimonials exist for the project
- Use same column layout as existing TestimonialsBlock for consistency

Co-authored-by: Jens Becker <jhb-dev@users.noreply.github.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Aug 2, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
website Ready Ready Preview, Comment Dec 20, 2025 4:49pm
1 Skipped Deployment
Project Deployment Review Updated (UTC)
website-cms Ignored Ignored Preview Dec 20, 2025 4:49pm

@claude
Copy link
Copy Markdown

claude Bot commented Aug 2, 2025

Claude finished @jhb-dev's task —— View job


PR Review: Related Testimonials Feature

Reviewing the implementation of the related testimonials feature on project pages.

Review Tasks:

  • Analyze code changes and implementation approach
  • Check code quality and best practices adherence
  • Look for potential bugs or issues
  • Assess performance considerations
  • Review security concerns
  • Evaluate test coverage needs
  • Provide constructive feedback and suggestions

✅ Overall Assessment

This is a solid implementation that follows repository conventions well. The feature correctly leverages the existing join field and maintains consistency with the existing TestimonialsBlock layout.

🎯 Code Quality & Best Practices

Strengths:

  • Follows repository patterns: Uses existing TestimonialCard component and matches the column layout from TestimonialsBlock
  • Type safety: Proper TypeScript filtering with type guards (testimonial is Testimonial)
  • Conditional rendering: Only shows section when testimonials exist
  • Internationalization: Adds testimonials label group to CMS with fallback text
  • Consistent styling: Uses same Tailwind classes as existing testimonials block
  • Accessibility: Proper semantic HTML with <section> and <h2> heading

Code Quality Score: 9/10

🐛 Potential Issues & Improvements

Critical Issues: None

Minor Improvements:

  1. Customer Population Issue (ProjectLayout.astro:25-27)

    // Current: Missing customer population
    const testimonials = pageData.testimonials?.docs?.filter((testimonial): testimonial is Testimonial => 
      typeof testimonial === 'object'
    ) || []

    Issue: The TestimonialCard component expects project.customer to be populated (see lines 24, 28 in TestimonialCard.astro), but testimonials from the join field likely have unpopulated customer references.

    Recommendation: Follow the pattern from TestimonialsBlock.astro:15-43 to populate customers:

    // Fetch customers and populate them manually
    const customers = await payloadSDK.find({
      collection: 'customers',
      locale: locale,
      draft: false,
      limit: 0,
      pagination: false,
    }, !preview)
    
    // Then populate testimonials with customer data
  2. Label Fallback (ProjectLayout.astro:58)

    {labels.testimonials?.['testimonials-title'] || 'Testimonials'}

    Minor: The fallback text should be localized. Consider using a more descriptive key name like 'related-testimonials' to distinguish from the general testimonials block.

⚡ Performance Considerations

  • No performance issues identified
  • Efficient filtering with early return
  • Conditional rendering prevents unnecessary DOM nodes

Note: If customer population is added (recommendation #1), consider caching the customer lookup if multiple testimonials reference the same customer.

🔒 Security Concerns

  • No security issues identified
  • Proper type checking prevents runtime errors
  • No direct user input handling

🧪 Test Coverage

Current State: No tests visible for this feature.

Recommendations:

  • Unit tests for testimonial filtering logic
  • Integration tests for customer population (if implemented)
  • Visual regression tests for the testimonials section layout
  • E2E tests to verify testimonials display correctly on project pages

📝 Summary

This is a well-implemented feature that follows established patterns. The main concern is the potential customer population issue that could cause runtime errors when TestimonialCard tries to access unpopulated customer data.

Priority Actions:

  1. High Priority: Implement customer population to prevent potential runtime errors
  2. Low Priority: Improve label localization

Files to verify after customer population fix:

  • ProjectLayout.astro:25-27 - Add customer population logic
  • Test on a project page with testimonials to ensure customer data displays correctly

@claude
Copy link
Copy Markdown

claude Bot commented Dec 20, 2025

PR Review: Related Testimonials on Project Pages

✅ Overall Assessment

This is a well-implemented feature that follows the repository's conventions. The code is clean, type-safe, and properly handles edge cases.


🎯 Strengths

  1. Excellent data handling: The refactor of TestimonialsBlock.astro to fetch testimonials with depth 2 resolves population issues elegantly
  2. Type safety: Proper error handling in TestimonialCard.astro when customer is not populated
  3. Consistent styling: Reuses existing layout patterns from TestimonialsBlock for visual consistency
  4. Responsive design: Adapts column layout based on number of testimonials (single testimonial gets centered, multiple get columns)
  5. Translation support: Properly added labels to CMS following the repository's multi-language architecture

🐛 Potential Issues

High Priority

1. Missing label translations in CMS

  • The new testimonials-title label was added to the schema but appears unused in the frontend
  • Location: web/src/layout/collections/ProjectLayout.astro
  • Issue: The testimonials section has no heading
  • Recommendation: Either add a heading using labels.testimonials['testimonials-title'] or remove the unused label from the CMS (per CLAUDE.md: "If some are unused after the frontend implementation, delete them")

Medium Priority

2. Inconsistent show-more button behavior

  • Location: TestimonialsBlock.astro:63-72
  • Issue: The show-more button logic checks populatedTestimonials.docs?.length > 1 but the masking/overflow styles use the same condition. This means with exactly 2 testimonials, you might get a show-more button even when content doesn't overflow.
  • Recommendation: Consider checking actual content height or adjust the threshold (e.g., > 2 or > 3)

3. Performance consideration

  • Location: TestimonialsBlock.astro:12-33
  • Issue: The component now makes an additional fetch to populate testimonials even when they might already be available
  • Impact: Minor performance overhead, but this is during build time for a static site, so acceptable
  • Note: The comment mentions this will be resolved when migrating to GraphQL/REST select API

🔒 Security

No security concerns identified. The implementation:

  • ✅ Properly validates data before rendering
  • ✅ Uses type-safe operations
  • ✅ No direct user input or XSS vulnerabilities

⚡ Performance

  • Build time: Slight increase due to additional fetch, but negligible for static generation
  • Runtime: No performance impact - all static HTML
  • Bundle size: No new dependencies added

📋 Recommendations

  1. Add section heading (or remove unused label):
{
  testimonials.length > 0 && (
    <section class="mt-16">
      <h2 class="text-3xl font-bold mb-8">{labels.testimonials['testimonials-title']}</h2>
      <TestimonialsBlock blockType="testimonials" testimonials={testimonials} />
    </section>
  )
}
  1. Consider adjusting show-more threshold if needed for better UX with 2-3 testimonials

✅ Pre-merge Checklist

Before merging, please verify:

  • Run pnpm lint in both cms and web folders
  • Run pnpm astro check in the web folder
  • Decide on the testimonials-title label (use it or remove it)
  • Test with 1, 2, 3, and 4+ testimonials to verify layout and show-more behavior

Verdict: Approve with minor changes suggested. The core implementation is solid and ready for merge once the unused label is addressed.

@jhb-dev jhb-dev marked this pull request as ready for review December 20, 2025 16:49
@jhb-dev jhb-dev merged commit a94f20a into main Dec 20, 2025
4 of 5 checks passed
@claude
Copy link
Copy Markdown

claude Bot commented Dec 20, 2025

PR Review: Related Testimonials on Projects Page

✅ Overall Assessment

Good implementation that successfully displays project-related testimonials. The code is clean and follows the repository's patterns. However, there are a few issues that should be addressed.


🐛 Critical Issues

1. Unused Label Added to CMS

Location: cms/src/globals/Labels.ts:73

A new label testimonials-title was added but is never used in the frontend code. According to CLAUDE.md rules:

When adding labels to the CMS, please ensure you use all of them. If some are unused after the frontend implementation, delete them.

Fix: Either:

  • Use the label to add a section title in ProjectLayout.astro (recommended), or
  • Remove the label from Labels.ts

Suggested implementation:

{
  testimonials.length > 0 && (
    <section class="mt-16">
      <h2 class="text-3xl font-bold mb-8">{labels.testimonials['testimonials-title']}</h2>
      <TestimonialsBlock blockType="testimonials" testimonials={testimonials} />
    </section>
  )
}

2. Missing Error Handling for Testimonial Fetch

Location: web/src/components/blocks/TestimonialsBlock.astro:13-33

The component fetches testimonials from the CMS but doesn't handle potential errors or empty results gracefully.

Risk: If the CMS request fails or returns no documents, populatedTestimonials.docs could be undefined, causing runtime errors.

Fix: Add validation:

const populatedTestimonials = await payloadSDK.find(/* ... */)

if (!populatedTestimonials.docs || populatedTestimonials.docs.length === 0) {
  return null // or throw an error
}

⚠️ Performance & Design Concerns

3. N+1 Query Problem

Location: web/src/components/blocks/TestimonialsBlock.astro:13-33

The component makes an additional CMS fetch for testimonials that are already partially available from the parent component. This creates unnecessary network overhead.

Issue: The ProjectLayout passes testimonials to TestimonialsBlock, which then re-fetches them with depth: 2. While this ensures proper population, it's inefficient.

Better approach:

  • Fetch the project with depth: 2 in getPageData() to populate testimonials upfront, or
  • Accept the populated testimonials directly from ProjectLayout without re-fetching

4. Breaking Change to TestimonialsBlock

Location: web/src/components/blocks/TestimonialsBlock.astro

The refactored TestimonialsBlock now requires testimonials to be pre-filtered (line 58 in ProjectLayout filters to objects only). The original block handled both string IDs and objects.

Risk: If this component is used elsewhere with string IDs, it will break.

Recommendation: Either:

  • Keep the component flexible to handle both types, or
  • Document this breaking change clearly

🔒 Security

✅ No security concerns identified. The code properly validates data types and uses safe CMS queries.


🎨 Code Quality

Positive aspects:

  • Clean separation of concerns
  • Proper TypeScript typing with type guards
  • Consistent with existing component patterns
  • Good conditional rendering

Minor improvements:

  • The useRowLayout logic is clever but might benefit from a comment explaining the UX reasoning
  • Consider extracting the layout threshold (2) to a constant for maintainability

📋 Task Completion Checklist

Before merging, ensure:

  • Run pnpm lint in both cms and web folders
  • Run pnpm astro check in the web folder
  • If CMS schema was modified (Labels.ts counts), run cd cms && pnpm generate:types (appears to be done ✅)
  • Actually use the testimonials-title label or remove it
  • Test with projects that have 0, 1, 2, and 3+ testimonials

🎯 Recommended Actions

Must fix before merge:

  1. Use or remove the unused testimonials-title label

Should consider:
2. Add error handling for the testimonials fetch
3. Optimize by avoiding the redundant CMS query
4. Document the breaking change to TestimonialsBlock

Overall: Good work! Just needs the label issue resolved and ideally the performance optimization.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

This PR is being reviewed by Cursor Bugbot

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

if (typeof testimonial === 'string') {
return testimonial
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Testimonial display order not preserved after refetch

The refactored code fetches testimonials using payloadSDK.find with a where.id.in filter but without a sort parameter. The database will return results in its default order (typically creation date), not the order specified in the original testimonials prop. The previous implementation used .map() on the original array to populate customer data, which preserved the intended order. This could cause testimonials to appear in an unexpected order compared to how they were arranged in the CMS.

Fix in Cursor Fix in Web

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: show related testimonial on projects page

1 participant