Code improvement

https://youtrack.weseek.co.jp/issue/GW-7808
- Add constant file for attachmentType enum
- Implement constant for attachmentType and remove USER_PAGE
- Rename attachmentId to brandLogoAttachmentId
- Remove customize:isDefaultLogo and customize:uploadedLogoSrc from config.ts
- Set nullable true to uploadedLogoSrc validator option
- Remove isDefaultLogo variable from GrowiNavbar

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

@@ -1,5 +1,6 @@
 import { Container } from 'unstated';
 
+import { AttachmentType } from '~/server/interfaces/attachment';
 import loggerFactory from '~/utils/logger';
 
 import { toastError } from '../util/apiNotification';
@@ -61,7 +62,7 @@ export default class AdminCustomizeContainer extends Container {
       uploadedLogoSrc: null,
       defaultLogoSrc: DEFAULT_LOGO,
       isDefaultLogo: true,
-      attachmentId: '',
+      brandLogoAttachmentId: '',
       /* eslint-enable quote-props, no-multi-spaces */
     };
     this.switchPageListLimitationS = this.switchPageListLimitationS.bind(this);
@@ -106,7 +107,7 @@ export default class AdminCustomizeContainer extends Container {
         currentCustomizeHeader: customizeParams.customizeHeader,
         currentCustomizeCss: customizeParams.customizeCss,
         currentCustomizeScript: customizeParams.customizeScript,
-        attachmentId: customizeParams.attachmentId,
+        brandLogoAttachmentId: customizeParams.brandLogoAttachmentId,
         isDefaultLogo: customizeParams.isDefaultLogo,
         uploadedLogoSrc: customizeParams.uploadedLogoSrc,
       });
