refactor(brand_image_setting): migrate BrandImageSetting to a function component (#34536)

* refactor(brand_image_setting): migrate to function component

* test(brand_image_setting): update tests

Migrated tests to React Testing Library.

* refactor(brand_image_setting): wrap functions with useCallback

* test(brand_image_setting): use nock to mock fetch api

* test(brand_image_setting): use findby query instead of getby

* test(brand_image_setting): remove unnecessary scope assertion

* chore(brand_image_setting): split useEffect into two

Also extracted the handleSave function and wrapped it in useCallback.

* test(brand_image_setting): add e2e test for deleting brand image

* test(brand_image_setting): use destructured functions

* chore: delete unnecessary comment

* Revert "test(brand_image_setting): use destructured functions"

This reverts commit 71dc6628ed.

* Fix bad merge

* Fully revert changes to test from merge

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
Co-authored-by: Harrison Healey <harrisonmhealey@gmail.com>
This commit is contained in:
Vicktor 2026-04-09 16:16:23 +03:00 committed by GitHub
parent cf102afc17
commit 6878d09547
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 244 additions and 267 deletions

View file

@ -49,10 +49,28 @@ describe('Customization', () => {
// # Save setting
saveSetting();
// # Verify that after page reload image exist
cy.reload();
cy.findByTestId('CustomBrandImage').should('be.visible').within(() => {
// * Verify that after page reload image exist
cy.get('img').should('have.attr', 'src').and('include', '/api/v4/brand/image?t=');
// * Verify that there's an option to delete the image.
cy.findByTestId('remove-image__btn').should('be.visible');
// # delete the image
cy.findByTestId('remove-image__btn').click();
});
// # Save setting
saveSetting();
cy.reload();
cy.findByTestId('CustomBrandImage').should('be.visible').within(() => {
// * Verify that after page reload, the image doesn't exist.
cy.findByAltText('brand image').should('not.exist');
// * Verify there's no option to delete the image.
cy.findByTestId('remove-image__btn').should('not.exist');
});
});

View file

@ -1,30 +1,18 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import nock from 'nock';
import React from 'react';
import {uploadBrandImage, deleteBrandImage} from 'actions/admin_actions.jsx';
import {Client4} from 'mattermost-redux/client';
import {renderWithContext, screen, userEvent, waitFor} from 'tests/react_testing_utils';
import BrandImageSetting from './brand_image_setting';
// Real implementations are async (await dispatch(...)); mocks must return Promises so handleSave can await them.
jest.mock('actions/admin_actions.jsx', () => ({
...jest.requireActual('actions/admin_actions.jsx'),
uploadBrandImage: jest.fn(async () => {}),
deleteBrandImage: jest.fn(async () => {}),
}));
Client4.setUrl('http://localhost:8065');
describe('components/admin_console/brand_image_setting', () => {
beforeEach(() => {
jest.spyOn(global, 'fetch').mockResolvedValue({status: 404} as Response);
});
afterEach(() => {
jest.restoreAllMocks();
});
const baseProps = {
disabled: false,
setSaveNeeded: jest.fn(),
@ -32,71 +20,59 @@ describe('components/admin_console/brand_image_setting', () => {
unRegisterSaveAction: jest.fn(),
};
test('should have called deleteBrandImage or uploadBrandImage on save depending on component state', async () => {
let saveAction: (() => Promise<unknown>) | undefined;
const registerSaveAction = jest.fn((fn: () => Promise<unknown>) => {
saveAction = fn;
});
const deleteButtonTestId = 'remove-image__btn';
const {container, unmount} = renderWithContext(
<BrandImageSetting
{...baseProps}
registerSaveAction={registerSaveAction}
/>,
);
let scope: nock.Scope;
// Wait for componentDidMount fetch to resolve
await waitFor(() => {
expect(registerSaveAction).toHaveBeenCalled();
});
expect(saveAction).toBeDefined();
beforeAll(() => {
scope = nock(Client4.getBaseRoute()).persist().get('/brand/image').query(true).reply(200);
});
// Simulate selecting a file via the file input to set brandImage
const file = new File(['brand_image_file'], 'brand.png', {type: 'image/png'});
const fileInput = container.querySelector('input[type="file"]');
expect(fileInput).toBeInTheDocument();
await userEvent.upload(fileInput as HTMLInputElement, file);
afterAll(() => {
nock.cleanAll();
});
// Now call save - should call uploadBrandImage
await saveAction!();
expect(deleteBrandImage).toHaveBeenCalledTimes(0);
expect(uploadBrandImage).toHaveBeenCalledTimes(1);
test('should register and unregister save handler when mounted and unmounted respectively', () => {
const {unmount} = renderWithContext(<BrandImageSetting {...baseProps}/>);
expect(baseProps.registerSaveAction).toHaveBeenCalledTimes(1);
// To test deleteBrandImage path, unmount then re-mount with fetch returning 200
unmount();
jest.clearAllMocks();
(global.fetch as jest.Mock).mockResolvedValueOnce({status: 200} as Response);
let saveAction2: (() => Promise<unknown>) | undefined;
const registerSaveAction2 = jest.fn((fn: () => Promise<unknown>) => {
saveAction2 = fn;
});
expect(baseProps.unRegisterSaveAction).toHaveBeenCalledTimes(1);
});
renderWithContext(
<BrandImageSetting
{...baseProps}
registerSaveAction={registerSaveAction2}
/>,
);
test('should show delete button if brand image exists', async () => {
renderWithContext(<BrandImageSetting {...baseProps}/>);
await waitFor(() => {
expect(registerSaveAction2).toHaveBeenCalled();
});
expect(saveAction2).toBeDefined();
await waitFor(() => expect(scope.isDone()).toBe(true));
expect(screen.getByTestId(deleteButtonTestId)).toBeVisible();
});
test('should hide delete button if the setting is disabled', async () => {
const props = {...baseProps, disabled: true};
renderWithContext(<BrandImageSetting {...props}/>);
await waitFor(() => expect(screen.queryByTestId(deleteButtonTestId)).toBe(null));
});
test('should call setSaveNeeded when a brand image is uploaded', async () => {
renderWithContext(<BrandImageSetting {...baseProps}/>);
await userEvent.upload(screen.getByTestId('file__upload-input'), new File(['brand_image_file'], 'brand_image_file.png', {type: 'image/png'}));
expect(baseProps.setSaveNeeded).toHaveBeenCalledTimes(1);
});
test('should call setSaveNeeded when the delete button is pressed', async () => {
renderWithContext(<BrandImageSetting {...baseProps}/>);
const deleteButton = await screen.findByTestId(deleteButtonTestId);
// Wait for the brand image to be detected and delete button to appear
await waitFor(() => {
expect(screen.getByText('×')).toBeInTheDocument();
});
const deleteButton = screen.getByText('×').closest('button')!;
await userEvent.click(deleteButton);
await waitFor(() => {
expect(screen.getByText('No brand image uploaded')).toBeInTheDocument();
});
await saveAction2!();
expect(deleteBrandImage).toHaveBeenCalledTimes(1);
expect(uploadBrandImage).toHaveBeenCalledTimes(0);
expect(baseProps.setSaveNeeded).toHaveBeenCalledTimes(1);
});
});

View file

@ -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, {memo, useCallback, useEffect, useRef, useState} from 'react';
import {FormattedMessage} from 'react-intl';
import {Client4} from 'mattermost-redux/client';
@ -9,6 +9,7 @@ import {Client4} from 'mattermost-redux/client';
import {uploadBrandImage, deleteBrandImage} from 'actions/admin_actions.jsx';
import SettingSet from 'components/admin_console/setting_set';
import useDidUpdate from 'components/common/hooks/useDidUpdate';
import FormError from 'components/form_error';
import WithTooltip from 'components/with_tooltip';
@ -44,242 +45,224 @@ type Props = {
unRegisterSaveAction: (saveAction: () => Promise<unknown>) => void;
};
type State = {
deleteBrandImage: boolean;
brandImage?: Blob;
brandImageExists: boolean;
brandImageTimestamp: number;
error: string;
};
const BrandImageSetting = ({
id,
disabled,
setSaveNeeded,
registerSaveAction,
unRegisterSaveAction,
}: Props) => {
const imageRef = useRef<HTMLImageElement>(null);
const fileInputRef = useRef<HTMLInputElement>(null);
export default class BrandImageSetting extends React.PureComponent<Props, State> {
private imageRef: React.RefObject<HTMLImageElement>;
private fileInputRef: React.RefObject<HTMLInputElement>;
const [brandImage, setBrandImage] = useState<Blob | undefined>();
const [shouldDeleteBrandImage, setShouldDeleteBrandImage] = useState(false);
const [brandImageExists, setBrandImageExists] = useState(false);
const [brandImageTimestamp, setBrandImageTimestamp] = useState(Date.now());
constructor(props: Props) {
super(props);
const [errorFromState, setErrorFromState] = useState('');
this.state = {
deleteBrandImage: false,
brandImageExists: false,
brandImageTimestamp: Date.now(),
error: '',
};
const handleSave = useCallback(async () => {
setErrorFromState('');
this.imageRef = React.createRef();
this.fileInputRef = React.createRef();
}
let error;
if (shouldDeleteBrandImage) {
await deleteBrandImage(
() => {
setShouldDeleteBrandImage(false);
setBrandImageExists(false);
setBrandImage(undefined);
},
(err: Error) => {
error = err;
setErrorFromState(err.message);
},
);
} else if (brandImage) {
await uploadBrandImage(
brandImage,
() => {
setBrandImageExists(true);
setBrandImage(undefined);
setBrandImageTimestamp(Date.now());
},
(err: Error) => {
error = err;
setErrorFromState(err.message);
},
);
}
return {error};
}, [brandImage, shouldDeleteBrandImage]);
componentDidMount() {
useEffect(() => {
fetch(
Client4.getBrandImageUrl(String(this.state.brandImageTimestamp)),
Client4.getBrandImageUrl(String(Date.now())),
).then((resp) => {
if (resp.status === HTTP_STATUS_OK) {
this.setState({brandImageExists: true});
setBrandImageExists(true);
} else {
this.setState({brandImageExists: false});
setBrandImageExists(false);
}
}).catch((err) => {
console.error(`unable to retrieve brand image: ${err}`); //eslint-disable-line no-console
this.setState({brandImageExists: false});
setBrandImageExists(false);
});
}, []);
this.props.registerSaveAction(this.handleSave);
}
useEffect(() => {
registerSaveAction(handleSave);
componentWillUnmount() {
this.props.unRegisterSaveAction(this.handleSave);
}
return () => {
unRegisterSaveAction(handleSave);
};
}, [handleSave, registerSaveAction, unRegisterSaveAction]);
componentDidUpdate() {
if (this.imageRef.current) {
useDidUpdate(() => {
if (imageRef.current) {
const reader = new FileReader();
const img = this.imageRef.current;
const img = imageRef.current;
reader.onload = (e) => {
const src =
e.target?.result instanceof ArrayBuffer ? e.target?.result.toString() : e.target?.result;
e.target?.result instanceof ArrayBuffer ? e.target?.result.toString() : e.target?.result;
if (src) {
img.setAttribute('src', src);
}
};
if (this.state.brandImage) {
reader.readAsDataURL(this.state.brandImage);
if (brandImage) {
reader.readAsDataURL(brandImage);
}
}
}
}, [brandImage]);
handleSelectClick = () => {
this.fileInputRef.current?.click();
};
const handleSelectClick = useCallback(() => {
fileInputRef.current?.click();
}, []);
handleImageChange = () => {
if (!this.fileInputRef.current) {
const handleImageChange = useCallback(() => {
if (!fileInputRef.current) {
return;
}
const element = this.fileInputRef.current;
const element = fileInputRef.current;
if (element.files && element.files.length > 0) {
this.props.setSaveNeeded();
this.setState({
brandImage: element.files[0],
deleteBrandImage: false,
});
setSaveNeeded();
setBrandImage(element.files[0]);
setShouldDeleteBrandImage(false);
}
};
}, [setSaveNeeded]);
handleDeleteButtonPressed = () => {
this.setState({
deleteBrandImage: true,
brandImage: undefined,
brandImageExists: false,
});
this.props.setSaveNeeded();
};
const handleDeleteButtonPressed = useCallback(() => {
setShouldDeleteBrandImage(true);
setBrandImage(undefined);
setBrandImageExists(false);
handleSave = async () => {
this.setState({
error: '',
});
setSaveNeeded();
}, [setSaveNeeded]);
let error;
if (this.state.deleteBrandImage) {
await deleteBrandImage(
() => {
this.setState({
deleteBrandImage: false,
brandImageExists: false,
brandImage: undefined,
});
},
(err: Error) => {
error = err;
this.setState({
error: err.message,
});
},
);
} else if (this.state.brandImage) {
await uploadBrandImage(
this.state.brandImage,
() => {
this.setState({
brandImageExists: true,
brandImage: undefined,
brandImageTimestamp: Date.now(),
});
},
(err: Error) => {
error = err;
this.setState({
error: err.message,
});
},
);
}
return {error};
};
render() {
let img = null;
if (this.state.brandImage) {
img = (
<div className='remove-image__img mb-5'>
<img
ref={this.imageRef}
alt='brand image'
src=''
/>
</div>
);
} else if (this.state.brandImageExists) {
let overlay;
if (!this.props.disabled) {
overlay = (
<WithTooltip
title={(
<FormattedMessage
id='admin.team.removeBrandImage'
defaultMessage='Remove brand image'
/>
)}
isVertical={false}
>
<button
type='button'
className='remove-image__btn'
onClick={this.handleDeleteButtonPressed}
>
<span aria-hidden={true}>{'×'}</span>
</button>
</WithTooltip>
);
}
img = (
<div className='remove-image__img mb-5'>
<img
alt='brand image'
src={Client4.getBrandImageUrl(
String(this.state.brandImageTimestamp),
)}
/>
{overlay}
</div>
);
} else {
img = (
<p className='mt-2'>
<FormattedMessage
id='admin.team.noBrandImage'
defaultMessage='No brand image uploaded'
/>
</p>
);
}
return (
<SettingSet
inputId={this.props.id}
helpText={
<FormattedMessage
id='admin.team.uploadDesc'
defaultMessage='Customize your user experience by adding a custom image to your login screen. Recommended maximum image size is less than 2 MB.'
/>
}
label={
<FormattedMessage
id='admin.team.brandImageTitle'
defaultMessage='Custom Brand Image:'
/>
}
setByEnv={false}
>
<div>
<div className='remove-image'>{img}</div>
</div>
<div className='file__upload mt-5'>
let img = null;
if (brandImage) {
img = (
<div className='remove-image__img mb-5'>
<img
ref={imageRef}
alt='brand image'
src=''
/>
</div>
);
} else if (brandImageExists) {
let overlay;
if (!disabled) {
overlay = (
<WithTooltip
title={(
<FormattedMessage
id='admin.team.removeBrandImage'
defaultMessage='Remove brand image'
/>
)}
isVertical={false}
>
<button
type='button'
className='btn btn-tertiary'
disabled={this.props.disabled}
onClick={this.handleSelectClick}
className='remove-image__btn'
data-testid='remove-image__btn'
onClick={handleDeleteButtonPressed}
>
<FormattedMessage
id='admin.team.chooseImage'
defaultMessage='Select Image'
/>
<span aria-hidden={true}>{'×'}</span>
</button>
<input
ref={this.fileInputRef}
type='file'
accept={Constants.ACCEPT_STATIC_IMAGE}
disabled={this.props.disabled}
onChange={this.handleImageChange}
/>
</div>
<FormError error={this.state.error}/>
</SettingSet>
</WithTooltip>
);
}
img = (
<div className='remove-image__img mb-5'>
<img
alt='brand image'
src={Client4.getBrandImageUrl(
String(brandImageTimestamp),
)}
/>
{overlay}
</div>
);
} else {
img = (
<p className='mt-2'>
<FormattedMessage
id='admin.team.noBrandImage'
defaultMessage='No brand image uploaded'
/>
</p>
);
}
}
return (
<SettingSet
inputId={id}
helpText={
<FormattedMessage
id='admin.team.uploadDesc'
defaultMessage='Customize your user experience by adding a custom image to your login screen. Recommended maximum image size is less than 2 MB.'
/>
}
label={
<FormattedMessage
id='admin.team.brandImageTitle'
defaultMessage='Custom Brand Image:'
/>
}
setByEnv={false}
>
<div>
<div className='remove-image'>{img}</div>
</div>
<div className='file__upload mt-5'>
<button
type='button'
className='btn btn-tertiary'
disabled={disabled}
onClick={handleSelectClick}
>
<FormattedMessage
id='admin.team.chooseImage'
defaultMessage='Select Image'
/>
</button>
<input
ref={fileInputRef}
data-testid='file__upload-input'
type='file'
accept={Constants.ACCEPT_STATIC_IMAGE}
disabled={disabled}
onChange={handleImageChange}
/>
</div>
<FormError error={errorFromState}/>
</SettingSet>
);
};
export default memo(BrandImageSetting);