Details modal refactor - fix type issues; Fix showing events synced back from gcal; Add loading limit for sidebar event fetch #56
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Comment |
|
problems remaining:
|
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| console.error("Error syncing event save status to gcal or DB, ", err); | ||
| // Roll back UI state upon failed sync | ||
| setSavedEventIds(prevSet => { | ||
| const newSet = new Set(prevSet); | ||
| if (isCurrentlySaved) newSet.add(event_occurrence.id); | ||
| else newSet.delete(event_occurrence.id); | ||
| return newSet; | ||
| }); | ||
| } | ||
|
|
||
| }; |
There was a problem hiding this comment.
Bug: The error handling logic for saving events uses the wrong ID (event_occurrence.id instead of event_occurrence.event_id), causing incorrect UI state rollback on API failure.
Severity: HIGH | Confidence: High
🔍 Detailed Analysis
In the error handling logic for toggling a saved event, the rollback mechanism incorrectly uses event_occurrence.id (the occurrence ID) to update the savedEventIds set. However, the rest of the application consistently uses event_occurrence.event_id (the event ID) to manage this set. This discrepancy means that if an API call to save or unsave an event fails, the UI state will not be correctly restored. The savedEventIds set will be corrupted with an occurrence ID, leading to incorrect save/unsave button states for events and preventing users from retrying the action.
💡 Suggested Fix
In the onError callback of the toggleAdded mutation, update the setSavedEventIds calls to use event_occurrence.event_id ?? -1 instead of event_occurrence.id. This will align the rollback logic with the rest of the component's state management for saved events.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/context/EventStateContext.tsx#L186-L196
Potential issue: In the error handling logic for toggling a saved event, the rollback
mechanism incorrectly uses `event_occurrence.id` (the occurrence ID) to update the
`savedEventIds` set. However, the rest of the application consistently uses
`event_occurrence.event_id` (the event ID) to manage this set. This discrepancy means
that if an API call to save or unsave an event fails, the UI state will not be correctly
restored. The `savedEventIds` set will be corrupted with an occurrence ID, leading to
incorrect save/unsave button states for events and preventing users from retrying the
action.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8261642
| const tagRes = await axios.get(`http://localhost:5001/api/events/${event_id}/tags`, { | ||
| withCredentials: true, | ||
| }); | ||
| const tags = tagRes.data; // e.g. [{ id: "1", name: "computer science" }, ...] | ||
| setSelectedTags( | ||
| tags.map((tag: any) => ({ | ||
| id: tag.id, | ||
| name: tag.name.toLowerCase(), |
There was a problem hiding this comment.
Bug: API calls in ModalEvent use a hardcoded localhost URL and a potentially undefined event_id prop, causing them to fail in production or when opening events from the calendar.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
In the ModalEvent component, API requests to fetch event tags and recurrence rules are constructed with two issues. First, they use a hardcoded http://localhost:5001 URL instead of the provided API_BASE_URL constant, which will cause the requests to fail in any non-localhost environment. Second, they use the event_id prop to build the URL path. This prop is undefined when a user opens an event from the main calendar view, resulting in malformed requests like /api/events/undefined/tags. Consequently, the event details modal will fail to display tags and recurrence information in production.
💡 Suggested Fix
Replace the hardcoded http://localhost:5001 with the API_BASE_URL constant. Instead of using the event_id prop, use eventDetails.event_id from the state, as eventDetails is guaranteed to be fetched and available in the first useEffect block. This ensures the correct API endpoint and event ID are used.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/app/components/ModalEvent.tsx#L106-L113
Potential issue: In the `ModalEvent` component, API requests to fetch event tags and
recurrence rules are constructed with two issues. First, they use a hardcoded
`http://localhost:5001` URL instead of the provided `API_BASE_URL` constant, which will
cause the requests to fail in any non-localhost environment. Second, they use the
`event_id` prop to build the URL path. This prop is `undefined` when a user opens an
event from the main calendar view, resulting in malformed requests like
`/api/events/undefined/tags`. Consequently, the event details modal will fail to display
tags and recurrence information in production.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8261642
Name of PR:
1. Issue
Link to the associated GitHub/Notion issue:
(or describe the issue if you don't have a link)
#39
2. Changes
What changes did you make to resolve the issue?
(bullet point format is fine)
Does your change introduce new dependencies to this project? If so, list here.
3. Validation
How did you trigger the change to show that it is working?
(bullet point format is fine)
Attach screenshot(s) of the logs and UI demonstrating the change.