Code improvement

https://youtrack.weseek.co.jp/issue/GW-7759
- Remove brandLogoAttachmentId completely
- Replace api v1 with apiv3 to upload and delete brand logo
- Update uploadBrandLogo and deleteLogo method
- Rename uploadedLogoSrc to customizedLogoSrc
- Remove createdAt from attachment model
- Move upload and delete logo route to customize-setting
- Remove uploadBrandLogo and  removeBrandLogo from attachments route

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

@@ -3,8 +3,10 @@ import { Container } from 'unstated';
 import loggerFactory from '~/utils/logger';
 
 import { toastError } from '../util/apiNotification';
-import { apiPost, apiPostForm } from '../util/apiv1-client';
-import { apiv3Get, apiv3Put } from '../util/apiv3-client';
+import {
+  apiv3Delete,
+  apiv3Get, apiv3PostForm, apiv3Put,
+} from '../util/apiv3-client';
 
 // eslint-disable-next-line no-unused-vars
 const logger = loggerFactory('growi:services:AdminCustomizeContainer');
@@ -58,10 +60,9 @@ export default class AdminCustomizeContainer extends Container {
         'tomorrow-night':   { name: '[Dark] Tomorrow Night',  border: false },
         'vs2015':           { name: '[Dark] Vs 2015',         border: false },
       },
-      uploadedLogoSrc: null,
+      customizedLogoSrc: null,
       defaultLogoSrc: DEFAULT_LOGO,
       isDefaultLogo: true,
-      brandLogoAttachmentId: '',
       /* eslint-enable quote-props, no-multi-spaces */
     };
     this.switchPageListLimitationS = this.switchPageListLimitationS.bind(this);
@@ -106,9 +107,8 @@ export default class AdminCustomizeContainer extends Container {
         currentCustomizeHeader: customizeParams.customizeHeader,
         currentCustomizeCss: customizeParams.customizeCss,
         currentCustomizeScript: customizeParams.customizeScript,
-        brandLogoAttachmentId: customizeParams.brandLogoAttachmentId,
         isDefaultLogo: customizeParams.isDefaultLogo,
-        uploadedLogoSrc: customizeParams.uploadedLogoSrc,
+        customizedLogoSrc: customizeParams.customizedLogoSrc,
       });
       // search style name from object for display
       this.setState({ currentHighlightJsStyleName: this.state.highlightJsCssSelectorOptions[customizeParams.styleName].name });
