Fix brand logo update

https://youtrack.weseek.co.jp/issue/GW-7759
- Add UNCATEGORIZED to attachmentType
- Set attachmentType UNCATEGORIZED by default
- Improve code writing style
- Fix delete logo on upload new logo
- Add circular props to image crop modal
- Fix issue on update twice , default logo will be selected

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

@@ -107,21 +107,13 @@ export default class AdminCustomizeContainer extends Container {
         currentCustomizeScript: customizeParams.customizeScript,
         attachmentId: customizeParams.attachmentLogoId,
         isDefaultLogo: customizeParams.isDefaultLogo,
+        uploadedLogoSrc: customizeParams.uploadedLogoSrc,
+        isUploadedLogo: customizeParams.uploadedLogoSrc,
       });
       // search style name from object for display
       this.setState({ currentHighlightJsStyleName: this.state.highlightJsCssSelectorOptions[customizeParams.styleName].name });
 
-      // set current uploaded logo
-      if (customizeParams.attachmentLogoId) {
-        const logoPath = `/attachment/${customizeParams.attachmentLogoId}`;
-
-        this.setState({ isUploadedLogo: true });
-        this.setState({ uploadedLogoSrc: logoPath });
-      }
-      else {
-        this.setState({ isUploadedLogo: false });
-        this.setState({ uploadedLogoSrc: DEFAULT_LOGO });
-      }
+
     }
     catch (err) {
       this.setState({ retrieveError: err });

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

@@ -81,68 +81,70 @@ const CustomizeLogoSetting: FC<Props> = (props: Props) => {
     <React.Fragment>
       <div className="row">
         <div className="col-12">
-          <h2 className="admin-setting-header">{t('admin:customize_setting.custom_logo')}</h2>
-          <div className="row">
-            <div className="col-md-6 col-12">
-              <h4>
-                <div className="custom-control custom-radio radio-primary">
-                  <input
-                    type="radio"
-                    id="radioDefaultLogo"
-                    className="custom-control-input"
-                    form="formImageType"
-                    name="imagetypeForm[isDefaultLogo]"
-                    checked={isDefaultLogo}
-                    onChange={() => { adminCustomizeContainer.switchDefaultLogo() }}
-                  />
-                  <label className="custom-control-label" htmlFor="radioDefaultLogo">
-                    {t('admin:customize_setting.default_logo')}
+          <div className="mb-5">
+            <h2 className="border-bottom my-4 admin-setting-header">{t('admin:customize_setting.custom_logo')}</h2>
+            <div className="row">
+              <div className="col-md-6 col-12 mb-3 mb-md-0">
+                <h4>
+                  <div className="custom-control custom-radio radio-primary">
+                    <input
+                      type="radio"
+                      id="radioDefaultLogo"
+                      className="custom-control-input"
+                      form="formImageType"
+                      name="imagetypeForm[isDefaultLogo]"
+                      checked={isDefaultLogo}
+                      onChange={() => { adminCustomizeContainer.switchDefaultLogo() }}
+                    />
+                    <label className="custom-control-label" htmlFor="radioDefaultLogo">
+                      {t('admin:customize_setting.default_logo')}
+                    </label>
+                  </div>
+                </h4>
+                <img src={defaultLogoSrc} width="64" />
+              </div>
+              <div className="col-md-6 col-12">
+                <h4>
+                  <div className="custom-control custom-radio radio-primary">
+                    <input
+                      type="radio"
+                      id="radioUploadLogo"
+                      className="custom-control-input"
+                      form="formImageType"
+                      name="imagetypeForm[isDefaultLogo]"
+                      checked={!isDefaultLogo}
+                      onChange={() => { adminCustomizeContainer.switchDefaultLogo() }}
+                    />
+                    <label className="custom-control-label" htmlFor="radioUploadLogo">
+                      { t('admin:customize_setting.upload_logo') }
+                    </label>
+                  </div>
+                </h4>
+                <div className="row mb-3">
+                  <label className="col-sm-4 col-12 col-form-label text-left">
+                    { t('admin:customize_setting.current_logo') }
                   </label>
+                  <div className="col-sm-8 col-12">
+                    {uploadedLogoSrc && (<p><img src={uploadedLogoSrc} className="picture picture-lg " id="settingBrandLogo" width="64" /></p>)}
+                    {isUploadedLogo && (
+                      <button type="button" className="btn btn-danger" onClick={onClickDeleteBtn}>
+                        { t('admin:customize_setting.delete_logo') }
+                      </button>
+                    )}
+                  </div>
                 </div>
-              </h4>
-              <img src={defaultLogoSrc} width="64" />
-            </div>
-            <div className="col-md-6 col-12">
-              <h4>
-                <div className="custom-control custom-radio radio-primary">
-                  <input
-                    type="radio"
-                    id="radioUploadLogo"
-                    className="custom-control-input"
-                    form="formImageType"
-                    name="imagetypeForm[isDefaultLogo]"
-                    checked={!isDefaultLogo}
-                    onChange={() => { adminCustomizeContainer.switchDefaultLogo() }}
-                  />
-                  <label className="custom-control-label" htmlFor="radioUploadLogo">
-                    { t('admin:customize_setting.upload_logo') }
+                <div className="row">
+                  <label className="col-sm-4 col-12 col-form-label text-left">
+                    { t('admin:customize_setting.upload_new_logo') }
                   </label>
-                </div>
-              </h4>
-              <div className="row mb-3">
-                <label className="col-sm-4 col-12 col-form-label text-left">
-                  { t('admin:customize_setting.current_logo') }
-                </label>
-                <div className="col-sm-8 col-12">
-                  {uploadedLogoSrc && (<p><img src={uploadedLogoSrc} className="picture picture-lg " id="settingBrandLogo" width="64" /></p>)}
-                  {isUploadedLogo && (
-                    <button type="button" className="btn btn-danger" onClick={onClickDeleteBtn}>
-                      { t('admin:customize_setting.delete_logo') }
-                    </button>
-                  )}
-                </div>
-              </div>
-              <div className="row">
-                <label className="col-sm-4 col-12 col-form-label text-left">
-                  { t('admin:customize_setting.upload_new_logo') }
-                </label>
-                <div className="col-sm-8 col-12">
-                  <input type="file" onChange={onSelectFile} name="brandLogo" accept="image/*" />
+                  <div className="col-sm-8 col-12">
+                    <input type="file" onChange={onSelectFile} name="brandLogo" accept="image/*" />
+                  </div>
                 </div>
               </div>
             </div>
+            <AdminUpdateButtonRow onClick={onClickSubmit} disabled={adminCustomizeContainer.state.retrieveError != null} />
           </div>
-          <AdminUpdateButtonRow onClick={onClickSubmit} disabled={adminCustomizeContainer.state.retrieveError != null} />
         </div>
       </div>
 
@@ -151,6 +153,7 @@ const CustomizeLogoSetting: FC<Props> = (props: Props) => {
         src={src}
         onModalClose={cancelModal}
         onCropCompleted={onCropCompleted}
+        circular={false}
       />
     </React.Fragment>
   );

packages/app/src/components/Common/ImageCropModal.tsx

@@ -33,12 +33,13 @@ type Props = {
   show: boolean,
   src: string | ArrayBuffer | null,
   onModalClose: () => void,
-  onCropCompleted: (res: any) => void
+  onCropCompleted: (res: any) => void,
+  circular: boolean,
 }
 const ImageCropModal: FC<Props> = (props: Props) => {
 
   const {
-    show, src, onModalClose, onCropCompleted,
+    show, src, onModalClose, onCropCompleted, circular,
   } = props;
 
   const [imageRef, setImageRef] = useState<HTMLImageElement>();
@@ -106,7 +107,7 @@ const ImageCropModal: FC<Props> = (props: Props) => {
         {t('crop_image_modal.image_crop')}
       </ModalHeader>
       <ModalBody className="my-4">
-        <ReactCrop src={src} crop={cropOptions} onImageLoaded={onImageLoaded} onChange={onCropChange} />
+        <ReactCrop src={src} crop={cropOptions} onImageLoaded={onImageLoaded} onChange={onCropChange} circularCrop={circular} />
       </ModalBody>
       <ModalFooter>
         <button type="button" className="btn btn-outline-danger rounded-pill mr-auto" onClick={reset}>

packages/app/src/components/Me/ProfileImageSettings.jsx

@@ -168,6 +168,7 @@ class ProfileImageSettings extends React.Component {
           src={this.state.src}
           onModalClose={this.cancelModal}
           onCropCompleted={this.onCropCompleted}
+          circular
         />
 
         <div className="row my-3">

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

@@ -6,10 +6,10 @@ import loggerFactory from '~/utils/logger';
 // eslint-disable-next-line no-unused-vars
 const path = require('path');
 
+const { addSeconds } = require('date-fns');
 const mongoose = require('mongoose');
-const uniqueValidator = require('mongoose-unique-validator');
 const mongoosePaginate = require('mongoose-paginate-v2');
-const { addSeconds } = require('date-fns');
+const uniqueValidator = require('mongoose-unique-validator');
 
 // eslint-disable-next-line no-unused-vars
 const logger = loggerFactory('growi:models:attachment');
@@ -37,8 +37,8 @@ module.exports = function(crowi) {
     temporaryUrlExpiredAt: { type: Date },
     attachmentType: {
       type: String,
-      enum: ['BRAND_LOGO', 'WIKI_PAGE', 'USER_PAGE', 'PROFILE_IMAGE', null],
-      default: null,
+      enum: ['BRAND_LOGO', 'WIKI_PAGE', 'USER_PAGE', 'PROFILE_IMAGE', 'UNCATEGORIZED'],
+      default: 'UNCATEGORIZED',
     },
   });
   attachmentSchema.plugin(uniqueValidator);
@@ -56,7 +56,7 @@ module.exports = function(crowi) {
   attachmentSchema.set('toJSON', { virtuals: true });
 
 
-  attachmentSchema.statics.createWithoutSave = function(pageId, user, fileStream, originalName, fileFormat, fileSize, attachmentType = null) {
+  attachmentSchema.statics.createWithoutSave = function(pageId, user, fileStream, originalName, fileFormat, fileSize, attachmentType = 'UNCATEGORIZED') {
     const Attachment = this;
 
     const extname = path.extname(originalName);

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

@@ -1,7 +1,7 @@
+import { getOrCreateModel } from '@growi/core';
 import { Types, Schema } from 'mongoose';
 import uniqueValidator from 'mongoose-unique-validator';
 
-import { getOrCreateModel } from '@growi/core';
 
 export interface Config {
   _id: Types.ObjectId;

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

@@ -10,6 +10,7 @@ const express = require('express');
 const router = express.Router();
 
 const { body, query } = require('express-validator');
+
 const ErrorV3 = require('../../models/vo/error-apiv3');
 
 /**
@@ -183,6 +184,7 @@ module.exports = (crowi) => {
       customizeScript: await crowi.configManager.getConfig('crowi', 'customize:script'),
       attachmentLogoId: await crowi.configManager.getConfig('crowi', 'customize:attachmentLogoId'),
       isDefaultLogo: await crowi.configManager.getConfig('crowi', 'customize:isDefaultLogo'),
+      uploadedLogoSrc: await crowi.configManager.getConfig('crowi', 'customize:uploadedLogoSrc'),
     };
 
     return res.apiv3({ customizeParams });
@@ -617,16 +619,19 @@ module.exports = (crowi) => {
 
   router.put('/customize-logo', loginRequiredStrictly, adminRequired, csrf, validator.logo, apiV3FormValidator, async(req, res) => {
 
+    const { isDefaultLogo, attachmentId } = req.body;
     // Set default logo when uploaded logo is empty
-    const isDefaultLogo = req.body.attachmentId ? req.body.isDefaultLogo : true;
+
+    const attachmentLogoId = isDefaultLogo || attachmentId == null ? null : attachmentId;
+
     const requestParams = {
-      'customize:attachmentLogoId': req.body.attachmentId,
+      'customize:attachmentLogoId': attachmentLogoId,
       'customize:isDefaultLogo': isDefaultLogo,
     };
     try {
       await crowi.configManager.updateConfigsInTheSameNamespace('crowi', requestParams);
       const customizedParams = {
-        attachmentLogoId: await crowi.configManager.getConfig('crowi', 'customize:attachmentLogoId'),
+        attachmentId: await crowi.configManager.getConfig('crowi', 'customize:attachmentLogoId'),
         isDefaultLogo: await crowi.configManager.getConfig('crowi', 'customize:isDefaultLogo'),
       };
       return res.apiv3({ customizedParams });

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

@@ -709,14 +709,14 @@ module.exports = function(crowi, app) {
     }
 
     const file = req.file;
-    const attachmentType = req.body.attachmentType;
+    const { attachmentType, attachmentId } = req.body;
 
-    let attachmentId;
-    if (req.body.attachmentId === 'null' || req.body.attachmentId === 'undefined') {
-      attachmentId = null;
+    let previousAttachmentId;
+    if (attachmentId === 'null' || attachmentId === 'undefined') {
+      previousAttachmentId = null;
     }
     else {
-      attachmentId = mongoose.Types.ObjectId(req.body.attachmentId);
+      previousAttachmentId = mongoose.Types.ObjectId(attachmentId);
     }
 
     // check type
@@ -725,9 +725,10 @@ module.exports = function(crowi, app) {
       return res.json(ApiResponse.error('File type error. Only image files is allowed to set as user picture.'));
     }
 
-    // remove previous attachment
-    if (attachmentId) {
-      await attachmentService.removeAttachment(attachmentId);
+    // Check if previous attachment exists and remove it
+    const previousAttachment = await Attachment.findById(previousAttachmentId);
+    if (previousAttachment !== null) {
+      await attachmentService.removeAttachment(previousAttachmentId);
     }
 
     let attachment;
@@ -752,15 +753,16 @@ module.exports = function(crowi, app) {
   };
 
   api.removeBrandLogo = async function(req, res) {
-    const attachmentId = mongoose.Types.ObjectId(req.body.attachmentId);
-    const attachment = await Attachment.findById(attachmentId);
+    const { attachmentId } = req.body;
+    const attachmentObjectId = mongoose.Types.ObjectId(attachmentId);
+    const attachment = await Attachment.findById(attachmentObjectId);
 
     if (attachment == null) {
       return res.json(ApiResponse.error('attachment not found'));
     }
 
     try {
-      await attachmentService.removeAttachment(attachmentId);
+      await attachmentService.removeAttachment(attachmentObjectId);
       // update attachmentLogoId immediately
       const attachmentConfigParams = {
         'customize:attachmentLogoId': null,

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

@@ -1,8 +1,9 @@
 import loggerFactory from '~/utils/logger';
 
-const mongoose = require('mongoose');
 const fs = require('fs');
 
+const mongoose = require('mongoose');
+
 // eslint-disable-next-line @typescript-eslint/no-unused-vars
 const logger = loggerFactory('growi:service:AttachmentService');
 
@@ -15,7 +16,7 @@ class AttachmentService {
     this.crowi = crowi;
   }
 
-  async createAttachment(file, user, pageId = null, attachmentType = null) {
+  async createAttachment(file, user, pageId = null, attachmentType = 'UNCATEGORIZED') {
     const { fileUploadService } = this.crowi;
 
     // check limit