IntenseWebs/mattermost
3
0
Fork 0
mirror of https://github.com/mattermost/mattermost.git synced 2025-02-25 18:55:24 -06:00
Code Issues Packages Projects Releases Wiki Activity

MM-55468 Ensure custom status emojis exist (#25501)

Browse Source 
* MM-55468 Ensure custom status emojis exist

* Fix plugin API unit test

* Print underlying error as detailed error message

* Convert CustomStatusModal tests to React Testing Library and improve a11y

* Don't suggest custom statuses with non-existent emojis

* Silence test error by providing fake translation strings
This commit is contained in:
Harrison Healey
2023-12-08 10:35:15 -05:00
committed by GitHub
parent 109f4643c6
commit b1e745894b
14 changed files with 320 additions and 115 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
server/channels/app/emoji.go
View File

@@ -366,3 +366,17 @@ func (a *App) deleteReactionsForEmoji(rctx request.CTX, emojiName string) {
rctx.Logger().Warn("Unable to delete reactions when deleting emoji", mlog.String("emoji_name", emojiName), mlog.Err(err))
}
}
func (a *App) confirmEmojiExists(c request.CTX, emojiName string) *model.AppError {
if model.IsSystemEmojiName(emojiName) {
return nil
}
err := model.IsValidEmojiName(emojiName)
if err != nil {
return err
}
_, err = a.GetEmojiByName(c, emojiName)
return err
}

2
server/channels/app/plugin_api_test.go
View File

@@ -513,7 +513,7 @@ func TestPluginAPIUserCustomStatus(t *testing.T) {
defer th.App.PermanentDeleteUser(th.Context, user1)
custom := &model.CustomStatus{
Emoji: ":tada:",
Emoji: "tada",
Text: "honk",
}

7
server/channels/app/status.go
View File

@@ -77,6 +77,13 @@ func (a *App) SetCustomStatus(c request.CTX, userID string, cs *model.CustomStat
return model.NewAppError("SetCustomStatus", "api.custom_status.set_custom_statuses.update.app_error", nil, "", http.StatusBadRequest)
}
// Ensure the emoji exists before saving the custom status even if it's deleted afterwards
if cs.Emoji != "" {
if err := a.confirmEmojiExists(c, cs.Emoji); err != nil {
return model.NewAppError("SetCustomStatus", "api.custom_status.set_custom_statuses.emoji_not_found", nil, err.Error(), http.StatusBadRequest)
}
}
user, err := a.GetUser(userID)
if err != nil {
return err

93
server/channels/app/status_test.go
View File

@@ -6,6 +6,7 @@ package app
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
@@ -22,7 +23,7 @@ func TestCustomStatus(t *testing.T) {
user := th.BasicUser
cs := &model.CustomStatus{
Emoji: ":smile:",
Emoji: "smile",
Text: "honk!",
}
@@ -91,7 +92,7 @@ func TestCustomStatusErrors(t *testing.T) {
require.NoError(t, err)
cs := &model.CustomStatus{
Emoji: ":smile:",
Emoji: "smile",
Text: "honk!",
}
@@ -108,3 +109,91 @@ func TestCustomStatusErrors(t *testing.T) {
})
}
}
func TestSetCustomStatus(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.EnableCustomEmoji = true
})
emoji := th.CreateEmoji()
for _, testCase := range []struct {
Name string
Input *model.CustomStatus
ExpectsError bool
}{
{
Name: "should be able to set custom status with text and emoji",
Input: &model.CustomStatus{
Emoji: "smile",
Text: "honk!",
},
ExpectsError: false,
},
{
Name: "should be able to set custom status with only text",
Input: &model.CustomStatus{
Text: "honk!",
},
ExpectsError: false,
},
{
Name: "should be able to set custom status with just a system emoji",
Input: &model.CustomStatus{
Emoji: "smile",
},
ExpectsError: false,
},
{
Name: "should be able to set custom status with just a custom emoji",
Input: &model.CustomStatus{
Emoji: emoji.Name,
},
ExpectsError: false,
},
{
Name: "should not be able to set custom status without text or emoji",
Input: &model.CustomStatus{},
ExpectsError: true,
},
{
Name: "should not be able to set custom status with a non-existent emoji name",
Input: &model.CustomStatus{
Emoji: "somethingthatdoesntexist",
},
ExpectsError: true,
},
{
Name: "should not be able to set custom status with an invalid emoji name",
Input: &model.CustomStatus{
Emoji: "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz",
Text: "honk!",
},
ExpectsError: true,
},
} {
t.Run(testCase.Name, func(t *testing.T) {
err := th.App.SetCustomStatus(th.Context, th.BasicUser.Id, testCase.Input)
defer th.App.RemoveCustomStatus(th.Context, th.BasicUser.Id)
if testCase.ExpectsError {
require.NotNil(t, err)
} else {
require.Nil(t, err)
}
customStatus, err := th.App.GetCustomStatus(th.BasicUser.Id)
require.Nil(t, err)
if testCase.ExpectsError {
assert.NotEqual(t, testCase.Input, customStatus)
} else {
assert.Equal(t, testCase.Input, customStatus)
}
})
}
}

