-
-
Notifications
You must be signed in to change notification settings - Fork 186
feat: blocked time layer metadata for events #287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
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 WalkthroughIntroduces a structured Metadata type with an optional blockedTime field and propagates blockedTime support across Google and Microsoft calendar providers, including parsing from and serializing to extended properties. Updates schemas to validate new metadata fields and adjusts Microsoft provider requests to expand singleValueExtendedProperties. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
| function parseBlockedTime(event: GoogleCalendarEvent) { | ||
| const extendedProperties = event.extendedProperties; | ||
| if (!extendedProperties?.private && !extendedProperties?.shared) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const blockedTimeData = | ||
| extendedProperties.private?.blockedTime || | ||
| extendedProperties.shared?.blockedTime; | ||
|
|
||
| if (!blockedTimeData) { | ||
| return undefined; | ||
| } | ||
|
|
||
| try { | ||
| const parsed = JSON.parse(blockedTimeData); | ||
| const result: { before?: number; after?: number } = {}; | ||
|
|
||
| if (typeof parsed.before === "number" && parsed.before > 0) { | ||
| result.before = parsed.before; | ||
| } | ||
| if (typeof parsed.after === "number" && parsed.after > 0) { | ||
| result.after = parsed.after; | ||
| } | ||
|
|
||
| return Object.keys(result).length > 0 ? result : undefined; | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you instead do the metadata using superjson (use the instance that is already declared in utils/superjson.ts, and zod/v3. We already use these with tRPC.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🧹 Nitpick comments (5)
packages/api/src/interfaces/events.ts (1)
29-34: Define a reusable BlockedTime type and document unitsTo make the contract unambiguous across providers and schemas, explicitly document the unit (minutes?) and re-use a shared type. This avoids drift with Zod schemas and provider implementations.
Apply this diff here:
export interface Metadata extends Record<string, unknown> { - blockedTime?: { - before?: number; - after?: number; - }; + /** + * Client-side buffer around the event in minutes. + * - before: minutes to block before start (must be a positive integer) + * - after: minutes to block after end (must be a positive integer) + */ + blockedTime?: BlockedTime; } + +/** Blocked/buffer time in minutes around an event. */ +export type BlockedTime = Readonly<{ + before?: number; + after?: number; +}>;Follow-up (optional): If you want stronger guarantees in TS, introduce a branded PositiveInt and use it for
before/after.packages/api/src/providers/calendars/microsoft-calendar/events.ts (1)
250-257: Minor: simplify blockedTime extraction from metadataCurrent type guards are verbose. Optional, but this reads cleaner and avoids repeated index checks.
Apply this diff:
- const blockedTimeProperties = - event.metadata && - "blockedTime" in event.metadata && - event.metadata.blockedTime - ? toMicrosoftBlockedTime( - event.metadata.blockedTime as { before?: number; after?: number }, - ) - : undefined; + const blockedTime = (event.metadata as MicrosoftEventMetadata | undefined) + ?.blockedTime as { before?: number; after?: number } | undefined; + const blockedTimeProperties = blockedTime + ? toMicrosoftBlockedTime(blockedTime) + : undefined;packages/api/src/providers/calendars/google-calendar/events.ts (3)
86-115: Robust JSON parsing and validation for blockedTime looks good; consider coercion from numeric stringsThe parser is defensive and returns undefined on invalid input. If you expect numeric strings (e.g.,
"15"), you can optionally coerce withNumber()and validate.Apply this diff if coercion is desirable:
- if (typeof parsed.before === "number" && parsed.before > 0) { + const before = typeof parsed.before === "string" ? Number(parsed.before) : parsed.before; + if (typeof before === "number" && Number.isFinite(before) && before > 0) { - result.before = parsed.before; + result.before = before; } - if (typeof parsed.after === "number" && parsed.after > 0) { - result.after = parsed.after; + const after = typeof parsed.after === "string" ? Number(parsed.after) : parsed.after; + if (typeof after === "number" && Number.isFinite(after) && after > 0) { + result.after = after; }
171-176: Potential duplication: blockedTime exists both as a parsed object and raw JSON in private/sharedYou currently include raw
extendedProperties.private/shared(which may contain theblockedTimeJSON string) and also a normalizedmetadata.blockedTime. That duplication can cause confusion when round-tripping.Either:
- strip
blockedTimefrom the rawprivate/sharedmaps when you also emitmetadata.blockedTime, or- keep both but prefer the normalized
metadata.blockedTimein serializers (see next comments).
I can provide a small sanitization helper if you want it.
208-217: Write to private vs shared: pick location deterministicallyYou always write to
extendedProperties.private. If the original event hadblockedTimeinshared, or if you want others to see it, consider preserving the source bucket or allowing a policy (default: private, unless shared already present).Example change (preserve existing location):
-function toGoogleCalendarBlockedTime(blockedTime: { +function toGoogleCalendarBlockedTime(blockedTime: { before?: number; after?: number; -}) { - return { - private: { - blockedTime: JSON.stringify(blockedTime), - }, - }; +}, target: "private" | "shared" = "private") { + const payload = { blockedTime: JSON.stringify(blockedTime) }; + return target === "shared" ? { shared: payload } : { private: payload }; }And pass the target based on
event.metadata?.shared?.blockedTimepresence.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/api/src/interfaces/events.ts(1 hunks)packages/api/src/providers/calendars/google-calendar/events.ts(5 hunks)packages/api/src/providers/calendars/microsoft-calendar.ts(3 hunks)packages/api/src/providers/calendars/microsoft-calendar/events.ts(5 hunks)packages/api/src/schemas/events.ts(2 hunks)
🔇 Additional comments (4)
packages/api/src/interfaces/events.ts (1)
23-27: Type alignment for CalendarEvent.metadata looks goodChanging
metadata?: Record<string, unknown>tometadata?: Metadatanarrows usages and enables typed access toblockedTimewhile preserving extensibility via the index signature. No functional issues spotted.packages/api/src/schemas/events.ts (1)
239-239: Confirm schema union is still exhaustive for metadata
metadatais a union of Microsoft and Google variants. With the addition ofprivate/sharedon Google andblockedTimeon both, this union remains accurate. Just ensure callers don’t attempt to pass a cross-provider mix.If you anticipate generic flows preparing
metadatawithout knowing the provider, consider a third “provider-agnostic” schema with only the shared fields (e.g.,blockedTime) to reduce friction when constructing requests upstream.packages/api/src/providers/calendars/microsoft-calendar/events.ts (1)
275-277: Guard against overwriting unrelated extended properties on updateWhen updating, if other
singleValueExtendedPropertiesexist, setting this field will replace the collection. If your update payload includes only blockedTime, consider merging with any known existing values (when your client has them) or limiting PATCH to only fields you intend to change.Would you like me to draft a helper that merges outgoing extended properties with the existing set when available, to avoid accidental deletion?
packages/api/src/providers/calendars/google-calendar/events.ts (1)
136-137: LGTM: blockedTime is conditionally merged into metadataNice, this keeps metadata concise and only adds
blockedTimewhen present.
| const blockedTimeExtendedProperties = | ||
| event.metadata && | ||
| "blockedTime" in event.metadata && | ||
| event.metadata.blockedTime | ||
| ? toGoogleCalendarBlockedTime( | ||
| event.metadata.blockedTime as { before?: number; after?: number }, | ||
| ) | ||
| : undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Compute extendedProperties once and merge with any user-provided maps
Right now you only include extendedProperties when blockedTime is present, and you don’t merge other keys from metadata.private/metadata.shared. On updates, this risks overwriting/losing existing keys.
Apply this diff to merge both sources:
- const blockedTimeExtendedProperties =
- event.metadata &&
- "blockedTime" in event.metadata &&
- event.metadata.blockedTime
- ? toGoogleCalendarBlockedTime(
- event.metadata.blockedTime as { before?: number; after?: number },
- )
- : undefined;
+ const md = event.metadata as
+ | { blockedTime?: { before?: number; after?: number }; private?: Record<string, string>; shared?: Record<string, string> }
+ | undefined;
+ const blockedTimeProps = md?.blockedTime
+ ? toGoogleCalendarBlockedTime(md.blockedTime, md?.shared?.blockedTime ? "shared" : "private")
+ : undefined;
+ const mergedExtendedProps =
+ md?.private || md?.shared || blockedTimeProps
+ ? {
+ private: { ...(md?.private ?? {}), ...(blockedTimeProps?.private ?? {}) },
+ shared: { ...(md?.shared ?? {}), ...(blockedTimeProps?.shared ?? {}) },
+ }
+ : undefined;Then below, emit extendedProperties: mergedExtendedProps when defined.
🤖 Prompt for AI Agents
In packages/api/src/providers/calendars/google-calendar/events.ts around lines
222-229, compute a single merged extendedProperties object instead of only
emitting when blockedTime exists and avoid clobbering user maps: create new
objects for mergedPrivate and mergedShared by shallow-copying metadata.private
and metadata.shared (if present), convert blockedTime via
toGoogleCalendarBlockedTime when present and merge its keys into the
mergedPrivate (or mergedShared if your convention dictates) map without mutating
originals, then set mergedExtendedProps = { private: mergedPrivate, shared:
mergedShared } and emit extendedProperties: mergedExtendedProps only when at
least one of those maps is non-empty.
| ...(blockedTimeExtendedProperties && { | ||
| extendedProperties: blockedTimeExtendedProperties, | ||
| }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Emit merged extendedProperties to avoid data loss on updates
Use the merged object from above rather than only blockedTime-derived properties.
Apply this diff:
- ...(blockedTimeExtendedProperties && {
- extendedProperties: blockedTimeExtendedProperties,
- }),
+ ...(mergedExtendedProps && { extendedProperties: mergedExtendedProps }),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ...(blockedTimeExtendedProperties && { | |
| extendedProperties: blockedTimeExtendedProperties, | |
| }), | |
| // … other props … | |
| - ...(blockedTimeExtendedProperties && { | |
| - extendedProperties: blockedTimeExtendedProperties, | |
| ...(mergedExtendedProps && { extendedProperties: mergedExtendedProps }), | |
| // … following props … |
🤖 Prompt for AI Agents
In packages/api/src/providers/calendars/google-calendar/events.ts around lines
251-253, the code currently sets extendedProperties to only
blockedTimeExtendedProperties which overwrites/loses previously merged
properties; change it to emit the merged extendedProperties object created
earlier (e.g., mergedExtendedProperties or the variable name used above) so the
final payload contains the full merged set of properties instead of only the
blocked-time subset, and keep the conditional presence check (only include
extendedProperties when the merged object exists).
| .filter( | ||
| `start/dateTime ge '${startTime}' and end/dateTime le '${endTime}'`, | ||
| ) | ||
| .expand("singleValueExtendedProperties") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
$expand for singleValueExtendedProperties should include an $filter and use a stable property ID
Microsoft Graph generally requires filtering extended properties by id when expanding; expanding without a filter is unsupported/inefficient and often rejected. Also, your current writer uses a random GUID for the property set ID, making it impossible to filter/round‑trip reliably.
Apply this diff (assumes you export a stable constant MICROSOFT_BLOCKED_TIME_PROP_ID from ./microsoft-calendar/events):
- .expand("singleValueExtendedProperties")
+ .expand(
+ `singleValueExtendedProperties($filter=id eq '${MICROSOFT_BLOCKED_TIME_PROP_ID}')`
+ )This ensures the events list returns only the blockedTime property and keeps OData semantics correct.
🌐 Web query:
Does Microsoft Graph require $filter on $expand=singleValueExtendedProperties for events? Provide an official doc snippet or example.
💡 Result:
Yes, Microsoft Graph requires a $filter when using $expand=singleValueExtendedProperties for events if you want to retrieve specific extended properties. This is explicitly documented in the official Microsoft Graph docs and confirmed by multiple support threads.
Official documentation states:
Using the query parameter
$expandallows you to get the specified resource instance expanded with a specific extended property. Use a$filterandeqoperator on the id property to specify the extended property. This is currently the only way to get the singleValueLegacyExtendedProperty object that represents an extended property[5].
Example from the official docs:
GET https://graph.microsoft.com/v1.0/me/events?$expand=singleValueExtendedProperties($filter=id eq 'String {GUID} Name CustomProperty')This request will expand the event with only the extended property matching the given id[5][8].
Community threads and support answers clarify:
- You cannot simply include all singleValueExtendedProperties by using
$expand=singleValueExtendedPropertieswithout a filter; you must specify the property id using a filter[3][8]. - Without a filter, the response will not include extended properties[3].
- This is by design; the API requires you to explicitly filter for the property you want to expand for events (and other resource types like messages)[5][8].
Representative code snippet from a support answer:
.Expand("singleValueExtendedProperties($filter=id eq 'String {GUID} Name MyExtendedProperty')")Retrieves only the extended property with the specified id for each event[8].
In summary:
Yes, a $filter is required on $expand=singleValueExtendedProperties when querying events in Microsoft Graph—there is no supported way to expand all extended properties without specifying a filter by property ID[3][5][8].
Reference - official documentation snippet:
"Use a
$filterandeqoperator on the id property to specify the extended property. This is currently the only way to get the singleValueLegacyExtendedProperty object that represents an extended property."
— [Microsoft Graph Docs: Get singleValueLegacyExtendedProperty][5]
If you need to retrieve all singleValueExtendedProperties for an event, the Graph API currently does not support a wildcard or omitting the filter; you must query those properties individually by their ID[3][5][8].
Citations:
- 1: https://learn.microsoft.com/en-us/answers/questions/1809278/issue-with-retrieving-singlevalueextendedpropertie
- 2: https://learn.microsoft.com/en-gb/answers/questions/2084527/how-to-expand-singlevalueextendedproperties-on-mes
- 3: https://learn.microsoft.com/en-us/answers/questions/1467708/when-using-singlevalueextendedproperties-can-i-get
- 4: https://learn.microsoft.com/en-us/answers/questions/2282685/cannot-get-singlevalueextendedproperties-from-to-t
- 5: https://learn.microsoft.com/en-us/graph/api/singlevaluelegacyextendedproperty-get?view=graph-rest-1.0
- 6: https://learn.microsoft.com/en-us/graph/api/singlevaluelegacyextendedproperty-post-singlevalueextendedproperties?view=graph-rest-1.0
- 7: https://learn.microsoft.com/en-us/answers/questions/1118527/ms-graph-api-filter-calendar-events
- 8: https://learn.microsoft.com/en-us/answers/questions/462964/how-to-expand-microsoft-graph-extended-properties
- 9: https://learn.microsoft.com/en-us/answers/questions/2278386/extended-properties-missing-from-attendee-calendar
- 10: https://learn.microsoft.com/en-us/answers/questions/1420756/add-singlevalueextendedproperties-or-open-extensio
Include $filter on singleValueExtendedProperties expand and use a stable property ID
Microsoft Graph requires specifying a $filter on the id when expanding singleValueExtendedProperties; expanding without a filter returns no extended properties. Additionally, avoid using a random GUID—define and reuse a stable constant MICROSOFT_BLOCKED_TIME_PROP_ID to reliably round-trip your custom property. (learn.microsoft.com)
Key location:
- packages/api/src/providers/calendars/microsoft-calendar.ts, around line 121: update the
.expandcall.
Suggested diff (assuming you export MICROSOFT_BLOCKED_TIME_PROP_ID from ./microsoft-calendar/events):
- .expand("singleValueExtendedProperties")
+ .expand(
+ `singleValueExtendedProperties($filter=id eq '${MICROSOFT_BLOCKED_TIME_PROP_ID}')`
+ )This ensures the events list returns only the blockedTime property and conforms to OData requirements.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .expand("singleValueExtendedProperties") | |
| .expand( | |
| `singleValueExtendedProperties($filter=id eq '${MICROSOFT_BLOCKED_TIME_PROP_ID}')` | |
| ) |
🤖 Prompt for AI Agents
In packages/api/src/providers/calendars/microsoft-calendar.ts around line 121,
the current .expand("singleValueExtendedProperties") call must specify a $filter
on the extended property id and use a stable property id constant; import
MICROSOFT_BLOCKED_TIME_PROP_ID from ./microsoft-calendar/events and replace the
expand call with one that filters singleValueExtendedProperties by id equal to
that constant so the API returns the custom blockedTime property reliably.
| .expand("singleValueExtendedProperties") | ||
| .post(toMicrosoftEvent(event)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Remove $expand from POST create; prefer return=representation if you need the created entity
OData options like $expand are not guaranteed on POST. To receive the created event, rely on Prefer: return=representation. Since you already serialize singleValueExtendedProperties in the request body, Graph will persist them; if you need them immediately, follow with a GET using $expand + $filter.
Apply this diff:
- .api(`${calendarPath(calendar.id)}/events`)
- .expand("singleValueExtendedProperties")
- .post(toMicrosoftEvent(event));
+ .api(`${calendarPath(calendar.id)}/events`)
+ .header("Prefer", 'return=representation')
+ .post(toMicrosoftEvent(event));Optional: If you must guarantee the extended property in the response, perform a subsequent GET with the same $expand($filter=...).
🌐 Web query:
Does Microsoft Graph support using $expand on POST requests? What is the recommended way to get a representation of the created entity, including extended properties?
💡 Result:
Microsoft Graph does not support using $expand on POST requests; $expand is limited to GET queries and cannot be used to get expanded data in the response of a create (POST) call[7][1]. To obtain a representation of the newly created entity—including extended properties—the recommended approach is to issue a GET request for the entity and use $expand in that request.
Details
-
$expand in Microsoft Graph:
The$expandquery parameter is documented as only working for GET requests where you fetch an entity and want to include its related (navigation) properties[1][7]. The documentation and SDK examples show usage only with GET endpoints, e.g.GET /me/messages/{message-id}?$expand=attachments[1][7]. There is no mention or example of$expandwith POST[1][7]. -
POST request behavior:
When you create an entity via a POST request (e.g., creating a message, event, etc. with extended properties), the API response generally contains only the representation of the entity itself—not its expanded navigation properties or extended properties unless those are part of the main entity or specified in the request body[2][5]. -
Extended properties (single- and multi-value):
To add extended properties when creating an entity, you include them in the initial POST request body[5][2]. However, to read all extended properties on the entity (especially those that are part of navigation properties), you need to do a follow-up GET with$expand[6][9]. For example:- After creating a message with extended properties, make a
GET /messages/{id}?$expand=singleValueExtendedProperties,multiValueExtendedPropertiesrequest to retrieve the full representation including those properties[6][9]. - This is described as "the only way to get the multiValueLegacyExtendedProperty object that represents an extended property"[6].
- After creating a message with extended properties, make a
Workflow Recommendation
- POST to create the entity (including any extended properties in the request body, if supported)[5][2].
- GET the entity with
$expandto retrieve the full representation, including extended properties:This pattern is necessary becauseGET /messages/{id}?$expand=singleValueExtendedProperties,multiValueExtendedProperties$expandis not permitted or supported in POST requests[7][1][6].
Additional Considerations
- This limitation applies to all query options in Graph: only certain ones (e.g.
$select,$expand,$filter) are permitted in GET requests[7][1]. - For some resources, extended properties must be specifically requested via
$expandor otherwise are not included in the default GET response[6][9].
Summary Table:
| HTTP Verb | $expand Supported? | How to get extended properties in response? |
|---|---|---|
| POST | No | POST with extended properties; follow with GET+expand |
| GET | Yes | Use $expand in query string |
This approach is explicitly confirmed in Microsoft Graph documentation regarding extended properties and $expand usage[1][5][6][7][9].
Citations:
- 1: https://learn.microsoft.com/en-us/graph/sdks/create-requests
- 2: https://github.com/microsoftgraph/microsoft-graph-docs-contrib/blob/main/api-reference/v1.0/api/multivaluelegacyextendedproperty-post-multivalueextendedproperties.md
- 3: https://learn.microsoft.com/en-us/graph/api/application-post-extensionproperty?view=graph-rest-1.0
- 4: https://learn.microsoft.com/en-us/answers/questions/1010008/filtering-and-expanding-users
- 5: https://learn.microsoft.com/en-us/graph/api/singlevaluelegacyextendedproperty-post-singlevalueextendedproperties?view=graph-rest-1.0
- 6: https://learn.microsoft.com/en-us/graph/api/multivaluelegacyextendedproperty-get?view=graph-rest-1.0
- 7: https://learn.microsoft.com/en-us/graph/query-parameters
- 8: https://learn.microsoft.com/en-us/graph/api/resources/extended-properties-overview?view=graph-rest-1.0
- 9: https://github.com/microsoftgraph/microsoft-graph-docs-contrib/blob/main/api-reference/beta/api/singlevaluelegacyextendedproperty-get.md
- 10: https://learn.microsoft.com/en-us/answers/questions/1015788/expand-sometimes-does-not-return-all-the-data
Remove unsupported $expand on POST and use Prefer: return=representation with subsequent GET for extended properties
The $expand query option is only supported on GET operations and has no effect on POST requests, so it should be removed from the create call. (learn.microsoft.com)
To receive the created event entity in the response, rely on the default representation returned by POST (or explicitly request it) using the Prefer: return=representation header. If you don’t need the response body, you can request return=minimal, but for creation you typically want the representation. (docs.oasis-open.org, learn.microsoft.com)
Extended properties included in the request body are persisted but not returned in the POST response. To retrieve them immediately, issue a follow-up GET on the event with $expand=singleValueExtendedProperties (and any necessary $filter). (dzone.com)
Apply this diff in microsoft-calendar.ts:
- .api(`${calendarPath(calendar.id)}/events`)
- .expand("singleValueExtendedProperties")
- .post(toMicrosoftEvent(event));
+ .api(`${calendarPath(calendar.id)}/events`)
+ .header("Prefer", "return=representation")
+ .post(toMicrosoftEvent(event));Optional: to guarantee extended properties in the response, perform:
const created = await client
.api(`${calendarPath(calendar.id)}/events/${createdEvent.id}`)
.expand("singleValueExtendedProperties($filter=id eq '<your-prop-id>')")
.get();🤖 Prompt for AI Agents
In packages/api/src/providers/calendars/microsoft-calendar.ts around lines
139-140, remove the unsupported .expand("singleValueExtendedProperties") from
the POST call, add the HTTP header Prefer: return=representation to ensure the
created event is returned (or Prefer: return=minimal if you want no body), and
after the POST issue a follow-up GET to retrieve extended properties (e.g. GET
the created event with .expand("singleValueExtendedProperties($filter=id eq
'<prop-id>')") or appropriate $filter) so extended properties persisted on
create are returned.
| .expand("singleValueExtendedProperties") | ||
| .patch(toMicrosoftEvent(event)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Remove $expand from PATCH update; ensure you get a representation
Similar to create, $expand on PATCH is not reliable. If you need the updated entity, use Prefer: return=representation, or re-fetch via GET with $expand($filter=...).
Apply this diff:
- .api(`${calendarPath(calendar.id)}/events/${eventId}`)
- .expand("singleValueExtendedProperties")
- .patch(toMicrosoftEvent(event));
+ .api(`${calendarPath(calendar.id)}/events/${eventId}`)
+ .header("Prefer", 'return=representation')
+ .patch(toMicrosoftEvent(event));Follow-up: If the API still returns 204 No Content depending on resource, perform a GET to fetch the updated record with $expand($filter=...).
🌐 Web query:
When updating an event via Microsoft Graph, does PATCH return a representation by default, or is Prefer: return=representation required? Cite official docs.
💡 Result:
When updating an event via Microsoft Graph using PATCH, the response does not return a resource representation by default; instead, it returns a 204 No Content by default unless you include the Prefer: return=representation header[2]. The official documentation for PATCHing events does not specify that a representation is returned unless the Prefer header is used[10].
Supporting details:
- According to Microsoft’s OData-conformant endpoint behavior, PATCH (for resources like users, events, planner plans, and others) typically responds with 204 No Content unless Prefer: return=representation is specified[2][8]. This is also observed for event update operations.
- The GitHub version of the official event-update doc and API request samples show PATCH requests without the Prefer header, and in practice, these result in a 204 response.
- An official Microsoft tech community discussion confirms: "When patching a user (and probably many other things), the Graph API will always return 204 No Content, even when adding the header Prefer: return=representation. This however is not following the OData specification"[2]. This suggests even specifying Prefer might not always work as expected for certain resources, but the documented intent is that Prefer is required for representation.
In summary: PATCHing an event in Microsoft Graph returns 204 No Content by default; use Prefer: return=representation if you require the updated resource returned in the response[2][10].
Citations:
- 1: https://learn.microsoft.com/en-us/answers/questions/2200306/urgent-issue-with-patching-of-calendar-invites-on
- 2: https://techcommunity.microsoft.com/t5/microsoft-365-developer-platform/graph-api-should-return-full-content-when-patching-with-prefer/idi-p/3771966
- 3: https://llvm.org/docs/LangRef.html
- 4: https://learn.microsoft.com/en-us/answers/questions/1192822/how-to-update-an-occurrence-of-a-meeting-using-gra
- 5: https://learn.microsoft.com/en-us/graph/api/plannerplandetails-update?view=graph-rest-1.0
- 6: https://www.keycloak.org/docs/latest/upgrading/index.html
- 7: https://learn.microsoft.com/en-us/graph/api/group-update-event?view=graph-rest-1.0
- 8: https://learn.microsoft.com/en-us/graph/api/plannerplan-update?view=graph-rest-1.0
- 9: https://cloud.google.com/chronicle/docs/reference/udm-field-list
- 10: https://github.com/microsoftgraph/microsoft-graph-docs-contrib/blob/main/api-reference/v1.0/api/event-update.md
Ensure PATCH returns the updated event by replacing .expand() with Prefer: return=representation
The Graph API’s PATCH method does not return a resource by default (it responds with HTTP 204 No Content unless you include a Prefer: return=representation header). Relying on .expand("singleValueExtendedProperties") during a PATCH is therefore ineffective.
• Location: packages/api/src/providers/calendars/microsoft-calendar.ts, around lines 167–168
• Issue: .expand(...) on a PATCH request is ignored and will not yield the updated entity.
Apply this fix:
- .api(`${calendarPath(calendar.id)}/events/${eventId}`)
- .expand("singleValueExtendedProperties")
- .patch(toMicrosoftEvent(event));
+ .api(`${calendarPath(calendar.id)}/events/${eventId}`)
+ .header("Prefer", "return=representation")
+ .patch(toMicrosoftEvent(event));Follow-up: If you still receive 204 No Content (which can occur for some resources even with the header), immediately issue a GET on the same endpoint using:
.api(`${calendarPath(calendar.id)}/events/${eventId}`)
.expand("singleValueExtendedProperties")
.get();to retrieve the fully populated event.
🤖 Prompt for AI Agents
In packages/api/src/providers/calendars/microsoft-calendar.ts around lines
167–168, the PATCH call currently uses .expand("singleValueExtendedProperties")
which is ignored by Graph PATCH (it returns 204 No Content by default); replace
the .expand usage on the PATCH with adding the request header "Prefer:
return=representation" so the PATCH returns the updated event (including
singleValueExtendedProperties) and, as a fallback if you still receive 204,
immediately issue a GET to the same event endpoint with
.expand("singleValueExtendedProperties") to retrieve the fully populated event.
| function parseBlockedTime(event: MicrosoftEvent) { | ||
| if (!event.singleValueExtendedProperties) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const blockedTimeProperty = event.singleValueExtendedProperties.find( | ||
| (prop) => prop && prop.id && prop.id.includes("Name blockedTime"), | ||
| ); | ||
|
|
||
| if (!blockedTimeProperty?.value) { | ||
| return undefined; | ||
| } | ||
|
|
||
| try { | ||
| const parsed = JSON.parse(blockedTimeProperty.value); | ||
| const result: { before?: number; after?: number } = {}; | ||
|
|
||
| if (typeof parsed.before === "number" && parsed.before > 0) { | ||
| result.before = parsed.before; | ||
| } | ||
| if (typeof parsed.after === "number" && parsed.after > 0) { | ||
| result.after = parsed.after; | ||
| } | ||
|
|
||
| return Object.keys(result).length > 0 ? result : undefined; | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Parsing extended properties should target a stable property ID, not a name substring
Filtering by prop.id.includes("Name blockedTime") is brittle and couples correctness to server-side behavior you don’t control. Use an exact, stable extended property ID and ensure your GET $expand filters by it.
Apply this diff (and export the constant for reuse by the provider’s GET):
+// Use a stable property set GUID that is constant for your app,
+// not a per-event random UUID. Generate once and commit it.
+export const MICROSOFT_BLOCKED_TIME_PROP_ID =
+ "String {f8a0fe66-219a-4e34-93da-e5edbe9801bc} Name blockedTime";
+
function parseBlockedTime(event: MicrosoftEvent) {
if (!event.singleValueExtendedProperties) {
return undefined;
}
- const blockedTimeProperty = event.singleValueExtendedProperties.find(
- (prop) => prop && prop.id && prop.id.includes("Name blockedTime"),
- );
+ const blockedTimeProperty = event.singleValueExtendedProperties.find(
+ (prop) => prop?.id === MICROSOFT_BLOCKED_TIME_PROP_ID,
+ );This aligns parsing with the $expand($filter=id eq '${MICROSOFT_BLOCKED_TIME_PROP_ID}') change.
| function toMicrosoftBlockedTime(blockedTime: { | ||
| before?: number; | ||
| after?: number; | ||
| }) { | ||
| return [ | ||
| { | ||
| id: `String {${crypto.randomUUID()}} Name blockedTime`, | ||
| value: JSON.stringify(blockedTime), | ||
| }, | ||
| ]; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t generate a new GUID per write; use a constant property set ID and import crypto correctly if needed
- Writing with a random GUID will make subsequent reads impossible to filter by ID and will accumulate orphaned properties.
- Also,
crypto.randomUUID()isn’t imported; relying on a global may fail type-checking in Node.
Apply this diff to use the same stable ID and drop the random UUID:
-function toMicrosoftBlockedTime(blockedTime: {
+function toMicrosoftBlockedTime(blockedTime: {
before?: number;
after?: number;
}) {
return [
{
- id: `String {${crypto.randomUUID()}} Name blockedTime`,
+ id: MICROSOFT_BLOCKED_TIME_PROP_ID,
value: JSON.stringify(blockedTime),
},
];
}If you still need randomUUID elsewhere, prefer import { randomUUID } from "node:crypto".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function toMicrosoftBlockedTime(blockedTime: { | |
| before?: number; | |
| after?: number; | |
| }) { | |
| return [ | |
| { | |
| id: `String {${crypto.randomUUID()}} Name blockedTime`, | |
| value: JSON.stringify(blockedTime), | |
| }, | |
| ]; | |
| } | |
| function toMicrosoftBlockedTime(blockedTime: { | |
| before?: number; | |
| after?: number; | |
| }) { | |
| return [ | |
| { | |
| id: MICROSOFT_BLOCKED_TIME_PROP_ID, | |
| value: JSON.stringify(blockedTime), | |
| }, | |
| ]; | |
| } |
🤖 Prompt for AI Agents
In packages/api/src/providers/calendars/microsoft-calendar/events.ts around
lines 234 to 244, the function currently generates a new GUID per write using
crypto.randomUUID(), which prevents reliable reads/filters and causes orphaned
properties; change it to use a stable constant property set ID (e.g., a fixed
string constant defined near the top of the module) instead of a random UUID,
remove the call to randomUUID(), and update the id value to use that constant;
if randomUUID is needed elsewhere, import it properly with import { randomUUID }
from "node:crypto" (but do not use it here).
| blockedTime: z | ||
| .object({ | ||
| before: z.number().int().positive().optional(), | ||
| after: z.number().int().positive().optional(), | ||
| }) | ||
| .optional(), | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Deduplicate and harden blockedTime schema
You define the same blockedTime shape twice. Extract a shared schema and also require at least one of before or after to avoid {} passing validation (which would serialize to {} in providers).
Apply this diff (add the shared schema once, then reuse):
import { z } from "zod/v3";
+const blockedTimeSchema = z
+ .object({
+ before: z.number().int().positive().optional(),
+ after: z.number().int().positive().optional(),
+ })
+ .refine((v) => typeof v.before === "number" || typeof v.after === "number", {
+ message: "blockedTime must include at least one of 'before' or 'after'",
+ path: [],
+ });
+
const microsoftMetadataSchema = z.object({
@@
- blockedTime: z
- .object({
- before: z.number().int().positive().optional(),
- after: z.number().int().positive().optional(),
- })
- .optional(),
+ blockedTime: blockedTimeSchema.optional(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| blockedTime: z | |
| .object({ | |
| before: z.number().int().positive().optional(), | |
| after: z.number().int().positive().optional(), | |
| }) | |
| .optional(), | |
| }); | |
| import { z } from "zod/v3"; | |
| const blockedTimeSchema = z | |
| .object({ | |
| before: z.number().int().positive().optional(), | |
| after: z.number().int().positive().optional(), | |
| }) | |
| .refine( | |
| (v) => typeof v.before === "number" || typeof v.after === "number", | |
| { | |
| message: "blockedTime must include at least one of 'before' or 'after'", | |
| path: [], | |
| } | |
| ); | |
| const microsoftMetadataSchema = z.object({ | |
| // ... other fields ... | |
| blockedTime: blockedTimeSchema.optional(), | |
| }); |
| // Extended properties for custom data | ||
| private: z.record(z.string(), z.string()).optional(), | ||
| shared: z.record(z.string(), z.string()).optional(), | ||
| blockedTime: z | ||
| .object({ | ||
| before: z.number().int().positive().optional(), | ||
| after: z.number().int().positive().optional(), | ||
| }) | ||
| .optional(), | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Reuse the shared blockedTime schema for Google metadata as well
Keep the constraints identical across providers.
Apply this diff:
- blockedTime: z
- .object({
- before: z.number().int().positive().optional(),
- after: z.number().int().positive().optional(),
- })
- .optional(),
+ blockedTime: blockedTimeSchema.optional(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Extended properties for custom data | |
| private: z.record(z.string(), z.string()).optional(), | |
| shared: z.record(z.string(), z.string()).optional(), | |
| blockedTime: z | |
| .object({ | |
| before: z.number().int().positive().optional(), | |
| after: z.number().int().positive().optional(), | |
| }) | |
| .optional(), | |
| }); | |
| // Extended properties for custom data | |
| private: z.record(z.string(), z.string()).optional(), | |
| shared: z.record(z.string(), z.string()).optional(), | |
| blockedTime: blockedTimeSchema.optional(), | |
| }); |
0d3aa57 to
1fbb819
Compare
Summary by CodeRabbit