Add presentation versioning with per-version reviews#654
Open
vanbroup wants to merge 7 commits into
Open
Conversation
Each presentation upload now creates an immutable version row in presentation_versions rather than overwriting the previous file reference. The current version is tracked via is_current; old versions are retained for audit and review purposes. New endpoints (all OpenAPI-registered, bearer-token capable for MCP): GET /admin/proposals/:id/presentation/versions — list versions GET /admin/proposals/:id/presentation/versions/:id/download POST /admin/proposals/:id/presentation/versions/:id/review DELETE /admin/proposals/:id/presentation/versions/:id — requires presentations:delete scope GET /proposals/speaker/:token/presentation/download — speaker self-download New scopes: presentations:review, presentations:delete Admin proposal detail gains a "Presentation" tab (shown only when the session type requires a presentation) with version cards, download links, inline review form, and soft-delete. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The three columns added for tracking the current presentation (presentation_r2_key, presentation_uploaded_at, presentation_uploaded_by_user_id) are superseded by presentation_versions. Migration 0033 now backfills existing uploads as version 1 rows and drops the old columns. All code that read from those columns is updated to query presentation_versions instead: - getSpeakerByManageToken: removes dropped columns from SQL + type - getPresentationUploader: joins presentation_versions instead of session_proposals - reminders: NOT EXISTS subquery replaces presentation_uploaded_at IS NULL - admin speakers endpoint: LEFT JOINs presentation_versions for upload status - remind-presentation endpoints: LEFT JOINs pv.id for hasPresentation flag - speaker token endpoint: presentationUploaded/At derived from uploader result Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces versioned presentation uploads and reviews for session proposals, replacing the previous single-upload fields with dedicated presentation_versions and presentation_version_reviews tables. It adds a new "Presentation" tab to the admin interface and updates the speaker-facing APIs to support multi-version history. The review feedback highlights several critical security and reliability issues, including Insecure Direct Object Reference (IDOR) vulnerabilities across multiple admin endpoints, potential HTTP Header Injection in file downloads due to unescaped filenames, a race condition in version number generation, a logical inconsistency when soft-deleting current versions, and a potential migration failure due to NOT NULL constraints on migrated timestamps.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
pkic-org | 9ba7a3f | Commit Preview URL Branch Preview URL |
Jun 25 2026, 04:08 PM |
Security (IDOR): all three version-scoped endpoints now verify that the requested version belongs to the proposalId in the URL path, returning 404 if not — prevents cross-proposal ID harvesting. Security (header injection): both download endpoints now sanitise the filename before embedding it in Content-Disposition and add the RFC 5987 filename* parameter for safe UTF-8 names. Race condition: version number generation is now an atomic subquery inside the INSERT rather than a separate SELECT + application-level increment, eliminating the UNIQUE constraint race on concurrent uploads. Soft-delete consistency: deletePresentationVersion now sets is_current = 0 on the deleted row in the same UPDATE, keeping is_current logically consistent even without the deleted_at filter. Migration safety: uploaded_at in the backfill INSERT now uses COALESCE(presentation_uploaded_at, updated_at, submitted_at) to handle any rows where the upload timestamp was never recorded. Data cleanliness: review note is trimmed and coerced to null when blank. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tests cover all seven scenarios from the test plan: 1. First upload creates a version row with is_current = 1 2. Second upload promotes the new version and demotes the previous one 3. Admin can list versions, download a file, and submit a review 4. Admin is blocked from deleting an approved version (409 CANNOT_DELETE_APPROVED) 5. Speaker GET response includes a presentationUrl field (with token) inside proposal 6. Speaker can download their current presentation via the download endpoint 7. Migration columns (presentation_r2_key etc.) are absent post-migration, confirming the backfill ran and data is accessible through presentation_versions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix finalize.ts to call processOutboxByIdBackground after each queueEmail so decision emails are actually delivered in wrangler dev - Add DEFAULT_MIN_PROPOSAL_REVIEWS=0 to e2e-start.sh env-file so the finalize step succeeds without requiring 2 reviews in the E2E env - Add missing presentation/versions mock route to admin-proposal-detail spec so the auth token is not cleared on the unmocked 401 response - Add full real-server E2E test: speaker uploads PDF, admin views Presentation tab and submits review, speaker downloads file - Fix reminder-cycle test: replace dropped presentation_uploaded_at column reference with INSERT into presentation_versions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…behalf Adds POST /api/v1/admin/proposals/:proposalId/presentation/versions so admins can upload a file directly without needing the speaker token. The Presentation tab now shows an "Upload on behalf of speaker" button in both the empty state and above the versions list. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ication Move file validation (MIME type, size) and R2 write into parsePresentationUpload() + storePresentationFile() in the presentation-versions service. Both the speaker and admin upload endpoints now call these shared helpers instead of duplicating the logic. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Collaborator
|
Is this ready to merge @vanbroup? |
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
2 participants
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.
Summary
presentation_versionsrather than overwritingpresentation_r2_keyonsession_proposalspresentation_r2_key,presentation_uploaded_at,presentation_uploaded_by_user_id) are backfilled and dropped in migration 0033New endpoints (OpenAPI-registered, bearer-token capable for MCP)
GET/admin/proposals/:id/presentation/versionsGET/admin/proposals/:id/presentation/versions/:id/downloadPOST/admin/proposals/:id/presentation/versions/:id/reviewpresentations:reviewscopeDELETE/admin/proposals/:id/presentation/versions/:idpresentations:deletescope; blocks deleting the only approved versionGET/proposals/speaker/:token/presentation/downloadNew scopes
presentations:review,presentations:deleteAdmin UI
The proposal detail page gains a Presentation tab (shown only when the session type has
requiresPresentation: true) with version cards showing upload time, file size, latest review status, download link, inline review form, and soft-delete.Test plan
presentation_versionsrow is created withis_current = 1is_current = 0, new one getsis_current = 1🤖 Generated with Claude Code