feat: Admin 'Mark Past as Completed' button — auto-complete past events from UI - #39
feat: Admin 'Mark Past as Completed' button — auto-complete past events from UI#39jerry-shimizutech wants to merge 1 commit into
Conversation
…ts from UI Admins can now mark all past-dated events as completed with one click. No more hardcoded migrations or rake tasks needed. - POST /api/v1/admin/events/auto_complete_past (requires staff auth) - Finds all upcoming/published events where date < today - Updates status to 'completed', returns summary of changes - Frontend: 'Mark Past as Completed' button in Events admin header - Shows spinner while running, displays success message with count - Reloads event table after completion Addresses inbox request: admin side should handle event status updates without hardcoded values.
✅ Deploy Preview for marianas-open ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can generate a title for your PR based on the changes.Add |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def auto_complete_past | ||
| today = Date.current | ||
| candidates = Event.where(status: %w[upcoming published]) | ||
| .where("COALESCE(end_date, date) < ?", today) | ||
|
|
||
| updated = [] | ||
| candidates.each do |event| | ||
| event.update!(status: "completed") | ||
| updated << { id: event.id, name: event.name, date: event.date, slug: event.slug } | ||
| end | ||
|
|
||
| render json: { | ||
| message: updated.any? ? "Marked #{updated.count} event(s) as completed." : "No past events needed updating.", | ||
| updated_count: updated.count, | ||
| updated_events: updated | ||
| } | ||
| end |
There was a problem hiding this comment.
The loop calls event.update! on each candidate individually with no wrapping transaction. If any update raises (e.g., a validation error on the 4th of 10 events), the first 3 events will already have been committed to completed while the remaining 7 are left unchanged. The endpoint will also return a 500 instead of a meaningful error, with no indication of which events were actually changed.
Wrap the block in a transaction so the whole operation succeeds or rolls back atomically:
def auto_complete_past
today = Date.current
candidates = Event.where(status: %w[upcoming published])
.where("COALESCE(end_date, date) < ?", today)
updated = []
Event.transaction do
candidates.each do |event|
event.update!(status: "completed")
updated << { id: event.id, name: event.name, date: event.date, slug: event.slug }
end
end
render json: {
message: updated.any? ? "Marked #{updated.count} event(s) as completed." : "No past events needed updating.",
updated_count: updated.count,
updated_events: updated
}
endPrompt To Fix With AI
This is a comment left during a code review.
Path: api/app/controllers/api/v1/admin/events_controller.rb
Line: 91-107
Comment:
**Missing transaction — partial updates on failure**
The loop calls `event.update!` on each candidate individually with no wrapping transaction. If any update raises (e.g., a validation error on the 4th of 10 events), the first 3 events will already have been committed to `completed` while the remaining 7 are left unchanged. The endpoint will also return a 500 instead of a meaningful error, with no indication of which events were actually changed.
Wrap the block in a transaction so the whole operation succeeds or rolls back atomically:
```ruby
def auto_complete_past
today = Date.current
candidates = Event.where(status: %w[upcoming published])
.where("COALESCE(end_date, date) < ?", today)
updated = []
Event.transaction do
candidates.each do |event|
event.update!(status: "completed")
updated << { id: event.id, name: event.name, date: event.date, slug: event.slug }
end
end
render json: {
message: updated.any? ? "Marked #{updated.count} event(s) as completed." : "No past events needed updating.",
updated_count: updated.count,
updated_events: updated
}
end
```
How can I resolve this? If you propose a fix, please make it concise.| candidates = Event.where(status: %w[upcoming published]) | ||
| .where("COALESCE(end_date, date) < ?", today) |
There was a problem hiding this comment.
Every other action in this controller scopes its queries to a specific organization (e.g., Organization.first → org.events). This action queries Event directly across all organizations. If the app ever has more than one organization in the database (even for testing or staging), this will blindly complete events that belong to a different tenant.
Scope the query to the organization for consistency:
org = Organization.first
return render json: { error: "No organization configured" }, status: :unprocessable_entity unless org
candidates = org.events.where(status: %w[upcoming published])
.where("COALESCE(end_date, date) < ?", today)Prompt To Fix With AI
This is a comment left during a code review.
Path: api/app/controllers/api/v1/admin/events_controller.rb
Line: 93-94
Comment:
**Unscoped query — affects all organizations**
Every other action in this controller scopes its queries to a specific organization (e.g., `Organization.first` → `org.events`). This action queries `Event` directly across **all** organizations. If the app ever has more than one organization in the database (even for testing or staging), this will blindly complete events that belong to a different tenant.
Scope the query to the organization for consistency:
```ruby
org = Organization.first
return render json: { error: "No organization configured" }, status: :unprocessable_entity unless org
candidates = org.events.where(status: %w[upcoming published])
.where("COALESCE(end_date, date) < ?", today)
```
How can I resolve this? If you propose a fix, please make it concise.| updated = [] | ||
| candidates.each do |event| | ||
| event.update!(status: "completed") | ||
| updated << { id: event.id, name: event.name, date: event.date, slug: event.slug } | ||
| end |
There was a problem hiding this comment.
Issuing one UPDATE query per event can be slow if many events need completing. update_all sends a single SQL statement. You can fetch the metadata you need in a separate (much cheaper) query before applying the bulk update:
updated = candidates.map { |e| { id: e.id, name: e.name, date: e.date, slug: e.slug } }
candidates.update_all(status: "completed")This trades individual round-trips for two queries total (one SELECT, one bulk UPDATE), which is significantly faster at any meaningful scale.
Prompt To Fix With AI
This is a comment left during a code review.
Path: api/app/controllers/api/v1/admin/events_controller.rb
Line: 96-100
Comment:
**N+1 updates — consider bulk update**
Issuing one `UPDATE` query per event can be slow if many events need completing. `update_all` sends a single SQL statement. You can fetch the metadata you need in a separate (much cheaper) query before applying the bulk update:
```ruby
updated = candidates.map { |e| { id: e.id, name: e.name, date: e.date, slug: e.slug } }
candidates.update_all(status: "completed")
```
This trades individual round-trips for two queries total (one `SELECT`, one bulk `UPDATE`), which is significantly faster at any meaningful scale.
How can I resolve this? If you propose a fix, please make it concise.| const handleAutoCompletePast = async () => { | ||
| setAutoCompleting(true) | ||
| setError('') | ||
| try { | ||
| const res = await api.admin.autoCompletePastEvents() | ||
| setSuccess(res.message) | ||
| await loadEvents() | ||
| setTimeout(() => setSuccess(''), 5000) | ||
| } catch (err) { | ||
| setError(err instanceof Error ? err.message : 'Auto-complete failed') | ||
| } finally { | ||
| setAutoCompleting(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
setTimeout not cleared on unmount
setTimeout(() => setSuccess(''), 5000) fires after 5 seconds, but if the user navigates away before it runs, it will call setSuccess on an unmounted component. While React 18 doesn't throw for this, it's still a state update after unmount that can cause confusion. Consider storing the timeout ref and clearing it in a cleanup effect, or using a pattern that cancels the timer on unmount.
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/src/pages/admin/EventsAdmin.tsx
Line: 51-64
Comment:
**`setTimeout` not cleared on unmount**
`setTimeout(() => setSuccess(''), 5000)` fires after 5 seconds, but if the user navigates away before it runs, it will call `setSuccess` on an unmounted component. While React 18 doesn't throw for this, it's still a state update after unmount that can cause confusion. Consider storing the timeout ref and clearing it in a cleanup effect, or using a pattern that cancels the timer on unmount.
How can I resolve this? If you propose a fix, please make it concise.1 participant
Summary
Addresses inbox request: 'fix up the admin side so the admin side is where we could do all this stuff... make sure nothing is hardcoded'
What This Does
Adds a "Mark Past as Completed" button to the Events admin page. One click automatically finds all upcoming/published events whose date has passed and marks them as
completed— no migrations, no rake tasks, no hardcoded values needed.Changes
Backend:
POST /api/v1/admin/events/auto_complete_past— new collection action (requires staff auth)COALESCE(end_date, date) < todayto find stale upcoming eventscompleted, returns count + names of updated eventsFrontend:
Usage
No more need to run rake tasks or write migrations when an event finishes.
Greptile Summary
This PR adds a "Mark Past as Completed" button to the Events admin page, backed by a new
POST /api/v1/admin/events/auto_complete_pastendpoint. The feature is a convenient replacement for manual rake tasks — it finds allupcoming/publishedevents whose date has passed and flips their status tocompleted.Issues found:
auto_complete_pastqueriesEvent.where(...)directly instead of scoping toorg.eventslike every other action in the controller. If more than one organization record exists in the database (e.g., staging data), this will affect events across all tenants.event.update!calls inside a plaineachloop mean that if any update raises (e.g., a validation error), earlier events will already be committed ascompletedwith no rollback — resulting in a partially-completed operation with no clear recovery path.UPDATEper event. A singleupdate_allcall would be significantly more efficient, especially as the event list grows.setTimeoutinhandleAutoCompletePast(P2): The 5-second timeout that clears the success message is never cancelled if the component unmounts before it fires.Confidence Score: 2/5
Event.where(...)query bypasses the organization boundary that every other action in this controller respects. Both issues are straightforward to fix but have meaningful data-integrity consequences.api/app/controllers/api/v1/admin/events_controller.rb— specifically theauto_complete_pastaction needs a transaction wrapper and organization scoping before this is safe to ship.Important Files Changed
auto_complete_pastaction has two P1 issues: unscopedEvent.where(...)query operates across all organizations instead of scoping toorg.events, and individualupdate!calls in a loop lack a transaction, risking partial updates on failure. Also performs N+1 updates instead of a singleupdate_all.collectionaction under admin events — no issues.setTimeoutthat could callsetSuccesson an unmounted component after navigation.autoCompletePastEventsAPI method is correctly typed and properly calls the new endpoint withPOSTand admin auth — no issues.Sequence Diagram
sequenceDiagram participant Admin as Admin UI (EventsAdmin.tsx) participant API as api.ts (autoCompletePastEvents) participant Controller as Admin::EventsController participant DB as Database (Event table) Admin->>API: POST /api/v1/admin/events/auto_complete_past API->>Controller: auto_complete_past (requires staff auth) Controller->>DB: SELECT * FROM events WHERE status IN ('upcoming','published')\nAND COALESCE(end_date, date) < today DB-->>Controller: candidate events loop For each candidate (N+1 updates) Controller->>DB: UPDATE events SET status='completed' WHERE id=? end DB-->>Controller: updated records Controller-->>API: { message, updated_count, updated_events } API-->>Admin: response Admin->>Admin: setSuccess(res.message) Admin->>Admin: await loadEvents() — reload table Admin->>Admin: setTimeout → clear success after 5sPrompt To Fix All With AI
Last reviewed commit: "feat: admin 'Mark Pa..."