Code improvement

https://youtrack.weseek.co.jp/issue/GW-7759
- Remove upload logo handling from AdminCustomizeContainer
- Move updateCustomizeLogo, deleteLogo and uploadBrandLogo to CustomizeLogoSetting
- Adjust logoSrc props in GrowiNavbar
- Remove undefined value from AttachmentType
- Set default attachmentType value to attachment schema
- Remove isDefaultLogo and customizedLogoSrc from config
- Add currentBrandLogo in config
- Update customize-logo route
- Add new route to get isDefaultLogo and customizedLogoSrc value
- Set currentBrandLogo on upload and delete logo
- Adjust createAttachment params

packages/app/src/client/services/AdminCustomizeContainer.js

@@ -3,15 +3,11 @@ import { Container } from 'unstated';
 import loggerFactory from '~/utils/logger';
 
 import { toastError } from '../util/apiNotification';
-import {
-  apiv3Delete,
-  apiv3Get, apiv3PostForm, apiv3Put,
-} from '../util/apiv3-client';
+import { apiv3Get, apiv3Put } from '../util/apiv3-client';
 
 // eslint-disable-next-line no-unused-vars
 const logger = loggerFactory('growi:services:AdminCustomizeContainer');
 
-const DEFAULT_LOGO = '/images/logo.svg';
 /**
  * Service container for admin customize setting page (Customize.jsx)
  * @extends {Container} unstated Container
@@ -60,18 +56,12 @@ export default class AdminCustomizeContainer extends Container {
         'tomorrow-night':   { name: '[Dark] Tomorrow Night',  border: false },
         'vs2015':           { name: '[Dark] Vs 2015',         border: false },
       },
-      customizedLogoSrc: null,
-      defaultLogoSrc: DEFAULT_LOGO,
-      isDefaultLogo: true,
       /* eslint-enable quote-props, no-multi-spaces */
     };
     this.switchPageListLimitationS = this.switchPageListLimitationS.bind(this);
     this.switchPageListLimitationM = this.switchPageListLimitationM.bind(this);
     this.switchPageListLimitationL = this.switchPageListLimitationL.bind(this);
     this.switchPageListLimitationXL = this.switchPageListLimitationXL.bind(this);
