[MM-12068] Add ability to remove custom branding image (mattermost#3207)

* [MM-12068] Added the ability to remove custom branding image * [MM-12068] Fixed lint error * [MM-12068] Update en.json * [MM-12068] Added styling that was accidently removed * [MM-12068] Addressed PR comments * [MM-12068] Fixed test case * [MM-12068] Addressed PR comments * [MM-12068] Fixed Asyncrhonous behaviour * [MM-12068] Add function comments * [MM-12068] Removed unnecessary code * [MM-12068] Fixed small code discrepancy * [MM-12068] Generalised code * [MM-12068] Address comments regarding incorrect ref handling and made more generalised * [MM-12068] Lint error fixes * [MM-12068] Addressed PR and used function pass instead of refs * [MM-12068] return error as object and refactor code * [MM-12068] Add unregister save action function * [MM-12068] Lint and snapshot fixes * add unit test
uber-archive · Oct 3, 2019 · 4da4025 · 4da4025
1 parent 1513818
commit 4da4025
Show file tree

Hide file tree

Showing 10 changed files with 259 additions and 82 deletions.
diff --git a/actions/admin_actions.jsx b/actions/admin_actions.jsx
@@ -162,6 +162,15 @@ export async function uploadBrandImage(brandImage, success, error) {
     }
 }
 