@@ -443,13 +443,9 @@ export default class AdminCustomizeContainer extends Container {
 
   async deleteLogo() {
     try {
-      const formData = {
-        brandLogoAttachmentId: this.state.brandLogoAttachmentId,
-      };
-      await apiPost('/attachments.removeBrandLogo', formData);
+      await apiv3Delete('/customize-setting/delete-brand-logo');
       this.setState({
-        uploadedLogoSrc: null,
-        brandLogoAttachmentId: null,
+        customizedLogoSrc: null,
       });
 
     }
@@ -464,12 +460,9 @@ export default class AdminCustomizeContainer extends Container {
     try {
       const formData = new FormData();
       formData.append('file', file);
-      formData.append('brandLogoAttachmentId', this.state.brandLogoAttachmentId);
-      const response = await apiPostForm('/attachments.uploadBrandLogo', formData);
-
+      const { data } = await apiv3PostForm('/customize-setting/upload-brand-logo', formData);
       this.setState({
-        uploadedLogoSrc: response.attachment.filePathProxied,
-        brandLogoAttachmentId: response.attachment.id,
+        customizedLogoSrc: data.attachment.filePathProxied,
       });
     }
     catch (err) {
@@ -487,14 +480,12 @@ export default class AdminCustomizeContainer extends Container {
     try {
       const response = await apiv3Put('/customize-setting/customize-logo', {
         isDefaultLogo: this.state.isDefaultLogo,
-        brandLogoAttachmentId: this.state.brandLogoAttachmentId,
-        uploadedLogoSrc: this.state.uploadedLogoSrc,
+        customizedLogoSrc: this.state.customizedLogoSrc,
       });
       const { customizedParams } = response.data;
       this.setState({
         isDefaultLogo: customizedParams.isDefaultLogo,
-        brandLogoAttachmentId:  customizedParams.brandLogoAttachmentId,
-        uploadedLogoSrc: customizedParams.uploadedLogoSrc,
+        customizedLogoSrc: customizedParams.customizedLogoSrc,
       });
     }
     catch (err) {

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

@@ -25,7 +25,7 @@ const CustomizeLogoSetting: FC<Props> = (props: Props) => {
   const [isShow, setIsShow] = useState<boolean>(false);
   const [src, setSrc] = useState<ArrayBuffer | string | null>(null);
   const {
-    uploadedLogoSrc, isDefaultLogo, defaultLogoSrc,
+    customizedLogoSrc, isDefaultLogo, defaultLogoSrc,
   } = adminCustomizeContainer.state;
 
   const hideModal = () => {
@@ -129,8 +129,8 @@ const CustomizeLogoSetting: FC<Props> = (props: Props) => {
                     { t('admin:customize_setting.current_logo') }
                   </label>
                   <div className="col-sm-8 col-12">
-                    <p><img src={uploadedLogoSrc || defaultLogoSrc} className="picture picture-lg " id="settingBrandLogo" width="64" /></p>
-                    {(uploadedLogoSrc != null) && (
+                    <p><img src={customizedLogoSrc || defaultLogoSrc} 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') }
                       </button>

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

@@ -122,12 +122,14 @@ const GrowiNavbar = (props) => {
 
   const { appContainer } = props;
   const {
-    crowi, isSearchServiceConfigured, uploadedLogoSrc, isDefaultLogo,
+    crowi, isSearchServiceConfigured, customizedLogoSrc, isDefaultLogo,
   } = appContainer.config;
-
+  console.log(customizedLogoSrc);
   const { data: isDeviceSmallerThanMd } = useIsDeviceSmallerThanMd();
   const { data: isSearchPage } = useIsSearchPage();
-  const logoSrc = (!isDefaultLogo && uploadedLogoSrc != null) ? uploadedLogoSrc : null;
+  const logoSrc = (!isDefaultLogo && customizedLogoSrc != null) ? customizedLogoSrc : null;
+  console.log(isDefaultLogo);
+  console.log(logoSrc);
   return (
     <>
       {/* Brand Logo  */}

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

@@ -75,7 +75,6 @@ module.exports = function(crowi) {
     attachment.fileName = fileName;
     attachment.fileFormat = fileFormat;
     attachment.fileSize = fileSize;
-    attachment.createdAt = Date.now();
     attachment.attachmentType = attachmentType;
     return attachment;
   };

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

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

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

@@ -1,6 +1,7 @@
 /* eslint-disable no-unused-vars */
 import rateLimit from 'express-rate-limit';
 
+import { AttachmentType } from '~/server/interfaces/attachment';
 import loggerFactory from '~/utils/logger';
 
 import { apiV3FormValidator } from '../../middlewares/apiv3-form-validator';
@@ -13,8 +14,6 @@ const router = express.Router();
 
 const { body, query } = require('express-validator');
 
-const ErrorV3 = require('../../models/vo/error-apiv3');
-
 const apiLimiter = rateLimit({
   windowMs: 15 * 60 * 1000, // 15 minutes
   max: 10, // limit each IP to 10 requests per windowMs
@@ -22,6 +21,10 @@ const apiLimiter = rateLimit({
     'Too many requests sent from this IP, please try again after 15 minutes',
 });
 
+const multer = require('multer');
+
+const ErrorV3 = require('../../models/vo/error-apiv3');
+
 /**
  * @swagger
  *  tags:
@@ -103,8 +106,9 @@ module.exports = (crowi) => {
   const adminRequired = require('../../middlewares/admin-required')(crowi);
   const csrf = require('../../middlewares/csrf')(crowi);
 
-  const { customizeService } = crowi;
-
+  const { customizeService, attachmentService } = crowi;
+  const Attachment = crowi.model('Attachment');
+  const uploads = multer({ dest: `${crowi.tmpDir}uploads` });
   const validator = {
     layout: [
       body('isContainerFluid').isBoolean(),
@@ -150,9 +154,8 @@ module.exports = (crowi) => {
       body('customizeScript').isString(),
     ],
     logo: [
-      body('brandLogoAttachmentId').isString().optional({ nullable: true }),
-      body('isDefaultLogo').isBoolean(),
-      body('uploadedLogoSrc').isString().optional({ nullable: true }),
+      body('isDefaultLogo').isBoolean().optional({ nullable: true }),
+      body('customizedLogoSrc').isString().optional({ nullable: true }),
     ],
   };
 
@@ -197,9 +200,8 @@ 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'),
-      brandLogoAttachmentId: await crowi.configManager.getConfig('crowi', 'customize:brandLogoAttachmentId'),
       isDefaultLogo,
-      uploadedLogoSrc: await crowi.configManager.getConfig('crowi', 'customize:uploadedLogoSrc'),
+      customizedLogoSrc: await crowi.configManager.getConfig('crowi', 'customize:customizedLogoSrc'),
     };
 
     return res.apiv3({ customizeParams });
@@ -671,20 +673,18 @@ module.exports = (crowi) => {
   router.put('/customize-logo', apiLimiter, loginRequiredStrictly, adminRequired, csrf, validator.logo, apiV3FormValidator, async(req, res) => {
 
     const {
-      isDefaultLogo, brandLogoAttachmentId, uploadedLogoSrc,
+      isDefaultLogo, customizedLogoSrc,
     } = req.body;
 
     const requestParams = {
-      'customize:brandLogoAttachmentId': brandLogoAttachmentId,
       'customize:isDefaultLogo': isDefaultLogo,
-      'customize:uploadedLogoSrc': uploadedLogoSrc,
+      'customize:customizedLogoSrc': customizedLogoSrc,
     };
     try {
       await crowi.configManager.updateConfigsInTheSameNamespace('crowi', requestParams);
       const customizedParams = {
-        brandLogoAttachmentId: await crowi.configManager.getConfig('crowi', 'customize:brandLogoAttachmentId'),
         isDefaultLogo: await crowi.configManager.getConfig('crowi', 'customize:isDefaultLogo'),
-        uploadedLogoSrc: await crowi.configManager.getConfig('crowi', 'customize:uploadedLogoSrc'),
+        customizedLogoSrc: await crowi.configManager.getConfig('crowi', 'customize:customizedLogoSrc'),
       };
       return res.apiv3({ customizedParams });
     }
@@ -695,5 +695,71 @@ module.exports = (crowi) => {
     }
   });
 
+  router.post('/upload-brand-logo', apiLimiter, uploads.single('file'), loginRequiredStrictly,
+    adminRequired, csrf, validator.logo, apiV3FormValidator, async(req, res) => {
+
+      if (req.file == null) {
+        return res.apiv3Err(new ErrorV3('File error.', 'upload-brand-logo-failed'));
+      }
+      if (req.user == null) {
+        return res.apiv3Err(new ErrorV3('param "user" must be set.', 'upload-brand-logo-failed'));
+      }
+
+      const file = req.file;
+
+      // check type
+      const acceptableFileType = /image\/.+/;
+      if (!file.mimetype.match(acceptableFileType)) {
+        const msg = 'File type error. Only image files is allowed to set as user picture.';
+        return res.apiv3Err(new ErrorV3(msg, 'upload-brand-logo-failed'));
+      }
+
+      // Check if previous attachment exists and remove it
+      const attachments = await Attachment.find({ attachmentType: AttachmentType.BRAND_LOGO });
+      if (attachments != null) {
+        await attachmentService.removeAllAttachments(attachments);
+      }
+
+      let attachment;
+      try {
+        attachment = await attachmentService.createAttachment(file, req.user, null, AttachmentType.BRAND_LOGO);
+        const attachmentConfigParams = {
+          'customize:customizedLogoSrc': attachment.filePathProxied,
+        };
+        await crowi.configManager.updateConfigsInTheSameNamespace('crowi', attachmentConfigParams);
+      }
+      catch (err) {
+        logger.error(err);
+        return res.apiv3Err(new ErrorV3(err.message, 'upload-brand-logo-failed'));
+      }
+      attachment.toObject({ virtuals: true });
+      return res.apiv3({ attachment });
+    });
+
+  router.delete('/delete-brand-logo', apiLimiter, loginRequiredStrictly,
+    adminRequired, csrf, async(req, res) => {
+
+      const attachments = await Attachment.find({ attachmentType: AttachmentType.BRAND_LOGO });
+
+      if (attachments == null) {
+        return res.apiv3Err(new ErrorV3('attachment not found', 'delete-brand-logo-failed'));
+      }
+
+      try {
+        await attachmentService.removeAllAttachments(attachments);
+        // update attachmentId immediately
+        const attachmentConfigParams = {
+          'customize:customizedLogoSrc': null,
+        };
+        await crowi.configManager.updateConfigsInTheSameNamespace('crowi', attachmentConfigParams);
+      }
+      catch (err) {
+        logger.error(err);
+        return res.status(500).apiv3Err(new ErrorV3('Error while deleting logo', 'delete-brand-logo-failed'));
+      }
+
+      return res.apiv3({ });
+    });
+
   return router;
 };

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

@@ -706,84 +706,5 @@ module.exports = function(crowi, app) {
     return res.json(ApiResponse.success({}));
   };
 
-  api.uploadBrandLogo = async function(req, res) {
-    // check params
-    if (req.file == null) {
-      return res.json(ApiResponse.error('File error.'));
-    }
-    if (req.user == null) {
-      return res.json(ApiResponse.error('param "user" must be set.'));
-    }
-
-    const file = req.file;
-    const { brandLogoAttachmentId } = req.body;
-
-    let previousAttachmentId;
-    if (brandLogoAttachmentId === 'null' || brandLogoAttachmentId === 'undefined') {
-      previousAttachmentId = null;
-    }
-    else {
-      previousAttachmentId = mongoose.Types.ObjectId(brandLogoAttachmentId);
-    }
-
-    // check type
-    const acceptableFileType = /image\/.+/;
-    if (!file.mimetype.match(acceptableFileType)) {
-      return res.json(ApiResponse.error('File type error. Only image files is allowed to set as user picture.'));
-    }
-
-    // Check if previous attachment exists and remove it
-    const previousAttachment = await Attachment.findById(previousAttachmentId);
-    if (previousAttachment !== null) {
-      await attachmentService.removeAttachment(previousAttachmentId);
-    }
-
-    let attachment;
-    try {
-      attachment = await attachmentService.createAttachment(file, req.user, null, AttachmentType.BRAND_LOGO);
-      const attachmentConfigParams = {
-        'customize:brandLogoAttachmentId': attachment.id,
-        'customize:uploadedLogoSrc': attachment.filePathProxied,
-      };
-      await crowi.configManager.updateConfigsInTheSameNamespace('crowi', attachmentConfigParams);
-    }
-    catch (err) {
-      logger.error(err);
-      return res.json(ApiResponse.error(err.message));
-    }
-
-    const result = {
-      attachment: attachment.toObject({ virtuals: true }),
-    };
-
-    return res.json(ApiResponse.success(result));
-  };
-
-  api.removeBrandLogo = async function(req, res) {
-    const { brandLogoAttachmentId } = req.body;
-    const attachmentObjectId = mongoose.Types.ObjectId(brandLogoAttachmentId);
-    const attachment = await Attachment.findById(attachmentObjectId);
-
-    if (attachment == null) {
-      return res.json(ApiResponse.error('attachment not found'));
-    }
-
-    try {
-      await attachmentService.removeAttachment(attachmentObjectId);
-      // update attachmentId immediately
-      const attachmentConfigParams = {
-        'customize:brandLogoAttachmentId': null,
-        'customize:uploadedLogoSrc': null,
-      };
-      await crowi.configManager.updateConfigsInTheSameNamespace('crowi', attachmentConfigParams);
-    }
-    catch (err) {
-      logger.error(err);
-      return res.status(500).json(ApiResponse.error('Error while deleting logo'));
-    }
-
-    return res.json(ApiResponse.success({}));
-  };
-
   return actions;
 };

packages/app/src/server/routes/index.js

@@ -189,8 +189,6 @@ module.exports = function(crowi, app) {
   apiV1Router.post('/attachments.remove'               , accessTokenParser , loginRequiredStrictly , csrf, attachment.api.remove);
   apiV1Router.post('/attachments.removeProfileImage'   , accessTokenParser , loginRequiredStrictly , csrf, attachment.api.removeProfileImage);
   apiV1Router.get('/attachments.limit'   , accessTokenParser , loginRequiredStrictly, attachment.api.limit);
-  apiV1Router.post('/attachments.uploadBrandLogo'   , apiLimiter, uploads.single('file'), autoReap, accessTokenParser, loginRequiredStrictly ,csrf, attachment.api.uploadBrandLogo);
-  apiV1Router.post('/attachments.removeBrandLogo'      , apiLimiter, accessTokenParser , loginRequiredStrictly , csrf, attachment.api.removeBrandLogo);
 
   // API v1
   app.use('/_api', unavailableWhenMaintenanceModeForApi, apiV1Router);