-    this.deleteLogo = this.deleteLogo.bind(this);
-    this.uploadBrandLogo = this.uploadBrandLogo.bind(this);
-
   }
 
   /**
@@ -107,8 +97,6 @@ export default class AdminCustomizeContainer extends Container {
         currentCustomizeHeader: customizeParams.customizeHeader,
         currentCustomizeCss: customizeParams.customizeCss,
         currentCustomizeScript: customizeParams.customizeScript,
-        isDefaultLogo: customizeParams.isDefaultLogo,
-        customizedLogoSrc: customizeParams.customizedLogoSrc,
       });
       // search style name from object for display
       this.setState({ currentHighlightJsStyleName: this.state.highlightJsCssSelectorOptions[customizeParams.styleName].name });
@@ -440,58 +428,4 @@ export default class AdminCustomizeContainer extends Container {
     }
   }
 
-
-  async deleteLogo() {
-    try {
-      await apiv3Delete('/customize-setting/delete-brand-logo');
-      this.setState({
-        customizedLogoSrc: null,
-      });
-
-    }
-    catch (err) {
-      this.setState({ retrieveError: err });
-      logger.error(err);
-      throw new Error('Failed to delete logo');
-    }
-  }
-
-  async uploadBrandLogo(file) {
-    try {
-      const formData = new FormData();
-      formData.append('file', file);
-      const { data } = await apiv3PostForm('/customize-setting/upload-brand-logo', formData);
-      this.setState({
-        customizedLogoSrc: data.attachment.filePathProxied,
-      });
-    }
-    catch (err) {
-      this.setState({ retrieveError: err });
-      logger.error(err);
-      throw new Error('Failed to upload brand logo');
-    }
-  }
-
-  switchDefaultLogo() {
-    this.setState({ isDefaultLogo: !this.state.isDefaultLogo });
-  }
-
-  async updateCustomizeLogo() {
-    try {
-      const response = await apiv3Put('/customize-setting/customize-logo', {
-        isDefaultLogo: this.state.isDefaultLogo,
-        customizedLogoSrc: this.state.customizedLogoSrc,
-      });
-      const { customizedParams } = response.data;
-      this.setState({
-        isDefaultLogo: customizedParams.isDefaultLogo,
-        customizedLogoSrc: customizedParams.customizedLogoSrc,
-      });
-    }
-    catch (err) {
-      logger.error(err);
-      throw new Error('Failed to update data');
-    }
-  }
-
 }

packages/app/src/components/Admin/Customize/CustomizeLogoSetting.tsx

@@ -1,84 +1,97 @@
-import React, { FC, useState } from 'react';
+import React, { useCallback, useEffect, useState } from 'react';
 
 import { useTranslation } from 'react-i18next';
 
-
-import AdminCustomizeContainer from '~/client/services/AdminCustomizeContainer';
-import AppContainer from '~/client/services/AppContainer';
+import { toastError, toastSuccess } from '~/client/util/apiNotification';
 import {
-  toastError, toastSuccess,
-} from '~/client/util/apiNotification';
+  apiv3Delete, apiv3Get, apiv3PostForm, apiv3Put,
+} from '~/client/util/apiv3-client';
 import ImageCropModal from '~/components/Common/ImageCropModal';
 
-import { withUnstatedContainers } from '../../UnstatedUtils';
 import AdminUpdateButtonRow from '../Common/AdminUpdateButtonRow';
 
+const DEFAULT_LOGO = '/images/logo.svg';
 
-type Props = {
-  adminCustomizeContainer : AdminCustomizeContainer
-}
-
-const CustomizeLogoSetting: FC<Props> = (props: Props) => {
+const CustomizeLogoSetting = (): JSX.Element => {
 
   const { t } = useTranslation();
-  const { adminCustomizeContainer } = props;
-  const [isShow, setIsShow] = useState<boolean>(false);
-  const [src, setSrc] = useState<ArrayBuffer | string | null>(null);
-  const {
-    customizedLogoSrc, isDefaultLogo, defaultLogoSrc,
-  } = adminCustomizeContainer.state;
-
-  const hideModal = () => {
-    setIsShow(false);
-  };
-
-  const cancelModal = () => {
-    hideModal();
-  };
-
-  const showModal = () => {
-    setIsShow(true);
-  };
-
-  const onSelectFile = (e) => {
+
+  const [uploadLogoSrc, setUploadLogoSrc] = useState<ArrayBuffer | string | null>(null);
+  const [customizedLogoSrc, setCustomizedLogoSrc] = useState<string | null>(null);
+  const [isImageCropModalShow, setIsImageCropModalShow] = useState<boolean>(false);
+  const [isDefaultLogo, setIsDefaultLogo] = useState<boolean>(true);
+  const [retrieveError, setRetrieveError] = useState<string | null>(null);
+
+  useEffect(() => {
+    (async() => {
+      try {
+        const response = await apiv3Get('/customize-setting/customize-logo');
+        const { isDefaultLogo, customizedLogoSrc } = response.data;
+        setIsDefaultLogo(isDefaultLogo);
+        setCustomizedLogoSrc(customizedLogoSrc);
+      }
+      catch (err) {
+        setRetrieveError(err);
+        throw new Error('Failed to fetch data');
+      }
+    })();
+  }, []);
+
+  const onSelectFile = useCallback((e: React.ChangeEvent<HTMLInputElement>) => {
     if (e.target.files != null && e.target.files.length > 0) {
       const reader = new FileReader();
-      reader.addEventListener('load', () => setSrc(reader.result));
+      reader.addEventListener('load', () => setUploadLogoSrc(reader.result));
       reader.readAsDataURL(e.target.files[0]);
-      showModal();
+      setIsImageCropModalShow(true);
     }
-  };
+  }, []);
 
-  const onClickSubmit = async() => {
+  const onClickSubmit = useCallback(async() => {
     try {
-      await adminCustomizeContainer.updateCustomizeLogo();
+      const response = await apiv3Put('/customize-setting/customize-logo', {
+        isDefaultLogo,
+        customizedLogoSrc,
+        currentBrandLogo: (!isDefaultLogo && customizedLogoSrc != null) ? customizedLogoSrc : null,
+      });
+      const { customizedParams } = response.data;
+      setIsDefaultLogo(customizedParams.isDefaultLogo);
+      setCustomizedLogoSrc(customizedParams.customizedLogoSrc);
       toastSuccess(t('toaster.update_successed', { target: t('admin:customize_setting.custom_logo') }));
     }
     catch (err) {
       toastError(err);
     }
-  };
+  }, [t, isDefaultLogo, customizedLogoSrc]);
+
 
-  const onClickDeleteBtn = async() => {
+  const onClickDeleteBtn = useCallback(async() => {
     try {
-      await adminCustomizeContainer.deleteLogo();
+      await apiv3Delete('/customize-setting/delete-brand-logo');
+      setCustomizedLogoSrc(null);
       toastSuccess(t('toaster.update_successed', { target: t('admin:customize_setting.current_logo') }));
     }
     catch (err) {
       toastError(err);
+      setRetrieveError(err);
+      throw new Error('Failed to delete logo');
     }
-  };
+  }, [t]);
 
-  const onCropCompleted = async(croppedImage) => {
+  const onCropCompleted = useCallback(async(croppedImage) => {
     try {
-      await adminCustomizeContainer.uploadBrandLogo(croppedImage);
+      const formData = new FormData();
+      formData.append('file', croppedImage);
+      const { data } = await apiv3PostForm('/customize-setting/upload-brand-logo', formData);
+      setCustomizedLogoSrc(data.attachment.filePathProxied);
       toastSuccess(t('toaster.update_successed', { target: t('admin:customize_setting.current_logo') }));
     }
     catch (err) {
       toastError(err);
+      setRetrieveError(err);
+      throw new Error('Failed to upload brand logo');
     }
-    hideModal();
-  };
+    setIsImageCropModalShow(false);
+  }, [t]);
 
 
   return (
@@ -98,14 +111,14 @@ const CustomizeLogoSetting: FC<Props> = (props: Props) => {
                       form="formImageType"
                       name="imagetypeForm[isDefaultLogo]"
                       checked={isDefaultLogo}
-                      onChange={() => { adminCustomizeContainer.switchDefaultLogo() }}
+                      onChange={() => { setIsDefaultLogo(true) }}
                     />
                     <label className="custom-control-label" htmlFor="radioDefaultLogo">
                       {t('admin:customize_setting.default_logo')}
                     </label>
                   </div>
                 </h4>
-                <img src={defaultLogoSrc} width="64" />
+                <img src={DEFAULT_LOGO} width="64" />
               </div>
               <div className="col-md-6 col-12">
                 <h4>
@@ -117,7 +130,7 @@ const CustomizeLogoSetting: FC<Props> = (props: Props) => {
                       form="formImageType"
                       name="imagetypeForm[isDefaultLogo]"
                       checked={!isDefaultLogo}
-                      onChange={() => { adminCustomizeContainer.switchDefaultLogo() }}
+                      onChange={() => { setIsDefaultLogo(false) }}
                     />
                     <label className="custom-control-label" htmlFor="radioUploadLogo">
                       { t('admin:customize_setting.upload_logo') }
@@ -129,7 +142,7 @@ const CustomizeLogoSetting: FC<Props> = (props: Props) => {
                     { t('admin:customize_setting.current_logo') }
                   </label>
                   <div className="col-sm-8 col-12">
-                    <p><img src={customizedLogoSrc || defaultLogoSrc} className="picture picture-lg " id="settingBrandLogo" width="64" /></p>
+                    <p><img src={customizedLogoSrc || DEFAULT_LOGO} className="picture picture-lg " id="settingBrandLogo" width="64" /></p>
                     {(customizedLogoSrc != null) && (
                       <button type="button" className="btn btn-danger" onClick={onClickDeleteBtn}>
                         { t('admin:customize_setting.delete_logo') }
@@ -147,15 +160,15 @@ const CustomizeLogoSetting: FC<Props> = (props: Props) => {
                 </div>
               </div>
             </div>
-            <AdminUpdateButtonRow onClick={onClickSubmit} disabled={adminCustomizeContainer.state.retrieveError != null} />
+            <AdminUpdateButtonRow onClick={onClickSubmit} disabled={retrieveError != null} />
           </div>
         </div>
       </div>
 
       <ImageCropModal
-        isShow={isShow}
-        src={src}
-        onModalClose={cancelModal}
+        isShow={isImageCropModalShow}
+        src={uploadLogoSrc}
+        onModalClose={() => setIsImageCropModalShow(false)}
         onCropCompleted={onCropCompleted}
         isCircular={false}
       />
@@ -165,6 +178,5 @@ const CustomizeLogoSetting: FC<Props> = (props: Props) => {
 
 };
 
-const CustomizeLogoSettingWrapper = withUnstatedContainers(CustomizeLogoSetting, [AppContainer, AdminCustomizeContainer]);
 
-export default CustomizeLogoSettingWrapper;
+export default CustomizeLogoSetting;

packages/app/src/components/Navbar/GrowiNavbar.tsx

@@ -122,20 +122,16 @@ const GrowiNavbar = (props) => {
 
   const { appContainer } = props;
   const {
-    crowi, isSearchServiceConfigured, customizedLogoSrc, isDefaultLogo,
+    crowi, isSearchServiceConfigured, currentBrandLogo,
   } = appContainer.config;
-  console.log(customizedLogoSrc);
   const { data: isDeviceSmallerThanMd } = useIsDeviceSmallerThanMd();
   const { data: isSearchPage } = useIsSearchPage();
-  const logoSrc = (!isDefaultLogo && customizedLogoSrc != null) ? customizedLogoSrc : null;
-  console.log(isDefaultLogo);
-  console.log(logoSrc);
   return (
     <>
       {/* Brand Logo  */}
       <div className="navbar-brand mr-0">
         <a className="grw-logo d-block" href="/">
