Skip to content

Commit ba58110

Browse files
tyler-daneCopilot
tyler-dane
and
Copilot
authored
Fix(web): allow user to save event without making changes (#1006)
* fix(web): log error details in handleError function for better debugging * feat(web): add utility function to create web-specific event schemas * feat(web): add WebEventParser and tests for event modification detection * refactor(web): remove unused draft conversion functions from sidebar actions * refactor(web): remove unused isEventDirty logic and related props from event forms * fix: make isSame check more robust in useDraftActions Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor(web): remove unused recurrence check methods from WebEventParser * refactor(web): remove unused openForm and discard calls in useDraftActions * refactor(web): add determineSubmitAction parameter to useDraftActions * fix: remove unnecessary deps in useDraftActions * fix(web): ensure draft ID is not optimistic in useDraftActions --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 01dbf16 commit ba58110

File tree

13 files changed

+426
-269
lines changed

13 files changed

+426
-269
lines changed

‎packages/web/src/__tests__/utils/event.util/test.event.util.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { faker } from "@faker-js/faker";
2+
import { Origin, Priorities } from "@core/constants/core.constants";
3+
import dayjs from "@core/util/date/dayjs";
4+
import { Schema_DraftEvent } from "@web/common/schemas/events/draft.event.schemas";
5+
import { Schema_WebEvent } from "@web/common/schemas/events/web.event.schemas";
6+
7+
/**
8+
* These utils focus on generating web-specific schemas.
9+
* For generating API-compatible events, see utils in `@core`
10+
*/
11+
12+
export const createWebEvent = (
13+
overrides: Partial<Schema_WebEvent> = {},
14+
): Schema_WebEvent => {
15+
const start = faker.date.future();
16+
const end = dayjs(start).add(1, "hour");
17+
18+
return {
19+
_id: faker.string.uuid(),
20+
origin: Origin.COMPASS,
21+
title: faker.lorem.sentence(),
22+
description: faker.lorem.paragraph(),
23+
startDate: start.toISOString(),
24+
endDate: end.toISOString(),
25+
priority: faker.helpers.arrayElement(Object.values(Priorities)),
26+
recurrence: undefined,
27+
user: faker.string.uuid(),
28+
...overrides,
29+
};
30+
};

‎packages/web/src/common/parsers/event.parser.test.ts‎

Lines changed: 259 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,259 @@
1+
import { faker } from "@faker-js/faker";
2+
import { Priorities } from "@core/constants/core.constants";
3+
import { createWebEvent } from "../../__tests__/utils/event.util/test.event.util";
4+
import { WebEventParser, isEventDirty } from "./event.parser";
5+
6+
describe("WebEventParser", () => {
7+
it("should return false when draft and original events are identical", () => {
8+
const originalEvent = createWebEvent();
9+
const draftEvent = createWebEvent({
10+
title: originalEvent.title,
11+
description: originalEvent.description,
12+
startDate: originalEvent.startDate,
13+
endDate: originalEvent.endDate,
14+
priority: originalEvent.priority,
15+
recurrence: originalEvent.recurrence,
16+
});
17+
18+
const parser = new WebEventParser(draftEvent, originalEvent);
19+
expect(parser.isDirty()).toBe(false);
20+
});
21+
22+
it("should return true when title has changed", () => {
23+
const originalEvent = createWebEvent();
24+
const draftEvent = createWebEvent({
25+
title: "Different Title",
26+
description: originalEvent.description,
27+
startDate: originalEvent.startDate,
28+
endDate: originalEvent.endDate,
29+
priority: originalEvent.priority,
30+
recurrence: originalEvent.recurrence,
31+
});
32+
33+
const parser = new WebEventParser(draftEvent, originalEvent);
34+
expect(parser.isDirty()).toBe(true);
35+
});
36+
37+
it("should return true when description has changed", () => {
38+
const originalEvent = createWebEvent();
39+
const draftEvent = createWebEvent({
40+
title: originalEvent.title,
41+
description: "Different Description",
42+
startDate: originalEvent.startDate,
43+
endDate: originalEvent.endDate,
44+
priority: originalEvent.priority,
45+
recurrence: originalEvent.recurrence,
46+
});
47+
48+
const parser = new WebEventParser(draftEvent, originalEvent);
49+
expect(parser.isDirty()).toBe(true);
50+
});
51+
52+
it("should return true when startDate has changed", () => {
53+
const originalEvent = createWebEvent();
54+
const draftEvent = createWebEvent({
55+
title: originalEvent.title,
56+
description: originalEvent.description,
57+
startDate: faker.date.future().toISOString(),
58+
endDate: originalEvent.endDate,
59+
priority: originalEvent.priority,
60+
recurrence: originalEvent.recurrence,
61+
});
62+
63+
const parser = new WebEventParser(draftEvent, originalEvent);
64+
expect(parser.isDirty()).toBe(true);
65+
});
66+
67+
it("should return true when endDate has changed", () => {
68+
const originalEvent = createWebEvent();
69+
const draftEvent = createWebEvent({
70+
title: originalEvent.title,
71+
description: originalEvent.description,
72+
startDate: originalEvent.startDate,
73+
endDate: faker.date.future().toISOString(),
74+
priority: originalEvent.priority,
75+
recurrence: originalEvent.recurrence,
76+
});
77+
78+
const parser = new WebEventParser(draftEvent, originalEvent);
79+
expect(parser.isDirty()).toBe(true);
80+
});
81+
82+
it("should return true when priority has changed", () => {
83+
const originalEvent = createWebEvent({ priority: Priorities.WORK });
84+
const draftEvent = createWebEvent({
85+
title: originalEvent.title,
86+
description: originalEvent.description,
87+
startDate: originalEvent.startDate,
88+
endDate: originalEvent.endDate,
89+
priority: Priorities.SELF,
90+
recurrence: originalEvent.recurrence,
91+
});
92+
93+
const parser = new WebEventParser(draftEvent, originalEvent);
94+
expect(parser.isDirty()).toBe(true);
95+
});
96+
97+
it("should return true when recurrence is added to non-recurring event", () => {
98+
const originalEvent = createWebEvent({ recurrence: undefined });
99+
const draftEvent = createWebEvent({
100+
title: originalEvent.title,
101+
description: originalEvent.description,
102+
startDate: originalEvent.startDate,
103+
endDate: originalEvent.endDate,
104+
priority: originalEvent.priority,
105+
recurrence: { rule: ["RRULE:FREQ=WEEKLY"] },
106+
});
107+
108+
const parser = new WebEventParser(draftEvent, originalEvent);
109+
expect(parser.isDirty()).toBe(true);
110+
});
111+
112+
it("should return true when recurrence is removed from recurring event", () => {
113+
const originalEvent = createWebEvent({
114+
recurrence: { rule: ["RRULE:FREQ=WEEKLY"] },
115+
});
116+
const draftEvent = createWebEvent({
117+
title: originalEvent.title,
118+
description: originalEvent.description,
119+
startDate: originalEvent.startDate,
120+
endDate: originalEvent.endDate,
121+
priority: originalEvent.priority,
122+
recurrence: undefined,
123+
});
124+
125+
const parser = new WebEventParser(draftEvent, originalEvent);
126+
expect(parser.isDirty()).toBe(true);
127+
});
128+
129+
it("should return true when recurrence rules have changed", () => {
130+
const originalEvent = createWebEvent({
131+
recurrence: { rule: ["RRULE:FREQ=WEEKLY"] },
132+
});
133+
const draftEvent = createWebEvent({
134+
title: originalEvent.title,
135+
description: originalEvent.description,
136+
startDate: originalEvent.startDate,
137+
endDate: originalEvent.endDate,
138+
priority: originalEvent.priority,
139+
recurrence: { rule: ["RRULE:FREQ=DAILY"] },
140+
});
141+
142+
const parser = new WebEventParser(draftEvent, originalEvent);
143+
expect(parser.isDirty()).toBe(true);
144+
});
145+
146+
it("should return true when recurrence rules array length has changed", () => {
147+
const originalEvent = createWebEvent({
148+
recurrence: { rule: ["RRULE:FREQ=WEEKLY"] },
149+
});
150+
const draftEvent = createWebEvent({
151+
title: originalEvent.title,
152+
description: originalEvent.description,
153+
startDate: originalEvent.startDate,
154+
endDate: originalEvent.endDate,
155+
priority: originalEvent.priority,
156+
recurrence: { rule: ["RRULE:FREQ=WEEKLY", "RRULE:BYDAY=MO"] },
157+
});
158+
159+
const parser = new WebEventParser(draftEvent, originalEvent);
160+
expect(parser.isDirty()).toBe(true);
161+
});
162+
163+
it("should return true when dates change in recurring event", () => {
164+
const originalEvent = createWebEvent({
165+
recurrence: { rule: ["RRULE:FREQ=WEEKLY"] },
166+
startDate: "2024-01-01T10:00:00Z",
167+
endDate: "2024-01-01T11:00:00Z",
168+
});
169+
const draftEvent = createWebEvent({
170+
title: originalEvent.title,
171+
description: originalEvent.description,
172+
startDate: "2024-01-01T11:00:00Z", // Different start time
173+
endDate: "2024-01-01T12:00:00Z", // Different end time
174+
priority: originalEvent.priority,
175+
recurrence: { rule: ["RRULE:FREQ=WEEKLY"] }, // Same recurrence
176+
});
177+
178+
const parser = new WebEventParser(draftEvent, originalEvent);
179+
expect(parser.isDirty()).toBe(true);
180+
});
181+
182+
it("should return false when only non-tracked fields change", () => {
183+
const originalEvent = createWebEvent();
184+
const draftEvent = createWebEvent({
185+
title: originalEvent.title,
186+
description: originalEvent.description,
187+
startDate: originalEvent.startDate,
188+
endDate: originalEvent.endDate,
189+
priority: originalEvent.priority,
190+
recurrence: originalEvent.recurrence,
191+
user: "different-user", // This field is not tracked
192+
});
193+
194+
const parser = new WebEventParser(draftEvent, originalEvent);
195+
expect(parser.isDirty()).toBe(false);
196+
});
197+
198+
it("should handle undefined recurrence gracefully", () => {
199+
const originalEvent = createWebEvent({ recurrence: undefined });
200+
const draftEvent = createWebEvent({
201+
title: originalEvent.title,
202+
description: originalEvent.description,
203+
startDate: originalEvent.startDate,
204+
endDate: originalEvent.endDate,
205+
priority: originalEvent.priority,
206+
recurrence: undefined,
207+
});
208+
209+
const parser = new WebEventParser(draftEvent, originalEvent);
210+
expect(parser.isDirty()).toBe(false);
211+
});
212+
213+
it("should handle empty recurrence rules", () => {
214+
const originalEvent = createWebEvent({
215+
recurrence: { rule: [] },
216+
});
217+
const draftEvent = createWebEvent({
218+
title: originalEvent.title,
219+
description: originalEvent.description,
220+
startDate: originalEvent.startDate,
221+
endDate: originalEvent.endDate,
222+
priority: originalEvent.priority,
223+
recurrence: { rule: [] },
224+
});
225+
226+
const parser = new WebEventParser(draftEvent, originalEvent);
227+
expect(parser.isDirty()).toBe(false);
228+
});
229+
});
230+
231+
describe("isDraftDirty", () => {
232+
it("should work as a standalone function", () => {
233+
const originalEvent = createWebEvent();
234+
const draftEvent = createWebEvent({
235+
title: "Different Title",
236+
description: originalEvent.description,
237+
startDate: originalEvent.startDate,
238+
endDate: originalEvent.endDate,
239+
priority: originalEvent.priority,
240+
recurrence: originalEvent.recurrence,
241+
});
242+
243+
expect(isEventDirty(draftEvent, originalEvent)).toBe(true);
244+
});
245+
246+
it("should return false for identical events", () => {
247+
const originalEvent = createWebEvent();
248+
const draftEvent = createWebEvent({
249+
title: originalEvent.title,
250+
description: originalEvent.description,
251+
startDate: originalEvent.startDate,
252+
endDate: originalEvent.endDate,
253+
priority: originalEvent.priority,
254+
recurrence: originalEvent.recurrence,
255+
});
256+
257+
expect(isEventDirty(draftEvent, originalEvent)).toBe(false);
258+
});
259+
});

‎packages/web/src/common/parsers/event.parser.ts‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
import { Schema_WebEvent } from "../types/web.event.types";
2+
3+
/**
4+
* Parser for determining if an event has been modified (is dirty)
5+
*/
6+
export class WebEventParser {
7+
private readonly curr: Schema_WebEvent;
8+
private readonly orig: Schema_WebEvent;
9+
10+
constructor(curr: Schema_WebEvent, orig: Schema_WebEvent) {
11+
this.curr = curr;
12+
this.orig = orig;
13+
}
14+
15+
/**
16+
* Public method to check if the event is dirty (has been modified)
17+
*/
18+
public isDirty(): boolean {
19+
// Compare relevant fields that can change in the form
20+
const fieldsToCompare = [
21+
"title",
22+
"description",
23+
"startDate",
24+
"endDate",
25+
"priority",
26+
"recurrence",
27+
] as const;
28+
29+
return fieldsToCompare.some((field) => {
30+
const current = this.curr[field];
31+
const original = this.orig[field];
32+
const isRecurrenceField = field === "recurrence";
33+
34+
return isRecurrenceField
35+
? this.isRecurrenceChanged()
36+
: current !== original;
37+
});
38+
}
39+
40+
/**
41+
* Private method to check if recurrence has changed
42+
*/
43+
private isRecurrenceChanged(): boolean {
44+
const isDateChanged = this.isDateChanged();
45+
const oldRecurrence = this.orig?.recurrence?.rule ?? [];
46+
const newRecurrence = this.curr?.recurrence?.rule ?? [];
47+
48+
// Check if recurrence existence has changed
49+
const oldHasRecurrence =
50+
Array.isArray(oldRecurrence) && oldRecurrence.length > 0;
51+
const newHasRecurrence =
52+
Array.isArray(newRecurrence) && newRecurrence.length > 0;
53+
54+
if (oldHasRecurrence !== newHasRecurrence) {
55+
return true;
56+
}
57+
58+
// If both have recurrence, compare the rules
59+
if (oldHasRecurrence && newHasRecurrence) {
60+
// First check if the arrays are different in length or content
61+
if (oldRecurrence.length !== newRecurrence.length) {
62+
return true;
63+
}
64+
65+
// Check if any rule is different
66+
for (let i = 0; i < oldRecurrence.length; i++) {
67+
if (oldRecurrence[i] !== newRecurrence[i]) {
68+
return true;
69+
}
70+
}
71+
72+
// Also check for date changes
73+
return isDateChanged;
74+
}
75+
76+
// If neither has recurrence, only check date changes
77+
return isDateChanged;
78+
}
79+
80+
/**
81+
* Private method to check if start or end dates have changed
82+
*/
83+
private isDateChanged(): boolean {
84+
const oldStartDate = this.orig?.startDate;
85+
const newStartDate = this.curr?.startDate;
86+
const oldEndDate = this.orig?.endDate;
87+
const newEndDate = this.curr?.endDate;
88+
89+
return oldStartDate !== newStartDate || oldEndDate !== newEndDate;
90+
}
91+
}
92+
93+
export const isEventDirty = (
94+
curr: Schema_WebEvent,
95+
orig: Schema_WebEvent,
96+
): boolean => {
97+
const parser = new WebEventParser(curr, orig);
98+
return parser.isDirty();
99+
};

‎packages/web/src/common/utils/event.util.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,8 @@ export const handleError = (error: Error) => {
150150
return;
151151
}
152152

153+
console.error(error);
154+
153155
if (code === Status.INTERNAL_SERVER) {
154156
alert("Something went wrong behind the scenes. Please try again later.");
155157
window.location.reload();

0 commit comments

Comments
 (0)