@@ -446,12 +447,12 @@ export default class AdminCustomizeContainer extends Container {
     try {
       const formData = {
         _csrf:  this.appContainer.csrfToken,
-        attachmentId: this.state.attachmentId,
+        brandLogoAttachmentId: this.state.brandLogoAttachmentId,
       };
       await apiPost('/attachments.removeBrandLogo', formData);
       this.setState({
         uploadedLogoSrc: null,
-        attachmentId: null,
+        brandLogoAttachmentId: null,
       });
 
     }
@@ -467,13 +468,13 @@ export default class AdminCustomizeContainer extends Container {
       const formData = new FormData();
       formData.append('file', file);
       formData.append('_csrf', this.appContainer.csrfToken);
-      formData.append('attachmentType', 'BRAND_LOGO');
-      formData.append('attachmentId', this.state.attachmentId);
+      formData.append('attachmentType', AttachmentType.BRAND_LOGO);
+      formData.append('brandLogoAttachmentId', this.state.brandLogoAttachmentId);
       const response = await apiPost('/attachments.uploadBrandLogo', formData);
 
       this.setState({
         uploadedLogoSrc: response.attachment.filePathProxied,
-        attachmentId: response.attachment.id,
+        brandLogoAttachmentId: response.attachment.id,
       });
     }
     catch (err) {
@@ -491,13 +492,13 @@ export default class AdminCustomizeContainer extends Container {
     try {
       const response = await apiv3Put('/customize-setting/customize-logo', {
         isDefaultLogo: this.state.isDefaultLogo,
-        attachmentId: this.state.attachmentId,
+        brandLogoAttachmentId: this.state.brandLogoAttachmentId,
         uploadedLogoSrc: this.state.uploadedLogoSrc,
       });
       const { customizedParams } = response.data;
       this.setState({
         isDefaultLogo: customizedParams.isDefaultLogo,
-        attachmentId:  customizedParams.attachmentId,
+        brandLogoAttachmentId:  customizedParams.brandLogoAttachmentId,
         uploadedLogoSrc: customizedParams.uploadedLogoSrc,
       });
     }

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

@@ -1,5 +1,6 @@
 import { Container } from 'unstated';
 
+import { AttachmentType } from '~/server/interfaces/attachment';
 import loggerFactory from '~/utils/logger';
 
 import { apiPost } from '../util/apiv1-client';
@@ -207,7 +208,7 @@ export default class PersonalContainer extends Container {
       const formData = new FormData();
       formData.append('file', file);
       formData.append('_csrf', this.appContainer.csrfToken);
-      formData.append('attachmentType', 'PROFILE_IMAGE');
+      formData.append('attachmentType', AttachmentType.PROFILE_IMAGE);
       const response = await apiPost('/attachments.uploadProfileImage', formData);
       this.setState({ isUploadedPicture: true, uploadedPictureSrc: response.attachment.filePathProxied });
     }

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

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

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

@@ -0,0 +1,8 @@
+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

@@ -1,5 +1,7 @@
 import loggerFactory from '~/utils/logger';
 
+import { AttachmentType } from '../interfaces/attachment';
+
 // disable no-return-await for model functions
 /* eslint-disable no-return-await */
 
@@ -37,8 +39,8 @@ module.exports = function(crowi) {
     temporaryUrlExpiredAt: { type: Date },
     attachmentType: {
       type: String,
-      enum: ['BRAND_LOGO', 'WIKI_PAGE', 'USER_PAGE', 'PROFILE_IMAGE', 'UNCATEGORIZED'],
-      default: 'UNCATEGORIZED',
+      enum: AttachmentType,
+      default: undefined,
     },
   });
   attachmentSchema.plugin(uniqueValidator);
@@ -56,7 +58,7 @@ module.exports = function(crowi) {
   attachmentSchema.set('toJSON', { virtuals: true });
 
 
-  attachmentSchema.statics.createWithoutSave = function(pageId, user, fileStream, originalName, fileFormat, fileSize, attachmentType = 'UNCATEGORIZED') {
+  attachmentSchema.statics.createWithoutSave = function(pageId, user, fileStream, originalName, fileFormat, fileSize, attachmentType = undefined) {
     const Attachment = this;
 
     const extname = path.extname(originalName);

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

@@ -135,8 +135,6 @@ export const defaultCrowiConfigs: { [key: string]: any } = {
   'customize:isEnabledStaleNotification': false,
   'customize:isAllReplyShown': false,
   'customize:isSearchScopeChildrenAsDefault': false,
-  'customize:isDefaultLogo': true,
-  'customize:uploadedLogoSrc': undefined,
 
   'notification:owner-page:isEnabled': false,
   'notification:group-page:isEnabled': false,

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

@@ -137,9 +137,9 @@ module.exports = (crowi) => {
       body('customizeScript').isString(),
     ],
     logo: [
-      body('attachmentId').isString().optional({ nullable: true }),
+      body('brandLogoAttachmentId').isString().optional({ nullable: true }),
       body('isDefaultLogo').isBoolean(),
-      body('uploadedLogoSrc').isString(),
+      body('uploadedLogoSrc').isString().optional({ nullable: true }),
     ],
   };
 
@@ -183,7 +183,7 @@ 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'),
-      attachmentId: await crowi.configManager.getConfig('crowi', 'customize:attachmentId'),
+      brandLogoAttachmentId: await crowi.configManager.getConfig('crowi', 'customize:brandLogoAttachmentId'),
       isDefaultLogo: await crowi.configManager.getConfig('crowi', 'customize:isDefaultLogo'),
       uploadedLogoSrc: await crowi.configManager.getConfig('crowi', 'customize:uploadedLogoSrc'),
     };
@@ -621,18 +621,18 @@ module.exports = (crowi) => {
   router.put('/customize-logo', loginRequiredStrictly, adminRequired, csrf, validator.logo, apiV3FormValidator, async(req, res) => {
 
     const {
-      isDefaultLogo, attachmentId, uploadedLogoSrc,
+      isDefaultLogo, brandLogoAttachmentId, uploadedLogoSrc,
     } = req.body;
 
     const requestParams = {
-      'customize:attachmentId': attachmentId,
+      'customize:brandLogoAttachmentId': brandLogoAttachmentId,
       'customize:isDefaultLogo': isDefaultLogo,
       'customize:uploadedLogoSrc': uploadedLogoSrc,
     };
     try {
       await crowi.configManager.updateConfigsInTheSameNamespace('crowi', requestParams);
       const customizedParams = {
-        attachmentId: await crowi.configManager.getConfig('crowi', 'customize:attachmentId'),
+        brandLogoAttachmentId: await crowi.configManager.getConfig('crowi', 'customize:brandLogoAttachmentId'),
         isDefaultLogo: await crowi.configManager.getConfig('crowi', 'customize:isDefaultLogo'),
         uploadedLogoSrc: await crowi.configManager.getConfig('crowi', 'customize:uploadedLogoSrc'),
       };

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

@@ -1,5 +1,6 @@
 import mongoose from 'mongoose';
 
+import { AttachmentType } from '~/server/interfaces/attachment';
 import loggerFactory from '~/utils/logger';
 /* eslint-disable no-use-before-define */
 
@@ -453,7 +454,7 @@ module.exports = function(crowi, app) {
 
     let attachment;
     try {
-      attachment = await attachmentService.createAttachment(file, req.user, pageId, 'WIKI_PAGE');
+      attachment = await attachmentService.createAttachment(file, req.user, pageId, AttachmentType.WIKI_PAGE);
     }
     catch (err) {
       logger.error(err);
@@ -709,14 +710,14 @@ module.exports = function(crowi, app) {
     }
 
     const file = req.file;
-    const { attachmentType, attachmentId } = req.body;
+    const { attachmentType, brandLogoAttachmentId } = req.body;
 
     let previousAttachmentId;
-    if (attachmentId === 'null' || attachmentId === 'undefined') {
+    if (brandLogoAttachmentId === 'null' || brandLogoAttachmentId === 'undefined') {
       previousAttachmentId = null;
     }
     else {
-      previousAttachmentId = mongoose.Types.ObjectId(attachmentId);
+      previousAttachmentId = mongoose.Types.ObjectId(brandLogoAttachmentId);
     }
 
     // check type
@@ -735,8 +736,7 @@ module.exports = function(crowi, app) {
     try {
       attachment = await attachmentService.createAttachment(file, req.user, null, attachmentType);
       const attachmentConfigParams = {
-        'customize:attachmentId': attachment.id,
-        'customize:uploadedLogoSrc': attachment.filePathProxied,
+        'customize:brandLogoAttachmentId': attachment.id,
       };
       await crowi.configManager.updateConfigsInTheSameNamespace('crowi', attachmentConfigParams);
     }
@@ -753,8 +753,8 @@ module.exports = function(crowi, app) {
   };
 
   api.removeBrandLogo = async function(req, res) {
-    const { attachmentId } = req.body;
-    const attachmentObjectId = mongoose.Types.ObjectId(attachmentId);
+    const { brandLogoAttachmentId } = req.body;
+    const attachmentObjectId = mongoose.Types.ObjectId(brandLogoAttachmentId);
     const attachment = await Attachment.findById(attachmentObjectId);
 
     if (attachment == null) {
@@ -765,7 +765,7 @@ module.exports = function(crowi, app) {
       await attachmentService.removeAttachment(attachmentObjectId);
       // update attachmentId immediately
       const attachmentConfigParams = {
-        'customize:attachmentId': null,
+        'customize:brandLogoAttachmentId': null,
         'customize:isDefaultLogo': true,
       };
       await crowi.configManager.updateConfigsInTheSameNamespace('crowi', attachmentConfigParams);

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

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