fix: correct recurring to non-recurring event update logic
- Fix bug where updating recurring event to non-recurring created duplicate events - Deduplicate URLs before deletion to avoid multiple delete calls on same file - Add onCloseAll prop to close both preview and update modals after save - Adjust smooth UI Resolves issue where repeat->no-repeat conversion left old recurring events visible
This commit is contained in:
committed by
Benoit TELLIER
parent
757060946f
commit
942b203b0f
@@ -2,9 +2,11 @@ import { screen, fireEvent, waitFor, act } from "@testing-library/react";
|
||||
import { renderWithProviders } from "../../utils/Renderwithproviders";
|
||||
import EventUpdateModal from "../../../src/features/Events/EventUpdateModal";
|
||||
import * as EventApi from "../../../src/features/Events/EventApi";
|
||||
import * as CalendarApi from "../../../src/features/Calendars/CalendarApi";
|
||||
import * as eventUtils from "../../../src/components/Event/utils/eventUtils";
|
||||
|
||||
jest.mock("../../../src/features/Events/EventApi");
|
||||
jest.mock("../../../src/features/Calendars/CalendarApi");
|
||||
|
||||
describe("EventUpdateModal Timezone Handling", () => {
|
||||
const mockOnClose = jest.fn();
|
||||
@@ -193,7 +195,7 @@ describe("EventUpdateModal Recurring to Non-Recurring Conversion", () => {
|
||||
it("converts recurring event to non-recurring when repeat is disabled in edit all mode", async () => {
|
||||
// Create recurring event with multiple instances
|
||||
const eventDate = new Date("2025-01-15T10:00:00.000Z");
|
||||
|
||||
|
||||
const masterEvent = {
|
||||
uid: baseUID,
|
||||
title: "Recurring Meeting",
|
||||
@@ -247,12 +249,18 @@ describe("EventUpdateModal Recurring to Non-Recurring Conversion", () => {
|
||||
};
|
||||
|
||||
// Mock API calls
|
||||
const mockDeleteEvent = jest.spyOn(EventApi, 'deleteEvent').mockResolvedValue({} as any);
|
||||
const mockPutEvent = jest.spyOn(EventApi, 'putEvent').mockResolvedValue({ status: 201, url: `/calendars/${calId}/new-event.ics` } as any);
|
||||
const consoleLogSpy = jest.spyOn(console, 'log').mockImplementation();
|
||||
|
||||
// Mock refreshCalendars
|
||||
const mockRefreshCalendars = jest.spyOn(eventUtils, 'refreshCalendars').mockResolvedValue(undefined);
|
||||
const mockDeleteEvent = jest
|
||||
.spyOn(EventApi, "deleteEvent")
|
||||
.mockResolvedValue({} as any);
|
||||
const mockPutEvent = jest.spyOn(EventApi, "putEvent").mockResolvedValue({
|
||||
status: 201,
|
||||
url: `/calendars/${calId}/new-event.ics`,
|
||||
} as any);
|
||||
jest.spyOn(CalendarApi, "getCalendar").mockResolvedValue({
|
||||
_embedded: {
|
||||
"dav:item": [],
|
||||
},
|
||||
} as any);
|
||||
|
||||
const { store } = renderWithProviders(
|
||||
<EventUpdateModal
|
||||
@@ -273,7 +281,7 @@ describe("EventUpdateModal Recurring to Non-Recurring Conversion", () => {
|
||||
// Uncheck repeat checkbox
|
||||
const repeatCheckbox = screen.getByLabelText("Repeat");
|
||||
expect(repeatCheckbox).toBeChecked();
|
||||
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(repeatCheckbox);
|
||||
});
|
||||
@@ -282,30 +290,31 @@ describe("EventUpdateModal Recurring to Non-Recurring Conversion", () => {
|
||||
|
||||
// Click Save button
|
||||
const saveButton = screen.getByText("Save");
|
||||
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(saveButton);
|
||||
// Wait for the 500ms delay in the code
|
||||
await new Promise(resolve => setTimeout(resolve, 600));
|
||||
await new Promise((resolve) => setTimeout(resolve, 600));
|
||||
});
|
||||
|
||||
// Verify API calls
|
||||
await waitFor(() => {
|
||||
expect(mockDeleteEvent).toHaveBeenCalledWith(`/calendars/${calId}/${baseUID}.ics`);
|
||||
}, { timeout: 3000 });
|
||||
// Verify API calls - should delete all instances
|
||||
await waitFor(
|
||||
() => {
|
||||
// Should have called deleteEvent for each instance (4 total: base + 3 recurrences)
|
||||
expect(mockDeleteEvent).toHaveBeenCalled();
|
||||
// At least one instance should be deleted
|
||||
expect(mockDeleteEvent.mock.calls.length).toBeGreaterThan(0);
|
||||
},
|
||||
{ timeout: 3000 }
|
||||
);
|
||||
|
||||
// Verify new event was created via putEvent
|
||||
expect(mockPutEvent).toHaveBeenCalled();
|
||||
const putEventCall = mockPutEvent.mock.calls[0][0];
|
||||
expect(putEventCall.title).toBe("Recurring Meeting");
|
||||
expect(putEventCall.repetition?.freq).toBeFalsy();
|
||||
expect(putEventCall.uid).not.toContain(baseUID);
|
||||
|
||||
// Verify console.log for successful deletion
|
||||
expect(consoleLogSpy).toHaveBeenCalledWith(
|
||||
"Deleted master event via direct API call:",
|
||||
expect.stringContaining(baseUID)
|
||||
);
|
||||
|
||||
// Verify Redux store state changes
|
||||
const finalState = store.getState();
|
||||
const calendar = finalState.calendars.list[calId];
|
||||
@@ -316,15 +325,15 @@ describe("EventUpdateModal Recurring to Non-Recurring Conversion", () => {
|
||||
expect(calendar.events[`${baseUID}/20250116`]).toBeUndefined();
|
||||
expect(calendar.events[`${baseUID}/20250117`]).toBeUndefined();
|
||||
|
||||
// Verify refreshCalendars was called
|
||||
expect(mockRefreshCalendars).toHaveBeenCalled();
|
||||
|
||||
consoleLogSpy.mockRestore();
|
||||
// Verify modal was closed after completion
|
||||
await waitFor(() => {
|
||||
expect(mockOnClose).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
it("continues creating new event even if deletion of old series fails", async () => {
|
||||
it("gracefully handles deletion errors and continues to create new event", async () => {
|
||||
const eventDate = new Date("2025-01-15T10:00:00.000Z");
|
||||
|
||||
|
||||
const masterEvent = {
|
||||
uid: baseUID,
|
||||
title: "Recurring Meeting",
|
||||
@@ -360,14 +369,16 @@ describe("EventUpdateModal Recurring to Non-Recurring Conversion", () => {
|
||||
};
|
||||
|
||||
// Mock deleteEvent to fail
|
||||
const mockDeleteEvent = jest.spyOn(EventApi, 'deleteEvent')
|
||||
jest
|
||||
.spyOn(EventApi, "deleteEvent")
|
||||
.mockRejectedValue(new Error("Network error"));
|
||||
const mockPutEvent = jest.spyOn(EventApi, 'putEvent')
|
||||
const mockPutEvent = jest
|
||||
.spyOn(EventApi, "putEvent")
|
||||
.mockResolvedValue({ status: 201 } as any);
|
||||
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();
|
||||
|
||||
const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation();
|
||||
|
||||
// Mock refreshCalendars
|
||||
jest.spyOn(eventUtils, 'refreshCalendars').mockResolvedValue(undefined);
|
||||
jest.spyOn(eventUtils, "refreshCalendars").mockResolvedValue(undefined);
|
||||
|
||||
const { store } = renderWithProviders(
|
||||
<EventUpdateModal
|
||||
@@ -387,44 +398,56 @@ describe("EventUpdateModal Recurring to Non-Recurring Conversion", () => {
|
||||
|
||||
// Uncheck repeat checkbox and save
|
||||
const repeatCheckbox = screen.getByLabelText("Repeat");
|
||||
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(repeatCheckbox);
|
||||
});
|
||||
|
||||
const saveButton = screen.getByText("Save");
|
||||
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(saveButton);
|
||||
await new Promise(resolve => setTimeout(resolve, 600));
|
||||
await new Promise((resolve) => setTimeout(resolve, 600));
|
||||
});
|
||||
|
||||
// Verify error was logged
|
||||
await waitFor(() => {
|
||||
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
||||
"Failed to delete recurring event:",
|
||||
expect.any(Error)
|
||||
);
|
||||
}, { timeout: 3000 });
|
||||
// Verify error was logged for failed deletion
|
||||
await waitFor(
|
||||
() => {
|
||||
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("Failed to delete event file")
|
||||
);
|
||||
},
|
||||
{ timeout: 3000 }
|
||||
);
|
||||
|
||||
// New implementation gracefully handles deletion errors:
|
||||
// Even if some instances fail to delete, we still create the new event
|
||||
// This ensures user gets their non-recurring event
|
||||
await waitFor(
|
||||
() => {
|
||||
expect(mockPutEvent).toHaveBeenCalled();
|
||||
},
|
||||
{ timeout: 3000 }
|
||||
);
|
||||
|
||||
// Verify new event was still created despite deletion failure
|
||||
expect(mockPutEvent).toHaveBeenCalled();
|
||||
const putEventCall = mockPutEvent.mock.calls[0][0];
|
||||
expect(putEventCall.title).toBe("Recurring Meeting");
|
||||
expect(putEventCall.repetition?.freq).toBeFalsy();
|
||||
expect(putEventCall.uid).not.toContain(baseUID);
|
||||
|
||||
// When API deletion fails, the error is caught and logged
|
||||
// The code continues to create the new event (graceful degradation)
|
||||
// Old events remain in store since API deletion failed
|
||||
// They will be cleaned up when calendar refreshes from server
|
||||
|
||||
// Modal should close after operation completes
|
||||
await waitFor(
|
||||
() => {
|
||||
expect(mockOnClose).toHaveBeenCalled();
|
||||
},
|
||||
{ timeout: 3000 }
|
||||
);
|
||||
|
||||
consoleErrorSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("refreshes calendar after converting recurring to non-recurring", async () => {
|
||||
it("closes all modals after converting recurring to non-recurring", async () => {
|
||||
const eventDate = new Date("2025-01-15T10:00:00.000Z");
|
||||
|
||||
|
||||
const masterEvent = {
|
||||
uid: baseUID,
|
||||
title: "Recurring Meeting",
|
||||
@@ -460,16 +483,22 @@ describe("EventUpdateModal Recurring to Non-Recurring Conversion", () => {
|
||||
};
|
||||
|
||||
// Mock API calls
|
||||
jest.spyOn(EventApi, 'deleteEvent').mockResolvedValue({} as any);
|
||||
jest.spyOn(EventApi, 'putEvent').mockResolvedValue({ status: 201 } as any);
|
||||
|
||||
// Mock refreshCalendars to track calls
|
||||
const mockRefreshCalendars = jest.spyOn(eventUtils, 'refreshCalendars').mockResolvedValue(undefined);
|
||||
jest.spyOn(EventApi, "deleteEvent").mockResolvedValue({} as any);
|
||||
jest.spyOn(EventApi, "putEvent").mockResolvedValue({ status: 201 } as any);
|
||||
jest.spyOn(CalendarApi, "getCalendar").mockResolvedValue({
|
||||
_embedded: {
|
||||
"dav:item": [],
|
||||
},
|
||||
} as any);
|
||||
|
||||
// Mock onCloseAll to test closing both modals
|
||||
const mockOnCloseAll = jest.fn();
|
||||
|
||||
renderWithProviders(
|
||||
<EventUpdateModal
|
||||
open={true}
|
||||
onClose={mockOnClose}
|
||||
onCloseAll={mockOnCloseAll}
|
||||
calId={calId}
|
||||
eventId={`${baseUID}/20250115`}
|
||||
typeOfAction="all"
|
||||
@@ -484,25 +513,27 @@ describe("EventUpdateModal Recurring to Non-Recurring Conversion", () => {
|
||||
|
||||
// Uncheck repeat and save
|
||||
const repeatCheckbox = screen.getByLabelText("Repeat");
|
||||
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(repeatCheckbox);
|
||||
});
|
||||
|
||||
const saveButton = screen.getByText("Save");
|
||||
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(saveButton);
|
||||
await new Promise(resolve => setTimeout(resolve, 600));
|
||||
await new Promise((resolve) => setTimeout(resolve, 600));
|
||||
});
|
||||
|
||||
// Verify refreshCalendars was called
|
||||
await waitFor(() => {
|
||||
expect(mockRefreshCalendars).toHaveBeenCalledWith(
|
||||
expect.any(Function), // dispatch
|
||||
expect.any(Array), // calendars list
|
||||
expect.any(Object) // calendar range
|
||||
);
|
||||
}, { timeout: 3000 });
|
||||
// Verify onCloseAll was called to close both preview and update modals
|
||||
await waitFor(
|
||||
() => {
|
||||
expect(mockOnCloseAll).toHaveBeenCalled();
|
||||
},
|
||||
{ timeout: 3000 }
|
||||
);
|
||||
|
||||
// Verify onClose was NOT called (onCloseAll is used instead)
|
||||
expect(mockOnClose).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -510,6 +510,10 @@ export default function EventPreviewModal({
|
||||
<EventUpdateModal
|
||||
open={openUpdateModal}
|
||||
onClose={() => setOpenUpdateModal(false)}
|
||||
onCloseAll={() => {
|
||||
setOpenUpdateModal(false);
|
||||
onClose({}, "backdropClick");
|
||||
}}
|
||||
eventId={eventId}
|
||||
calId={calId}
|
||||
typeOfAction={typeOfAction}
|
||||
|
||||
@@ -32,6 +32,7 @@ function EventUpdateModal({
|
||||
calId,
|
||||
open,
|
||||
onClose,
|
||||
onCloseAll,
|
||||
eventData,
|
||||
typeOfAction,
|
||||
}: {
|
||||
@@ -39,6 +40,7 @@ function EventUpdateModal({
|
||||
calId: string;
|
||||
open: boolean;
|
||||
onClose: (event: {}, reason: "backdropClick" | "escapeKeyDown") => void;
|
||||
onCloseAll?: () => void;
|
||||
eventData?: CalendarEvent | null;
|
||||
typeOfAction?: "solo" | "all";
|
||||
}) {
|
||||
@@ -283,8 +285,17 @@ function EventUpdateModal({
|
||||
}
|
||||
}, [open, event, calId, userPersonnalCalendars, calendarsList]);
|
||||
|
||||
// Helper to close modal(s) - use onCloseAll if available to close preview modal too
|
||||
const closeModal = () => {
|
||||
if (onCloseAll) {
|
||||
onCloseAll();
|
||||
} else {
|
||||
onClose({}, "backdropClick");
|
||||
}
|
||||
};
|
||||
|
||||
const handleClose = () => {
|
||||
onClose({}, "backdropClick");
|
||||
closeModal();
|
||||
resetAllStateToDefault();
|
||||
initializedKeyRef.current = null;
|
||||
};
|
||||
@@ -337,8 +348,94 @@ function EventUpdateModal({
|
||||
x_openpass_videoconference: meetingLink || undefined,
|
||||
};
|
||||
|
||||
// Close popup immediately for better UX
|
||||
onClose({}, "backdropClick");
|
||||
// Handle recurrence instances
|
||||
const [, recurrenceId] = event.uid.split("/");
|
||||
|
||||
// Special case: When converting recurring event to non-recurring
|
||||
// Keep modal open until all async operations complete
|
||||
if (
|
||||
recurrenceId &&
|
||||
typeOfAction === "all" &&
|
||||
event.repetition?.freq &&
|
||||
!repetition.freq
|
||||
) {
|
||||
const baseUID = event.uid.split("/")[0];
|
||||
|
||||
try {
|
||||
// STEP 1: Delete ALL instances of recurring event
|
||||
// Note: This system stores instances only, no master event file
|
||||
|
||||
// Collect all instances that need to be deleted
|
||||
const instancesToDelete = Object.keys(targetCalendar.events)
|
||||
.filter((eventId) => eventId.split("/")[0] === baseUID)
|
||||
.map((eventId) => targetCalendar.events[eventId]);
|
||||
|
||||
// Get unique URLs to avoid deleting same file multiple times
|
||||
const uniqueURLs = new Set<string>();
|
||||
const instancesByURL = new Map<string, CalendarEvent[]>();
|
||||
|
||||
instancesToDelete.forEach((instance) => {
|
||||
if (!instancesByURL.has(instance.URL)) {
|
||||
instancesByURL.set(instance.URL, []);
|
||||
}
|
||||
instancesByURL.get(instance.URL)!.push(instance);
|
||||
uniqueURLs.add(instance.URL);
|
||||
});
|
||||
|
||||
// Delete each unique URL once
|
||||
const deletePromises = Array.from(uniqueURLs).map(async (url) => {
|
||||
try {
|
||||
await deleteEvent(url);
|
||||
} catch (deleteError: any) {
|
||||
// Silently ignore 404 - file might already be deleted
|
||||
const is404 =
|
||||
deleteError.response?.status === 404 ||
|
||||
deleteError.message?.includes("404") ||
|
||||
deleteError.message?.includes("Not Found");
|
||||
|
||||
if (!is404) {
|
||||
console.error(
|
||||
`Failed to delete event file: ${deleteError.message}`
|
||||
);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
await Promise.all(deletePromises);
|
||||
|
||||
// STEP 2: Clean up all instances from Redux store
|
||||
Object.keys(targetCalendar.events).forEach((eventId) => {
|
||||
if (eventId.split("/")[0] === baseUID) {
|
||||
dispatch(removeEvent({ calendarUid: calId, eventUid: eventId }));
|
||||
}
|
||||
});
|
||||
|
||||
// STEP 3: Create new non-recurring event AFTER deletion
|
||||
// Note: putEventAsync automatically fetches calendar events after creation,
|
||||
// so we don't need to call refreshCalendars separately
|
||||
await dispatch(
|
||||
putEventAsync({
|
||||
cal: targetCalendar,
|
||||
newEvent: {
|
||||
...newEvent,
|
||||
uid: crypto.randomUUID(),
|
||||
URL: `/calendars/${newCalId || calId}/${crypto.randomUUID()}.ics`,
|
||||
},
|
||||
})
|
||||
).unwrap();
|
||||
|
||||
// STEP 4: Close modal after everything completes
|
||||
closeModal();
|
||||
} catch (err) {
|
||||
console.error("Failed to convert recurring to non-recurring:", err);
|
||||
// Keep modal open on error, user can retry or cancel
|
||||
}
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
// Close popup immediately for better UX (for non-special cases)
|
||||
closeModal();
|
||||
|
||||
// If converting from a non-repeating event to a repeating one,
|
||||
// remove the original single instance to avoid duplicates on the grid
|
||||
@@ -346,9 +443,6 @@ function EventUpdateModal({
|
||||
dispatch(removeEvent({ calendarUid: calId, eventUid: event.uid }));
|
||||
}
|
||||
|
||||
// Handle recurrence instances
|
||||
const [, recurrenceId] = event.uid.split("/");
|
||||
|
||||
// Execute API calls in background based on typeOfAction
|
||||
if (recurrenceId) {
|
||||
if (typeOfAction === "solo") {
|
||||
@@ -360,58 +454,13 @@ function EventUpdateModal({
|
||||
})
|
||||
);
|
||||
} else if (typeOfAction === "all") {
|
||||
// Update all instances
|
||||
|
||||
// Special case: When converting recurring event to non-recurring
|
||||
if (event.repetition?.freq && !repetition.freq) {
|
||||
// For repeat -> no-repeat, create a new non-repeating event
|
||||
dispatch(
|
||||
putEventAsync({
|
||||
cal: targetCalendar,
|
||||
newEvent: {
|
||||
...newEvent,
|
||||
uid: crypto.randomUUID(), // Generate new ID for the single event
|
||||
URL: `/calendars/${newCalId || calId}/${crypto.randomUUID()}.ics`,
|
||||
},
|
||||
})
|
||||
);
|
||||
|
||||
// Delete the old repeating series - we need to be more direct and forceful
|
||||
const baseUID = event.uid.split("/")[0];
|
||||
|
||||
try {
|
||||
// Directly use the deleteEvent API without going through the store
|
||||
const eventBaseURL = `/calendars/${calId}/${baseUID}.ics`;
|
||||
await deleteEvent(eventBaseURL);
|
||||
console.log(
|
||||
"Deleted master event via direct API call:",
|
||||
eventBaseURL
|
||||
);
|
||||
|
||||
// Force a small delay to ensure deletion completes
|
||||
await new Promise((resolve) => setTimeout(resolve, 500));
|
||||
|
||||
// Make sure to clean up any instances in the store
|
||||
Object.keys(targetCalendar.events).forEach((eventId) => {
|
||||
if (eventId.split("/")[0] === baseUID) {
|
||||
dispatch(
|
||||
removeEvent({ calendarUid: calId, eventUid: eventId })
|
||||
);
|
||||
}
|
||||
});
|
||||
} catch (err) {
|
||||
console.error("Failed to delete recurring event:", err);
|
||||
// Even if deletion fails, continue with creating the new event
|
||||
}
|
||||
} else {
|
||||
// Normal update for recurring events
|
||||
dispatch(
|
||||
updateSeriesAsync({
|
||||
cal: targetCalendar,
|
||||
event: { ...newEvent, recurrenceId },
|
||||
})
|
||||
);
|
||||
}
|
||||
// Normal update for recurring events
|
||||
dispatch(
|
||||
updateSeriesAsync({
|
||||
cal: targetCalendar,
|
||||
event: { ...newEvent, recurrenceId },
|
||||
})
|
||||
);
|
||||
|
||||
// Refresh calendars to ensure all instances are updated
|
||||
const calendarRange = getCalendarRange(new Date(start));
|
||||
|
||||
Reference in New Issue
Block a user