Fix post-review findings: stale calendar leak, aria-label, color dot, loading state
- Add access check to display calendar batch query (Security L-01) - Add aria-label, color dot, disabled-during-mutation, h-8 height (UI W-01/W-02/W-03/S-01) - Add display_calendar_id to EventInvitationResponse schema (Code W-02) - Invalidate event-invitations cache on display calendar update (Code S-03) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
68a609ee50
commit
29c2cbbec8
@ -31,8 +31,23 @@ def _event_to_dict(
|
||||
is_invited: bool = False,
|
||||
invitation_status: str | None = None,
|
||||
invitation_id: int | None = None,
|
||||
display_calendar_id: int | None = None,
|
||||
display_calendar_name: str | None = None,
|
||||
display_calendar_color: str | None = None,
|
||||
) -> dict:
|
||||
"""Serialize a CalendarEvent ORM object to a response dict including calendar info."""
|
||||
# For invited events: use display calendar if set, otherwise fallback to "Invited"/gray
|
||||
if is_invited:
|
||||
if display_calendar_name:
|
||||
cal_name = display_calendar_name
|
||||
cal_color = display_calendar_color or "#6B7280"
|
||||
else:
|
||||
cal_name = "Invited"
|
||||
cal_color = "#6B7280"
|
||||
else:
|
||||
cal_name = event.calendar.name if event.calendar else ""
|
||||
cal_color = event.calendar.color if event.calendar else ""
|
||||
|
||||
d = {
|
||||
"id": event.id,
|
||||
"title": event.title,
|
||||
@ -45,8 +60,8 @@ def _event_to_dict(
|
||||
"recurrence_rule": event.recurrence_rule,
|
||||
"is_starred": event.is_starred,
|
||||
"calendar_id": event.calendar_id,
|
||||
"calendar_name": "Invited" if is_invited else (event.calendar.name if event.calendar else ""),
|
||||
"calendar_color": "#6B7280" if is_invited else (event.calendar.color if event.calendar else ""),
|
||||
"calendar_name": cal_name,
|
||||
"calendar_color": cal_color,
|
||||
"is_virtual": False,
|
||||
"parent_event_id": event.parent_event_id,
|
||||
"is_recurring": event.is_recurring,
|
||||
@ -56,6 +71,7 @@ def _event_to_dict(
|
||||
"is_invited": is_invited,
|
||||
"invitation_status": invitation_status,
|
||||
"invitation_id": invitation_id,
|
||||
"display_calendar_id": display_calendar_id,
|
||||
}
|
||||
return d
|
||||
|
||||
@ -191,16 +207,34 @@ async def get_events(
|
||||
|
||||
# Build invitation lookup for the current user
|
||||
invited_event_id_set = set(invited_event_ids)
|
||||
invitation_map: dict[int, tuple[str, int]] = {} # event_id -> (status, invitation_id)
|
||||
invitation_map: dict[int, tuple[str, int, int | None]] = {} # event_id -> (status, invitation_id, display_calendar_id)
|
||||
if invited_event_ids:
|
||||
inv_result = await db.execute(
|
||||
select(EventInvitation.event_id, EventInvitation.status, EventInvitation.id).where(
|
||||
select(
|
||||
EventInvitation.event_id,
|
||||
EventInvitation.status,
|
||||
EventInvitation.id,
|
||||
EventInvitation.display_calendar_id,
|
||||
).where(
|
||||
EventInvitation.user_id == current_user.id,
|
||||
EventInvitation.event_id.in_(invited_event_ids),
|
||||
)
|
||||
)
|
||||
for eid, status, inv_id in inv_result.all():
|
||||
invitation_map[eid] = (status, inv_id)
|
||||
for eid, status, inv_id, disp_cal_id in inv_result.all():
|
||||
invitation_map[eid] = (status, inv_id, disp_cal_id)
|
||||
|
||||
# Batch-fetch display calendars for invited events
|
||||
display_cal_ids = {t[2] for t in invitation_map.values() if t[2] is not None}
|
||||
display_cal_map: dict[int, dict] = {} # cal_id -> {name, color}
|
||||
if display_cal_ids:
|
||||
cal_result = await db.execute(
|
||||
select(Calendar.id, Calendar.name, Calendar.color).where(
|
||||
Calendar.id.in_(display_cal_ids),
|
||||
Calendar.id.in_(all_calendar_ids),
|
||||
)
|
||||
)
|
||||
for cal_id, cal_name, cal_color in cal_result.all():
|
||||
display_cal_map[cal_id] = {"name": cal_name, "color": cal_color}
|
||||
|
||||
# Get per-occurrence overrides for invited events
|
||||
all_event_ids = [e.id for e in events]
|
||||
@ -213,12 +247,27 @@ async def get_events(
|
||||
is_invited = parent_id in invited_event_id_set
|
||||
inv_status = None
|
||||
inv_id = None
|
||||
disp_cal_id = None
|
||||
disp_cal_name = None
|
||||
disp_cal_color = None
|
||||
if is_invited and parent_id in invitation_map:
|
||||
inv_status, inv_id = invitation_map[parent_id]
|
||||
inv_status, inv_id, disp_cal_id = invitation_map[parent_id]
|
||||
# Check for per-occurrence override
|
||||
if e.id in override_map:
|
||||
inv_status = override_map[e.id]
|
||||
response.append(_event_to_dict(e, is_invited=is_invited, invitation_status=inv_status, invitation_id=inv_id))
|
||||
# Resolve display calendar info
|
||||
if disp_cal_id and disp_cal_id in display_cal_map:
|
||||
disp_cal_name = display_cal_map[disp_cal_id]["name"]
|
||||
disp_cal_color = display_cal_map[disp_cal_id]["color"]
|
||||
response.append(_event_to_dict(
|
||||
e,
|
||||
is_invited=is_invited,
|
||||
invitation_status=inv_status,
|
||||
invitation_id=inv_id,
|
||||
display_calendar_id=disp_cal_id,
|
||||
display_calendar_name=disp_cal_name,
|
||||
display_calendar_color=disp_cal_color,
|
||||
))
|
||||
|
||||
# Fetch the user's Birthdays system calendar; only generate virtual events if visible
|
||||
bday_result = await db.execute(
|
||||
|
||||
@ -19,6 +19,11 @@ class EventInvitationOverrideCreate(BaseModel):
|
||||
status: Literal["accepted", "tentative", "declined"]
|
||||
|
||||
|
||||
class UpdateDisplayCalendar(BaseModel):
|
||||
model_config = ConfigDict(extra="forbid")
|
||||
calendar_id: Annotated[int, Field(ge=1, le=2147483647)]
|
||||
|
||||
|
||||
class EventInvitationResponse(BaseModel):
|
||||
model_config = ConfigDict(from_attributes=True)
|
||||
id: int
|
||||
@ -30,3 +35,4 @@ class EventInvitationResponse(BaseModel):
|
||||
responded_at: Optional[datetime]
|
||||
invitee_name: Optional[str] = None
|
||||
invitee_umbral_name: Optional[str] = None
|
||||
display_calendar_id: Optional[int] = None
|
||||
|
||||
@ -259,7 +259,8 @@ export default function EventDetailPanel({
|
||||
const parentEventId = event?.parent_event_id ?? eventNumericId;
|
||||
const {
|
||||
invitees, invite, isInviting, respond: respondInvitation,
|
||||
isResponding, override: overrideInvitation, leave: leaveInvitation, isLeaving,
|
||||
isResponding, override: overrideInvitation, updateDisplayCalendar,
|
||||
isUpdatingDisplayCalendar, leave: leaveInvitation, isLeaving,
|
||||
} = useEventInvitations(parentEventId);
|
||||
const { connections } = useConnectedUsersSearch();
|
||||
const [showLeaveDialog, setShowLeaveDialog] = useState(false);
|
||||
@ -929,12 +930,39 @@ export default function EventDetailPanel({
|
||||
<>
|
||||
{/* 2-column grid: Calendar, Starred, Start, End, Location, Recurrence */}
|
||||
<div className="grid grid-cols-2 gap-3">
|
||||
{/* Calendar */}
|
||||
{/* Calendar — for invited events with accepted/tentative, show picker */}
|
||||
<div className="space-y-1">
|
||||
<div className="flex items-center gap-1.5 text-[11px] text-muted-foreground uppercase tracking-wider">
|
||||
<Calendar className="h-3 w-3" />
|
||||
Calendar
|
||||
</div>
|
||||
{isInvitedEvent && myInvitationId && (myInvitationStatus === 'accepted' || myInvitationStatus === 'tentative') ? (
|
||||
<div className="flex items-center gap-2">
|
||||
<div
|
||||
className="w-2 h-2 rounded-full shrink-0"
|
||||
style={{ backgroundColor: event?.calendar_color || '#6B7280' }}
|
||||
/>
|
||||
<Select
|
||||
aria-label="Display calendar"
|
||||
value={event?.display_calendar_id?.toString() || ''}
|
||||
onChange={(e) => {
|
||||
const calId = parseInt(e.target.value);
|
||||
if (calId && myInvitationId) {
|
||||
updateDisplayCalendar({ invitationId: myInvitationId, calendarId: calId });
|
||||
}
|
||||
}}
|
||||
className="text-xs h-8"
|
||||
disabled={isUpdatingDisplayCalendar}
|
||||
>
|
||||
{!event?.display_calendar_id && (
|
||||
<option value="" disabled>Assign to calendar...</option>
|
||||
)}
|
||||
{selectableCalendars.map((cal) => (
|
||||
<option key={cal.id} value={cal.id}>{cal.name}</option>
|
||||
))}
|
||||
</Select>
|
||||
</div>
|
||||
) : (
|
||||
<div className="flex items-center gap-2">
|
||||
<div
|
||||
className="w-2 h-2 rounded-full shrink-0"
|
||||
@ -942,6 +970,7 @@ export default function EventDetailPanel({
|
||||
/>
|
||||
<span className="text-sm">{event?.calendar_name}</span>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{/* Starred */}
|
||||
|
||||
@ -59,6 +59,21 @@ export function useEventInvitations(eventId: number | null) {
|
||||
},
|
||||
});
|
||||
|
||||
const updateDisplayCalendarMutation = useMutation({
|
||||
mutationFn: async ({ invitationId, calendarId }: { invitationId: number; calendarId: number }) => {
|
||||
const { data } = await api.put(`/event-invitations/${invitationId}/display-calendar`, { calendar_id: calendarId });
|
||||
return data;
|
||||
},
|
||||
onSuccess: () => {
|
||||
queryClient.invalidateQueries({ queryKey: ['calendar-events'] });
|
||||
queryClient.invalidateQueries({ queryKey: ['event-invitations'] });
|
||||
toast.success('Display calendar updated');
|
||||
},
|
||||
onError: (error) => {
|
||||
toast.error(getErrorMessage(error, 'Failed to update display calendar'));
|
||||
},
|
||||
});
|
||||
|
||||
const leaveMutation = useMutation({
|
||||
mutationFn: async (invitationId: number) => {
|
||||
await api.delete(`/event-invitations/${invitationId}`);
|
||||
@ -83,6 +98,8 @@ export function useEventInvitations(eventId: number | null) {
|
||||
respond: respondMutation.mutateAsync,
|
||||
isResponding: respondMutation.isPending,
|
||||
override: overrideMutation.mutateAsync,
|
||||
updateDisplayCalendar: updateDisplayCalendarMutation.mutateAsync,
|
||||
isUpdatingDisplayCalendar: updateDisplayCalendarMutation.isPending,
|
||||
leave: leaveMutation.mutateAsync,
|
||||
isLeaving: leaveMutation.isPending,
|
||||
};
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user