Merge pull request #1150 from weseek/master

release v3.5.6

CHANGES.md

@@ -1,6 +1,12 @@
 # CHANGES
 
-## 3.5.5-RC
+## 3.5.6-RC
+
+* Fix: Saving new page is failed when empty string tag is set
+* Fix: Link of Create template page button in New Page Modal is broken
+* Fix: Global Notification dows not work when creating/moving/deleting/like/comment
+
+## 3.5.5
 
 * Feature: Support S3-compatible object storage (e.g. MinIO)
 * Feature: Enable/Disable ID/Password Authentication

package.json

@@ -1,6 +1,6 @@
 {
   "name": "growi",
-  "version": "3.5.5-RC",
+  "version": "3.5.6-RC",
   "description": "Team collaboration software using markdown",
   "tags": [
     "wiki",

src/client/js/services/TagContainer.js

@@ -49,7 +49,9 @@ export default class TagContainer extends Container {
     }
     // when the page not exist
     else if (templateTagData != null) {
-      tags = templateTagData.split(',');
+      tags = templateTagData.split(',').filter((str) => {
+        return str !== ''; // filter empty values
+      });
     }
 
     logger.debug('tags data has been initialized');

src/server/models/page-tag-relation.js

@@ -31,12 +31,6 @@ schema.plugin(mongoosePaginate);
  */
 class PageTagRelation {
 
-  static async createIfNotExist(pageId, tagId) {
-    if (!await this.findOne({ relatedPage: pageId, relatedTag: tagId })) {
-      await this.create({ relatedPage: pageId, relatedTag: tagId });
-    }
-  }
-
   static async createTagListWithCount(option) {
     const Tag = mongoose.model('Tag');
     const opt = option || {};
@@ -56,35 +50,54 @@ class PageTagRelation {
     return { list, totalCount };
   }
 
-  static async listTagsByPage(pageId) {
-    const tags = await this.find({ relatedPage: pageId }).populate('relatedTag').select('-_id relatedTag');
-    return tags.filter((tag) => { return tag.relatedTag !== null });
+  static async findByPageId(pageId) {
+    const relations = await this.find({ relatedPage: pageId }).populate('relatedTag').select('-_id relatedTag');
+    return relations.filter((relation) => { return relation.relatedTag !== null });
   }
 
   static async listTagNamesByPage(pageId) {
-    const tags = await this.listTagsByPage(pageId);
-    return tags.map((tag) => { return tag.relatedTag.name });
+    const relations = await this.findByPageId(pageId);
+    return relations.map((relation) => { return relation.relatedTag.name });
   }
 
   static async updatePageTags(pageId, tags) {
+    if (pageId == null || tags == null) {
+      throw new Error('args \'pageId\' and \'tags\' are required.');
+    }
+
+    // filter empty string
+    // eslint-disable-next-line no-param-reassign
+    tags = tags.filter((tag) => { return tag !== '' });
+
     const Tag = mongoose.model('Tag');
 
-    // get tags relate this page
-    const relatedTags = await this.listTagsByPage(pageId);
+    // get relations for this page
+    const relations = await this.findByPageId(pageId);
 
     // unlink relations
-    const unlinkTagRelations = relatedTags.filter((tag) => { return !tags.includes(tag.relatedTag.name) });
-    await this.deleteMany({
+    const unlinkTagRelations = relations.filter((relation) => { return !tags.includes(relation.relatedTag.name) });
+    const bulkDeletePromise = this.deleteMany({
       relatedPage: pageId,
       relatedTag: { $in: unlinkTagRelations.map((relation) => { return relation.relatedTag._id }) },
     });
 
-    // create tag and relations
-    /* eslint-disable no-await-in-loop */
-    for (const tag of tags) {
-      const setTag = await Tag.findOrCreate(tag);
-      await this.createIfNotExist(pageId, setTag._id);
-    }
+    // filter tags to create
+    const relatedTagNames = relations.map((relation) => { return relation.relatedTag.name });
+    // find or create tags
+    const tagsToCreate = tags.filter((tag) => { return !relatedTagNames.includes(tag) });
+    const tagEntities = await Tag.findOrCreateMany(tagsToCreate);
+
+    // create relations
+    const bulkCreatePromise = this.insertMany(
+      tagEntities.map((relatedTag) => {
+        return {
+          relatedPage: pageId,
+          relatedTag,
+        };
+      }),
+    );
+
+    return Promise.all([bulkDeletePromise, bulkCreatePromise]);
   }
 
 }

src/server/models/tag.js

@@ -11,6 +11,7 @@ const schema = new mongoose.Schema({
   name: {
     type: String,
     required: true,
+    unique: true,
   },
 });
 schema.plugin(mongoosePaginate);
@@ -25,11 +26,26 @@ class Tag {
   static async findOrCreate(tagName) {
     const tag = await this.findOne({ name: tagName });
     if (!tag) {
-      return await this.create({ name: tagName });
+      return this.create({ name: tagName });
     }
     return tag;
   }
 
+  static async findOrCreateMany(tagNames) {
+    const existTags = await this.find({ name: { $in: tagNames } });
+    const existTagNames = existTags.map((tag) => { return tag.name });
+
+    // bulk insert
+    const tagsToCreate = tagNames.filter((tagName) => { return !existTagNames.includes(tagName) });
+    await this.insertMany(
+      tagsToCreate.map((tag) => {
+        return { name: tag };
+      }),
+    );
+
+    return this.find({ name: { $in: tagNames } });
+  }
+
 }
 
 module.exports = function(crowi) {

src/server/service/global-notification.js

@@ -1,5 +1,5 @@
 const logger = require('@alias/logger')('growi:service:GlobalNotification');
-const path = require('path');
+const nodePath = require('path');
 /**
  * the service class of GlobalNotificationSetting
  */
@@ -43,7 +43,7 @@ class GlobalNotificationService {
     const option = {
       mail: {
         subject: `#pageCreate - ${page.creator.username} created ${page.path}`,
-        template: `../../locales/${lang}/notifications/pageCreate.txt`,
+        template: nodePath.join(this.crowi.localeDir, `${lang}/notifications/pageCreate.txt`),
         vars: {
           appTitle: this.appTitle,
           path: page.path,
@@ -69,7 +69,7 @@ class GlobalNotificationService {
     const option = {
       mail: {
         subject: `#pageEdit - ${page.creator.username} edited ${page.path}`,
-        template: path.join(this.crowi.localeDir, `${lang}/notifications/pageEdit.txt`),
+        template: nodePath.join(this.crowi.localeDir, `${lang}/notifications/pageEdit.txt`),
         vars: {
           appTitle: this.appTitle,
           path: page.path,
@@ -95,7 +95,7 @@ class GlobalNotificationService {
     const option = {
       mail: {
         subject: `#pageDelete - ${page.creator.username} deleted ${page.path}`, // FIXME
-        template: `../../locales/${lang}/notifications/pageDelete.txt`,
+        template: nodePath.join(this.crowi.localeDir, `${lang}/notifications/pageDelete.txt`),
         vars: {
           appTitle: this.appTitle,
           path: page.path,
@@ -119,7 +119,7 @@ class GlobalNotificationService {
     const option = {
       mail: {
         subject: `#pageMove - ${user.username} moved ${page.path} to ${page.path}`, // FIXME
-        template: `../../locales/${lang}/notifications/pageMove.txt`,
+        template: nodePath.join(this.crowi.localeDir, `${lang}/notifications/pageMove.txt`),
         vars: {
           appTitle: this.appTitle,
           oldPath: oldPagePath,
@@ -144,7 +144,7 @@ class GlobalNotificationService {
     const option = {
       mail: {
         subject: `#pageLike - ${user.username} liked ${page.path}`,
-        template: `../../locales/${lang}/notifications/pageLike.txt`,
+        template: nodePath.join(this.crowi.localeDir, `${lang}/notifications/pageLike.txt`),
         vars: {
           appTitle: this.appTitle,
           path: page.path,
@@ -170,7 +170,7 @@ class GlobalNotificationService {
     const option = {
       mail: {
         subject: `#comment - ${user.username} commented on ${path}`,
-        template: `../../locales/${lang}/notifications/comment.txt`,
+        template: nodePath.join(this.crowi.localeDir, `${lang}/notifications/comment.txt`),
         vars: {
           appTitle: this.appTitle,
           path,

src/server/util/mailer.js

@@ -3,7 +3,7 @@
  */
 
 module.exports = function(crowi) {
-  const debug = require('debug')('growi:lib:mailer');
+  const logger = require('@alias/logger')('growi:lib:mailer');
   const nodemailer = require('nodemailer');
   const swig = require('swig-templates');
 
@@ -13,7 +13,7 @@ module.exports = function(crowi) {
   let mailer = {};
 
   function createSMTPClient(option) {
-    debug('createSMTPClient option', option);
+    logger.debug('createSMTPClient option', option);
     if (!option) {
       option = { // eslint-disable-line no-param-reassign
         host: configManager.getConfig('crowi', 'mail:smtpHost'),
@@ -34,7 +34,7 @@ module.exports = function(crowi) {
 
     const client = nodemailer.createTransport(option);
 
-    debug('mailer set up for SMTP', client);
+    logger.debug('mailer set up for SMTP', client);
     return client;
   }
 
@@ -49,7 +49,7 @@ module.exports = function(crowi) {
     const ses = require('nodemailer-ses-transport');
     const client = nodemailer.createTransport(ses(option));
 
-    debug('mailer set up for SES', client);
+    logger.debug('mailer set up for SES', client);
     return client;
   }
 
@@ -75,7 +75,7 @@ module.exports = function(crowi) {
     mailConfig.from = configManager.getConfig('crowi', 'mail:from');
     mailConfig.subject = `${appService.getAppTitle()}からのメール`;
 
-    debug('mailer initialized');
+    logger.debug('mailer initialized');
   }
 
   function setupMailConfig(overrideConfig) {
@@ -110,7 +110,7 @@ module.exports = function(crowi) {
       );
     }
 
-    debug('Mailer is not completed to set up. Please set up SMTP or AWS setting.');
+    logger.debug('Mailer is not completed to set up. Please set up SMTP or AWS setting.');
     return callback(new Error('Mailer is not completed to set up. Please set up SMTP or AWS setting.'), null);
   }
 

src/server/views/modal/create_page.html

@@ -44,9 +44,10 @@
           </fieldset>
         </form>
 
+        {% set templateParentPath = parentPath(path | preventXss | escape) %}
         <div id="template-form" class="row form-horizontal m-t-15">
           <fieldset class="col-xs-12">
-            <legend>{{ t('template.modal_label.Create template under', parentPath(path | preventXss | escape)) }}</legend>
+            <legend>{{ t('template.modal_label.Create template under', templateParentPath) }}</legend>
             <div class="d-flex create-page-input-container">
               <div class="create-page-input-row d-flex align-items-center">
                 <select id="template-type" class="form-control selectpicker" title="{{ t('template.option_label.select') }}">
@@ -72,8 +73,8 @@
             // modify href
             const value = $(this).val();
             const pageName = (value === 'children') ? '_template' : '__template';
-            const truePath = "{% if page.path || path %}{{ page.path || path | addTrailingSlash }}{% else %}{{ page.path || path }}{% endif %}"
-            const link = truePath + pageName + '#edit-form';
+            const parentPath = '{{templateParentPath}}';
+            const link = parentPath + pageName + '#edit-form';
             $('#link-to-template').attr('href', link);
           });
         </script>

src/server/views/modal/create_template.html

@@ -1,3 +1,5 @@
+{% set templateParentPath = parentPath(page.path | preventXss | escape) %}
+
 <div class="modal create-template" id="create-template">
   <div class="modal-dialog">
     <div class="modal-content">
@@ -8,7 +10,7 @@
       </div>
       <div class="modal-body">
         <div class="form-group">
-          <label class="mb-4">{{ t('template.modal_label.Create template under', page.path ) }}</label>
+          <label class="mb-4">{{ t('template.modal_label.Create template under', templateParentPath ) }}</label>
           <div class="row">
             <div class="col-sm-6">
               <div class="panel panel-default panel-select-template">
@@ -18,7 +20,7 @@
                   <p class="help-block text-center"><small>{{ t('template.children.desc') }}</small></p>
                 </div>
                 <div class="panel-footer text-center">
-                  <a href="{% if page.path.endsWith('/') %}{{ page.path }}{% else %}{{ page.path}}/{% endif %}_template#edit"
+                  <a href="{{templateParentPath}}_template#edit"
                       class="btn btn-sm btn-primary" id="template-button-children">
                       {{ t("Edit") }}
                   </a>
@@ -33,7 +35,7 @@
                   <p class="help-block text-center"><small>{{ t('template.decendants.desc') }}</small></p>
                 </div>
                 <div class="panel-footer text-center">
-                  <a href="{% if page.path.endsWith('/') %}{{ page.path }}{% else %}{{ page.path }}/{% endif %}__template#edit"
+                  <a href="{{templateParentPath}}__template#edit"
                       class="btn btn-sm btn-primary" id="template-button-decendants">
                       {{ t("Edit") }}
                   </a>