feat(ui): New user donations list in donor section!#629
Open
uziff wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughThis PR introduces a complete donor management system: a Ko-fi webhook handler that captures and stores donations in KV storage, an API endpoint that retrieves and sorts donors by contribution type and recency, and corresponding frontend components to fetch and display the donor list on the page. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client<br/>(Browser)
participant Pages as Cloudflare<br/>Pages Functions
participant KV as KV Storage
participant KoFi as Ko-fi<br/>Platform
rect rgba(100, 200, 150, 0.5)
Note over Client,KoFi: Donation Flow (Webhook)
KoFi->>Pages: POST /api/kofi-webhook<br/>(donation form data)
Pages->>Pages: Extract & parse donation data<br/>Verify token
Pages->>KV: Read current donors list
KV-->>Pages: Return existing donors
Pages->>Pages: Upsert donor record<br/>Promote to monthly if needed<br/>Slice to 100 entries
Pages->>KV: Write updated donors list
KV-->>Pages: Acknowledge
Pages->>KoFi: Return 200 { ok: true }
end
rect rgba(150, 180, 220, 0.5)
Note over Client,KV: Display Flow (Page Load)
Client->>Pages: GET /api/donors
Pages->>KV: Read donors list
KV-->>Pages: Return donors
Pages->>Pages: Sort by type (monthly first)<br/>then by timestamp
Pages->>Client: Return sorted JSON
Client->>Client: Render donor links<br/>in UI
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.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: 6
🧹 Nitpick comments (1)
index.html (1)
6003-6004: ⚡ Quick winAdd live-region semantics for dynamically inserted donor/error content.
Since these containers are populated after load, adding ARIA live attributes improves screen-reader feedback with minimal change.
♿ Suggested markup tweak
- <div class="about-contributors donate-donors-list"></div> - <div class="donate-donors-failed"></div> + <div class="about-contributors donate-donors-list" aria-live="polite"></div> + <div class="donate-donors-failed" role="status" aria-live="polite"></div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@index.html` around lines 6003 - 6004, The two empty divs that receive content dynamically (.about-contributors donate-donors-list and .donate-donors-failed) need ARIA live-region semantics so screen readers announce updates; update the markup for the element with class "donate-donors-list" to include aria-live="polite" and aria-atomic="true" (and optionally role="status") and update the element with class "donate-donors-failed" to include aria-live="assertive" and aria-atomic="true" (and optionally role="alert") so donor additions and error messages are announced appropriately.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@functions/api/donors.js`:
- Around line 37-39: The KV value parsing can throw and crash the onRequestGet
handler; wrap the JSON.parse(stored) call (when using env.DONORS_KV.get and
assigning to donors) in a try/catch so malformed JSON doesn't bubble up—on parse
error fall back to a safe default (e.g., [] or previously initialized donors)
and optionally log the parse error; update the block that reads from
env.DONORS_KV.get and assigns donors to handle this failure path safely.
In `@functions/api/kofi-webhook.js`:
- Around line 62-73: The current read-modify-write against env.DONORS_KV
(reading 'donors', updating `existing`/`idx` and then put) can lose concurrent
updates; replace this KV-side concurrency pattern with a single-writer Durable
Object: create a Durable Object class (e.g., DonorsDO) that exposes an update
endpoint (e.g., handleUpdate or fetch) which receives the donor payload and
performs the same logic (findIndex by donor.name, update type/timestamp or
unshift, slice(0,100)), and have the webhook handler forward updates to the
Durable Object instance (via env.DONORS_DO.idFromName/ get and fetch) instead of
directly reading/writing env.DONORS_KV; inside the DO you may still persist to
env.DONORS_KV for storage but all mutations are serialized by the DO so
concurrent webhook deliveries won't overwrite each other.
- Line 62: Wrap the JSON.parse of the KV payload in a safe try/catch so a
malformed value in env.DONORS_KV.get('donors') doesn't throw and return 500;
specifically, replace the direct parse used to set the existing variable with
logic that fetches the string with env.DONORS_KV.get('donors'), checks for
null/undefined, then attempts JSON.parse inside a try/catch (on error set
existing = [] and log the parse error) so the webhook processing continues.
Ensure you update the code path that uses the existing variable (the same scope
where existing is declared) to rely on this fallback behavior.
- Around line 35-40: The webhook currently allows requests through when
env.KOFI_VERIFICATION_TOKEN is unset, so change the auth check in
functions/api/kofi-webhook.js to fail closed by rejecting requests if the token
env var is missing or if donation.verification_token does not match: update the
condition around the existing verification block (the code referencing
env.KOFI_VERIFICATION_TOKEN and donation.verification_token) to return a 401
(and log or include an explanatory error) when env.KOFI_VERIFICATION_TOKEN is
falsy OR the tokens don't match so no requests are accepted when the expected
token isn't configured.
In `@js/app.js`:
- Line 517: The call to fetchDonors() is a floating promise; update the caller
to handle its returned Promise to satisfy lint and surface errors — either make
the surrounding function async and await fetchDonors() or append a catch handler
(e.g. fetchDonors().catch(err => console.error('fetchDonors failed', err))) so
failures are logged; locate the invocation of fetchDonors() and apply one of
these fixes.
- Around line 122-126: The donor.name value is being directly interpolated into
div.innerHTML (XSS risk); stop using innerHTML to insert untrusted donor.name
and instead construct the anchor and its child spans via createElement and set
textContent for the donor name and other user-supplied values (or use
Element.textContent / node.appendChild) so the donor.name is escaped; update the
code paths that set div.innerHTML (the place referencing div.innerHTML and
donor.name) to build DOM nodes safely and only use innerHTML for static trusted
markup like the icon if needed.
---
Nitpick comments:
In `@index.html`:
- Around line 6003-6004: The two empty divs that receive content dynamically
(.about-contributors donate-donors-list and .donate-donors-failed) need ARIA
live-region semantics so screen readers announce updates; update the markup for
the element with class "donate-donors-list" to include aria-live="polite" and
aria-atomic="true" (and optionally role="status") and update the element with
class "donate-donors-failed" to include aria-live="assertive" and
aria-atomic="true" (and optionally role="alert") so donor additions and error
messages are announced appropriately.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: fec86a58-ff4c-422e-b982-3a5b3d91de08
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
functions/api/donors.jsfunctions/api/kofi-webhook.jsindex.htmljs/app.jsstyles.css
Comment on lines
+37
to
+39
| const stored = await env.DONORS_KV.get('donors').catch(() => null); | ||
| if (stored) donors = JSON.parse(stored); | ||
| } |
There was a problem hiding this comment.
Guard KV JSON parsing to prevent endpoint-wide failure on malformed data.
JSON.parse(stored) can throw and currently bubbles out of onRequestGet, which turns donor list fetches into 500s.
Suggested fix
- if (env.DONORS_KV) {
- const stored = await env.DONORS_KV.get('donors').catch(() => null);
- if (stored) donors = JSON.parse(stored);
- }
+ if (env.DONORS_KV) {
+ const stored = await env.DONORS_KV.get('donors').catch(() => null);
+ if (stored) {
+ try {
+ const parsed = JSON.parse(stored);
+ if (Array.isArray(parsed)) donors = parsed;
+ } catch {
+ donors = PLACEHOLDERS_DONORS;
+ }
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@functions/api/donors.js` around lines 37 - 39, The KV value parsing can throw
and crash the onRequestGet handler; wrap the JSON.parse(stored) call (when using
env.DONORS_KV.get and assigning to donors) in a try/catch so malformed JSON
doesn't bubble up—on parse error fall back to a safe default (e.g., [] or
previously initialized donors) and optionally log the parse error; update the
block that reads from env.DONORS_KV.get and assigns donors to handle this
failure path safely.
Comment on lines
+35
to
+40
| if (env.KOFI_VERIFICATION_TOKEN && donation.verification_token !== env.KOFI_VERIFICATION_TOKEN) { | ||
| return new Response(JSON.stringify({ error: 'Invalid verification token' }), { | ||
| status: 401, | ||
| headers: CORS_HEADERS, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Webhook auth is fail-open when KOFI_VERIFICATION_TOKEN is unset.
If the env var is missing, any caller can write donor records. This should fail closed for production safety.
Suggested fix
+ if (!env.KOFI_VERIFICATION_TOKEN) {
+ return new Response(JSON.stringify({ error: 'Webhook verification token is not configured' }), {
+ status: 503,
+ headers: CORS_HEADERS,
+ });
+ }
- if (env.KOFI_VERIFICATION_TOKEN && donation.verification_token !== env.KOFI_VERIFICATION_TOKEN) {
+ if (donation.verification_token !== env.KOFI_VERIFICATION_TOKEN) {
return new Response(JSON.stringify({ error: 'Invalid verification token' }), {
status: 401,
headers: CORS_HEADERS,
});
}📝 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.
Suggested change
| if (env.KOFI_VERIFICATION_TOKEN && donation.verification_token !== env.KOFI_VERIFICATION_TOKEN) { | |
| return new Response(JSON.stringify({ error: 'Invalid verification token' }), { | |
| status: 401, | |
| headers: CORS_HEADERS, | |
| }); | |
| } | |
| if (!env.KOFI_VERIFICATION_TOKEN) { | |
| return new Response(JSON.stringify({ error: 'Webhook verification token is not configured' }), { | |
| status: 503, | |
| headers: CORS_HEADERS, | |
| }); | |
| } | |
| if (donation.verification_token !== env.KOFI_VERIFICATION_TOKEN) { | |
| return new Response(JSON.stringify({ error: 'Invalid verification token' }), { | |
| status: 401, | |
| headers: CORS_HEADERS, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@functions/api/kofi-webhook.js` around lines 35 - 40, The webhook currently
allows requests through when env.KOFI_VERIFICATION_TOKEN is unset, so change the
auth check in functions/api/kofi-webhook.js to fail closed by rejecting requests
if the token env var is missing or if donation.verification_token does not
match: update the condition around the existing verification block (the code
referencing env.KOFI_VERIFICATION_TOKEN and donation.verification_token) to
return a 401 (and log or include an explanatory error) when
env.KOFI_VERIFICATION_TOKEN is falsy OR the tokens don't match so no requests
are accepted when the expected token isn't configured.
Comment on lines
+62
to
+73
| const existing = JSON.parse((await env.DONORS_KV.get('donors').catch(() => null)) || '[]'); | ||
|
|
||
| const idx = existing.findIndex((d) => d.name === donor.name); | ||
| if (idx >= 0) { | ||
| if (donor.type === 'monthly') existing[idx].type = 'monthly'; | ||
| existing[idx].timestamp = donor.timestamp; | ||
| } else { | ||
| existing.unshift(donor); | ||
| } | ||
|
|
||
| //modify the values to show more or less ocntributors (0, 100) | ||
| await env.DONORS_KV.put('donors', JSON.stringify(existing.slice(0, 100))); |
There was a problem hiding this comment.
KV read-modify-write can lose updates under concurrent webhook deliveries.
Two near-simultaneous requests can read the same old list and overwrite each other. This is a data-loss risk for donor history.
Consider moving this path to a single-writer primitive (e.g., Durable Object) or a transactional store so updates are serialized.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@functions/api/kofi-webhook.js` around lines 62 - 73, The current
read-modify-write against env.DONORS_KV (reading 'donors', updating
`existing`/`idx` and then put) can lose concurrent updates; replace this KV-side
concurrency pattern with a single-writer Durable Object: create a Durable Object
class (e.g., DonorsDO) that exposes an update endpoint (e.g., handleUpdate or
fetch) which receives the donor payload and performs the same logic (findIndex
by donor.name, update type/timestamp or unshift, slice(0,100)), and have the
webhook handler forward updates to the Durable Object instance (via
env.DONORS_DO.idFromName/ get and fetch) instead of directly reading/writing
env.DONORS_KV; inside the DO you may still persist to env.DONORS_KV for storage
but all mutations are serialized by the DO so concurrent webhook deliveries
won't overwrite each other.
| }); | ||
| } | ||
|
|
||
| const existing = JSON.parse((await env.DONORS_KV.get('donors').catch(() => null)) || '[]'); |
There was a problem hiding this comment.
Protect against malformed KV payloads when parsing existing donors.
This parse can throw and drop valid webhook events with a 500 response.
Suggested fix
- const existing = JSON.parse((await env.DONORS_KV.get('donors').catch(() => null)) || '[]');
+ const rawExisting = (await env.DONORS_KV.get('donors').catch(() => null)) || '[]';
+ let existing = [];
+ try {
+ const parsed = JSON.parse(rawExisting);
+ if (Array.isArray(parsed)) existing = parsed;
+ } catch {
+ existing = [];
+ }📝 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.
Suggested change
| const existing = JSON.parse((await env.DONORS_KV.get('donors').catch(() => null)) || '[]'); | |
| const rawExisting = (await env.DONORS_KV.get('donors').catch(() => null)) || '[]'; | |
| let existing = []; | |
| try { | |
| const parsed = JSON.parse(rawExisting); | |
| if (Array.isArray(parsed)) existing = parsed; | |
| } catch { | |
| existing = []; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@functions/api/kofi-webhook.js` at line 62, Wrap the JSON.parse of the KV
payload in a safe try/catch so a malformed value in env.DONORS_KV.get('donors')
doesn't throw and return 500; specifically, replace the direct parse used to set
the existing variable with logic that fetches the string with
env.DONORS_KV.get('donors'), checks for null/undefined, then attempts JSON.parse
inside a try/catch (on error set existing = [] and log the parse error) so the
webhook processing continues. Ensure you update the code path that uses the
existing variable (the same scope where existing is declared) to rely on this
fallback behavior.
Comment on lines
+122
to
+126
| div.innerHTML = ` | ||
| <a href="https://ko-fi.com/monochrometf" target="_blank" rel="noopener noreferrer"> | ||
| <span class="donor-icon">${icon}</span> | ||
| <span>${donor.name}</span> | ||
| <span class="contrib">${label}</span> |
There was a problem hiding this comment.
Untrusted donor name is rendered via innerHTML (XSS risk).
donor.name comes from webhook payloads (functions/api/kofi-webhook.js Line 50) and is inserted directly into HTML here. That allows script/HTML injection in the donors section.
Suggested fix
- donors.forEach((donor) => {
- const div = document.createElement('div');
- const icon = donor.type === 'monthly' ? '♥' : '☕';
- const label = donor.type === 'monthly' ? 'Monthly Supporter' : 'One-time Donor';
- div.innerHTML = `
- <a href="https://ko-fi.com/monochrometf" target="_blank" rel="noopener noreferrer">
- <span class="donor-icon">${icon}</span>
- <span>${donor.name}</span>
- <span class="contrib">${label}</span>
- </a>
- `;
- con.appendChild(div);
- });
+ donors.forEach((donor) => {
+ const div = document.createElement('div');
+ const icon = donor.type === 'monthly' ? '♥' : '☕';
+ const label = donor.type === 'monthly' ? 'Monthly Supporter' : 'One-time Donor';
+
+ const a = document.createElement('a');
+ a.href = 'https://ko-fi.com/monochrometf';
+ a.target = '_blank';
+ a.rel = 'noopener noreferrer';
+
+ const iconSpan = document.createElement('span');
+ iconSpan.className = 'donor-icon';
+ iconSpan.textContent = icon;
+
+ const nameSpan = document.createElement('span');
+ nameSpan.textContent = donor.name || 'Anonymous';
+
+ const labelSpan = document.createElement('span');
+ labelSpan.className = 'contrib';
+ labelSpan.textContent = label;
+
+ a.append(iconSpan, nameSpan, labelSpan);
+ div.appendChild(a);
+ con.appendChild(div);
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app.js` around lines 122 - 126, The donor.name value is being directly
interpolated into div.innerHTML (XSS risk); stop using innerHTML to insert
untrusted donor.name and instead construct the anchor and its child spans via
createElement and set textContent for the donor name and other user-supplied
values (or use Element.textContent / node.appendChild) so the donor.name is
escaped; update the code paths that set div.innerHTML (the place referencing
div.innerHTML and donor.name) to build DOM nodes safely and only use innerHTML
for static trusted markup like the icon if needed.
| initTracker().catch(console.error); | ||
|
|
||
| await fetchcontributors(); | ||
| fetchDonors(); |
There was a problem hiding this comment.
Handle the fetchDonors() promise to satisfy lint and avoid silent async failures.
This line currently triggers @typescript-eslint/no-floating-promises in CI.
Suggested fix
- fetchDonors();
+ void fetchDonors();📝 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.
Suggested change
| fetchDonors(); | |
| void fetchDonors(); |
🧰 Tools
🪛 GitHub Actions: Lint Codebase
[error] 517-517: ESLint: Promises must be awaited / handled with .catch or .then with rejection handler (or marked with void) (@typescript-eslint/no-floating-promises)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app.js` at line 517, The call to fetchDonors() is a floating promise;
update the caller to handle its returned Promise to satisfy lint and surface
errors — either make the surrounding function async and await fetchDonors() or
append a catch handler (e.g. fetchDonors().catch(err =>
console.error('fetchDonors failed', err))) so failures are logged; locate the
invocation of fetchDonors() and apply one of these fixes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
lists recent users who have donated in the donor section by fetching the ko-fi webhook. It uses Cloudflare's KV, instructions are the following:
Configure the webhook URL in your Ko-fi settings: https://ko-fi.com/manage/webhooks
Set the URL to: https://monochrome.tf/api/kofi-webhook
Set KOFI_VERIFICATION_TOKEN in your Cloudflare Pages environment variables
Bind a KV namespace named DONORS_KV in your Cloudflare Pages settings
I got help from Claude but i did review and verify that the code was working by running
npx wrangler pages dev dist/ --kv DONORS_KVand sending requests to myself that looked like thesecurl -X POST http://localhost:8788/api/kofi-webhook ^ -d "data={\"from_name\":\"Chroma\",\"is_subscription_payment\":true,\"is_public\":true,\"timestamp\":\"2026-04-30T12:00:00Z\"}", simulating the webhook.The instructions can also appear in the new file /functions/api/kofi-webhook.js
Type of Change
Checklist
By submitting this PR, I agree to follow the guidelines. I understand that the final decision to merge rests with the maintainers and that not all contributions can be accepted.
Summary by CodeRabbit