Skip to content

feat(microsoft): add microsoft people sdk - #471

Open
JeanMeijer wants to merge 2 commits into
mainfrom
feat/add-microsoft-people-sdk
Open

feat(microsoft): add microsoft people sdk#471
JeanMeijer wants to merge 2 commits into
mainfrom
feat/add-microsoft-people-sdk

Conversation

@JeanMeijer

@JeanMeijer JeanMeijer commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Description

Briefly describe what you did and why.

Screenshots / Recordings

Add screenshots or recordings here to help reviewers understand your changes.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • UI/UX update
  • Docs update
  • Refactor / Cleanup

Related Areas

  • Authentication
  • Calendar UI
  • Data/API
  • Docs

Testing

  • Manual testing performed
  • Cross-browser testing (if UI changes)
  • Mobile responsiveness verified (if UI changes)

Checklist

  • I’ve read the CONTRIBUTING guide
  • My code works and is understandable and follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in complex areas
  • I have updated the documentation
  • Any dependent changes are merged and published

Notes

(Optional) Add anything else you'd like to share.

By submitting, I confirm I understand and stand behind this code. If AI was used, I’ve reviewed and verified everything myself.


Summary by cubic

Adds @analog/microsoft-people, a typed client for Microsoft Graph People to list, page, count, and fetch person details. Strengthens reliability with robust transport/API error handling and clear types.

  • New Features
    • MicrosoftPeople client with users.people: list, listMore(nextLink), $count, get.
    • Supports Graph params: $top, $skip, $search, $filter, $count, $orderby, $select, $expand.
    • Standardized errors (APIError, AuthenticationError, RateLimitError, TimeoutError, ConnectionError, etc.).
    • Request options support AbortSignal and custom headers; handles userId: "me" alias and validates absolute @odata.nextLink origins.

Written for commit 1a12509. Summary will update on new commits.

Review in cubic

@vercel

vercel Bot commented Jul 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
analog Ready Ready Preview, Comment Jul 25, 2026 9:26am

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found across 12 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/microsoft-people/src/client.ts">

<violation number="1" location="packages/microsoft-people/src/client.ts:5">
P3: This new client duplicates the existing Microsoft Graph transport implementation, creating a second maintenance path for auth, pagination URLs, and error handling. A shared internal Graph client/helper would keep those behaviors aligned.</violation>

<violation number="2" location="packages/microsoft-people/src/client.ts:51">
P2: The `Authorization` header is set first and then user-supplied headers are spread after it (`{ Authorization: ..., ...headers }`). This allows callers to the public `get`/`number` methods to pass an `Authorization` header that overrides the client's access token. If a caller unintentionally (or maliciously) includes an `Authorization` header in their `RequestHeaders`, it would silently replace the bearer token that was configured at construction time, leading to unexpected authentication behavior. Consider spreading user headers first then setting `Authorization` last so it cannot be overridden.</violation>
</file>

<file name="packages/microsoft-people/src/error.ts">

<violation number="1" location="packages/microsoft-people/src/error.ts:31">
P3: Microsoft Graph error handling now has two near-identical implementations that have already drifted in malformed-body validation. Extract the shared OData/API error implementation into a common module so fixes apply consistently to both SDKs.</violation>

<violation number="2" location="packages/microsoft-people/src/error.ts:177">
P2: Malformed JSON error bodies can be returned as typed `ODataError`s and produce messages such as `400 [object Object]` when `message` is non-string. Validate the parsed value's object and string fields before returning it, matching the Microsoft Calendar client.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/microsoft-people/src/client.ts Outdated
Comment thread packages/microsoft-people/src/client.ts Outdated

function parseErrorBody(text: string): ODataError | undefined {
try {
const error: ODataError = JSON.parse(text);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Malformed JSON error bodies can be returned as typed ODataErrors and produce messages such as 400 [object Object] when message is non-string. Validate the parsed value's object and string fields before returning it, matching the Microsoft Calendar client.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/microsoft-people/src/error.ts, line 177:

<comment>Malformed JSON error bodies can be returned as typed `ODataError`s and produce messages such as `400 [object Object]` when `message` is non-string. Validate the parsed value's object and string fields before returning it, matching the Microsoft Calendar client.</comment>

<file context>
@@ -0,0 +1,244 @@
+
+function parseErrorBody(text: string): ODataError | undefined {
+  try {
+    const error: ODataError = JSON.parse(text);
+
+    if (!error.error.code) {
</file context>

Comment thread packages/microsoft-people/src/interfaces.ts

private buildHeaders(headers?: RequestHeaders) {
return {
Authorization: `Bearer ${this.accessToken}`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The Authorization header is set first and then user-supplied headers are spread after it ({ Authorization: ..., ...headers }). This allows callers to the public get/number methods to pass an Authorization header that overrides the client's access token. If a caller unintentionally (or maliciously) includes an Authorization header in their RequestHeaders, it would silently replace the bearer token that was configured at construction time, leading to unexpected authentication behavior. Consider spreading user headers first then setting Authorization last so it cannot be overridden.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/microsoft-people/src/client.ts, line 51:

<comment>The `Authorization` header is set first and then user-supplied headers are spread after it (`{ Authorization: ..., ...headers }`). This allows callers to the public `get`/`number` methods to pass an `Authorization` header that overrides the client's access token. If a caller unintentionally (or maliciously) includes an `Authorization` header in their `RequestHeaders`, it would silently replace the bearer token that was configured at construction time, leading to unexpected authentication behavior. Consider spreading user headers first then setting `Authorization` last so it cannot be overridden.</comment>

<file context>
@@ -0,0 +1,163 @@
+
+  private buildHeaders(headers?: RequestHeaders) {
+    return {
+      Authorization: `Bearer ${this.accessToken}`,
+      ...headers,
+    };
</file context>

@@ -0,0 +1,163 @@
import { APIError, ConnectionError, TimeoutError } from "./error";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: This new client duplicates the existing Microsoft Graph transport implementation, creating a second maintenance path for auth, pagination URLs, and error handling. A shared internal Graph client/helper would keep those behaviors aligned.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/microsoft-people/src/client.ts, line 5:

<comment>This new client duplicates the existing Microsoft Graph transport implementation, creating a second maintenance path for auth, pagination URLs, and error handling. A shared internal Graph client/helper would keep those behaviors aligned.</comment>

<file context>
@@ -0,0 +1,163 @@
+import type { QueryParams, RequestHeaders } from "./interfaces";
+import { Users } from "./users";
+
+export class MicrosoftPeople {
+  private static readonly BASE_URL = "https://graph.microsoft.com/v1.0";
+  private static readonly ORIGIN_URL = "https://graph.microsoft.com";
</file context>

[key: string]: unknown;
}

export class APIError<

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Microsoft Graph error handling now has two near-identical implementations that have already drifted in malformed-body validation. Extract the shared OData/API error implementation into a common module so fixes apply consistently to both SDKs.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/microsoft-people/src/error.ts, line 31:

<comment>Microsoft Graph error handling now has two near-identical implementations that have already drifted in malformed-body validation. Extract the shared OData/API error implementation into a common module so fixes apply consistently to both SDKs.</comment>

<file context>
@@ -0,0 +1,244 @@
+  [key: string]: unknown;
+}
+
+export class APIError<
+  TStatus extends number | undefined = number | undefined,
+  THeaders extends Headers | undefined = Headers | undefined,
</file context>

Comment thread packages/microsoft-people/src/client.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant