From 826c2cef0f9729d9799894342f5b9fc4b36d7a7f Mon Sep 17 00:00:00 2001 From: Caleb Roseland Date: Thu, 7 May 2026 16:52:57 -0500 Subject: [PATCH 1/3] fix(bookmarks): disable DnD with one item; don't open menu without overflow (#36457) --- .../channel/channel_bookmarks_spec.ts | 27 ++++ .../bookmarks_bar_menu.test.tsx | 100 +++++++++++++ .../channel_bookmarks/bookmarks_bar_menu.tsx | 7 +- .../channel_bookmarks/channel_bookmarks.tsx | 11 +- .../hooks/use_bookmark_drag_drop.test.ts | 90 ++++++++++++ .../hooks/use_bookmarks_dnd.test.ts | 137 ++++++++++++++++++ .../hooks/use_bookmarks_dnd.ts | 14 +- 7 files changed, 375 insertions(+), 11 deletions(-) create mode 100644 webapp/channels/src/components/channel_bookmarks/bookmarks_bar_menu.test.tsx create mode 100644 webapp/channels/src/components/channel_bookmarks/hooks/use_bookmark_drag_drop.test.ts create mode 100644 webapp/channels/src/components/channel_bookmarks/hooks/use_bookmarks_dnd.test.ts diff --git a/e2e-tests/cypress/tests/integration/channels/channel/channel_bookmarks_spec.ts b/e2e-tests/cypress/tests/integration/channels/channel/channel_bookmarks_spec.ts index 10b3c6743762..972e93064346 100644 --- a/e2e-tests/cypress/tests/integration/channels/channel/channel_bookmarks_spec.ts +++ b/e2e-tests/cypress/tests/integration/channels/channel/channel_bookmarks_spec.ts @@ -340,6 +340,33 @@ describe('Channel Bookmarks', () => { cy.visit(`/${testTeam.name}/channels/${publicChannel.name}`); }); }); + + it('reorder is disabled when only one bookmark exists', () => { + // # Use a fresh channel with exactly one bookmark + cy.apiCreateChannel(testTeam.id, `single-${getRandomId(4)}`, 'single bookmark', 'O').then((result) => { + cy.visit(`/${testTeam.name}/channels/${result.channel.name}`); + + const {displayName} = createLinkBookmark({displayName: 'Solo Bookmark', fromChannelMenu: true}); + + // # Press Space on the only bookmark + cy.findByTestId('channel-bookmarks-container').findByRole('link', {name: displayName}).focus(). + trigger('keydown', {key: ' ', code: 'Space', bubbles: true}); + + cy.wait(TIMEOUTS.HALF_SEC); + + // * Reorder visual state did not engage (no 3px reorder outline) + cy.findByTestId('channel-bookmarks-container'). + find('[data-bookmark-id] > div').first(). + should('not.have.css', 'outline-width', '3px'); + + // * Bookmark count and identity unchanged + cy.findByTestId('channel-bookmarks-container').findAllByRole('link'). + should('have.length', 1).first().should('contain', displayName); + + // # Return to original channel for subsequent tests + cy.visit(`/${testTeam.name}/channels/${publicChannel.name}`); + }); + }); }); describe('manage bookmarks - permissions enforcement', () => { diff --git a/webapp/channels/src/components/channel_bookmarks/bookmarks_bar_menu.test.tsx b/webapp/channels/src/components/channel_bookmarks/bookmarks_bar_menu.test.tsx new file mode 100644 index 000000000000..f68773cc99cb --- /dev/null +++ b/webapp/channels/src/components/channel_bookmarks/bookmarks_bar_menu.test.tsx @@ -0,0 +1,100 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react'; + +import type {ChannelBookmark} from '@mattermost/types/channel_bookmarks'; + +import {renderWithContext} from 'tests/react_testing_utils'; + +import BookmarksBarMenu from './bookmarks_bar_menu'; + +const mockDropTargetForElements: jest.Mock = jest.fn(() => () => undefined); + +jest.mock('@atlaskit/pragmatic-drag-and-drop/element/adapter', () => ({ + dropTargetForElements: (arg: unknown) => mockDropTargetForElements(arg), +})); + +function makeBookmark(id: string): ChannelBookmark { + return { + id, + channel_id: 'c1', + owner_id: 'u1', + type: 'link', + link_url: 'https://example.com', + display_name: `bm-${id}`, + sort_order: 0, + create_at: 0, + update_at: 0, + delete_at: 0, + } as ChannelBookmark; +} + +const baseProps = { + channelId: 'c1', + bookmarks: {} as Record, + hasBookmarks: true, + limitReached: false, + canUploadFiles: true, + canReorder: true, + isDragging: false, + canAdd: true, +}; + +type DropTargetConfig = { + canDrop: (args: {source: {data: Record}}) => boolean; + getData: () => Record; +}; + +describe('BookmarksBarMenu — overflow drop target', () => { + beforeEach(() => { + mockDropTargetForElements.mockClear(); + }); + + test('canDrop returns false when there is no overflow', () => { + renderWithContext( + , + ); + + expect(mockDropTargetForElements).toHaveBeenCalledTimes(1); + const config = mockDropTargetForElements.mock.calls[0][0] as unknown as DropTargetConfig; + expect(config.getData()).toEqual({type: 'overflow-trigger'}); + expect(config.canDrop({source: {data: {type: 'bookmark'}}})).toBe(false); + }); + + test('canDrop returns true for bookmark sources when overflow items exist', () => { + const overflowItems = ['a', 'b']; + const bookmarks = {a: makeBookmark('a'), b: makeBookmark('b')}; + + renderWithContext( + , + ); + + expect(mockDropTargetForElements).toHaveBeenCalledTimes(1); + const config = mockDropTargetForElements.mock.calls[0][0] as unknown as DropTargetConfig; + expect(config.canDrop({source: {data: {type: 'bookmark'}}})).toBe(true); + }); + + test('canDrop returns false for non-bookmark sources even with overflow', () => { + const overflowItems = ['a']; + const bookmarks = {a: makeBookmark('a')}; + + renderWithContext( + , + ); + + const config = mockDropTargetForElements.mock.calls[0][0] as unknown as DropTargetConfig; + expect(config.canDrop({source: {data: {type: 'something-else'}}})).toBe(false); + }); +}); diff --git a/webapp/channels/src/components/channel_bookmarks/bookmarks_bar_menu.tsx b/webapp/channels/src/components/channel_bookmarks/bookmarks_bar_menu.tsx index d18cf46b9432..6ac49a6ae2c5 100644 --- a/webapp/channels/src/components/channel_bookmarks/bookmarks_bar_menu.tsx +++ b/webapp/channels/src/components/channel_bookmarks/bookmarks_bar_menu.tsx @@ -77,7 +77,10 @@ function BookmarksBarMenu({ } }, []); - // Register as drop target for overflow auto-open trigger + // Drops are rejected via canDrop when there is no overflow so the trailing + // add-bookmark button never auto-opens the menu during a drag. + const hasOverflowRef = useRef(hasOverflow); + hasOverflowRef.current = hasOverflow; useEffect(() => { const el = triggerRef.current; if (!el) { @@ -86,7 +89,7 @@ function BookmarksBarMenu({ return dropTargetForElements({ element: el, getData: () => ({type: 'overflow-trigger'}), - canDrop: ({source}) => source.data.type === 'bookmark', + canDrop: ({source}) => source.data.type === 'bookmark' && hasOverflowRef.current, }); }, []); diff --git a/webapp/channels/src/components/channel_bookmarks/channel_bookmarks.tsx b/webapp/channels/src/components/channel_bookmarks/channel_bookmarks.tsx index 3563c74faf4b..8d509f80d18d 100644 --- a/webapp/channels/src/components/channel_bookmarks/channel_bookmarks.tsx +++ b/webapp/channels/src/components/channel_bookmarks/channel_bookmarks.tsx @@ -24,6 +24,7 @@ function ChannelBookmarks({channelId}: Props) { const canUploadFiles = useCanUploadFiles(); const hasBookmarks = Boolean(order?.length); const limitReached = order.length >= MAX_BOOKMARKS_PER_CHANNEL; + const canDrag = canReorder && order.length > 1; // --- Overflow detection --- const { @@ -54,7 +55,7 @@ function ChannelBookmarks({channelId}: Props) { onReorder: reorder, getName: useCallback((id: string) => bookmarks[id]?.display_name ?? '', [bookmarks]), onOverflowOpenChange: setForceOverflowOpen, - canReorder, + canReorder: canDrag, }); // Pause overflow recalculation while dragging or keyboard reordering. @@ -89,9 +90,9 @@ function ChannelBookmarks({channelId}: Props) { key={id} id={id} bookmark={bookmark} - disabled={!canReorder} + disabled={!canDrag} isDraggingGlobal={isDragging} - keyboardReorderProps={!isHidden && canReorder ? getItemProps(id) : undefined} + keyboardReorderProps={!isHidden && canDrag ? getItemProps(id) : undefined} isKeyboardReordering={!isHidden && reorderState.isReordering && reorderState.itemId === id} hidden={isHidden} onMount={registerItemRef} @@ -106,13 +107,13 @@ function ChannelBookmarks({channelId}: Props) { hasBookmarks={hasBookmarks} limitReached={limitReached} canUploadFiles={canUploadFiles} - canReorder={canReorder} + canReorder={canDrag} isDragging={isDragging} canAdd={canAdd} forceOpen={forceOverflowOpen} onOpenChange={setForceOverflowOpen} reorderState={reorderState} - getItemProps={canReorder ? getItemProps : undefined} + getItemProps={canDrag ? getItemProps : undefined} /> diff --git a/webapp/channels/src/components/channel_bookmarks/hooks/use_bookmark_drag_drop.test.ts b/webapp/channels/src/components/channel_bookmarks/hooks/use_bookmark_drag_drop.test.ts new file mode 100644 index 000000000000..68022743ee24 --- /dev/null +++ b/webapp/channels/src/components/channel_bookmarks/hooks/use_bookmark_drag_drop.test.ts @@ -0,0 +1,90 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {renderHookWithContext} from 'tests/react_testing_utils'; + +import {useBookmarkDragDrop} from './use_bookmark_drag_drop'; + +const mockDraggable: jest.Mock = jest.fn(() => () => undefined); +const mockDropTargetForElements: jest.Mock = jest.fn(() => () => undefined); + +jest.mock('@atlaskit/pragmatic-drag-and-drop/element/adapter', () => ({ + draggable: (arg: unknown) => mockDraggable(arg), + dropTargetForElements: (arg: unknown) => mockDropTargetForElements(arg), +})); + +jest.mock('@atlaskit/pragmatic-drag-and-drop/combine', () => ({ + combine: (...cleanups: Array<() => void>) => () => cleanups.forEach((c) => c()), +})); + +jest.mock('@atlaskit/pragmatic-drag-and-drop/element/set-custom-native-drag-preview', () => ({ + setCustomNativeDragPreview: jest.fn(), +})); + +jest.mock('@atlaskit/pragmatic-drag-and-drop/prevent-unhandled', () => ({ + preventUnhandled: {start: jest.fn(), stop: jest.fn()}, +})); + +jest.mock('@atlaskit/pragmatic-drag-and-drop-hitbox/closest-edge', () => ({ + attachClosestEdge: (data: unknown) => data, + extractClosestEdge: () => null, +})); + +describe('useBookmarkDragDrop', () => { + beforeEach(() => { + mockDraggable.mockClear(); + mockDropTargetForElements.mockClear(); + }); + + test('does not register draggable or drop target when canReorder is false', () => { + const element = document.createElement('div'); + + renderHookWithContext( + () => useBookmarkDragDrop({ + id: 'bm1', + container: 'bar', + allowedEdges: ['left', 'right'], + displayName: 'Example', + canReorder: false, + element, + }), + ); + + expect(mockDraggable).not.toHaveBeenCalled(); + expect(mockDropTargetForElements).not.toHaveBeenCalled(); + }); + + test('registers draggable and drop target when canReorder is true', () => { + const element = document.createElement('div'); + + renderHookWithContext( + () => useBookmarkDragDrop({ + id: 'bm1', + container: 'bar', + allowedEdges: ['left', 'right'], + displayName: 'Example', + canReorder: true, + element, + }), + ); + + expect(mockDraggable).toHaveBeenCalledTimes(1); + expect(mockDropTargetForElements).toHaveBeenCalledTimes(1); + }); + + test('does not register when element is null even if canReorder is true', () => { + renderHookWithContext( + () => useBookmarkDragDrop({ + id: 'bm1', + container: 'bar', + allowedEdges: ['left', 'right'], + displayName: 'Example', + canReorder: true, + element: null, + }), + ); + + expect(mockDraggable).not.toHaveBeenCalled(); + expect(mockDropTargetForElements).not.toHaveBeenCalled(); + }); +}); diff --git a/webapp/channels/src/components/channel_bookmarks/hooks/use_bookmarks_dnd.test.ts b/webapp/channels/src/components/channel_bookmarks/hooks/use_bookmarks_dnd.test.ts new file mode 100644 index 000000000000..885367e7fb26 --- /dev/null +++ b/webapp/channels/src/components/channel_bookmarks/hooks/use_bookmarks_dnd.test.ts @@ -0,0 +1,137 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {act} from '@testing-library/react'; + +import {renderHookWithContext} from 'tests/react_testing_utils'; + +import {useBookmarksDnd} from './use_bookmarks_dnd'; + +type MonitorConfig = { + canMonitor: (args: {source: {data: Record}}) => boolean; + onDragStart: (args: {source: {data: Record}}) => void; + onDrop: (args: { + source: {data: Record}; + location: {current: {dropTargets: Array<{data: Record}>}}; + }) => void; + onDropTargetChange: (args: { + location: {current: {dropTargets: Array<{data: Record}>}}; + }) => void; +}; + +const registeredMonitors: MonitorConfig[] = []; + +jest.mock('@atlaskit/pragmatic-drag-and-drop/element/adapter', () => ({ + monitorForElements: (config: MonitorConfig) => { + registeredMonitors.push(config); + return () => { + const idx = registeredMonitors.indexOf(config); + if (idx !== -1) { + registeredMonitors.splice(idx, 1); + } + }; + }, +})); + +jest.mock('@atlaskit/pragmatic-drag-and-drop-hitbox/closest-edge', () => ({ + extractClosestEdge: () => null, +})); + +function dropTargets(...targets: Array>) { + return {current: {dropTargets: targets.map((data) => ({data}))}}; +} + +describe('useBookmarksDnd — overflow open guard', () => { + beforeEach(() => { + registeredMonitors.length = 0; + }); + + test('onDropTargetChange forces overflow open when overflow exists', () => { + const onReorder = jest.fn().mockResolvedValue(undefined); + + const {result} = renderHookWithContext(() => useBookmarksDnd({ + order: ['a', 'b', 'c', 'd'], + visibleItems: ['a', 'b'], + onReorder, + })); + + expect(registeredMonitors).toHaveLength(1); + const monitor = registeredMonitors[0]; + + act(() => { + monitor.onDropTargetChange({ + location: dropTargets({type: 'overflow-trigger'}), + }); + }); + + expect(result.current.forceOverflowOpen).toBe(true); + }); + + test('onDropTargetChange does NOT force overflow open when there is no overflow', () => { + const onReorder = jest.fn().mockResolvedValue(undefined); + + const {result} = renderHookWithContext(() => useBookmarksDnd({ + order: ['a', 'b', 'c'], + visibleItems: ['a', 'b', 'c'], + onReorder, + })); + + const monitor = registeredMonitors[0]; + + act(() => { + monitor.onDropTargetChange({ + location: dropTargets({type: 'overflow-trigger'}), + }); + }); + + expect(result.current.forceOverflowOpen).toBeUndefined(); + }); + + test('onDrop does NOT force overflow open when dropping on overflow-trigger without overflow', () => { + const onReorder = jest.fn().mockResolvedValue(undefined); + + const {result} = renderHookWithContext(() => useBookmarksDnd({ + order: ['a', 'b', 'c'], + visibleItems: ['a', 'b', 'c'], + onReorder, + })); + + const monitor = registeredMonitors[0]; + + act(() => { + monitor.onDragStart({source: {data: {bookmarkId: 'a', type: 'bookmark'}}}); + }); + act(() => { + monitor.onDrop({ + source: {data: {bookmarkId: 'a', type: 'bookmark'}}, + location: dropTargets({type: 'overflow-trigger'}), + }); + }); + + expect(result.current.forceOverflowOpen).toBe(false); + }); + + test('onDrop does NOT call onReorder when dropping on overflow-trigger without overflow', () => { + const onReorder = jest.fn().mockResolvedValue(undefined); + + renderHookWithContext(() => useBookmarksDnd({ + order: ['a', 'b', 'c'], + visibleItems: ['a', 'b', 'c'], + onReorder, + })); + + const monitor = registeredMonitors[0]; + + act(() => { + monitor.onDragStart({source: {data: {bookmarkId: 'a', type: 'bookmark'}}}); + }); + act(() => { + monitor.onDrop({ + source: {data: {bookmarkId: 'a', type: 'bookmark'}}, + location: dropTargets({type: 'overflow-trigger'}), + }); + }); + + expect(onReorder).not.toHaveBeenCalled(); + }); +}); diff --git a/webapp/channels/src/components/channel_bookmarks/hooks/use_bookmarks_dnd.ts b/webapp/channels/src/components/channel_bookmarks/hooks/use_bookmarks_dnd.ts index 591f4e8811f9..2b5cef952555 100644 --- a/webapp/channels/src/components/channel_bookmarks/hooks/use_bookmarks_dnd.ts +++ b/webapp/channels/src/components/channel_bookmarks/hooks/use_bookmarks_dnd.ts @@ -76,9 +76,11 @@ export function useBookmarksDnd({ onDrop: ({source, location}) => { setActiveId(null); - // Keep overflow menu open if dropped into overflow; close if dropped into bar const dropTarget = location.current.dropTargets[0]; - const droppedInOverflow = dropTarget?.data.container === 'overflow' || dropTarget?.data.type === 'overflow-trigger'; + const hasOverflow = orderRef.current.length > visibleItemsRef.current.length; + const droppedInOverflow = + dropTarget?.data.container === 'overflow' || + (dropTarget?.data.type === 'overflow-trigger' && hasOverflow); setForceOverflowOpen(droppedInOverflow); const sourceId = source.data.bookmarkId as string; @@ -98,6 +100,10 @@ export function useBookmarksDnd({ let newIndex: number; if (target.data.type === 'overflow-trigger') { + if (!hasOverflow) { + return; + } + // Dropped on the overflow trigger — place at the beginning of overflow newIndex = visibleItemsRef.current.length; } else if (target.data.type === 'bookmark') { @@ -121,9 +127,9 @@ export function useBookmarksDnd({ }, onDropTargetChange: ({location}) => { - // Detect when drag enters overflow-trigger zone const target = location.current.dropTargets[0]; - if (target?.data.type === 'overflow-trigger') { + const hasOverflow = orderRef.current.length > visibleItemsRef.current.length; + if (target?.data.type === 'overflow-trigger' && hasOverflow) { setForceOverflowOpen(true); } }, From 6d30bff0a6887832444e2812c3bb7f72fc7f5c85 Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Fri, 8 May 2026 00:38:07 +0200 Subject: [PATCH 2/3] fix: use Blob with application/json type for client_perf sendBeacon (#36466) When navigator.sendBeacon() is called with a plain string body, the browser defaults to Content-Type: text/plain;charset=UTF-8. This causes legitimate WAF alerts because the payload is JSON, not plain text. Many deployments have been blocking users because of this mismatch (MM-61530 / mattermost/mattermost#29101). Fix: wrap the JSON string in a Blob with type 'application/json' before passing it to sendBeacon. Also add the matching Content-Type: application/json header to the fallback fetch call that fires when sendBeacon returns false. Tests: add three new focused tests in the PerformanceReporter.sendReport content-type suite that verify: 1. sendBeacon receives a Blob with type application/json 2. The Blob's text content is valid JSON matching the report 3. The fallback fetch includes Content-Type: application/json Also update the existing (currently skipped) tests to parse the Blob body via FileReader instead of JSON.parse(string). Co-authored-by: Cursor Agent Co-authored-by: Miguel de la Cruz --- .../performance_telemetry/reporter.test.ts | 70 +++++++++++++++++-- .../utils/performance_telemetry/reporter.ts | 7 +- 2 files changed, 69 insertions(+), 8 deletions(-) diff --git a/webapp/channels/src/utils/performance_telemetry/reporter.test.ts b/webapp/channels/src/utils/performance_telemetry/reporter.test.ts index 07d177095c55..dc7639f09516 100644 --- a/webapp/channels/src/utils/performance_telemetry/reporter.test.ts +++ b/webapp/channels/src/utils/performance_telemetry/reporter.test.ts @@ -69,7 +69,7 @@ describe.skip('PerformanceReporter', () => { expect(sendBeacon).toHaveBeenCalled(); expect(sendBeacon.mock.calls[0][0]).toEqual(siteUrl + '/api/v4/client_perf'); - const report = JSON.parse(sendBeacon.mock.calls[0][1]); + const report = JSON.parse(await (sendBeacon.mock.calls[0][1] as Blob).text()); expect(report).toMatchObject({ histograms: [ { @@ -116,7 +116,7 @@ describe.skip('PerformanceReporter', () => { expect(sendBeacon).toHaveBeenCalled(); expect(sendBeacon.mock.calls[0][0]).toEqual(siteUrl + '/api/v4/client_perf'); - const report = JSON.parse(sendBeacon.mock.calls[0][1]); + const report = JSON.parse(await (sendBeacon.mock.calls[0][1] as Blob).text()); expect(report).toMatchObject({ counters: [ { @@ -168,7 +168,7 @@ describe.skip('PerformanceReporter', () => { expect(sendBeacon).toHaveBeenCalled(); expect(sendBeacon.mock.calls[0][0]).toEqual(siteUrl + '/api/v4/client_perf'); - const report = JSON.parse(sendBeacon.mock.calls[0][1]); + const report = JSON.parse(await (sendBeacon.mock.calls[0][1] as Blob).text()); expect(report).toMatchObject({ counters: [ { @@ -196,7 +196,7 @@ describe.skip('PerformanceReporter', () => { expect(sendBeacon).toHaveBeenCalled(); expect(sendBeacon.mock.calls[0][0]).toEqual(siteUrl + '/api/v4/client_perf'); - let report = JSON.parse(sendBeacon.mock.calls[0][1]); + let report = JSON.parse(await (sendBeacon.mock.calls[0][1] as Blob).text()); expect(report).toMatchObject({ histograms: [ { @@ -221,7 +221,7 @@ describe.skip('PerformanceReporter', () => { expect(sendBeacon).toHaveBeenCalled(); expect(sendBeacon.mock.calls[0][0]).toEqual(siteUrl + '/api/v4/client_perf'); - report = JSON.parse(sendBeacon.mock.calls[0][1]); + report = JSON.parse(await (sendBeacon.mock.calls[0][1] as Blob).text()); expect(report).toMatchObject({ histograms: [ { @@ -313,7 +313,7 @@ describe.skip('PerformanceReporter', () => { expect(sendBeacon).toHaveBeenCalled(); expect(sendBeacon.mock.calls[0][0]).toEqual(siteUrl + '/api/v4/client_perf'); - const report = JSON.parse(sendBeacon.mock.calls[0][1]); + const report = JSON.parse(await (sendBeacon.mock.calls[0][1] as Blob).text()); expect(report).toMatchObject({ labels: { agent: 'firefox', @@ -357,6 +357,64 @@ describe.skip('PerformanceReporter', () => { }); }); +describe('PerformanceReporter.sendReport content-type', () => { + const sampleReport = { + version: '0.1.0' as const, + labels: {platform: 'other' as const, agent: 'other' as const}, + start: 1000, + end: 2000, + counters: [{metric: 'test_counter', value: 1, timestamp: 1500}], + histograms: [], + }; + + test('should pass a Blob with application/json type to sendBeacon', () => { + const {reporter, sendBeacon} = newTestReporter(); + + (reporter as any).sendReport(sampleReport); + + expect(sendBeacon).toHaveBeenCalledTimes(1); + const [url, body] = sendBeacon.mock.calls[0]; + expect(url).toContain('/api/v4/client_perf'); + expect(body).toBeInstanceOf(Blob); + expect((body as Blob).type).toBe('application/json'); + }); + + test('should send a Blob whose content is valid JSON matching the report', async () => { + const {reporter, sendBeacon} = newTestReporter(); + + (reporter as any).sendReport(sampleReport); + + const blob = sendBeacon.mock.calls[0][1] as Blob; + const text = await new Promise((resolve, reject) => { + const reader = new FileReader(); + reader.onload = () => resolve(reader.result as string); + reader.onerror = reject; + reader.readAsText(blob); + }); + const parsed = JSON.parse(text); + expect(parsed).toMatchObject(sampleReport); + }); + + test('should include Content-Type application/json header in fallback fetch', () => { + const {reporter, sendBeacon} = newTestReporter(); + sendBeacon.mockReturnValue(false); + + const fetchSpy = jest.spyOn(global, 'fetch').mockImplementation(() => Promise.resolve({} as Response)); + + (reporter as any).sendReport(sampleReport); + + expect(fetchSpy).toHaveBeenCalledWith( + expect.stringContaining('/api/v4/client_perf'), + expect.objectContaining({ + method: 'POST', + headers: expect.objectContaining({'Content-Type': 'application/json'}), + }), + ); + + fetchSpy.mockRestore(); + }); +}); + class TestPerformanceReporter extends PerformanceReporter { public sendBeacon: jest.Mock = jest.fn(() => true); public reportPeriodBase = 10; diff --git a/webapp/channels/src/utils/performance_telemetry/reporter.ts b/webapp/channels/src/utils/performance_telemetry/reporter.ts index ff015be8f32d..f11738805704 100644 --- a/webapp/channels/src/utils/performance_telemetry/reporter.ts +++ b/webapp/channels/src/utils/performance_telemetry/reporter.ts @@ -378,11 +378,14 @@ export default class PerformanceReporter { const url = this.client.getClientMetricsRoute(); const data = JSON.stringify(report); - const beaconSent = this.sendBeacon(url, data); + // Wrap the JSON string in a Blob so that sendBeacon sends Content-Type: application/json + // instead of the default text/plain, which triggers WAF rules on many deployments. + const blob = new Blob([data], {type: 'application/json'}); + const beaconSent = this.sendBeacon(url, blob); if (!beaconSent) { // The data couldn't be queued as a beacon for some reason, so fall back to sending an immediate fetch - fetch(url, {method: 'POST', body: data}); + fetch(url, {method: 'POST', body: data, headers: {'Content-Type': 'application/json'}}); } } From 97f0ad7c3bd3a5a1583cf2d3f8ca8c43adbd8a33 Mon Sep 17 00:00:00 2001 From: Doug Lauder Date: Thu, 7 May 2026 21:04:10 -0400 Subject: [PATCH 3/3] [MM-68697] Preserve sender file ID in plugin-relayed shared channel attachments (#36468) Set ReqFileId on the UploadSession created in ReceiveSharedChannelAttachmentSyncMsg so the attachment is persisted under the sender's file ID. Without this, the receiving server stored the bytes under a freshly generated ID while the synced post's FileIds still referenced the sender's ID, leaving the attachment invisible in the UI even though the file and FileInfo row existed on disk. Mirrors the existing cluster-to-cluster path in platform/services/sharedchannel/attachment.go. Also tightens TestPluginAPIReceiveSharedChannelAttachmentSyncMsg to pass a sender-side fi.Id and assert the saved FileInfo keeps it. The prior assertion only checked that some ID was assigned, which the buggy code also satisfied. --- server/channels/app/shared_channel.go | 1 + server/channels/app/shared_channel_test.go | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/server/channels/app/shared_channel.go b/server/channels/app/shared_channel.go index c25f03e30718..f861265c789c 100644 --- a/server/channels/app/shared_channel.go +++ b/server/channels/app/shared_channel.go @@ -339,6 +339,7 @@ func (a *App) ReceiveSharedChannelAttachmentSyncMsg(rctx request.CTX, pluginID, Filename: fi.Name, FileSize: fi.Size, RemoteId: rc.RemoteId, + ReqFileId: fi.Id, } us, appErr := a.CreateUploadSession(rctx, us) diff --git a/server/channels/app/shared_channel_test.go b/server/channels/app/shared_channel_test.go index 73adac0c1b3b..5acdc384fc19 100644 --- a/server/channels/app/shared_channel_test.go +++ b/server/channels/app/shared_channel_test.go @@ -1360,7 +1360,11 @@ func TestPluginAPIReceiveSharedChannelAttachmentSyncMsg(t *testing.T) { remoteUser, appErr := th.App.CreateUser(th.Context, remoteUser) require.Nil(t, appErr) + // The sender-side file ID must be preserved on the receiving server so the + // post's FileIds (which reference the sender ID) resolve to the saved file. + senderFileID := model.NewId() fi := &model.FileInfo{ + Id: senderFileID, CreatorId: remoteUser.Id, Name: "hello.txt", Size: 13, @@ -1369,12 +1373,13 @@ func TestPluginAPIReceiveSharedChannelAttachmentSyncMsg(t *testing.T) { saved, err := api.ReceiveSharedChannelAttachmentSyncMsg(rc.RemoteId, channel.Id, fi, bytes.NewReader([]byte("hello, world!"))) require.NoError(t, err) require.NotNil(t, saved) - assert.NotEmpty(t, saved.Id) + assert.Equal(t, senderFileID, saved.Id, "saved FileInfo must keep the sender's file ID") assert.Equal(t, rc.RemoteId, *saved.RemoteId) // Verify the FileInfo was persisted with a server-constructed path storedFI, appErr := th.App.GetFileInfo(th.Context, saved.Id) require.Nil(t, appErr) + assert.Equal(t, senderFileID, storedFI.Id) assert.Equal(t, "hello.txt", storedFI.Name) assert.NotEmpty(t, storedFI.Path) assert.Contains(t, storedFI.Path, "hello.txt")