[pull] master from mattermost:master

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', () => {

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)

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")

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<string, ChannelBookmark>,
+    hasBookmarks: true,
+    limitReached: false,
+    canUploadFiles: true,
+    canReorder: true,
+    isDragging: false,
+    canAdd: true,
+};
+
+type DropTargetConfig = {
+    canDrop: (args: {source: {data: Record<string, unknown>}}) => boolean;
+    getData: () => Record<string, unknown>;
+};
+
+describe('BookmarksBarMenu — overflow drop target', () => {
+    beforeEach(() => {
+        mockDropTargetForElements.mockClear();
+    });
+
+    test('canDrop returns false when there is no overflow', () => {
+        renderWithContext(
+            <BookmarksBarMenu
+                {...baseProps}
+                overflowItems={[]}
+            />,
+        );
+
+        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(
+            <BookmarksBarMenu
+                {...baseProps}
+                overflowItems={overflowItems}
+                bookmarks={bookmarks}
+            />,
+        );
+
+        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(
+            <BookmarksBarMenu
+                {...baseProps}
+                overflowItems={overflowItems}
+                bookmarks={bookmarks}
+            />,
+        );
+
+        const config = mockDropTargetForElements.mock.calls[0][0] as unknown as DropTargetConfig;
+        expect(config.canDrop({source: {data: {type: 'something-else'}}})).toBe(false);
+    });
+});

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,
         });
     }, []);
 

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}
                 />
             </BookmarksBarContent>
         </Container>

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();
+    });
+});