-          <GrowiNavbarLogo logoSrc={...logoSrc} />
+          <GrowiNavbarLogo logoSrc={...currentBrandLogo} />
         </a>
       </div>
 

packages/app/src/server/interfaces/attachment.ts

@@ -2,7 +2,6 @@ export const AttachmentType = {
   BRAND_LOGO: 'BRAND_LOGO',
   WIKI_PAGE: 'WIKI_PAGE',
   PROFILE_IMAGE: 'PROFILE_IMAGE',
-  undefined,
 } as const;
 
 export type AttachmentType = typeof AttachmentType[keyof typeof AttachmentType];

packages/app/src/server/models/attachment.js

@@ -39,7 +39,7 @@ module.exports = function(crowi) {
     attachmentType: {
       type: String,
       enum: AttachmentType,
-      default: undefined,
+      default: AttachmentType.WIKI_PAGE,
     },
   }, {
     timestamps: { createdAt: true, updatedAt: false },
@@ -59,7 +59,7 @@ module.exports = function(crowi) {
   attachmentSchema.set('toJSON', { virtuals: true });
 
 
-  attachmentSchema.statics.createWithoutSave = function(pageId, user, fileStream, originalName, fileFormat, fileSize, attachmentType = undefined) {
+  attachmentSchema.statics.createWithoutSave = function(pageId, user, fileStream, originalName, fileFormat, fileSize, attachmentType) {
     const Attachment = this;
 
     const extname = path.extname(originalName);

packages/app/src/server/models/config.ts

@@ -245,8 +245,7 @@ schema.statics.getLocalconfig = function(crowi) {
     globalLang: crowi.configManager.getConfig('crowi', 'app:globalLang'),
     pageLimitationL: crowi.configManager.getConfig('crowi', 'customize:showPageLimitationL'),
     pageLimitationXL: crowi.configManager.getConfig('crowi', 'customize:showPageLimitationXL'),
-    isDefaultLogo:  crowi.configManager.getConfig('crowi', 'customize:isDefaultLogo'),
-    customizedLogoSrc: crowi.configManager.getConfig('crowi', 'customize:customizedLogoSrc'),
+    currentBrandLogo: crowi.configManager.getConfig('crowi', 'customize:currentBrandLogo'),
     isSidebarDrawerMode: crowi.configManager.getConfig('crowi', 'customize:isSidebarDrawerMode'),
     isSidebarClosedAtDockMode: crowi.configManager.getConfig('crowi', 'customize:isSidebarClosedAtDockMode'),
   };

packages/app/src/server/routes/apiv3/customize-setting.js

@@ -156,6 +156,7 @@ module.exports = (crowi) => {
     logo: [
       body('isDefaultLogo').isBoolean().optional({ nullable: true }),
       body('customizedLogoSrc').isString().optional({ nullable: true }),
+      body('currentBrandLogo').isString().optional({ nullable: true }),
     ],
   };
 
@@ -180,8 +181,6 @@ module.exports = (crowi) => {
    *                      description: customize params
    */
   router.get('/', loginRequiredStrictly, adminRequired, async(req, res) => {
-    const defaultLogoConfig = await crowi.configManager.getConfig('crowi', 'customize:isDefaultLogo');
-    const isDefaultLogo = defaultLogoConfig === undefined || defaultLogoConfig;
     const customizeParams = {
       themeType: await crowi.configManager.getConfig('crowi', 'customize:theme'),
       isEnabledTimeline: await crowi.configManager.getConfig('crowi', 'customize:isEnabledTimeline'),
@@ -200,8 +199,6 @@ module.exports = (crowi) => {
       customizeHeader: await crowi.configManager.getConfig('crowi', 'customize:header'),
       customizeCss: await crowi.configManager.getConfig('crowi', 'customize:css'),
       customizeScript: await crowi.configManager.getConfig('crowi', 'customize:script'),
-      isDefaultLogo,
-      customizedLogoSrc: await crowi.configManager.getConfig('crowi', 'customize:customizedLogoSrc'),
     };
 
     return res.apiv3({ customizeParams });
@@ -670,15 +667,23 @@ module.exports = (crowi) => {
     }
   });
 
+  router.get('/customize-logo', loginRequiredStrictly, adminRequired, async(req, res) => {
+    const defaultLogoConfig = await crowi.configManager.getConfig('crowi', 'customize:isDefaultLogo');
+    const isDefaultLogo = defaultLogoConfig === undefined || defaultLogoConfig;
+    const customizedLogoSrc = await crowi.configManager.getConfig('crowi', 'customize:customizedLogoSrc');
+    return res.apiv3({ isDefaultLogo, customizedLogoSrc });
+  });
+
   router.put('/customize-logo', apiLimiter, loginRequiredStrictly, adminRequired, csrf, validator.logo, apiV3FormValidator, async(req, res) => {
 
     const {
-      isDefaultLogo, customizedLogoSrc,
+      isDefaultLogo, customizedLogoSrc, currentBrandLogo,
     } = req.body;
 
     const requestParams = {
       'customize:isDefaultLogo': isDefaultLogo,
       'customize:customizedLogoSrc': customizedLogoSrc,
+      'customize:currentBrandLogo': currentBrandLogo,
     };
     try {
       await crowi.configManager.updateConfigsInTheSameNamespace('crowi', requestParams);
@@ -723,9 +728,13 @@ module.exports = (crowi) => {
       let attachment;
       try {
         attachment = await attachmentService.createAttachment(file, req.user, null, AttachmentType.BRAND_LOGO);
+        const isDefaultLogo = await crowi.configManager.getConfig('crowi', 'customize:isDefaultLogo');
         const attachmentConfigParams = {
           'customize:customizedLogoSrc': attachment.filePathProxied,
         };
+        if (!isDefaultLogo) {
+          attachmentConfigParams['customize:currentBrandLogo'] = attachment.filePathProxied;
+        }
         await crowi.configManager.updateConfigsInTheSameNamespace('crowi', attachmentConfigParams);
       }
       catch (err) {
@@ -746,11 +755,15 @@ module.exports = (crowi) => {
       }
 
       try {
+        const isDefaultLogo = await crowi.configManager.getConfig('crowi', 'customize:isDefaultLogo');
         await attachmentService.removeAllAttachments(attachments);
         // update attachmentId immediately
         const attachmentConfigParams = {
           'customize:customizedLogoSrc': null,
         };
+        if (!isDefaultLogo) {
+          attachmentConfigParams['customize:currentBrandLogo'] = null;
+        }
         await crowi.configManager.updateConfigsInTheSameNamespace('crowi', attachmentConfigParams);
       }
       catch (err) {

packages/app/src/server/service/attachment.js

@@ -16,7 +16,7 @@ class AttachmentService {
     this.crowi = crowi;
   }
 
-  async createAttachment(file, user, pageId = null, attachmentType = undefined) {
+  async createAttachment(file, user, pageId = null, attachmentType = null) {
     const { fileUploadService } = this.crowi;
 
     // check limit