diff --git a/server/platform/services/remotecluster/recv.go b/server/platform/services/remotecluster/recv.go index b9db91e9406..ad183fabbb8 100644 --- a/server/platform/services/remotecluster/recv.go +++ b/server/platform/services/remotecluster/recv.go @@ -71,6 +71,9 @@ func (rcs *Service) ReceiveInviteConfirmation(confirm model.RemoteClusterInvite) // If the accepting cluster sent a RefreshedToken (its RemoteToken), set it as our Token if confirm.Version >= 2 && confirm.RefreshedToken != "" { + if confirm.RefreshedToken == rc.Token { + return nil, fmt.Errorf("cannot accept invite confirmation for remote %s: RefreshedToken must be different from the original invite token", confirm.RemoteId) + } rc.Token = confirm.RefreshedToken } else { // For older versions or if no RefreshedToken was provided, generate a new token diff --git a/server/platform/services/remotecluster/recv_test.go b/server/platform/services/remotecluster/recv_test.go index 8fa18073da6..bcbb66d37e6 100644 --- a/server/platform/services/remotecluster/recv_test.go +++ b/server/platform/services/remotecluster/recv_test.go @@ -258,6 +258,47 @@ func TestReceiveInviteConfirmation_TokenInvalidation(t *testing.T) { remoteClusterStoreMock.AssertExpectations(t) }) + + t.Run("Protocol v2+ with RefreshedToken equal to original token - rejected (MM-67098)", func(t *testing.T) { + // Security: RefreshedToken must be different from the original invite token. + // If the remote sends back the same token, they did not actually refresh; reject. + originalToken := model.NewId() + remoteId := model.NewId() + + originalRC := &model.RemoteCluster{ + RemoteId: remoteId, + Token: originalToken, + SiteURL: model.SiteURLPending + model.NewId(), + CreateAt: model.GetMillis(), + } + + remoteClusterStoreMock := &mocks.RemoteClusterStore{} + remoteClusterStoreMock.On("Get", remoteId, false).Return(originalRC, nil) + // Update must NOT be called when RefreshedToken == rc.Token + + storeMock := &mocks.Store{} + storeMock.On("RemoteCluster").Return(remoteClusterStoreMock) + + mockServer := newMockServerWithStore(t, storeMock) + mockApp := newMockApp(t, nil) + service, err := NewRemoteClusterService(mockServer, mockApp) + require.NoError(t, err) + + confirm := model.RemoteClusterInvite{ + RemoteId: remoteId, + SiteURL: "http://example.com", + Token: model.NewId(), + RefreshedToken: originalToken, // Same as rc.Token - invalid, must be different + Version: 3, + } + + rcUpdated, err := service.ReceiveInviteConfirmation(confirm) + + require.Error(t, err) + assert.Nil(t, rcUpdated) + assert.Contains(t, err.Error(), "RefreshedToken must be different from the original invite token") + remoteClusterStoreMock.AssertNotCalled(t, "Update", mock.Anything) + }) } // TestReceiveInviteConfirmation_EdgeCases tests various edge cases diff --git a/webapp/channels/src/components/create_recap_modal/create_recap_modal.scss b/webapp/channels/src/components/create_recap_modal/create_recap_modal.scss index a54ca756b0f..1985b7adabe 100644 --- a/webapp/channels/src/components/create_recap_modal/create_recap_modal.scss +++ b/webapp/channels/src/components/create_recap_modal/create_recap_modal.scss @@ -14,7 +14,6 @@ .create-recap-modal-body { max-height: 500px; padding: 0px 0px; - overflow-y: auto; } .create-recap-modal-header { @@ -22,6 +21,7 @@ width: 100%; align-items: center; justify-content: space-between; + margin-top: -9px; .create-recap-modal-header-actions { display: flex; @@ -136,13 +136,15 @@ .input-container { position: relative; display: flex; - align-items: center; + flex-direction: column; .icon { position: absolute; + top: 50%; left: 16px; color: rgba(var(--center-channel-color-rgb), 0.64); font-size: 16px; + transform: translateY(-50%); } .form-control { @@ -153,6 +155,36 @@ &:focus { border-color: var(--button-bg); } + + &.input-error { + border-color: var(--error-text); + + &:focus { + border-color: var(--error-text); + box-shadow: 0 0 0 1px var(--error-text); + } + } + } + + &.has-error { + .icon { + color: var(--error-text); + } + } + + .input-error-message { + display: flex; + align-items: center; + margin-top: 6px; + color: var(--error-text); + font-size: 12px; + gap: 4px; + + .icon { + position: static; + font-size: 14px; + transform: none; + } } } } diff --git a/webapp/channels/src/components/create_recap_modal/create_recap_modal.test.tsx b/webapp/channels/src/components/create_recap_modal/create_recap_modal.test.tsx index cdd899fe75d..273b6089eb6 100644 --- a/webapp/channels/src/components/create_recap_modal/create_recap_modal.test.tsx +++ b/webapp/channels/src/components/create_recap_modal/create_recap_modal.test.tsx @@ -267,6 +267,27 @@ describe('CreateRecapModal', () => { }); }); + test('should not advance to next step when name is empty and show validation error', async () => { + renderWithContext(, initialState); + + await waitFor(() => { + const dropdownButton = screen.getByLabelText('Agent selector'); + expect(dropdownButton).toHaveTextContent('Copilot'); + }); + + // Select a recap type but leave the name empty + const allUnreadsButton = screen.getByText('Recap all my unreads'); + await userEvent.click(allUnreadsButton); + + // Next button should be disabled since name is empty + const nextButton = screen.getByRole('button', {name: /next/i}); + expect(nextButton).toBeDisabled(); + + // We should still be on step 1 + expect(screen.getByText('Give your recap a name')).toBeInTheDocument(); + expect(screen.getByText('What type of recap would you like?')).toBeInTheDocument(); + }); + test('should enable Next on channel selection step when a checkbox is clicked', async () => { renderWithContext(, initialState); diff --git a/webapp/channels/src/components/create_recap_modal/recap_configuration.test.tsx b/webapp/channels/src/components/create_recap_modal/recap_configuration.test.tsx index 105095bf9a4..8e014159257 100644 --- a/webapp/channels/src/components/create_recap_modal/recap_configuration.test.tsx +++ b/webapp/channels/src/components/create_recap_modal/recap_configuration.test.tsx @@ -215,6 +215,68 @@ describe('RecapConfiguration', () => { }); }); + describe('Auto-focus', () => { + it('should auto-focus the name input on mount', () => { + renderWithContext(); + + const input = screen.getByPlaceholderText('Give your recap a name'); + expect(input).toHaveFocus(); + }); + }); + + describe('Name Validation', () => { + it('should not show error before input is blurred', () => { + renderWithContext(); + + expect(screen.queryByText('This field is required')).not.toBeInTheDocument(); + }); + + it('should show error message when input is blurred with empty name', async () => { + renderWithContext(); + + const input = screen.getByPlaceholderText('Give your recap a name'); + await userEvent.click(input); + await userEvent.tab(); + + expect(screen.getByText('This field is required')).toBeInTheDocument(); + }); + + it('should not show error when blurred with a valid name', async () => { + renderWithContext( + , + ); + + const input = screen.getByPlaceholderText('Give your recap a name'); + await userEvent.click(input); + await userEvent.tab(); + + expect(screen.queryByText('This field is required')).not.toBeInTheDocument(); + }); + + it('should add input-error class when error is shown', async () => { + renderWithContext(); + + const input = screen.getByPlaceholderText('Give your recap a name'); + await userEvent.click(input); + await userEvent.tab(); + + expect(input).toHaveClass('input-error'); + }); + + it('should set aria-invalid when error is shown', async () => { + renderWithContext(); + + const input = screen.getByPlaceholderText('Give your recap a name'); + await userEvent.click(input); + await userEvent.tab(); + + expect(input).toHaveAttribute('aria-invalid', 'true'); + }); + }); + describe('Form Labels', () => { it('should display name label', () => { renderWithContext(); diff --git a/webapp/channels/src/components/create_recap_modal/recap_configuration.tsx b/webapp/channels/src/components/create_recap_modal/recap_configuration.tsx index ea99b7b427e..53f869fc454 100644 --- a/webapp/channels/src/components/create_recap_modal/recap_configuration.tsx +++ b/webapp/channels/src/components/create_recap_modal/recap_configuration.tsx @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import React from 'react'; +import React, {useCallback, useState} from 'react'; import {useIntl, FormattedMessage} from 'react-intl'; import {ProductChannelsIcon, LightningBoltOutlineIcon, CheckCircleIcon} from '@mattermost/compass-icons/components'; @@ -21,8 +21,15 @@ type Props = { const RecapConfiguration = ({recapName, setRecapName, recapType, setRecapType, unreadChannels}: Props) => { const {formatMessage} = useIntl(); + const [touched, setTouched] = useState(false); const hasUnreadChannels = unreadChannels.length > 0; + const showError = touched && recapName.trim().length === 0; + + const handleBlur = useCallback(() => { + setTouched(true); + }, []); + const allUnreadsButton = ( -
- +
e.stopPropagation()} + > ({ })); jest.mock('mattermost-redux/selectors/entities/recaps', () => ({ + getAllRecaps: jest.fn(() => []), getUnreadRecaps: jest.fn(() => []), getReadRecaps: jest.fn(() => []), })); diff --git a/webapp/channels/src/components/recaps/recaps.tsx b/webapp/channels/src/components/recaps/recaps.tsx index eba7e8a55c4..6683468493c 100644 --- a/webapp/channels/src/components/recaps/recaps.tsx +++ b/webapp/channels/src/components/recaps/recaps.tsx @@ -10,7 +10,7 @@ import {PlusIcon} from '@mattermost/compass-icons/components'; import {getAgents} from 'mattermost-redux/actions/agents'; import {getRecaps} from 'mattermost-redux/actions/recaps'; -import {getUnreadRecaps, getReadRecaps} from 'mattermost-redux/selectors/entities/recaps'; +import {getAllRecaps, getUnreadRecaps, getReadRecaps} from 'mattermost-redux/selectors/entities/recaps'; import {selectLhsItem} from 'actions/views/lhs'; import {openModal} from 'actions/views/modals'; @@ -23,6 +23,7 @@ import {ModalIdentifiers} from 'utils/constants'; import {LhsItemType, LhsPage} from 'types/store/lhs'; +import AICopilotIntroSvg from './ai_copilot_intro_svg'; import RecapsList from './recaps_list'; import './recaps.scss'; @@ -31,15 +32,23 @@ const Recaps = () => { const {formatMessage} = useIntl(); const dispatch = useDispatch(); const [activeTab, setActiveTab] = useState<'unread' | 'read'>('unread'); + const [isLoading, setIsLoading] = useState(true); const enableAIRecaps = useGetFeatureFlagValue('EnableAIRecaps'); const agentsBridgeEnabled = useGetAgentsBridgeEnabled(); + const allRecaps = useSelector(getAllRecaps); const unreadRecaps = useSelector(getUnreadRecaps); const readRecaps = useSelector(getReadRecaps); + const hasNoRecaps = !isLoading && allRecaps.length === 0; + useEffect(() => { dispatch(selectLhsItem(LhsItemType.Page, LhsPage.Recaps)); - dispatch(getRecaps(0, 60)); + const fetchData = async () => { + await dispatch(getRecaps(0, 60)); + setIsLoading(false); + }; + fetchData(); dispatch(getAgents()); }, [dispatch]); @@ -67,34 +76,58 @@ const Recaps = () => { {formatMessage({id: 'recaps.title', defaultMessage: 'Recaps'})}
-
- - -
+ {!hasNoRecaps && ( +
+ + +
+ )}
- + {!hasNoRecaps && ( + + )}
- + {hasNoRecaps ? ( +
+ +

+ {formatMessage({id: 'recaps.placeholder.title', defaultMessage: 'Set up your recap'})} +

+

+ {formatMessage({id: 'recaps.placeholder.description', defaultMessage: 'Recaps help you get caught up quickly on discussions that are most important to you with a summarized report.'})} +

+ +
+ ) : ( + + )}
); diff --git a/webapp/channels/src/components/recaps/recaps_list.test.tsx b/webapp/channels/src/components/recaps/recaps_list.test.tsx index 7ab66f4b9f8..b66cd51e9e6 100644 --- a/webapp/channels/src/components/recaps/recaps_list.test.tsx +++ b/webapp/channels/src/components/recaps/recaps_list.test.tsx @@ -57,12 +57,5 @@ describe('RecapsList', () => { expect(screen.getByText('Morning Standup')).toBeInTheDocument(); expect(screen.getByText('Weekly Review')).toBeInTheDocument(); }); - - test('should show "all caught up" message at the bottom', () => { - renderWithContext(); - - const allCaughtUpMessages = screen.getAllByText("You're all caught up"); - expect(allCaughtUpMessages.length).toBeGreaterThan(0); - }); }); diff --git a/webapp/channels/src/components/recaps/recaps_list.tsx b/webapp/channels/src/components/recaps/recaps_list.tsx index ba8dae238fc..dfcfab2b261 100644 --- a/webapp/channels/src/components/recaps/recaps_list.tsx +++ b/webapp/channels/src/components/recaps/recaps_list.tsx @@ -87,11 +87,6 @@ const RecapsList = ({recaps}: Props) => { onToggle={() => toggleRecap(recap.id)} /> ))} - -
- - {formatMessage({id: 'recaps.allCaughtUp', defaultMessage: "You're all caught up"})} -
); }; diff --git a/webapp/channels/src/components/recaps_link/recaps_link.scss b/webapp/channels/src/components/recaps_link/recaps_link.scss index 7f9e2705eb5..b13bbb6017e 100644 --- a/webapp/channels/src/components/recaps_link/recaps_link.scss +++ b/webapp/channels/src/components/recaps_link/recaps_link.scss @@ -41,7 +41,8 @@ display: flex; align-items: center; padding: 3px; - margin-right: 4px; + margin-right: 1px; + color: rgba(var(--sidebar-text-rgb), 0.64); font-size: 18px; opacity: 0.64; @@ -69,11 +70,11 @@ color: var(--sidebar-text); .icon { - opacity: 1; + color: var(--sidebar-text); } .SidebarChannelLinkLabel { - font-weight: 600; + font-weight: 400; } } } diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index 5642a05b161..3b67749402a 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -5744,7 +5744,6 @@ "recaps.actionItems": "Action items:", "recaps.addRecap": "Add a recap", "recaps.addRecap.disabled": "Agents Bridge is not enabled", - "recaps.allCaughtUp": "You're all caught up", "recaps.channelMenu.ariaLabel": "Options for {channelName}", "recaps.defaultAgent": "Copilot", "recaps.delete.confirm.button": "Delete", @@ -5769,6 +5768,7 @@ "recaps.modal.error.noChannels": "Please select at least one channel.", "recaps.modal.nameLabel": "Give your recap a name", "recaps.modal.namePlaceholder": "Give your recap a name", + "recaps.modal.nameRequired": "This field is required", "recaps.modal.noChannels": "No channels found", "recaps.modal.noUnreadsAvailable": "No unread channels available", "recaps.modal.noUnreadsAvailableHint": "You currently have no unread messages in any channels", @@ -5780,6 +5780,9 @@ "recaps.modal.summaryTitle": "The following channels will be included in your recap", "recaps.modal.title": "Set up your recap", "recaps.modal.typeLabel": "What type of recap would you like?", + "recaps.placeholder.createRecap": "Create a recap", + "recaps.placeholder.description": "Recaps help you get caught up quickly on discussions that are most important to you with a summarized report.", + "recaps.placeholder.title": "Set up your recap", "recaps.processing.message": "We're working on your recap. Check back shortly", "recaps.processing.subtitle": "Recap created. You'll receive a summary shortly", "recaps.readTab": "Read",