From 66db7c5e2ae27eae740b9096fae10e463d4d60d4 Mon Sep 17 00:00:00 2001 From: Naman Yadav Date: Thu, 4 Jun 2026 21:09:14 +0530 Subject: [PATCH] fix(office365video): use PATCH for updateMeeting and implement deleteMeeting updateMeeting was calling POST /onlineMeetings, creating a new meeting on every reschedule instead of updating the existing one via PATCH. deleteMeeting was a no-op that returned an empty array without making any HTTP request, leaving the remote Teams meeting alive after deletion. Also updates existing updateMeeting tests (which were asserting the broken POST behavior) and adds deleteMeeting tests covering success, 404, and error. Fixes #29498 --- .../lib/VideoApiAdapter.test.ts | 92 ++++++++++++++++--- .../office365video/lib/VideoApiAdapter.ts | 41 +++++++-- 2 files changed, 113 insertions(+), 20 deletions(-) diff --git a/packages/app-store/office365video/lib/VideoApiAdapter.test.ts b/packages/app-store/office365video/lib/VideoApiAdapter.test.ts index e84b3e51ed36e0..528b5e4f013c63 100644 --- a/packages/app-store/office365video/lib/VideoApiAdapter.test.ts +++ b/packages/app-store/office365video/lib/VideoApiAdapter.test.ts @@ -7,14 +7,21 @@ import { internalServerErrorResponse, successResponse } from "../../_utils/testU import config from "../config.json"; import VideoApiAdapter from "./VideoApiAdapter"; +const FAKE_MEETING_ID = "FAKE_MEETING_ID"; +const FAKE_BOOKING_REF = { type: "office365_video", uid: "FAKE_UID", meetingId: FAKE_MEETING_ID }; + const URLS = { CREATE_MEETING: { url: "https://graph.microsoft.com/v1.0/me/onlineMeetings", method: "POST", }, UPDATE_MEETING: { - url: "https://graph.microsoft.com/v1.0/me/onlineMeetings", - method: "POST", + url: `https://graph.microsoft.com/v1.0/me/onlineMeetings/${FAKE_MEETING_ID}`, + method: "PATCH", + }, + DELETE_MEETING: { + url: `https://graph.microsoft.com/v1.0/me/onlineMeetings/${FAKE_MEETING_ID}`, + method: "DELETE", }, }; @@ -199,11 +206,11 @@ describe("updateMeeting", () => { const videoApi = VideoApiAdapter(testCredential); mockRequestRaw.mockImplementation(({ url }) => { - if (url === URLS.CREATE_MEETING.url) { + if (url === URLS.UPDATE_MEETING.url) { return Promise.resolve( successResponse({ json: { - id: 1, + id: FAKE_MEETING_ID, joinWebUrl: "https://join_web_url.example.com", joinUrl: "https://join_url.example.com", }, @@ -220,12 +227,12 @@ describe("updateMeeting", () => { endTime: new Date(), }; - const updatedMeeting = await videoApi?.updateMeeting(null, event); + const updatedMeeting = await videoApi?.updateMeeting(FAKE_BOOKING_REF, event); expect(OAuthManager).toHaveBeenCalled(); expect(mockRequestRaw).toHaveBeenCalledWith({ - url: URLS.CREATE_MEETING.url, + url: URLS.UPDATE_MEETING.url, options: { - method: "POST", + method: "PATCH", body: JSON.stringify({ startDateTime: event.startTime, endDateTime: event.endTime, @@ -234,7 +241,7 @@ describe("updateMeeting", () => { }, }); expect(updatedMeeting).toEqual({ - id: 1, + id: FAKE_MEETING_ID, password: "", type: config.type, url: "https://join_web_url.example.com", @@ -245,11 +252,11 @@ describe("updateMeeting", () => { const videoApi = VideoApiAdapter(testCredential); mockRequestRaw.mockImplementation(({ url }) => { - if (url === URLS.CREATE_MEETING.url) { + if (url === URLS.UPDATE_MEETING.url) { return Promise.resolve( internalServerErrorResponse({ json: { - id: 1, + id: FAKE_MEETING_ID, joinWebUrl: "https://join_web_url.example.com", joinUrl: "https://join_url.example.com", }, @@ -266,12 +273,14 @@ describe("updateMeeting", () => { endTime: new Date(), }; - await expect(() => videoApi?.updateMeeting(null, event)).rejects.toThrowError("Internal Server Error"); + await expect(() => videoApi?.updateMeeting(FAKE_BOOKING_REF, event)).rejects.toThrowError( + "Internal Server Error" + ); expect(OAuthManager).toHaveBeenCalled(); expect(mockRequestRaw).toHaveBeenCalledWith({ - url: URLS.CREATE_MEETING.url, + url: URLS.UPDATE_MEETING.url, options: { - method: "POST", + method: "PATCH", body: JSON.stringify({ startDateTime: event.startTime, endDateTime: event.endTime, @@ -280,4 +289,61 @@ describe("updateMeeting", () => { }, }); }); + + test("`updateMeeting` throws when meetingId is missing", async () => { + const videoApi = VideoApiAdapter(testCredential); + const event = { + title: "Test Meeting", + description: "Test Description", + startTime: new Date(), + endTime: new Date(), + }; + await expect(() => + videoApi?.updateMeeting({ type: "office365_video", uid: "uid", meetingId: null }, event) + ).rejects.toThrowError("Meeting ID is required"); + }); +}); + +describe("deleteMeeting", () => { + test("Successful `deleteMeeting` call", async () => { + const videoApi = VideoApiAdapter(testCredential); + + mockRequestRaw.mockImplementation(({ url }) => { + if (url === URLS.DELETE_MEETING.url) { + return Promise.resolve(successResponse({ json: {} })); + } + throw new Error("Unexpected URL"); + }); + + await expect(videoApi?.deleteMeeting(FAKE_MEETING_ID)).resolves.toBeDefined(); + expect(mockRequestRaw).toHaveBeenCalledWith({ + url: URLS.DELETE_MEETING.url, + options: { method: "DELETE" }, + }); + }); + + test("`deleteMeeting` is idempotent on 404", async () => { + const videoApi = VideoApiAdapter(testCredential); + + mockRequestRaw.mockImplementation(() => + Promise.resolve({ ok: false, status: 404, statusText: "Not Found", text: async () => "" }) + ); + + await expect(videoApi?.deleteMeeting(FAKE_MEETING_ID)).resolves.toBeDefined(); + }); + + test("Failing `deleteMeeting` call", async () => { + const videoApi = VideoApiAdapter(testCredential); + + mockRequestRaw.mockImplementation(({ url }) => { + if (url === URLS.DELETE_MEETING.url) { + return Promise.resolve(internalServerErrorResponse({ json: {} })); + } + throw new Error("Unexpected URL"); + }); + + await expect(() => videoApi?.deleteMeeting(FAKE_MEETING_ID)).rejects.toThrowError( + "Internal Server Error" + ); + }); }); diff --git a/packages/app-store/office365video/lib/VideoApiAdapter.ts b/packages/app-store/office365video/lib/VideoApiAdapter.ts index 04bc6ad348d5f3..a789e1f7328fd6 100644 --- a/packages/app-store/office365video/lib/VideoApiAdapter.ts +++ b/packages/app-store/office365video/lib/VideoApiAdapter.ts @@ -228,11 +228,18 @@ const TeamsVideoApiAdapter = (credential: CredentialForCalendarServiceWithTenant return Promise.resolve([]); }, updateMeeting: async (bookingRef: PartialReference, event: CalendarEvent) => { + const meetingId = bookingRef?.meetingId; + if (!meetingId) { + throw new HttpError({ + statusCode: 400, + message: "Meeting ID is required to update a Teams meeting", + }); + } try { const response = await auth.requestRaw({ - url: `${await getUserEndpoint()}/onlineMeetings`, + url: `${await getUserEndpoint()}/onlineMeetings/${meetingId}`, options: { - method: "POST", + method: "PATCH", body: JSON.stringify(translateEvent(event)), }, }); @@ -244,12 +251,11 @@ const TeamsVideoApiAdapter = (credential: CredentialForCalendarServiceWithTenant }); } - const resultString = await response.text(); - const resultObject = JSON.parse(resultString); + const resultObject = JSON.parse(await response.text()); return Promise.resolve({ type: "office365_video", - id: resultObject.id, + id: resultObject.id ?? meetingId, password: "", url: resultObject.joinWebUrl || resultObject.joinUrl, }); @@ -264,8 +270,29 @@ const TeamsVideoApiAdapter = (credential: CredentialForCalendarServiceWithTenant }); } }, - deleteMeeting:() => { - return Promise.resolve([]); + deleteMeeting: async (uid: string) => { + try { + const response = await auth.requestRaw({ + url: `${await getUserEndpoint()}/onlineMeetings/${uid}`, + options: { method: "DELETE" }, + }); + if (!response.ok && response.status !== 404) { + throw new HttpError({ + statusCode: response.status, + message: response.statusText, + }); + } + return Promise.resolve([]); + } catch (error) { + log.error(`Error deleting MS Teams meeting ${uid}`, error); + if (error instanceof HttpError) { + throw error; + } + throw new HttpError({ + statusCode: 500, + message: `Error deleting MS Teams meeting ${uid}`, + }); + } }, createMeeting: async (event: CalendarEvent): Promise => { const url = `${await getUserEndpoint()}/onlineMeetings`;