4
server/i18n/en.json
View File

@@ -1717,6 +1717,10 @@
"id": "api.custom_status.recent_custom_statuses.delete.app_error",
"translation": "Failed to delete the recent status. Please try adding the status first or contact your system administrator for details."
},
{
"id": "api.custom_status.set_custom_statuses.emoji_not_found",
"translation": "Failed to update the custom status. An emoji with the given name does not exist."
},
{
"id": "api.custom_status.set_custom_statuses.update.app_error",
"translation": "Failed to update the custom status. Please add either emoji or custom text status or both."

4
server/public/model/emoji.go
View File

@@ -36,7 +36,7 @@ func (emoji *Emoji) Auditable() map[string]interface{} {
}
}
func inSystemEmoji(emojiName string) bool {
func IsSystemEmojiName(emojiName string) bool {
_, ok := SystemEmojis[emojiName]
return ok
}
@@ -92,7 +92,7 @@ func IsValidEmojiName(name string) *AppError {
if name == "" || len(name) > EmojiNameMaxLength || !IsValidAlphaNumHyphenUnderscorePlus(name) {
return NewAppError("Emoji.IsValid", "model.emoji.name.app_error", nil, "", http.StatusBadRequest)
}
if inSystemEmoji(name) {
if IsSystemEmojiName(name) {
return NewAppError("Emoji.IsValid", "model.emoji.system_emoji_name.app_error", nil, "", http.StatusBadRequest)
}

31
webapp/channels/src/actions/emoji_actions.js
View File

@@ -3,7 +3,9 @@
import * as EmojiActions from 'mattermost-redux/actions/emojis';
import {savePreferences} from 'mattermost-redux/actions/preferences';
import {Preferences as ReduxPreferences} from 'mattermost-redux/constants';
import {getCustomEmojisByName as selectCustomEmojisByName, getCustomEmojisEnabled} from 'mattermost-redux/selectors/entities/emojis';
import {get} from 'mattermost-redux/selectors/entities/preferences';
import {getCurrentUserId} from 'mattermost-redux/selectors/entities/users';
import {getEmojiMap, getRecentEmojisData, getRecentEmojisNames, isCustomEmojiEnabled} from 'selectors/emojis';
@@ -132,6 +134,35 @@ export function loadCustomEmojisForCustomStatusesByUserIds(userIds) {
};
}
export function loadCustomEmojisForRecentCustomStatuses() {
return (dispatch, getState) => {
const state = getState();
const customEmojiEnabled = isCustomEmojiEnabled(state);
const customStatusEnabled = isCustomStatusEnabled(state);
if (!customEmojiEnabled || !customStatusEnabled) {
return {data: false};
}
const recentCustomStatusesValue = get(state, ReduxPreferences.CATEGORY_CUSTOM_STATUS, ReduxPreferences.NAME_RECENT_CUSTOM_STATUSES);
if (!recentCustomStatusesValue) {
return {data: false};
}
const recentCustomStatuses = JSON.parse(recentCustomStatusesValue);
const emojisToLoad = new Set();
for (const customStatus of recentCustomStatuses) {
if (!customStatus || !customStatus.emoji) {
continue;
}
emojisToLoad.add(customStatus.emoji);
}
return dispatch(loadCustomEmojisIfNeeded(Array.from(emojisToLoad)));
};
}
export function loadCustomEmojisIfNeeded(emojis) {
return (dispatch, getState) => {
if (!emojis || emojis.length === 0) {

77
webapp/channels/src/components/custom_status/__snapshots__/custom_status_modal.test.tsx.snap
View File

@@ -1,77 +0,0 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`components/custom_status/custom_status_modal should match snapshot 1`] = `
<ContextProvider
value={
Object {
"store": Object {
"clearActions": [Function],
"dispatch": [Function],
"getActions": [Function],
"getState": [Function],
"replaceReducer": [Function],
"subscribe": [Function],
},
"subscription": Subscription {
"handleChangeWrapper": [Function],
"listeners": Object {
"notify": [Function],
},
"onStateChange": [Function],
"parentSub": undefined,
"store": Object {
"clearActions": [Function],
"dispatch": [Function],
"getActions": [Function],
"getState": [Function],
"replaceReducer": [Function],
"subscribe": [Function],
},
"unsubscribe": null,
},
}
}
>
<CustomStatusModal
onExited={[MockFunction]}
/>
</ContextProvider>
`;
exports[`components/custom_status/custom_status_modal should match snapshot when user has custom status set 1`] = `
<ContextProvider
value={
Object {
"store": Object {
"clearActions": [Function],
"dispatch": [Function],
"getActions": [Function],
"getState": [Function],
"replaceReducer": [Function],
"subscribe": [Function],
},
"subscription": Subscription {
"handleChangeWrapper": [Function],
"listeners": Object {
"notify": [Function],
},
"onStateChange": [Function],
"parentSub": undefined,
"store": Object {
"clearActions": [Function],
"dispatch": [Function],
"getActions": [Function],
"getState": [Function],
"replaceReducer": [Function],
"subscribe": [Function],
},
"unsubscribe": null,
},
}
}
>
<CustomStatusModal
onExited={[MockFunction]}
/>
</ContextProvider>
`;

158
webapp/channels/src/components/custom_status/custom_status_modal.test.tsx
View File

@@ -1,46 +1,156 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import {shallow} from 'enzyme';
import React from 'react';
import {Provider} from 'react-redux';
import type {AutoSizerProps} from 'react-virtualized-auto-sizer';
import * as StatusSelectors from 'selectors/views/custom_status';
import type {DeepPartial} from '@mattermost/types/utilities';
import mockStore from 'tests/test_store';
import {Preferences} from 'mattermost-redux/constants';
import mergeObjects from 'packages/mattermost-redux/test/merge_objects';
import {act, renderWithContext, screen, userEvent} from 'tests/react_testing_utils';
import {TestHelper} from 'utils/test_helper';
import type {GlobalState} from 'types/store';
import CustomStatusModal from './custom_status_modal';
jest.mock('selectors/views/custom_status');
jest.mock('react-virtualized-auto-sizer', () => (props: AutoSizerProps) => props.children({height: 100, width: 100}));
jest.mock('images/img_trans.gif', () => 'img_trans.gif');
describe('components/custom_status/custom_status_modal', () => {
const store = mockStore({});
describe('CustomStatusModal', () => {
const baseProps = {
onExited: jest.fn(),
};
it('should match snapshot', () => {
const wrapper = shallow(
<Provider store={store}>
<CustomStatusModal {...baseProps}/>
</Provider>,
const initialState: DeepPartial<GlobalState> = {
entities: {
general: {
config: {
EnableCustomEmoji: 'true',
EnableCustomUserStatuses: 'true',
},
},
},
};
// The emoji picker renders emoji categories without passing a defaultMessage, and we don't pass translation strings
// into the provider by default, so we need to pass something for this string to silence errors from FormatJS.
const renderOptions = {
intlMessages: {
'emoji_picker.smileys-emotion': 'Smileys & Emotions',
},
};
test('should render suggested statuses until the user starts typing', () => {
renderWithContext(
<CustomStatusModal
{...baseProps}
/>,
initialState,
renderOptions,
);
expect(wrapper).toMatchSnapshot();
expect(screen.getByText('SUGGESTIONS')).toBeInTheDocument();
expect(screen.getByText('Out for lunch')).toBeInTheDocument();
expect(screen.getByLabelText(':hamburger:')).toBeInTheDocument();
userEvent.type(screen.getByPlaceholderText('Set a status'), 'Test status, please ignore');
expect(screen.queryByText('SUGGESTIONS')).not.toBeInTheDocument();
expect(screen.queryByText('Out for lunch')).not.toBeInTheDocument();
expect(screen.queryByLabelText(':hamburger:')).not.toBeInTheDocument();
});
it('should match snapshot when user has custom status set', () => {
const customStatus = {
emoji: 'speech_balloon',
text: 'speaking',
};
(StatusSelectors.makeGetCustomStatus as jest.Mock).mockReturnValue(() => customStatus);
const wrapper = shallow(
<Provider store={store}>
<CustomStatusModal {...baseProps}/>
</Provider>,
test('should render suggested statuses until the user selects an emoji', () => {
renderWithContext(
<CustomStatusModal
{...baseProps}
/>,
initialState,
renderOptions,
);
expect(wrapper).toMatchSnapshot();
expect(screen.getByText('SUGGESTIONS')).toBeInTheDocument();
expect(screen.getByLabelText(':hamburger:')).toBeInTheDocument();
expect(screen.getByText('Out for lunch')).toBeInTheDocument();
userEvent.click(screen.getByLabelText('select an emoji'));
act(() => userEvent.click(screen.getByLabelText('grinning emoji')));
expect(screen.queryByText('SUGGESTIONS')).not.toBeInTheDocument();
expect(screen.queryByText('Out for lunch')).not.toBeInTheDocument();
expect(screen.queryByLabelText(':hamburger:')).not.toBeInTheDocument();
});
test('should render recently used statuses as suggestions', () => {
const testState = mergeObjects(initialState, {
entities: {
preferences: {
myPreferences: TestHelper.getPreferencesMock([
{
category: Preferences.CATEGORY_CUSTOM_STATUS,
name: Preferences.NAME_RECENT_CUSTOM_STATUSES,
value: JSON.stringify([
TestHelper.getCustomStatusMock({emoji: 'taco', text: 'Eating a taco'}),
]),
},
]),
},
},
});
renderWithContext(
<CustomStatusModal
{...baseProps}
/>,
testState,
renderOptions,
);
expect(screen.getByText('SUGGESTIONS')).toBeInTheDocument();
expect(screen.getByText('Eating a taco')).toBeInTheDocument();
expect(screen.getByLabelText(':taco:')).toBeInTheDocument();
});
test('should render recently used statuses with custom emojis which exist', () => {
const existentEmoji = TestHelper.getCustomEmojiMock({name: 'existent'});
const testState = mergeObjects(initialState, {
entities: {
emojis: {
customEmoji: {
[existentEmoji.id]: existentEmoji,
},
},
preferences: {
myPreferences: TestHelper.getPreferencesMock([
{
category: Preferences.CATEGORY_CUSTOM_STATUS,
name: Preferences.NAME_RECENT_CUSTOM_STATUSES,
value: JSON.stringify([
TestHelper.getCustomStatusMock({emoji: 'existent', text: 'Existing'}),
TestHelper.getCustomStatusMock({emoji: 'nonexistent', text: 'Not existing'}),
]),
},
]),
},
},
});
renderWithContext(
<CustomStatusModal
{...baseProps}
/>,
testState,
renderOptions,
);
expect(screen.getByText('SUGGESTIONS')).toBeInTheDocument();
expect(screen.getByText('Existing')).toBeInTheDocument();
expect(screen.getByLabelText(':existent:')).toBeInTheDocument();
expect(screen.queryByText('Not existing')).not.toBeInTheDocument();
expect(screen.queryByLabelText(':nonexistent:')).not.toBeInTheDocument();
});
});

7
webapp/channels/src/components/custom_status/custom_status_modal.tsx
View File

@@ -19,7 +19,7 @@ import {setCustomStatus, unsetCustomStatus, removeRecentCustomStatus} from 'matt
import {Preferences} from 'mattermost-redux/constants';
import {getCurrentTimezone} from 'mattermost-redux/selectors/entities/timezone';
import {loadCustomEmojisIfNeeded} from 'actions/emoji_actions';
import {loadCustomEmojisForRecentCustomStatuses} from 'actions/emoji_actions';
import {closeModal} from 'actions/views/modals';
import {makeGetCustomStatus, getRecentCustomStatuses, showStatusDropdownPulsatingDot, isCustomStatusExpired} from 'selectors/views/custom_status';
@@ -153,9 +153,7 @@ const CustomStatusModal: React.FC<Props> = (props: Props) => {
};
const loadCustomEmojisForRecentStatuses = () => {
const emojisToLoad = new Set<string>();
recentCustomStatuses.forEach((customStatus: UserCustomStatus) => emojisToLoad.add(customStatus.emoji));
dispatch(loadCustomEmojisIfNeeded(Array.from(emojisToLoad)));
dispatch(loadCustomEmojisForRecentCustomStatuses());
};
const handleStatusExpired = () => {
@@ -427,6 +425,7 @@ const CustomStatusModal: React.FC<Props> = (props: Props) => {
type='button'
onClick={toggleEmojiPicker}
ref={emojiButtonRef}
aria-label={formatMessage({id: 'emoji_picker.emojiPicker.button.ariaLabel', defaultMessage: 'select an emoji'})}
className={classNames('emoji-picker__container', 'StatusModal__emoji-button', {
'StatusModal__emoji-button--active': showEmojiPicker,
})}

2
webapp/channels/src/components/emoji/render_emoji.tsx
View File

@@ -34,7 +34,7 @@ const RenderEmoji = ({emojiName, emojiStyle, size, onClick}: ComponentProps) =>
<span
onClick={onClick}
className='emoticon'
alt={`:${emojiName}:`}
aria-label={`:${emojiName}:`}
data-emoticon={emojiName}
style={{
backgroundImage: `url(${emojiImageUrl})`,

20
webapp/channels/src/selectors/views/custom_status.ts
View File

@@ -13,6 +13,8 @@ import {get} from 'mattermost-redux/selectors/entities/preferences';
import {getCurrentTimezone} from 'mattermost-redux/selectors/entities/timezone';
import {getCurrentUser, getUser} from 'mattermost-redux/selectors/entities/users';
import {getEmojiMap} from 'selectors/emojis';
import {getCurrentMomentForTimezone} from 'utils/timezone';
import {isDateWithinDaysRange, TimeInformation} from 'utils/utils';
@@ -44,11 +46,23 @@ export function isCustomStatusExpired(state: GlobalState, customStatus?: UserCus
return currentTime.isSameOrAfter(expiryTime);
}
export const getRecentCustomStatuses = createSelector(
/**
* getRecentCustomStatuses returns an array of the current user's recent custom statuses with any statuses using
* non-loaded or non-existent emojis filtered out.
*/
export const getRecentCustomStatuses: (state: GlobalState) => UserCustomStatus[] = createSelector(
'getRecentCustomStatuses',
(state: GlobalState) => get(state, Preferences.CATEGORY_CUSTOM_STATUS, Preferences.NAME_RECENT_CUSTOM_STATUSES),
(value) => {
return value ? JSON.parse(value) : [];
getEmojiMap,
(value, emojiMap) => {
if (!value) {
return [];
}
let recentCustomStatuses: UserCustomStatus[] = JSON.parse(value);
recentCustomStatuses = recentCustomStatuses.filter((customStatus) => emojiMap.has(customStatus.emoji));
return recentCustomStatuses;
},
);

3
webapp/channels/src/tests/react_testing_utils.tsx
View File

@@ -23,6 +23,7 @@ export * from '@testing-library/react';
export {userEvent};
export type FullContextOptions = {
intlMessages?: Record<string, string>;
locale?: string;
useMockedStore?: boolean;
}
@@ -33,6 +34,7 @@ export const renderWithContext = (
partialOptions?: FullContextOptions,
) => {
const options = {
intlMessages: partialOptions?.intlMessages,
locale: partialOptions?.locale ?? 'en',
useMockedStore: partialOptions?.useMockedStore ?? false,
};
@@ -55,6 +57,7 @@ export const renderWithContext = (
<Router history={renderState.history}>
<IntlProvider
locale={renderState.options.locale}
messages={renderState.options.intlMessages}
>
<WebSocketContext.Provider value={WebSocketClient}>
{props.children}

13
webapp/channels/src/utils/test_helper.ts
View File

@@ -17,7 +17,8 @@ import type {Reaction} from '@mattermost/types/reactions';
import type {Role} from '@mattermost/types/roles';
import type {Session} from '@mattermost/types/sessions';
import type {Team, TeamMembership} from '@mattermost/types/teams';
import type {UserProfile, UserAccessToken} from '@mattermost/types/users';
import {CustomStatusDuration} from '@mattermost/types/users';
import type {UserProfile, UserAccessToken, UserCustomStatus} from '@mattermost/types/users';
import {CategoryTypes} from 'mattermost-redux/constants/channel_categories';
import {getPreferenceKey} from 'mattermost-redux/utils/preference_utils';
@@ -81,6 +82,16 @@ export class TestHelper {
return Object.assign({}, defaultUser, override);
}
public static getCustomStatusMock(override?: Partial<UserCustomStatus>): UserCustomStatus {
const defaultCustomStatus: UserCustomStatus = {
emoji: 'neutral_face',
text: 'text',
duration: CustomStatusDuration.DONT_CLEAR,
};
return Object.assign({}, defaultCustomStatus, override);
}
public static getUserAccessTokenMock(override?: Partial<UserAccessToken>): UserAccessToken {
const defaultUserAccessToken: UserAccessToken = {
id: 'token_id',