feat(events): high priority fixes — approval lock, sort, slug collision, computed status - #21
feat(events): high priority fixes — approval lock, sort, slug collision, computed status#21julianromli wants to merge 1 commit into
Conversation
- Lock detail page to approved events only (getEventBySlug filters approved=true) - Connect sort control (nearest/latest) to client-side filtering via createdAt - Handle slug collisions with auto-suffix unique strategy (retry on 23505) - Compute event status (past/ongoing/upcoming) at read time based on date
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughEvent status is now computed server-side from event dates (past/ongoing/upcoming) rather than fetched from the database. Event submission now handles slug collisions by appending an incrementing suffix and retrying insertion. Sorting functionality is integrated into the filter pipeline, mapping "latest" to creation-date ordering. Direct URL access to events is gated by an approval check. Changes span client components, server actions, utilities, and type definitions. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server as Server Action<br/>(submitEvent)
participant Database
Client->>Server: Submit event with slug
Server->>Database: Insert event (attempt 1)
alt Unique constraint violation (slug exists)
Database-->>Server: Error 23505
Server->>Server: Append suffix to slug<br/>(retry count++)
Note over Server: Retry up to 5 times
Server->>Database: Insert event (next attempt)
Database-->>Server: Success or next error
else Other database error
Database-->>Server: Error (non-unique)
Server-->>Client: Return failure
else Success
Database-->>Server: Event created
Server-->>Client: Return success
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 0/1 reviews remaining, refill in 60 minutes.Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8722773d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| coverImage: data.cover_image, | ||
| category: data.category as EventCategory, | ||
| status: data.status, | ||
| status: computeEventStatus(data.date), |
There was a problem hiding this comment.
mapEventFromDB now overwrites DB status with computeEventStatus(data.date), which only compares the start date to today. For events that span multiple days (end_date is populated), day 2+ will be marked past even while the event is still running, so detail-page state that depends on event.status (e.g. past-event CTA behavior) becomes incorrect.
Useful? React with 👍 / 👎.
| currentSlug = `${dbData.slug}-${i + 2}` | ||
| continue |
There was a problem hiding this comment.
On a 23505 collision, the loop assigns a new suffix and continues, but on the final iteration that new value is never inserted because the loop exits immediately afterward. With maxRetries = 5, collisions on base, -2, -3, -4, and -5 return failure even if -6 is available, causing avoidable submission failures for common names.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/server/events-public.ts`:
- Around line 6-15: The computeEventStatus function is duplicated; extract it
into a single shared utility module (e.g., events-utils) as an exported function
with the same signature and EventStatus return type, then remove the local
implementations and replace them with imports of computeEventStatus in both
places that currently define it (the server-side and actions code) so both files
call the shared computeEventStatus implementation.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6aeb93a3-93a6-40a0-a584-16c602070566
📒 Files selected for processing (6)
app/event/list/event-list-client.tsxdocs/events-feature/next-steps.mdlib/actions/events.tslib/events-utils.tslib/server/events-public.tstypes/events.ts
| function computeEventStatus(date: string): EventStatus { | ||
| const eventDate = new Date(date) | ||
| eventDate.setHours(0, 0, 0, 0) | ||
| const today = new Date() | ||
| today.setHours(0, 0, 0, 0) | ||
|
|
||
| if (eventDate < today) return 'past' | ||
| if (eventDate.getTime() === today.getTime()) return 'ongoing' | ||
| return 'upcoming' | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Extract duplicated computeEventStatus to a shared utility.
This function is duplicated verbatim in lib/actions/events.ts (lines 10-19). Consider extracting it to a shared location (e.g., lib/events-utils.ts) to maintain a single source of truth for event status computation.
♻️ Suggested approach
Add to lib/events-utils.ts:
import type { EventStatus } from '@/types/events'
export function computeEventStatus(date: string): EventStatus {
const eventDate = new Date(date)
eventDate.setHours(0, 0, 0, 0)
const today = new Date()
today.setHours(0, 0, 0, 0)
if (eventDate < today) return 'past'
if (eventDate.getTime() === today.getTime()) return 'ongoing'
return 'upcoming'
}Then import from both lib/server/events-public.ts and lib/actions/events.ts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/server/events-public.ts` around lines 6 - 15, The computeEventStatus
function is duplicated; extract it into a single shared utility module (e.g.,
events-utils) as an exported function with the same signature and EventStatus
return type, then remove the local implementations and replace them with imports
of computeEventStatus in both places that currently define it (the server-side
and actions code) so both files call the shared computeEventStatus
implementation.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f872277. Configure here.
|
|
||
| if (eventDate < today) return 'past' | ||
| if (eventDate.getTime() === today.getTime()) return 'ongoing' | ||
| return 'upcoming' |
There was a problem hiding this comment.
Status computation ignores end date for multi-day events
Medium Severity
computeEventStatus only considers the start date, ignoring endDate. Multi-day events (e.g., a 3-day conference) are marked 'past' the day after they start, even though they're still running. The end_date column exists in the DB, is mapped to endDate in mapEventFromDB, and the codebase already handles multi-day display via formatEventDateRange. The status override now replaces whatever the DB had, so there's no fallback for these events.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f872277. Configure here.
|
|
||
| if (eventDate < today) return 'past' | ||
| if (eventDate.getTime() === today.getTime()) return 'ongoing' | ||
| return 'upcoming' |
There was a problem hiding this comment.
Duplicated computeEventStatus across two files
Low Severity
computeEventStatus is identically defined in both lib/actions/events.ts and lib/server/events-public.ts. This duplication means any future fix (e.g., adding endDate support) needs to be applied in both places, risking inconsistency. A shared utility module could house this function.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f872277. Configure here.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/events-feature/next-steps.md">
<violation number="1" location="docs/events-feature/next-steps.md:1">
P3: New docs file was added without updating `docs/README.md`, which breaks the documented docs-index convention.</violation>
</file>
<file name="lib/actions/events.ts">
<violation number="1" location="lib/actions/events.ts:11">
P1: Parsing a date-only string with `new Date(date)` can shift the event day by timezone, producing incorrect computed statuses. Construct the date in local time from components instead.</violation>
</file>
<file name="lib/server/events-public.ts">
<violation number="1" location="lib/server/events-public.ts:7">
P2: Parsing a date-only string with `new Date(date)` can misclassify event status by one day in non-UTC timezones. Parse as local date explicitly before comparing.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| import type { AIEvent, EventCategory, EventFormData, EventLocationType, EventStatus } from '@/types/events' | ||
|
|
||
| function computeEventStatus(date: string): EventStatus { | ||
| const eventDate = new Date(date) |
There was a problem hiding this comment.
P1: Parsing a date-only string with new Date(date) can shift the event day by timezone, producing incorrect computed statuses. Construct the date in local time from components instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/actions/events.ts, line 11:
<comment>Parsing a date-only string with `new Date(date)` can shift the event day by timezone, producing incorrect computed statuses. Construct the date in local time from components instead.</comment>
<file context>
@@ -5,7 +5,18 @@ import { ROLES } from '@/lib/actions/admin/schemas'
+import type { AIEvent, EventCategory, EventFormData, EventLocationType, EventStatus } from '@/types/events'
+
+function computeEventStatus(date: string): EventStatus {
+ const eventDate = new Date(date)
+ eventDate.setHours(0, 0, 0, 0)
+ const today = new Date()
</file context>
| const eventDate = new Date(date) | |
| const [year, month, day] = date.split('-').map(Number) | |
| const eventDate = new Date(year, month - 1, day) |
| import type { AIEvent, EventCategory, EventLocationType, EventStatus } from '@/types/events' | ||
|
|
||
| function computeEventStatus(date: string): EventStatus { | ||
| const eventDate = new Date(date) |
There was a problem hiding this comment.
P2: Parsing a date-only string with new Date(date) can misclassify event status by one day in non-UTC timezones. Parse as local date explicitly before comparing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/server/events-public.ts, line 7:
<comment>Parsing a date-only string with `new Date(date)` can misclassify event status by one day in non-UTC timezones. Parse as local date explicitly before comparing.</comment>
<file context>
@@ -1,7 +1,18 @@
+import type { AIEvent, EventCategory, EventLocationType, EventStatus } from '@/types/events'
+
+function computeEventStatus(date: string): EventStatus {
+ const eventDate = new Date(date)
+ eventDate.setHours(0, 0, 0, 0)
+ const today = new Date()
</file context>
| const eventDate = new Date(date) | |
| const eventDate = new Date(`${date}T00:00:00`) |
| @@ -0,0 +1,41 @@ | |||
| # Events feature — next steps | |||
There was a problem hiding this comment.
P3: New docs file was added without updating docs/README.md, which breaks the documented docs-index convention.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/events-feature/next-steps.md, line 1:
<comment>New docs file was added without updating `docs/README.md`, which breaks the documented docs-index convention.</comment>
<file context>
@@ -0,0 +1,41 @@
+# Events feature — next steps
+
+Prioritas: **High**, **Medium**, **Low**. Ringkasan dari review fitur AI Events (listing, detail, submit, approval admin).
</file context>
1 participant
Changes
🔒 Detail page locked to approved content
getEventBySlugnow filters.eq('approved', true)so pending events are inaccessible via direct URL.🔃 Sort control connected to data
AIEventtype extended withcreatedAt.applyFiltersacceptssort: 'nearest' | 'latest'.event-list-clientpassesselectedSortinto filter pipeline.latestsorts bycreatedAtdesc;nearestkeeps existing date-based logic.🏷️ Slug collision handling
submitEventretries insert up to 5× on unique-violation (23505).-2,-3, etc. until success.📅 Event status computed at read time
computeEventStatus(date)compares event date to today.status\' inmapEventFromDB` for both public cache and detail actions.Closes high-priority items 1–4 from
docs/events-feature/next-steps.md.Note
Medium Risk
Changes Supabase read/write behavior (locking detail reads to
approved, retrying inserts on unique violations, and computingstatusat read time), which can affect event visibility and ordering. Risk is moderate due to date/time edge cases and potential differences from stored DBstatus/created_atsemantics.Overview
Tightens public event visibility by updating
getEventBySlugto only return events whereapproved = true, preventing direct access to pending events by URL.Wires the list “Sort” control to real data by passing
selectedSortthroughapplyFilters, addingsortsupport (nearestvslatest), and extendingAIEventwithcreatedAtto enable client-side “latest” sorting.Improves data consistency and submission robustness by computing
statusfrom the event date when mapping DB rows (instead of trusting the stored DBstatus) and retryingsubmitEventinserts with-2/-3/...slug suffixes on unique-constraint collisions.Adds
docs/events-feature/next-steps.mddocumenting prioritized follow-ups and noting these high-priority fixes.Reviewed by Cursor Bugbot for commit f872277. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation