Wire Banter to Supabase — dynamic deadlines, on-site signup, admin dashboard, deck/match review - #181
Open
moterodiaz wants to merge 12 commits into
Open
moterodiaz wants to merge 12 commits into
moterodiaz wants to merge 12 commits into
Conversation
…n dashboard Replaces the hand-edited deadline table and forms.gle links with a site_config-backed timeline, a ported signup form that posts to the existing submit-response Edge Function, and a magic-link admin dashboard for editing deadlines and reviewing applications. Also fixes: timeline invisible below 510px, malformed <tr> before <tbody>, "current" highlighting every future row instead of just the next one, stale "May 2024" copy, dead riot/card-list.tag 404, unused Facebook SDK and duplicate Google Analytics script, and the plain-HTTP html5shim include.
…e overflow Public /decks/ page lists approved mentor profiles via the mentor-decks function. Admin dashboard gets a decks review queue (preview, approve), a matches view, and buttons to trigger the deck-generation and matching GitHub Actions workflows through a new admin-only dispatch-workflow proxy function (the GitHub token never reaches the browser). Also fixes the admin Applications/timeline tables cutting off on the right: drag-to-scroll and a visible scrollbar on wide tables, wrapped JSON detail dumps instead of blowing out table width, and a fixed-height sticky-header panel so the applications list stays compact. Also fixes .gitignore never having been committed (it was ignoring itself).
Anyone can reach the admin login from the normal Banter page now; Supabase auth (admin-role claim) is still the actual gate. Non-admin sign-in shows "No admin privileges provisioned" instead of a vaguer message.
site_config.banter_cycle.deadlines replaces the single global applications_open/close pair with one open/close window per role (mentor_alums, students, young_alums, alumni mirrors mentor_alums). apply/index.html gates per the selected role and re-checks at submit time; admin dashboard gets three date-input pairs to edit them, wired into the save/load/preview cycle so Save no longer silently drops the field. submit-response (deployed separately, source belongs to the banter repo) now enforces the same window server-side instead of relying on the client-side gate alone.
Admin can download all current decks (native multi-download, no zip dependency) and re-import corrected .pptx files afterward. Files are matched by filename only, against decks that already exist -- a storage RLS policy grants admin UPDATE but not INSERT, so this path can only overwrite existing decks, never create new ones; new decks still only come from the generation pipeline. Importing a deck resets its approval, same as a regenerated one.
Approving a mentee cohort's decks for the first time now opens that cohort's own application window (deadlines.<cohort>.open), independent per audience -- regenerating and reapproving never reopens or resets it. A visible per-audience status block shows each window's state with its own "close now" mistake-recovery button, separate from the static close date. Replaces the confusing single checkbox+button in Matches with three distinct actions: Generate (algorithm only, never emails), Download/ Import as CSV (admin's manual correction loop, whole-file validation before any write, matches the DB's own check constraints so a bad row can't leave the table half-replaced), and Approve & send emails (separate button, explicit warning text, targets the backend's upcoming notify-only path so a CSV correction can't be silently discarded by a fresh algorithm run).
Two real bugs from the first live click-through: closeWindowNow never checked its write's error, so a failed save looked like nothing happened; and windowState treats close as inclusive of that day everywhere else in the system, so "Close now" writing today's date would not actually read as closed until tomorrow -- now writes yesterday so it takes effect immediately without changing that shared convention. Adds the missing "Open now" counterpart (clears close entirely, same "blank means no restriction" rule already used elsewhere), shown whenever a window reads as closed. Download all decks / Download matches now report to a status line directly under their own button instead of a shared status line that sits above the table, and download-matches gets the same "nothing to download" feedback download-all-decks already had. Application windows section restructured into one grouped column per cohort (heading, then its own Opens/Closes pair) instead of three cramped label+input rows. STE100 pass (asd-ste100 skill) over the page's visible text -- shorter sentences, active voice, no em dashes joining two ideas, no double negatives.
Matches CSV import was delete-then-insert: a failed insert after a successful delete left the matches table empty with no rollback. Now upserts the new set first (mentor/mentee/cohort/score keyed on the mentee primary key) and only deletes rows that were actually dropped from the file, so a failure partway through never removes data that wasn't going to be removed anyway. onAuthStateChange re-ran loadDashboard() on every auth event including TOKEN_REFRESHED, which fires silently in the background -- reloading the deadline editor's rows wiped any unsaved edits in an open tab with no warning. Now only reacts to events that change who's signed in. addRow()'s template literal escaped the `"` character in display, label, and href values before interpolating into innerHTML, but left `date` unescaped -- inconsistent with its siblings in the same template and a theoretical injection path if a stored date value ever contained a quote. Also removes two duplicate implementations flagged independently by both a Ponytail pass and a correctness review: SAFE_HREF was defined separately in banter.js (unexported) and admin/index.html; the CSV formula-injection escaper existed as both toCsv's inline `esc` and a standalone csvEscape. Both are now single, shared definitions.
Deck import uploaded the replacement file, then reset the approval flag in a separate request. A failed reset after a successful upload left an edited, unreviewed deck marked approved and publicly visible. Reordered: reset the flag first, upload second, so a failure always lands on the safe side (unapproved, unchanged content), never the unsafe one. Matches import computed which rows to delete from the dashboard's already-loaded matches array -- a snapshot from page load, not live state. A row added by a concurrent matching run or another admin between that snapshot and the write would survive the import unnoticed. Now reads the current mentee set immediately before deciding what's stale, shrinking the race window from "however long the tab's open" to one round trip. Not a full transaction (would need a Postgres RPC) -- narrowed deliberately rather than fully closed, low realistic collision risk at this system's actual scale.
Greptile's re-review confirmed the client-side upsert-then-select- then-delete sequence narrows the race window but isn't atomic: a concurrent insert between the read and the delete can still survive, and a failure partway through can still leave mixed state that the notify-matches workflow might later email from. Adds public.replace_matches(rows jsonb), a security-definer Postgres function that does the delete-not-in + upsert in one statement, one transaction: any constraint violation or admin-role check failure rolls back the whole call, so no partial state is reachable. The client now makes one sb.rpc() call instead of three separate REST requests. Function is EXECUTE-granted to authenticated only (explicitly revoked from public/anon, closing a gap the security advisor caught on first deploy -- Postgres grants EXECUTE to PUBLIC by default on function creation unless revoked). The admin-role check happens inside the function itself, same as every other admin-gated path in this schema.
Wire Banter to Supabase — dynamic deadlines, on-site signup, admin dashboard, deck/match review
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate admin-dashboard findings affect data integrity, security, and workflow correctness.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR replaces Banter’s static signup workflow with Supabase-backed timelines, applications, mentor browsing, and an authenticated admin dashboard.
Changes:
- Adds dynamic timelines and server-gated on-site applications.
- Adds mentor deck browsing and deck/match review workflows.
- Adds administrative configuration, pipeline controls, styling, and documentation updates.
File summaries
| File | Reviewed change |
|---|---|
resources/banter/js/banter.js |
Shared Supabase configuration and timeline rendering. |
resources/banter/index.html |
Dynamic public timeline and navigation cleanup. |
resources/banter/decks/index.html |
Public mentor profile browser. |
resources/banter/css/banterstyle.css |
Timeline, table, responsive, and social-link styling. |
resources/banter/apply/index.html |
On-site signup form with deadline gating. |
resources/banter/admin/index.html |
Authenticated configuration, deck, application, and match administration. |
README.md |
Updated local serving instructions. |
.gitignore |
Local environment and tooling exclusions. |
Review details
Suppressed comments (4)
resources/banter/admin/index.html:463
loadCycleIntoEditor()is also called after deck approval and the window-control actions, so this unconditional clear/rebuild discards any unsaved milestone, header, or deadline edits without warning. Track a dirty state and prompt before reloading, or refresh only the window-status data for those actions instead of resetting the editor.
editorRows.innerHTML = '';
(cycle.milestones || []).forEach((m) => addRow(m));
if (!cycle.milestones?.length) addRow();
renderWindowStatus(cycle);
resources/banter/admin/index.html:697
- The signed URL is fetched before
window.openruns. Because the call is now outside the synchronous click activation, popup blockers can reject it and make the Preview action appear to do nothing. Open a blank window synchronously on click, then navigate it aftercreateSignedUrlresolves and close it on error.
previewBtn.addEventListener('click', async () => {
const { data: signed, error } = await sb.storage.from('banter-decks').createSignedUrl(d.object_path, 300);
if (error) { alert(`Couldn't open preview: ${error.message}`); return; }
window.open(signed.signedUrl, '_blank');
});
resources/banter/admin/index.html:934
Number('')is0, so deleting a score from an edited CSV silently imports it as a valid zero instead of reporting a malformed row. Reject an empty score string before converting it to a number at this manual-import boundary.
const score = Number(r[col('score')]);
resources/banter/admin/index.html:390
- The Save button is not a form submission and never calls
checkValidity()/reportValidity(), so themin/maxconstraints on the year input are not enforced. Clearing it savesNumber('') === 0(or typing1saves1), and the public timeline then renders an invalidDate (...)header; validate the year before writing.
function validateCycle(cycle) {
if (!cycle.milestones.length) return 'Add at least one milestone.';
for (let i = 0; i < cycle.milestones.length; i++) {
- Files reviewed: 7/8 changed files
- Comments generated: 7
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+419
to
+421
| const { error } = await sb.from('site_config') | ||
| .update({ value: cycle, updated_at: new Date().toISOString() }) | ||
| .eq('key', CYCLE_KEY); |
| const state = windowState(d); | ||
| const div = document.createElement('div'); | ||
| div.className = 'window-item'; | ||
| div.innerHTML = `<strong>${labels[audience]}:</strong> ${state} `; |
Comment on lines
+512
to
+516
| ...cycle, | ||
| deadlines: { ...cycle.deadlines, [audience]: { ...cycle.deadlines?.[audience], ...patch } }, | ||
| }; | ||
| const { error: writeError } = await sb.from('site_config') | ||
| .update({ value: updated, updated_at: new Date().toISOString() }).eq('key', CYCLE_KEY); |
Comment on lines
+468
to
+473
| function windowState(d) { | ||
| if (!d?.open) return 'not yet open'; | ||
| const today = new Date().toISOString().slice(0, 10); | ||
| if (d.close && today > d.close) return `closed ${d.close}`; | ||
| return `open since ${d.open}`; | ||
| } |
Comment on lines
+531
to
+533
| const { data } = await sb.from('site_config').select('value').eq('key', CYCLE_KEY).maybeSingle(); | ||
| if (!data || data.value.deadlines?.[audience]?.open) return; | ||
| await patchWindow(audience, { open: new Date().toISOString().slice(0, 10) }); |
Comment on lines
+969
to
+981
| document.getElementById('approve-matches').addEventListener('click', async () => { | ||
| if (!matches.length) { document.getElementById('matching-pipeline-status').textContent = 'No matches to send.'; return; } | ||
| if (!confirm('This emails every matched pair in the table. Are you sure?')) return; | ||
| const status = document.getElementById('matching-pipeline-status'); | ||
| status.textContent = 'Sending…'; | ||
| // Notify-only path: reads matches JOIN survey_responses and emails from | ||
| // that, never re-runs process.match_students_mentors, never writes | ||
| // `matches` -- confirmed by the backend, verified locally on their end. | ||
| const { ok, body } = await dispatchWorkflow('notify-matches.yml'); | ||
| status.textContent = ok | ||
| ? 'Started. Announcement emails are going out.' | ||
| : `Couldn't start: ${body.error || 'unknown error'}`; | ||
| }); |
Comment on lines
+511
to
+514
| const updated = { | ||
| ...cycle, | ||
| deadlines: { ...cycle.deadlines, [audience]: { ...cycle.deadlines?.[audience], ...patch } }, | ||
| }; |
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
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.
Replaces the Banter program's hand-edited HTML deadline table and three Google Forms with a Supabase-backed system: a dynamic timeline, an on-site signup form, and an admin dashboard for organizers.
Public site
site_configinstead of hand-edited dates (fixes it being invisible on phones below 510px, a long-standing bug)/resources/banter/apply/, replacing threeforms.glelinks/resources/banter/decks/Admin dashboard (
/resources/banter/admin/, magic-link auth)Backend integration: Coordinated live with a companion session on the
~/projects/bantermatching backend (separate repo). This PR is the client half only — schema, RLS, and Edge Functions live there and are already deployed to the shared Supabase project.Also fixes: malformed
<tr>nesting, a "current" highlight applied to every future row instead of just the next one, stale "May 2024" copy, a dead 404'ing script include, duplicate analytics — cleanup found during the same rewrite.Review: Ponytail (2 simplifications), a correctness-focused review (5 findings), and three rounds of Greptile on a staging PR — final confidence 5/5, no outstanding findings. Every finding across all passes was fixed, not dismissed.
Known gaps, not blocking: no rate limiting on the public intake endpoint (accepted risk at this scale).