+export async function deleteBrandImage(success, error) {
+    const {data, error: err} = await AdminActions.deleteBrandImage()(dispatch, getState);
+    if (data && success) {
+        success(data);
+    } else if (err && error) {
+        error({id: err.server_error_id, ...err});
+    }
+}
+
 export async function uploadPublicSamlCertificate(file, success, error) {
     const {data, error: err} = await AdminActions.uploadPublicSamlCertificate(file)(dispatch, getState);
     if (data && success) {

diff --git a/components/admin_console/__snapshots__/schema_admin_settings.test.jsx.snap b/components/admin_console/__snapshots__/schema_admin_settings.test.jsx.snap
@@ -562,7 +562,10 @@ exports[`components/admin_console/SchemaAdminSettings should match snapshot with
             id="custom"
             key="Config_userautocomplete_custom"
             onChange={[Function]}
+            registerSaveAction={[Function]}
             setByEnv={false}
+            setSaveNeeded={[Function]}
+            unRegisterSaveAction={[Function]}
             value=""
           />
           <Connect(InjectIntl(JobTable))

diff --git a/components/admin_console/brand_image_setting/brand_image_setting.jsx b/components/admin_console/brand_image_setting/brand_image_setting.jsx
@@ -5,14 +5,13 @@ import $ from 'jquery';
 import PropTypes from 'prop-types';
 import React from 'react';
 import {FormattedHTMLMessage, FormattedMessage} from 'react-intl';
+import {OverlayTrigger, Tooltip} from 'react-bootstrap';
 import {Client4} from 'mattermost-redux/client';
 
-import {uploadBrandImage} from 'actions/admin_actions.jsx';
-import {UploadStatuses} from 'utils/constants.jsx';
+import {uploadBrandImage, deleteBrandImage} from 'actions/admin_actions.jsx';
+import {Constants} from 'utils/constants.jsx';
 import FormError from 'components/form_error.jsx';
 
-import UploadButton from './upload_button.jsx';
-
 const HTTP_STATUS_OK = 200;
 
 export default class BrandImageSetting extends React.PureComponent {
@@ -22,20 +21,35 @@ export default class BrandImageSetting extends React.PureComponent {
          * Set to disable the setting
          */
         disabled: PropTypes.bool.isRequired,
+
+        /*
+        * Set the save needed in the admin schema settings to trigger the save button to turn on
+        */
+        setSaveNeeded: PropTypes.func.isRequired,
+
+        /*
+        * Registers the function suppose to be run when the save button is pressed
+        */
+        registerSaveAction: PropTypes.func.isRequired,
+
+        /*
+        * Unregisters the function on unmount of the component suppose to be run when the save button is pressed
+        */
+        unRegisterSaveAction: PropTypes.func.isRequired,
     }
 
     constructor(props) {
         super(props);
 
         this.handleImageChange = this.handleImageChange.bind(this);
-        this.handleImageSubmit = this.handleImageSubmit.bind(this);
+        this.handleDeleteButtonPressed = this.handleDeleteButtonPressed.bind(this);
 
         this.state = {
+            deleteBrandImage: false,
             brandImage: null,
             brandImageExists: false,
             brandImageTimestamp: Date.now(),
             error: '',
-            status: UploadStatuses.DEFAULT,
         };
     }
 
@@ -51,6 +65,14 @@ export default class BrandImageSetting extends React.PureComponent {
         );
     }
 
+    componentDidMount() {
+        this.props.registerSaveAction(this.handleSave);
+    }
+
+    componentWillUnmount() {
+        this.props.unRegisterSaveAction(this.handleSave);
+    }
+
     componentDidUpdate() {
         if (this.refs.image) {
             const reader = new FileReader();
@@ -66,56 +88,64 @@ export default class BrandImageSetting extends React.PureComponent {
 
     handleImageChange() {
         const element = $(this.refs.fileInput);
-
         if (element.prop('files').length > 0) {
+            this.props.setSaveNeeded();
             this.setState({
                 brandImage: element.prop('files')[0],
-                status: UploadStatuses.DEFAULT,
+                deleteBrandImage: false,
             });
         }
     }
 
-    handleImageSubmit(e) {
-        e.preventDefault();
-
-        if (!this.state.brandImage) {
-            return;
-        }
-
-        if (this.state.status === UploadStatuses.LOADING) {
-            return;
-        }
+    handleDeleteButtonPressed() {
+        this.setState({deleteBrandImage: true, brandImage: null, brandImageExists: false});
+        this.props.setSaveNeeded();
+    }
 
+    handleSave = async () => {
         this.setState({
             error: '',
-            status: UploadStatuses.LOADING,
         });
 
-        uploadBrandImage(
-            this.state.brandImage,
-            () => {
-                this.setState({
-                    brandImageExists: true,
-                    brandImage: null,
-                    brandImageTimestamp: Date.now(),
-                    status: UploadStatuses.COMPLETE,
-                });
-            },
-            (err) => {
-                this.setState({
-                    error: err.message,
-                    status: UploadStatuses.DEFAULT,
-                });
-            }
-        );
+        let error;
+        if (this.state.deleteBrandImage) {
+            await deleteBrandImage(
+                () => {
+                    this.setState({
+                        deleteBrandImage: false,
+                        brandImageExists: false,
+                        brandImage: null,
+                    });
+                },
+                (err) => {
+                    error = err;
+                    this.setState({
+                        error: err.message,
+                    });
+                }
+            );
+        } else if (this.state.brandImage) {
+            await uploadBrandImage(
+                this.state.brandImage,
+                () => {
+                    this.setState({
+                        brandImageExists: true,
+                        brandImage: null,
+                        brandImageTimestamp: Date.now(),
+                    });
+                },
+                (err) => {
+                    error = err;
+                    this.setState({
+                        error: err.message,
+                    });
+                }
+            );
+        }
+        return {error};
     }
 
     render() {
-        let btnPrimaryClass = 'btn';
-        if (this.state.brandImage) {
-            btnPrimaryClass += ' btn-primary';
-        }
-
         let letbtnDefaultClass = 'btn';
         if (!this.props.disabled) {
             letbtnDefaultClass += ' btn-default';
@@ -124,24 +154,53 @@ export default class BrandImageSetting extends React.PureComponent {
         let img = null;
         if (this.state.brandImage) {
             img = (
-                <img
-                    ref='image'
-                    className='brand-img'
-                    alt='brand image'
-                    src=''
-                />
+                <div className='remove-image__img margin-bottom x3'>
+                    <img
+                        ref='image'
+                        alt='brand image'
+                        src=''
+                    />
+                </div>
             );
         } else if (this.state.brandImageExists) {
+            let overlay;
+            if (!this.props.disabled) {
+                overlay = (
+                    <OverlayTrigger
+                        delayShow={Constants.OVERLAY_TIME_DELAY}
+                        placement='right'
+                        overlay={(
+                            <Tooltip id='removeIcon'>
+                                <div aria-hidden={true}>
+                                    <FormattedMessage
+                                        id='admin.team.removeBrandImage'
+                                        defaultMessage='Remove brand image'
+                                    />
+                                </div>
+                            </Tooltip>
+                        )}
+                    >
+                        <button
+                            className='remove-image__btn'
+                            onClick={this.handleDeleteButtonPressed}
+                        >
+                            <span aria-hidden={true}>{'×'}</span>
+                        </button>
+                    </OverlayTrigger>
+                );
+            }
             img = (
-                <img
-                    className='brand-img'
-                    alt='brand image'
-                    src={Client4.getBrandImageUrl(this.state.brandImageTimestamp)}
-                />
+                <div className='remove-image__img margin-bottom x3'>
+                    <img
+                        alt='brand image'
+                        src={Client4.getBrandImageUrl(this.state.brandImageTimestamp)}
+                    />
+                    {overlay}
+                </div>
             );
         } else {
             img = (
-                <p>
+                <p className='margin-top'>
                     <FormattedMessage
                         id='admin.team.noBrandImage'
                         defaultMessage='No brand image uploaded'
@@ -159,18 +218,20 @@ export default class BrandImageSetting extends React.PureComponent {
                     />
                 </label>
                 <div className='col-sm-8'>
-                    {img}
+                    <div className='remove-image'>
+                        {img}
+                    </div>
                 </div>
                 <div className='col-sm-4'/>
                 <div className='col-sm-8'>
-                    <div className='file__upload'>
+                    <div className='file__upload margin-top x3'>
                         <button
                             className={letbtnDefaultClass}
                             disabled={this.props.disabled}
                         >
                             <FormattedMessage
                                 id='admin.team.chooseImage'
-                                defaultMessage='Choose New Image'
+                                defaultMessage='Select Image'
                             />
                         </button>
                         <input
@@ -181,12 +242,6 @@ export default class BrandImageSetting extends React.PureComponent {
                             onChange={this.handleImageChange}
                         />
                     </div>
-                    <UploadButton
-                        primaryClass={btnPrimaryClass}
-                        status={this.state.status}
-                        disabled={this.props.disabled || !this.state.brandImage}
-                        onClick={this.handleImageSubmit}
-                    />
                     <br/>
                     <FormError error={this.state.error}/>
                     <p className='help-text no-margin'>

diff --git a/components/admin_console/brand_image_setting/brand_image_setting.test.jsx b/components/admin_console/brand_image_setting/brand_image_setting.test.jsx
@@ -0,0 +1,42 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+import React from 'react';
+import {shallow} from 'enzyme';
+
+import {uploadBrandImage, deleteBrandImage} from 'actions/admin_actions.jsx';
+
+import BrandImageSetting from './brand_image_setting.jsx';
+
+jest.mock('actions/admin_actions.jsx', () => ({
+    ...jest.requireActual('actions/admin_actions.jsx'),
+    uploadBrandImage: jest.fn(),
+    deleteBrandImage: jest.fn(),
+}));
+
+describe('components/admin_console/brand_image_setting', () => {
+    const baseProps = {
+        disabled: false,
+        setSaveNeeded: jest.fn(),
+        registerSaveAction: jest.fn(),
+        unRegisterSaveAction: jest.fn(),
+    };
+
+    test('should have called deleteBrandImage or uploadBrandImage on save depending on component state', () => {
+        const wrapper = shallow(
+            <BrandImageSetting {...baseProps}/>
+        );
+
+        const instance = wrapper.instance();
+
+        wrapper.setState({deleteBrandImage: false, brandImage: 'brand_image_file'});
+        instance.handleSave();
+        expect(deleteBrandImage).toHaveBeenCalledTimes(0);
+        expect(uploadBrandImage).toHaveBeenCalledTimes(1);
+
+        wrapper.setState({deleteBrandImage: true, brandImage: null});
+        instance.handleSave();
+        expect(deleteBrandImage).toHaveBeenCalledTimes(1);
+        expect(uploadBrandImage).toHaveBeenCalledTimes(1);
+    });
+});