Add Dynamic Certificate Templates #3073
Conversation
Create 2 new certificate templates Modify pdf viewer to only show pagination when there are more than 1 page Refactor Certificate Viewer to view the react-pdf component instead of using a seperate file Add certTemplate column and its logic
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
WalkthroughThis update introduces support for multiple certificate templates in the platform. It adds a new database enum and schema field for selecting certificate designs, implements three distinct PDF certificate templates, updates the certificate generation and display logic to use the selected template, and enhances the UI with template selection and live preview features. Localization files and types are expanded accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CourseForm
participant DB
participant CertificatePage
participant PDFTemplates
participant CertificateViewer
User->>CourseForm: Open course form (with enableCerts)
CourseForm->>DB: Fetch certificate template options
User->>CourseForm: Select certificate template
CourseForm->>DB: Save course with cert_template
User->>CertificatePage: View certificate
CertificatePage->>DB: Fetch certificate details (incl. cert_template)
CertificatePage->>PDFTemplates: Render PDF with selected template
PDFTemplates-->>CertificatePage: Return PDF buffer
CertificatePage->>CertificateViewer: Display PDF (data URL)
User->>CertificateViewer: Download as PDF/PNG
Possibly related PRs
Suggested labels
Poem
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
apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsxOops! Something went wrong! :( ESLint: 9.28.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsxOops! Something went wrong! :( ESLint: 9.28.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs packages/ui/src/components/ui/custom/education/certificates/types.tsOops! Something went wrong! :( ESLint: 9.28.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs
✨ Finishing Touches
🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3073 +/- ##
==========================================
- Coverage 0.84% 0.83% -0.01%
==========================================
Files 2488 2491 +3
Lines 306773 307328 +555
Branches 2990 2993 +3
==========================================
+ Hits 2577 2581 +4
- Misses 302068 302620 +552
+ Partials 2128 2127 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Can see preview of the certificate template when creating/updating course details
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🔭 Outside diff range comments (2)
apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx (1)
12-15: 🛠️ Refactor suggestion
paramsshould not be a PromiseNext Route Handlers pass a plain object, not a Promise.
Typing it asPromise<…>forces callers toawaitan ordinary object and breaks type-safety.-{ params }: { params: Promise<{ certId: string }> } +{ params }: { params: { certId: string } }apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx (1)
13-18:⚠️ Potential issue
paramsshould not be typed as aPromise
paramsis synchronously supplied by the Next-JS App-Router. Declaring it asPromise<…>(and thenawait-ing) works at runtime only becauseawaiton a non-thenable is a no-op, but it’s misleading and breaks type-safety (TS will happily let youawaitany object).-interface PageProps { - params: Promise<{ - certificateId: string; - wsId: string; - }>; -} +interface PageProps { + params: { + certificateId: string; + wsId: string; + }; +}
♻️ Duplicate comments (1)
apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx (1)
14-22: Same font-registration pitfalls as in Modern templateSee previous comment – duplicate registration and non-numeric weights.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 14-22: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L14-L22
Added lines #L14 - L22 were not covered by tests
🧹 Nitpick comments (15)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/page.tsx (1)
14-14: Verify alias configuration and add test coverage forDownloadButtonPDFimport
The absolute import is correct given the shared UI refactor, but please confirm yourtsconfig.jsonpath aliases include@tuturuuu/ui/custom/education/certificates/download-button-pdf. Static analysis flags this line as untested—add a unit or integration test to assert that the PDF download button renders and invokes the correct download logic.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 14-14: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/page.tsx#L14
Added line #L14 was not covered by testsapps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/page.tsx (1)
60-60: Boolean prop can be expressed more idiomatically
enableCertsis a boolean that defaults tofalse; to passtrueyou can omit the explicit value and just writeenableCerts.
No functional difference, but keeps the JSX a bit cleaner.- form={<CourseForm wsId={wsId} enableCerts={true} />} + form={<CourseForm wsId={wsId} enableCerts />}🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 60-60: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/page.tsx#L60
Added line #L60 was not covered by testsapps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/columns.tsx (1)
94-96: Same nit ‒ shorthand boolean & keep config central
- JSX shorthand applies here too:
- <WorkspaceCourseRowActions row={row} enableCerts={true} /> + <WorkspaceCourseRowActions row={row} enableCerts />
- The feature flag is now duplicated in two places (
page.tsxand this file).
Consider lifting it to a single config/constant (e.g.CERTS_ENABLED) so it cannot drift.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 94-96: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/columns.tsx#L94-L96
Added lines #L94 - L96 were not covered by testsapps/web/messages/vi.json (1)
2008-2010: Translation accuracy: refine phrasing for certificate template keys.The Vietnamese strings for
"certificate_template"and"select_certificate_template"may read unnaturally as"Thiết kế bằng chứng chỉ"and"Chọn một mẫu thiết kế chứng chỉ". Consider using more idiomatic translations like"Mẫu chứng chỉ"and"Chọn mẫu chứng chỉ"(or"Chọn mẫu thiết kế chứng chỉ") to clearly convey “Certificate Template” and “Select a certificate template.”apps/db/supabase/migrations/20250610140712_new_migration.sql (1)
1-1: Ensure enum creation is idempotent
To prevent failures if this migration is ever re-run, consider addingIF NOT EXISTSto the enum creation.Example adjustment:
- create type "public"."certificate_templates" as enum ('original', 'modern', 'elegant'); + create type if not exists "public"."certificate_templates" as enum ('original', 'modern', 'elegant');packages/ui/src/components/ui/custom/education/modules/resources/pdf-viewer.tsx (1)
50-52: Using height: 100 % relies on parent layoutThe container is now
h-full; if any ancestor lacks an explicit height the viewer collapses.
Consider either:• keeping the previous intrinsic aspect-ratio approach, or
• usingmin-h-[calc(…)]/ a fixed max-height, or
• documenting the height contract in a comment.apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx (1)
55-69: Template selection is fine but misses exhaustive checkingBecause
certificate_templatesis an enum with three variants, aswitchwithout aneverexhaustiveness check will silently default for future additions.switch (certTemplate) { case 'elegant': … case 'modern': … case 'original': … /* no default */ }This causes a compile-time error when the enum grows — safer long-term.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 50-69: apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx#L50-L69
Added lines #L50 - L69 were not covered by testspackages/ui/src/components/ui/custom/education/certificates/download-button-pdf.tsx (2)
26-36: Good UX – avoids redundant network hitUsing the data-URL when available removes an extra request.
Minor: addrel="noopener"and setstyle="display:none"on the temporary anchor to avoid visible layout shifts.-const link = document.createElement('a'); +const link = document.createElement('a'); +link.style.display = 'none'; +link.rel = 'noopener';
70-70: Missing dependency warning suppressed
handleDownloadcloses overvariantandclassName, but they’re not in the dependency array.
React’s ESLint rule will warn. Include them or mark as stable.apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx (1)
75-76: Consider streaming instead of base-64 inlining for large PDFsSerialising the entire PDF to a base-64 data-URL inflates payload size (~33%) and may blow past 4 MB HTML limits on slower connections. If SEO/preview is not required, returning a
Responsewith the correctContent-Type(or a presigned URL) and letting theCertificateViewerfetch it lazily will be far more memory-efficient.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 40-76: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx#L40-L76
Added lines #L40 - L76 were not covered by testsapps/upskii/src/lib/certificate-helper.ts (1)
65-67: Replaceconsole.logwith structured logging / error reporting
console.logwill be lost in production (depending on log sinks). Use your project’s logger or at leastconsole.errorso that the message is surfaced by APM tools.- console.log(error); + console.error('[getCertificateDetails] Supabase error:', error);packages/ui/src/components/ui/custom/education/certificates/certificate-viewer.tsx (1)
195-202: Hard-coded English strings bypass localisation
'Hide'/'Show'should come fromnext-intl, otherwise the form becomes partially untranslated.- {showPreview ? 'Hide' : 'Show'} Certificate Preview + {showPreview + ? t('hide_certificate_preview') + : t('show_certificate_preview')}packages/ui/src/components/ui/custom/education/courses/course-form.tsx (2)
49-55: Usez.nativeEnumfor cleaner enum validationConstructing a tuple spread is brittle and loses exhaustiveness checks. Zod supports enums directly:
-cert_template: z - .enum([...certificateTemplateOptions] as [ - CertificateTemplate, - ...CertificateTemplate[], - ]) - .default('original'), +cert_template: z + .nativeEnum(Constants.public.Enums.certificate_templates) + .default('original'),This keeps the schema in sync even if new templates are added.
221-226: Missingpriorityorloading="lazy"on large preview imageThe preview is 400 × 300 px. Mark it as
loading="lazy"to avoid unnecessary bandwidth on first paint.- width={400} - height={300} + width={400} + height={300} + loading="lazy"packages/ui/src/components/ui/custom/education/certificates/types.ts (1)
7-10: Consider typingcompletionDateasDateinstead ofstringUsing
Date(or a branded ISO-string type) prevents accidental locale-formatted strings leaking into storage and makes date-logic safer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
apps/upskii/public/media/certificate-previews/en/elegant-preview.pngis excluded by!**/*.pngapps/upskii/public/media/certificate-previews/en/modern-preview.pngis excluded by!**/*.pngapps/upskii/public/media/certificate-previews/en/original-preview.pngis excluded by!**/*.pngapps/upskii/public/media/certificate-previews/vi/elegant-preview.pngis excluded by!**/*.pngapps/upskii/public/media/certificate-previews/vi/modern-preview.pngis excluded by!**/*.pngapps/upskii/public/media/certificate-previews/vi/original-preview.pngis excluded by!**/*.png
📒 Files selected for processing (24)
apps/db/supabase/migrations/20250610140712_new_migration.sql(1 hunks)apps/upskii/messages/en.json(1 hunks)apps/upskii/messages/vi.json(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx(3 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/certificate-page.tsx(0 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/page.tsx(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/columns.tsx(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/page.tsx(1 hunks)apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx(2 hunks)apps/upskii/src/app/api/v1/certificates/[certId]/generate/types.ts(0 hunks)apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx(1 hunks)apps/upskii/src/app/api/v1/certificates/templates/modern-certificate.tsx(1 hunks)apps/upskii/src/app/api/v1/certificates/templates/og-certificate.tsx(2 hunks)apps/upskii/src/lib/certificate-helper.ts(4 hunks)apps/web/messages/en.json(1 hunks)apps/web/messages/vi.json(1 hunks)packages/types/src/supabase.ts(20 hunks)packages/ui/package.json(1 hunks)packages/ui/src/components/ui/custom/education/certificates/certificate-viewer.tsx(1 hunks)packages/ui/src/components/ui/custom/education/certificates/download-button-pdf.tsx(2 hunks)packages/ui/src/components/ui/custom/education/certificates/types.ts(1 hunks)packages/ui/src/components/ui/custom/education/courses/course-form.tsx(3 hunks)packages/ui/src/components/ui/custom/education/courses/course-row-actions.tsx(2 hunks)packages/ui/src/components/ui/custom/education/modules/resources/pdf-viewer.tsx(3 hunks)
💤 Files with no reviewable changes (2)
- apps/upskii/src/app/api/v1/certificates/[certId]/generate/types.ts
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/certificate-page.tsx
🧰 Additional context used
🧬 Code Graph Analysis (7)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/page.tsx (1)
packages/ui/src/components/ui/custom/education/courses/course-form.tsx (1)
CourseForm(57-234)
apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx (4)
packages/types/src/supabase.ts (1)
Database(9-7840)apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx (1)
ElegantCertificateDocument(286-315)apps/upskii/src/app/api/v1/certificates/templates/modern-certificate.tsx (1)
ModernCertificateDocument(284-300)apps/upskii/src/app/api/v1/certificates/templates/og-certificate.tsx (1)
OGCertificateDocument(170-186)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/columns.tsx (1)
packages/ui/src/components/ui/custom/education/courses/course-row-actions.tsx (1)
WorkspaceCourseRowActions(26-137)
apps/upskii/src/app/api/v1/certificates/templates/og-certificate.tsx (1)
packages/ui/src/components/ui/custom/education/certificates/types.ts (1)
CertificateData(12-20)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx (7)
apps/upskii/src/lib/certificate-helper.ts (1)
getCertificateDetails(31-82)packages/ui/src/components/ui/custom/education/certificates/types.ts (1)
CertificateData(12-20)packages/types/src/supabase.ts (1)
Database(9-7840)apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx (1)
ElegantCertificateDocument(286-315)apps/upskii/src/app/api/v1/certificates/templates/modern-certificate.tsx (1)
ModernCertificateDocument(284-300)apps/upskii/src/app/api/v1/certificates/templates/og-certificate.tsx (1)
OGCertificateDocument(170-186)packages/ui/src/components/ui/custom/education/certificates/certificate-viewer.tsx (1)
CertificateViewer(25-98)
apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx (2)
apps/web/src/constants/common.ts (1)
BASE_URL(11-13)packages/ui/src/components/ui/custom/education/certificates/types.ts (1)
CertificateData(12-20)
apps/upskii/src/app/api/v1/certificates/templates/modern-certificate.tsx (2)
apps/web/src/constants/common.ts (1)
BASE_URL(11-13)packages/ui/src/components/ui/custom/education/certificates/types.ts (1)
CertificateData(12-20)
🪛 GitHub Check: codecov/patch
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/page.tsx
[warning] 14-14: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/page.tsx#L14
Added line #L14 was not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/page.tsx
[warning] 60-60: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/page.tsx#L60
Added line #L60 was not covered by tests
apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx
[warning] 2-3: apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx#L2-L3
Added lines #L2 - L3 were not covered by tests
[warning] 7-8: apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx#L7-L8
Added lines #L7 - L8 were not covered by tests
[warning] 50-69: apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx#L50-L69
Added lines #L50 - L69 were not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/columns.tsx
[warning] 94-96: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/columns.tsx#L94-L96
Added lines #L94 - L96 were not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx
[warning] 2-3: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx#L2-L3
Added lines #L2 - L3 were not covered by tests
[warning] 5-5: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx#L5
Added line #L5 was not covered by tests
[warning] 7-10: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx#L7-L10
Added lines #L7 - L10 were not covered by tests
[warning] 23-23: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx#L23
Added line #L23 was not covered by tests
[warning] 34-34: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx#L34
Added line #L34 was not covered by tests
[warning] 40-76: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx#L40-L76
Added lines #L40 - L76 were not covered by tests
[warning] 78-82: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx#L78-L82
Added lines #L78 - L82 were not covered by tests
[warning] 85-85: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx#L85
Added line #L85 was not covered by tests
apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx
[warning] 2-2: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L2
Added line #L2 was not covered by tests
[warning] 14-22: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L14-L22
Added lines #L14 - L22 were not covered by tests
[warning] 25-25: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L25
Added line #L25 was not covered by tests
[warning] 27-39: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L27-L39
Added lines #L27 - L39 were not covered by tests
[warning] 42-73: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L42-L73
Added lines #L42 - L73 were not covered by tests
[warning] 75-96: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L75-L96
Added lines #L75 - L96 were not covered by tests
[warning] 98-106: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L98-L106
Added lines #L98 - L106 were not covered by tests
[warning] 109-129: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L109-L129
Added lines #L109 - L129 were not covered by tests
[warning] 132-139: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L132-L139
Added lines #L132 - L139 were not covered by tests
[warning] 141-166: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L141-L166
Added lines #L141 - L166 were not covered by tests
[warning] 168-198: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L168-L198
Added lines #L168 - L198 were not covered by tests
[warning] 201-234: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L201-L234
Added lines #L201 - L234 were not covered by tests
[warning] 237-243: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L237-L243
Added lines #L237 - L243 were not covered by tests
[warning] 246-250: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L246-L250
Added lines #L246 - L250 were not covered by tests
[warning] 253-261: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L253-L261
Added lines #L253 - L261 were not covered by tests
[warning] 263-268: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L263-L268
Added lines #L263 - L268 were not covered by tests
🪛 Biome (1.9.4)
packages/ui/src/components/ui/custom/education/certificates/certificate-viewer.tsx
[error] 36-36: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (14)
apps/upskii/messages/en.json (2)
3542-3543: New localization keys follow conventions and are syntactically correct.The
certificate_templateandselect_certificate_templateentries are named consistently with existing keys and correctly punctuated.
3536-3552: Verify translations exist in other locales.Ensure that these new keys are also added to all other language JSON files (e.g.,
vi.json) so that non-English users see the corresponding labels.apps/upskii/src/app/api/v1/certificates/templates/og-certificate.tsx (2)
11-11: Centralize typing for certificate data
ImportingCertificateDatafrom the shared UI package ensures consistent type definitions across all certificate templates and UI components. Please confirm that yourtsconfig.jsonis configured to resolve the@tuturuuu/ui/custom/education/certificates/typespath correctly.
170-174:✅ Verification successful
Disambiguate component name to
OGCertificateDocument
Renaming the component toOGCertificateDocumentaligns it with the file name and differentiates it from other certificate variants. Verify that all imports and usages in the PDF generation route and certificate viewer have been updated accordingly.Run this script to catch any lingering references or stale local imports:
🏁 Script executed:
#!/bin/bash # Check for old component imports or relative types rg -n "CertificateDocument" -n "from '\\./types'" apps/upskii/src/app/api/v1/certificates/templatesLength of output: 598
🏁 Script executed:
#!/bin/bash # Search for any lingering imports of the old 'CertificateDocument' rg -n "import.*CertificateDocument" -g "*.tsx" apps/upskii/src # Search for any direct references to 'CertificateDocument' outside of template definitions rg -n "CertificateDocument" apps/upskii/src | grep -v "OGCertificateDocument" | grep -v "ElegantCertificateDocument" | grep -v "ModernCertificateDocument" # Verify usages of the new 'OGCertificateDocument' rg -n "OGCertificateDocument" apps/upskii/src # Ensure 'CertificateData' type is being imported correctly rg -n "CertificateData" apps/upskii/srcLength of output: 4412
All references updated to
OGCertificateDocument
- No lingering
CertificateDocumentimports or usages found in apps/upskii/src.OGCertificateDocumentis correctly imported and used in:
• apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx
• apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsxCertificateDatatype imports remain consistent across all templates.No further action required.
apps/web/messages/en.json (1)
1997-1999: New localization keys for certificate templates
The entries"certificate_template": "Certificate Design"and"select_certificate_template": "Choose a certificate template"have been added underws-courses. They align with the new certificate template selection feature. Ensure these keys are also present in all other locale files for full translation coverage.packages/ui/src/components/ui/custom/education/courses/course-row-actions.tsx (2)
23-29: Prop wiring looks good
enableCertsis optional, defaults tofalse, and is forwarded toCourseForm.
No issues spotted.
110-111: ConfirmCourseFormsupports the new propPlease verify that
CourseFormacceptsenableCerts?: booleanor the prop will be silently dropped.apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx (1)
50-52: Potential camelCase / snake_case mismatch
certData.certTemplateassumes the helper returnscamelCase.
The DB column name iscert_template; if the helper does not rename it this line will beundefined, falling through to the default template every time.const certTemplate = (certData.certTemplate ?? certData.cert_template) as Database['public']['Enums']['certificate_templates'];Double-check
getCertificateDetails.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 50-69: apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx#L50-L69
Added lines #L50 - L69 were not covered by testsapps/upskii/src/lib/certificate-helper.ts (1)
74-81:cert_templatemight benull– add a fallbackIf existing rows pre-date the migration the column can be
NULL. Returningnullwill later hit aswitchthat expects concrete strings. Either coerce to'original'or validate here.- certTemplate: certificate.workspace_courses.cert_template, + certTemplate: + certificate.workspace_courses.cert_template ?? 'original',apps/upskii/src/app/api/v1/certificates/templates/modern-certificate.tsx (1)
36-42:gapstyle may break on older @react-pdf versions
gapwas introduced only in v3.1+. If the runtime still uses ≤3.0, PDFs will fail to compile withUnknown prop. Verify the deployed version or replace withmargin*utilities.apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx (1)
132-139:gapcompatibility checkSame caveat as modern template – ensure renderer ≥3.1 or replace.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 132-139: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L132-L139
Added lines #L132 - L139 were not covered by testspackages/types/src/supabase.ts (3)
5043-5065: Newcert_templatecolumn is non-nullable – double-check all insertsThe
Rowtype rightly models the DB default and makescert_templatemandatory, whileInsert/Updatekeep it optional.
Any code that previously inserted intoworkspace_courseswithout specifying this field will now fail type-checking (and will also violate the DB constraint if you bypass the types). Make sure everysupabase.from('workspace_courses').insert(...)path either:
- Omits the property entirely (and lets Postgres default to
'original'), or- Supplies one of the enum values explicitly.
7805-7808: Enum wiring is consistent – good!
certificate_templatesis exposed inEnumsand inConstants.public.Enums, keeping generated types and helper arrays in sync. No action required.Also applies to: 7960-7961
7335-7362:✅ Verification successful
Function signatures changed – audit call-sites
count_search_users,create_ai_chat,generate_cross_app_token, etc., now accept extended parameter objects (additional optional filters / session data). Any stale call-sites will compile-fail or, worse, pass incorrect positional arguments at runtime ifanyis involved.
🏁 Script executed:
#!/bin/bash # Scan for outdated positional calls to a few updated functions. rg --line-number -e 'create_ai_chat\(' -e 'generate_cross_app_token\(' -e 'count_search_users\(' | headLength of output: 1069
No outdated RPC call-sites detected; TS unions preserve backward compatibility
I ran a search for positional calls to
create_ai_chat(...),generate_cross_app_token(...), andcount_search_users(...)in the codebase and only found their SQL definitions in migration files—no JS/TS call-sites using positional arguments. Since the TypeScript definitions now use unioned object shapes (with optional extras), existing calls passing just the required fields still satisfy one of the unions and will continue to compile and run correctly.
packages/ui/src/components/ui/custom/education/modules/resources/pdf-viewer.tsx
Outdated
Show resolved
Hide resolved
apps/upskii/src/app/api/v1/certificates/templates/modern-certificate.tsx
Show resolved
Hide resolved
apps/upskii/src/app/api/v1/certificates/templates/modern-certificate.tsx
Outdated
Show resolved
Hide resolved
apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx
Show resolved
Hide resolved
apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx
Outdated
Show resolved
Hide resolved
apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx
Outdated
Show resolved
Hide resolved
…u/platform into feat/certificate-template
Fallback component for if the image for preview is not available Add missing translations
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx (1)
14-19:⚠️ Potential issueIncorrect
paramstyping – drops type-safety and misleads readers
Next.jspasses{ params: { certId: string } }synchronously.
Typing it asPromise<...>andawait-ing it works at runtime (a non-promise is awaited as-is) but fools TypeScript and future readers.-export async function POST( - req: NextRequest, - { params }: { params: Promise<{ certId: string }> } +export async function POST( + req: NextRequest, + { params }: { params: { certId: string } } )This restores the correct signature and eliminates the unnecessary
await params.
🧹 Nitpick comments (2)
apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx (1)
55-74: Reduce repetition with a template mapThe
switchduplicatesrenderToStreamcalls. A map keeps this O(1) to extend and removes the default fall-through.-let stream; -switch (certTemplate) { - case 'elegant': - stream = await renderToStream(<ElegantCertificateDocument data={data} />); - break; - case 'modern': - stream = await renderToStream(<ModernCertificateDocument data={data} />); - break; - case 'original': - stream = await renderToStream(<OGCertificateDocument data={data} />); - break; - default: - console.log('Unhandled template:', certTemplate); - console.log('Using original template as fallback'); - stream = await renderToStream(<OGCertificateDocument data={data} />); - break; -} +const templateMap = { + elegant: ElegantCertificateDocument, + modern: ModernCertificateDocument, + original: OGCertificateDocument, +} as const; + +const Component = templateMap[certTemplate] ?? OGCertificateDocument; +const stream = await renderToStream(<Component data={data} />); <details> <summary>🧰 Tools</summary> <details> <summary>🪛 GitHub Check: codecov/patch</summary> [warning] 50-74: apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx#L50-L74 Added lines #L50 - L74 were not covered by tests </details> </details> </blockquote></details> <details> <summary>apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx (1)</summary><blockquote> `147-153`: **Potential runtime issue – mixed numeric & string padding** `paddingBottom: '4pt'` and `paddingHorizontal: '16pt'` are accepted, but keeping units consistent (numbers) avoids accidental string coercion and makes math easier later. ```diff - paddingBottom: '4pt', - paddingHorizontal: '16pt', + paddingBottom: 4, + paddingHorizontal: 16,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/upskii/messages/en.json(1 hunks)apps/upskii/messages/vi.json(1 hunks)apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx(2 hunks)apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx(1 hunks)apps/upskii/src/app/api/v1/certificates/templates/modern-certificate.tsx(1 hunks)apps/upskii/src/app/api/v1/certificates/templates/og-certificate.tsx(2 hunks)apps/upskii/src/lib/font-register-pdf.ts(1 hunks)apps/web/messages/en.json(1 hunks)apps/web/messages/vi.json(1 hunks)packages/types/src/supabase.ts(7 hunks)packages/ui/src/components/ui/custom/education/certificates/certificate-viewer.tsx(1 hunks)packages/ui/src/components/ui/custom/education/courses/course-form.tsx(3 hunks)packages/ui/src/components/ui/custom/education/modules/resources/pdf-viewer.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/upskii/src/lib/font-register-pdf.ts
- apps/web/messages/vi.json
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/ui/src/components/ui/custom/education/modules/resources/pdf-viewer.tsx
- apps/upskii/src/app/api/v1/certificates/templates/og-certificate.tsx
- packages/types/src/supabase.ts
- apps/upskii/messages/en.json
- packages/ui/src/components/ui/custom/education/courses/course-form.tsx
- packages/ui/src/components/ui/custom/education/certificates/certificate-viewer.tsx
- apps/upskii/src/app/api/v1/certificates/templates/modern-certificate.tsx
- apps/web/messages/en.json
- apps/upskii/messages/vi.json
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx (4)
packages/types/src/supabase.ts (1)
Database(9-7867)apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx (1)
ElegantCertificateDocument(283-312)apps/upskii/src/app/api/v1/certificates/templates/modern-certificate.tsx (1)
ModernCertificateDocument(275-291)apps/upskii/src/app/api/v1/certificates/templates/og-certificate.tsx (1)
OGCertificateDocument(161-177)
apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx (3)
apps/upskii/src/lib/font-register-pdf.ts (1)
registerRobotoFonts(7-21)packages/ui/src/components/ui/custom/education/certificates/types.ts (1)
CertificateData(12-20)apps/web/src/constants/common.ts (1)
BASE_URL(11-13)
🪛 GitHub Check: codecov/patch
apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx
[warning] 2-3: apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx#L2-L3
Added lines #L2 - L3 were not covered by tests
[warning] 7-8: apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx#L7-L8
Added lines #L7 - L8 were not covered by tests
[warning] 50-74: apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx#L50-L74
Added lines #L50 - L74 were not covered by tests
apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx
[warning] 2-3: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L2-L3
Added lines #L2 - L3 were not covered by tests
[warning] 13-13: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L13
Added line #L13 was not covered by tests
[warning] 16-16: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L16
Added line #L16 was not covered by tests
[warning] 18-32: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L18-L32
Added lines #L18 - L32 were not covered by tests
[warning] 35-66: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L35-L66
Added lines #L35 - L66 were not covered by tests
[warning] 68-89: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L68-L89
Added lines #L68 - L89 were not covered by tests
[warning] 91-99: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L91-L99
Added lines #L91 - L99 were not covered by tests
[warning] 102-122: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L102-L122
Added lines #L102 - L122 were not covered by tests
[warning] 125-132: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L125-L132
Added lines #L125 - L132 were not covered by tests
[warning] 134-161: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L134-L161
Added lines #L134 - L161 were not covered by tests
[warning] 163-195: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L163-L195
Added lines #L163 - L195 were not covered by tests
[warning] 198-231: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L198-L231
Added lines #L198 - L231 were not covered by tests
[warning] 234-240: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L234-L240
Added lines #L234 - L240 were not covered by tests
[warning] 243-247: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L243-L247
Added lines #L243 - L247 were not covered by tests
[warning] 250-258: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L250-L258
Added lines #L250 - L258 were not covered by tests
[warning] 260-265: apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx#L260-L265
Added lines #L260 - L265 were not covered by tests
apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx
Outdated
Show resolved
Hide resolved
apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx
Show resolved
Hide resolved
apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx
Show resolved
Hide resolved
apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/db/supabase/migrations/20250612040712_add_certificate_templates.sql (2)
1-1: Use IF NOT EXISTS for enum creation
To avoid migration failures if the enum already exists (e.g., on repeat runs), prefix withIF NOT EXISTS.Apply:
- create type "public"."certificate_templates" as enum ('original', 'modern', 'elegant'); + create type if not exists "public"."certificate_templates" as enum ('original', 'modern', 'elegant');
3-3: Verify RLS policies and consider indexing
Adding a non-nullable column under RLS may block inserts/updates if policies aren’t updated; please confirm your policies allow operations oncert_template. Additionally, if you’ll frequently filter by this field in queries, an index could improve performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/db/supabase/migrations/20250612040712_add_certificate_templates.sql(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Prettier Check (23)
- GitHub Check: Verify generated types
…late type in certificate-related files
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx (1)
50-51:⚠️ Potential issueGuard against
null/undefinedenum values
certData.certTemplatecan still benullif the DB column is nullable.
Cast-away typing (CertificateTemplate) hides that possibility and theswitch
below would immediately hit thedefaultbranch. Add a defensive fallback
before theswitch.-const certTemplate: CertificateTemplate = certData.certTemplate; +const certTemplate: CertificateTemplate = + (certData.certTemplate ?? 'original') as CertificateTemplate;🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 50-73: apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx#L50-L73
Added lines #L50 - L73 were not covered by testsapps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx (1)
58-71: Explicitly handle'original'case instead of lumping it intodefault
Having a dedicatedcase 'original':improves readability and guarantees the
compiler warns on future enum additions. This was flagged earlier and still
applies.
🧹 Nitpick comments (3)
apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx (2)
52-52: Missing type forstreamobscures API contract
let stream;is implicitlyany. Declare an explicit type
(ReadableStream<Uint8Array>or the concrete type returned by
renderToStream) to preserve type-safety and future refactor confidence.-let stream; +let stream: ReadableStream<Uint8Array>;
69-71: Useconsole.error(or a logger) for unexpected templatesUnknown templates represent an anomaly; surface them at an error level rather
thanconsole.logto aid alerting / monitoring.- console.log('Unhandled template:', certTemplate); - console.log('Using original template as fallback'); + console.error(`Unhandled certificate template: ${certTemplate}`); + console.info('Falling back to original template');apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx (1)
73-75: Base64-encoding large PDFs causes avoidable memory pressure
pdfBuffer.toString('base64')doubles memory usage for each request and pushes
the entire file through the HTML. Consider:
- Streaming the PDF to the client (especially if you already expose the
/generateendpoint), or- Storing the buffer in object storage and serving a signed URL.
Both approaches cut server memory spikes and reduce page payload.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 40-75: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx#L40-L75
Added lines #L40 - L75 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx(3 hunks)apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx(2 hunks)apps/upskii/src/lib/certificate-helper.ts(5 hunks)packages/types/src/db.ts(1 hunks)packages/ui/src/components/ui/custom/education/certificates/types.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/types/src/db.ts
- packages/ui/src/components/ui/custom/education/certificates/types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/upskii/src/lib/certificate-helper.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx (4)
packages/types/src/db.ts (1)
CertificateTemplate(143-144)apps/upskii/src/app/api/v1/certificates/templates/elegant-certificate.tsx (1)
ElegantCertificateDocument(283-312)apps/upskii/src/app/api/v1/certificates/templates/modern-certificate.tsx (1)
ModernCertificateDocument(275-291)apps/upskii/src/app/api/v1/certificates/templates/og-certificate.tsx (1)
OGCertificateDocument(161-177)
🪛 GitHub Check: codecov/patch
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx
[warning] 2-3: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx#L2-L3
Added lines #L2 - L3 were not covered by tests
[warning] 5-5: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx#L5
Added line #L5 was not covered by tests
[warning] 7-10: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx#L7-L10
Added lines #L7 - L10 were not covered by tests
[warning] 23-23: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx#L23
Added line #L23 was not covered by tests
[warning] 34-34: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx#L34
Added line #L34 was not covered by tests
[warning] 40-75: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx#L40-L75
Added lines #L40 - L75 were not covered by tests
[warning] 77-81: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx#L77-L81
Added lines #L77 - L81 were not covered by tests
[warning] 84-84: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx#L84
Added line #L84 was not covered by tests
apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx
[warning] 2-3: apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx#L2-L3
Added lines #L2 - L3 were not covered by tests
[warning] 7-8: apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx#L7-L8
Added lines #L7 - L8 were not covered by tests
[warning] 50-73: apps/upskii/src/app/api/v1/certificates/[certId]/generate/route.tsx#L50-L73
Added lines #L50 - L73 were not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Verify generated types
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/certificates/[certificateId]/page.tsx
Show resolved
Hide resolved
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Database