Skip to content

feat: Admin 'Mark Past as Completed' button — auto-complete past events from UI #39

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jerry-shimizutech wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: Admin 'Mark Past as Completed' button — auto-complete past events from UI #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions api/app/controllers/api/v1/admin/events_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,27 @@ def upload_image
render json: { event: @event.reload.as_json }
end

# POST /api/v1/admin/events/auto_complete_past
# Finds all upcoming/published events whose end_date (or date) is before today
# and marks them as completed. Returns a summary of what changed.
def auto_complete_past
today = Date.current
candidates = Event.where(status: %w[upcoming published])
.where("COALESCE(end_date, date) < ?", today)
Comment on lines +93 to +94

@greptile-apps greptile-apps Bot Mar 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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:

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
Comment on lines +96 to +100

@greptile-apps greptile-apps Bot Mar 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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:

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.


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
Comment on lines +91 to +107

@greptile-apps greptile-apps Bot Mar 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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:

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
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: 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.


private

def set_event
Expand Down
3 changes: 3 additions & 0 deletions api/config/routes.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
get :import_results_preview
post :import_results
end
collection do
post :auto_complete_past
end
resources :event_results, only: [:index, :create, :update, :destroy], path: 'results' do
collection do
post :bulk_create
Expand Down
43 changes: 35 additions & 8 deletions web/src/pages/admin/EventsAdmin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useEffect, useState, useCallback } from 'react'
import { CalendarDays, Plus, Pencil, Trash2, X, Loader2, Star, Clock, Trophy, Save, ChevronDown, ChevronUp, Radio, Hotel } from 'lucide-react'
import { CalendarDays, Plus, Pencil, Trash2, X, Loader2, Star, Clock, Trophy, Save, ChevronDown, ChevronUp, Radio, Hotel, CheckCircle2 } from 'lucide-react'
import { motion, AnimatePresence } from 'framer-motion'
import { Link } from 'react-router-dom'
import { api } from '../../services/api'
Expand Down Expand Up @@ -46,6 +46,22 @@ export default function EventsAdmin() {
const [success, setSuccess] = useState('')
const [deleteConfirm, setDeleteConfirm] = useState<number | null>(null)
const [expandedSections, setExpandedSections] = useState<Record<string, boolean>>({})
const [autoCompleting, setAutoCompleting] = useState(false)

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)
}
}
Comment on lines +51 to +64

@greptile-apps greptile-apps Bot Mar 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.


const loadEvents = useCallback(async () => {
try {
Expand Down Expand Up @@ -199,13 +215,24 @@ export default function EventsAdmin() {
<h1 className="font-heading text-2xl font-bold text-text-primary">Events</h1>
</div>
{!editing && (
<button
onClick={() => { setForm(emptyForm); setEditing('new'); setError('') }}
className="flex items-center gap-2 px-4 py-2 bg-gold/10 text-gold text-sm font-medium hover:bg-gold/15 transition-colors"
>
<Plus className="w-4 h-4" />
Create Event
</button>
<div className="flex items-center gap-2">
<button
onClick={handleAutoCompletePast}
disabled={autoCompleting}
className="flex items-center gap-2 px-4 py-2 bg-white/5 text-text-secondary text-sm font-medium hover:bg-white/10 hover:text-text-primary transition-colors disabled:opacity-50"
title="Automatically mark any events whose date has passed as 'completed'"
>
{autoCompleting ? <Loader2 className="w-4 h-4 animate-spin" /> : <CheckCircle2 className="w-4 h-4" />}
{autoCompleting ? 'Updating...' : 'Mark Past as Completed'}
</button>
<button
onClick={() => { setForm(emptyForm); setEditing('new'); setError('') }}
className="flex items-center gap-2 px-4 py-2 bg-gold/10 text-gold text-sm font-medium hover:bg-gold/15 transition-colors"
>
<Plus className="w-4 h-4" />
Create Event
</button>
</div>
)}
</div>

Expand Down
4 changes: 4 additions & 0 deletions web/src/services/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,10 @@ export const api = {
fetchApi<ImportPreview>(`/api/v1/admin/events/${eventId}/import_results_preview`, {}, true),
importResults: (eventId: number) =>
fetchApi<ImportResult>(`/api/v1/admin/events/${eventId}/import_results`, { method: 'POST' }, true),
autoCompletePastEvents: () =>
fetchApi<{ message: string; updated_count: number; updated_events: Array<{ id: number; name: string; date: string; slug: string }> }>(
'/api/v1/admin/events/auto_complete_past', { method: 'POST' }, true
),

// Event Accommodations
getAccommodations: (eventId: number) =>
Expand Down