Implement Certificates feature - #67
Conversation
|
Warning Review limit reached
Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
WalkthroughThis PR adds end-to-end certificate support: database persistence, authenticated generation and retrieval APIs, activity-page controls, and a standalone certificate page with QR verification, sharing, dark mode, and print-to-PDF support. ChangesCertificate Issuance and Display
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds certificate issuance and verification support for completed activity enrollments, including persistence, API endpoints, and UI for generating/downloading certificates.
Changes:
- Adds
certificatestable + related index via schema + migration + runtime bootstrap. - Adds API endpoints to generate a certificate for an enrollment and to fetch certificate details by UUID.
- Adds UI in activity details to generate certificates and a new certificate page with print/PDF and share features.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/worker.py | Creates certificates table/index at startup and adds certificate generate/fetch APIs + routing. |
| schema.sql | Adds certificates table and index to the base schema. |
| migrations/0005_add_certificates.sql | Introduces migration to create certificates table and index. |
| public/activity.html | Adds certificate section in activity UI and calls certificate generation endpoint. |
| public/certificate.html | Adds certificate viewer/print page that fetches certificate details and renders QR/share links. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@public/certificate.html`:
- Around line 101-103: The <button id="download-btn"> currently lacks an
explicit type which can cause unintended form submission; update the button
element with type="button" (i.e., add the type attribute to the element with id
"download-btn") so it’s explicitly a non-submit button and defensive against
future wrapping in a <form>.
- Around line 197-200: The showError function currently assigns HTML into
document.getElementById('status').innerHTML which risks XSS; change it to set
the element's textContent to the message and apply the styling classes directly
(e.g., set element.className or use classList.add with "text-red-600
dark:text-red-400 font-semibold") so the visible styling is preserved without
injecting HTML; update the showError function to get the status element, set
statusEl.textContent = message, and then set statusEl.className (or classList)
accordingly.
In `@src/worker.py`:
- Around line 1556-1607: Replace the direct use of uuid.uuid4() in
api_generate_certificate with the project's helper new_id() for consistent ID
generation: change the creation of cert_uuid (currently cert_uuid =
str(uuid.uuid4())) to call new_id() so the rest of the insertion and response
logic (INSERT INTO certificates and returned cert_uuid) continue to work
unchanged; ensure you import or reference new_id() the same way other modules do
if not already available in this file.
🪄 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: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 50d78876-72e2-47f4-a2a7-9a7127d96a83
⛔ Files ignored due to path filters (1)
migrations/0005_add_certificates.sqlis excluded by!**/migrations/**
📒 Files selected for processing (4)
public/activity.htmlpublic/certificate.htmlschema.sqlsrc/worker.py
|
@Ananya44444 please fix the conflicts |
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 (3)
public/activity-detail.html (3)
1-1: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDynamic status/error text lacks ARIA live-region hooks for screen readers. Root cause: neither the certificate note/error paragraphs nor the loading/error
#statuscontainer announce updates to assistive technology when their content changes via JS.
public/activity-detail.html#L61-62: addaria-live="polite"to#certificate-noteand#certificate-errorso generation results are announced.public/certificate.html#L106-108: addaria-live="polite"(orrole="status") to#statusso load/error state changes are announced.As per path instructions,
**/*.htmlchanges should be "reviewed for accessibility (ARIA attributes, semantic elements)."🤖 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 `@public/activity-detail.html` at line 1, Add aria-live="polite" to the certificate-note and certificate-error elements in activity-detail.html and the `#status` container in certificate.html so JavaScript-driven generation, loading, and error updates are announced to screen readers.Source: Path instructions
1-1: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winBoth
generateCertificatecopies discard specific backend error text for any non-400 status. Root cause: the identicalif (res.status === 400) {...} else { error.textContent = 'Something went wrong...' }branch ignorespayload.errorfor 401/403/404/500 responses that the backend can legitimately return (e.g. "Enrollment not found", "Forbidden").
public/activity-detail.html#L461-466: fall back topayload.errorfor all non-ok responses, not juststatus === 400.public/activity.html#L800-805: apply the same change here.🤖 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 `@public/activity-detail.html` at line 1, Update both generateCertificate implementations in activity-detail.html and activity.html so every non-ok response uses payload.error when available, while retaining the existing generic message as a fallback; do not limit backend error propagation to status 400.
1-1: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
window.open()fired after an awaitedfetchis duplicated in both files and both copies risk being blocked by popup blockers. Root cause: the identicalgenerateCertificateimplementation callswindow.open(...)only after twoawaits resolve, which strict browsers (notably Safari) treat as not tied to the original user gesture and block.
public/activity-detail.html#L447-459: open a blank window synchronously in the click handler, then set.location.hrefonce the fetch resolves, instead of callingwindow.open()after theawaits.public/activity.html#L787-798: apply the same fix here.This is also a good opportunity to extract the duplicated
resetCertificateSection/renderCertificateSection/generateCertificateblock (public/activity-detail.html#L384-476,public/activity.html#L728-814) into a shared script included by both pages — the duplication is already the reasonactivity.html's copy independently drifted (missing thedata.is_completedfallback and theresetCertificateSectionnull-guard, flagged separately above).🤖 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 `@public/activity-detail.html` at line 1, Update both generateCertificate implementations in activity-detail.html and activity.html to open a blank popup synchronously within the user-triggered handler, then assign its location after the awaited fetch operations complete; handle popup creation failure appropriately. Extract the shared resetCertificateSection, renderCertificateSection, and generateCertificate logic into one shared script included by both pages, preserving the data.is_completed fallback and resetCertificateSection null-guard in the consolidated implementation.
🤖 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 `@public/activity-detail.html`:
- Around line 426-476: Update generateCertificate to open a blank certificate
window synchronously before the first await, capturing the returned handle.
After a successful response, navigate that handle to the encoded certificate URL
instead of calling window.open again; handle a null handle by displaying an
appropriate visible error and avoid leaving the button in a misleading state.
Ensure failed requests also close the placeholder window when one was opened.
In `@public/activity.html`:
- Around line 742-764: Update renderCertificateSection and its call site so
certificate eligibility also recognizes the page-level data.is_completed value,
while retaining the enrollment.status === 'completed' check. Pass the fuller
data object or an equivalent completion flag from the existing caller, and
ensure users meeting either completion condition receive the enabled
certificate-generation UI.
In `@public/certificate.html`:
- Around line 8-10: Replace the development-only cdn.tailwindcss.com script in
certificate.html with a production-ready, precompiled Tailwind CSS stylesheet
containing the classes used by the page. Remove the Play CDN runtime while
preserving the existing Font Awesome and QR code dependencies.
In `@schema.sql`:
- Line 101: Remove the redundant idx_certificates_enrollment index definition
from schema.sql and remove the matching entry from the _DDL list in
src/worker.py; retain the certificates.enrollment_id UNIQUE constraint as the
sole index-providing definition.
In `@src/worker.py`:
- Around line 2513-2618: Provide tests for api_generate_certificate and
api_get_certificate covering authentication and enrollment/host authorization,
missing and incomplete enrollments, completion-status updates,
existing-certificate idempotency, concurrent insert recovery, insert failures,
successful certificate responses, and missing certificate lookup; verify
expected database calls, status codes, response payloads, and notifications.
- Around line 2513-2593: Update api_generate_certificate with explicit type
hints for request, env, and target_id, plus its return type, matching the
conventions used by _create_notification. In the certificate INSERT block,
replace the broad except Exception with the specific D1 constraint-violation
exception exposed by the binding, preserving the existing duplicate lookup only
for that conflict and propagating or logging unrelated database failures
normally.
- Around line 2539-2551: Update the completion-check logic around
enrollment.status so cancelled or any other non-active/non-completed status is
rejected before querying or honoring activity_completions. Preserve the existing
completed path and only allow an active enrollment with a valid completion
record to transition to completed and issue a certificate.
---
Outside diff comments:
In `@public/activity-detail.html`:
- Line 1: Add aria-live="polite" to the certificate-note and certificate-error
elements in activity-detail.html and the `#status` container in certificate.html
so JavaScript-driven generation, loading, and error updates are announced to
screen readers.
- Line 1: Update both generateCertificate implementations in
activity-detail.html and activity.html so every non-ok response uses
payload.error when available, while retaining the existing generic message as a
fallback; do not limit backend error propagation to status 400.
- Line 1: Update both generateCertificate implementations in
activity-detail.html and activity.html to open a blank popup synchronously
within the user-triggered handler, then assign its location after the awaited
fetch operations complete; handle popup creation failure appropriately. Extract
the shared resetCertificateSection, renderCertificateSection, and
generateCertificate logic into one shared script included by both pages,
preserving the data.is_completed fallback and resetCertificateSection null-guard
in the consolidated implementation.
🪄 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: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 93e3fe51-41a6-432a-8e38-2b80a703dcea
⛔ Files ignored due to path filters (1)
migrations/0013_add_certificates.sqlis excluded by!**/migrations/**
📒 Files selected for processing (5)
public/activity-detail.htmlpublic/activity.htmlpublic/certificate.htmlschema.sqlsrc/worker.py
| <script src="https://cdn.tailwindcss.com"></script> | ||
| <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.0.0/css/all.min.css"> | ||
| <script src="https://cdnjs.cloudflare.com/ajax/libs/qrcodejs/1.0.0/qrcode.min.js"></script> |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff
🧩 Analysis chain
🌐 Web query:
Is cdn.tailwindcss.com Play CDN safe to use in production for a low-traffic page in 2026?
💡 Result:
The official guidance from Tailwind Labs is that the Play CDN (cdn.tailwindcss.com) is designed for development and prototyping purposes only and is not intended for use in production environments [1][2][3][4]. While some users may argue that it can be technically "safe" in terms of security for a low-traffic page, it is strongly discouraged for several performance, reliability, and architectural reasons [5][6][4]: Performance: The Play CDN requires the browser to download the entire Tailwind CSS library and perform runtime processing to generate styles [7][6]. This significantly increases page load times and can cause a flash of unstyled content (FOUC), which negatively impacts user experience and SEO [5][7][6]. In contrast, a standard production build uses a build step to scan your code and generate a tiny, optimized CSS file containing only the styles you actually use [1][7][6][3]. Reliability: Relying on an external CDN introduces a third-party dependency [6]. If the CDN experiences downtime or network latency, your website's styling will break until the service is restored [6][4]. Resource Inefficiency: The CDN approach forces every visitor's browser to redundantly re-process the same CSS on every page load, which is inefficient compared to serving a pre-compiled static CSS file [4]. Feature Limitations: Certain features available in the standard production build, such as specific build-time optimizations and configurations, may be unavailable, limited, or behave differently when using the Play CDN [3][4]. For a production environment—regardless of traffic volume—the recommended best practice is to install Tailwind CSS using the CLI, Vite, or a PostCSS plugin [1][2][3]. This allows you to generate a static, production-ready CSS file that is optimized for performance and reliability [1][7][6][3].
Citations:
- 1: https://tailwindcss.com/docs/installation/play-cdn
- 2: The browser console warnings persist when using TailwindCSS's CDN link, with no built-in way to disable them. tailwindlabs/tailwindcss#18731
- 3: https://tailkits.com/blog/tailwind-css-v4-cdn-setup/
- 4: Prefix don't work in PlayCDN tailwindlabs/tailwindcss#9935
- 5: Using Play CDN on Production site, is it safe? tailwindlabs/tailwindcss#7637
- 6: https://webtech.tools/can-you-use-tailwind-cdn-in-production-sites
- 7: https://stackoverflow.com/questions/71818499/how-bad-is-it-to-use-tailwind-cdn-in-production
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)public/(certificate\.html|.*certificate.*)|(^|/)templates/.*\.html$|(^|/)static/' || true
echo
echo "== certificate.html tailwind refs =="
if [ -f public/certificate.html ]; then
nl -ba public/certificate.html | sed -n '1,80p'
echo
echo "== search certificate.html Tailwind classes/context =="
rg -n "tailwind|@|class=|font-|text-|bg-|w-|h-|flex|grid|hidden" public/certificate.html || true
fi
echo
echo "== all Play CDN scripts in repo =="
rg -n "cdn\.tailwindcss\.com|tailwindcss\.com" . || true
echo
echo "== template inheritance check in certificate.html =="
rg -n "{% extends|{% block|{% include" public/certificate.html || trueRepository: alphaonelabs/learn
Length of output: 271
🏁 Script executed:
#!/bin/bash
set -euo pipefail
export LC_ALL=C
echo "== files =="
git ls-files | grep -E '(^|/)public/(certificate\.html|.*certificate.*|templates/.*)|(^|/)static/' || true
echo
echo "== certificate.html relevant header/content =="
if [ -f public/certificate.html ]; then
awk '{printf "%5d\t%s\n", NR, $0}' public/certificate.html | sed -n '1,120p'
fi
echo
echo "== Play CDN references in repository =="
grep -RInE "cdn\.tailwindcss\.com|tailwindcss\.com|tailwind" . --include='*.html' --include='*.css' --include='*.js' --include='*.py' --include='*.md' --include='*.txt' --include='*.json' 2>/dev/null || true
echo
echo "== template inheritance references in certificate.html =="
grep -Ein "{% *(extends|block|include) " public/certificate.html 2>/dev/null || true
echo
echo "== static Tailwind stylesheet references =="
grep -EIn '(^|[/"'\''=`]|\b)\.css(?=["'\'`> \)])' public templates 2>/dev/null | grep -i tailwind || trueRepository: alphaonelabs/learn
Length of output: 8888
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== certificate Tailwind usage count =="
grep -oEi '([A-Za-z0-9_:.-]+)' public/certificate.html | grep -Ey '^\s*(class|href|src)\s*$|^[A-Za-z0-9_:./-]+$' || true
grep -Eo '\b[A-Za-z0-9_-]+-[A-Za-z0-9_.:-]+\b|hover:|focus:|active:|disabled:|aria-[A-Za-z0-9_-]+' public/certificate.html | sort -u | sed -n '1,200p'
echo
echo "== base template inheritance references =="
grep -En "{% *(extends|block|include) " public/base.html 2>/dev/null || trueRepository: alphaonelabs/learn
Length of output: 12650
Use a production-ready Tailwind stylesheet instead of the Play CDN.
cdn.tailwindcss.com is Tailwind’s Play CDN, intended for development/prototyping. It downloads the full Tailwind runtime and generates styles in the browser, which can slow load times and cause flashes of unstyled content; serving a static, precompiled/purged CSS file for certificate.html is more suitable for this public-facing certificate page.
🤖 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 `@public/certificate.html` around lines 8 - 10, Replace the development-only
cdn.tailwindcss.com script in certificate.html with a production-ready,
precompiled Tailwind CSS stylesheet containing the classes used by the page.
Remove the Play CDN runtime while preserving the existing Font Awesome and QR
code dependencies.
Summary
This PR implements the Certificates feature. Students who complete an activity can generate a shareable, printable certificate of completion.
Changes
Database Schema
API Endpoints (src/worker.py)
Certificate Page (public/certificate.html)
Activity Page (public/activity.html)
Notes
Certificates Feature Implementation
This PR adds an end-to-end Certificates feature that lets students generate shareable, printable certificates after completing activities, and allows anyone to verify certificate authenticity via a public verification endpoint.
Key Changes
Backend (schema + API)
Database: Introduces a new
certificatestable with a uniqueenrollment_id(FK toenrollments(id)with cascade delete) plusissued_at/created_attimestamps, and an index onenrollment_id.Certificate generation: Adds
POST /api/certificates/generate/:enrollment_idwith:Public verification: Adds
GET /api/certificates/:cert_uuidto fetch certificate details by UUID, including:Enrollment data exposure: Extends activity-related responses so the frontend can access
enrollment.idfor certificate generation.Frontend (certificate + activity flows)
public/certificate.html):uuidquery parameter (/api/certificates/:uuid)window.print()beforeprinthandler)POST /api/certificates/generate/{enrollmentId}with Bearer auth), then openscertificate.html?uuid=...on success, with proper button/error state handling.Impact