Merge branch 'master' into password-length-test

client/coral-admin/src/actions/comments.js

@@ -55,9 +55,9 @@ export const fetchFlaggedQueue = () => {
     dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_REQUEST});
 
     return coralApi('/queue/comments/flagged')
-      .then(comments => {
-        comments.forEach(comment => comment.flagged = true);
-        return comments;
+      .then(results => {
+        results.comments.forEach(comment => comment.flagged = true);
+        return results;
       })
       .then(addUsersCommentsActions.bind(this, dispatch));
   };

client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js

@@ -22,49 +22,61 @@ export default ({onTabClick, ...props}) => (
            className={`mdl-tabs__tab ${styles.tab}`}>{lang.t('modqueue.flagged')}</a>
       </div>
       <div className={`mdl-tabs__panel is-active ${styles.listContainer}`} id='pending'>
-        <CommentList
-          suspectWords={props.settings.settings.wordlist.suspect}
-          isActive={props.activeTab === 'pending'}
-          singleView={props.singleView}
-          commentIds={props.premodIds}
-          comments={props.comments.byId}
-          users={props.users.byId}
-          onClickAction={props.updateStatus}
-          onClickShowBanDialog={props.showBanUserDialog}
-          modActions={['reject', 'approve', 'ban']}
-          loading={props.comments.loading}/>
-        <BanUserDialog
-          open={props.comments.showBanUserDialog}
-          handleClose={props.hideBanUserDialog}
-          onClickBanUser={props.banUser}
-          user={props.comments.banUser}
-        />
+        {
+          props.activeTab === 'pending'
+          ? <div>
+              <CommentList
+                suspectWords={props.settings.settings.wordlist.suspect}
+                isActive={props.activeTab === 'pending'}
+                singleView={props.singleView}
+                commentIds={props.premodIds}
+                comments={props.comments.byId}
+                users={props.users.byId}
+                onClickAction={props.updateStatus}
+                onClickShowBanDialog={props.showBanUserDialog}
+                modActions={['reject', 'approve', 'ban']}
+                loading={props.comments.loading} />
+              <BanUserDialog
+                open={props.comments.showBanUserDialog}
+                handleClose={props.hideBanUserDialog}
+                onClickBanUser={props.banUser}
+                user={props.comments.banUser} />
+            </div>
+          : null
+        }
       </div>
       <div className={`mdl-tabs__panel ${styles.listContainer}`} id='rejected'>
-        <CommentList
-          suspectWords={props.settings.settings.wordlist.suspect}
-          isActive={props.activeTab === 'rejected'}
-          singleView={props.singleView}
-          commentIds={props.rejectedIds}
-          comments={props.comments.byId}
-          users={props.users.byId}
-          onClickAction={props.updateStatus}
-          modActions={['approve']}
-          loading={props.comments.loading}
-        />
+      {
+        props.activeTab === 'rejected'
+        ? <CommentList
+            suspectWords={props.settings.settings.wordlist.suspect}
+            isActive={props.activeTab === 'rejected'}
+            singleView={props.singleView}
+            commentIds={props.rejectedIds}
+            comments={props.comments.byId}
+            users={props.users.byId}
+            onClickAction={props.updateStatus}
+            modActions={['approve']}
+            loading={props.comments.loading} />
+        : null
+      }
       </div>
       <div className={`mdl-tabs__panel ${styles.listContainer}`} id='flagged'>
-        <CommentList
-          suspectWords={props.settings.settings.wordlist.suspect}
-          isActive={props.activeTab === 'flagged'}
-          singleView={props.singleView}
-          commentIds={props.flaggedIds}
-          comments={props.comments.byId}
-          users={props.users.byId}
-          onClickAction={props.updateStatus}
-          modActions={['reject', 'approve']}
-          loading={props.comments.loading}/>
-      </div>
+      {
+        props.activeTab === 'flagged'
+        ? <CommentList
+            suspectWords={props.settings.settings.wordlist.suspect}
+            isActive={props.activeTab === 'flagged'}
+            singleView={props.singleView}
+            commentIds={props.flaggedIds}
+            comments={props.comments.byId}
+            users={props.users.byId}
+            onClickAction={props.updateStatus}
+            modActions={['reject', 'approve']}
+            loading={props.comments.loading} />
+        : null
+      }
+       </div>
       <ModerationKeysModal open={props.modalOpen} onClose={props.closeModal} />
     </div>
   </div>

client/coral-framework/translations.json

@@ -14,6 +14,8 @@
       "PASSWORD_REQUIRED": "Must input a password",
       "PASSWORD_LENGTH": "Password is too short",
       "EMAIL_IN_USE": "Email address already in use",
+      "EMAIL_DISPLAY_NAME_IN_USE": "Email address or display name already in use",
+      "DISPLAYNAME_IN_USE": "Display name already in use",
       "DISPLAY_NAME_REQUIRED": "Must input a display name",
       "NO_SPECIAL_CHARACTERS": "Display names can contain letters, numbers and _ only",
       "PROFANITY_ERROR": "Display names must not contain profanity. Please contact the administrator if you believe this to be in error."
@@ -34,6 +36,8 @@
       "PASSWORD_REQUIRED": "Debe ingresar una contraseña",
       "PASSWORD_LENGTH": "La contraseña es muy corta",
       "EMAIL_IN_USE": "La dirección de correo electrónico se encuentra en uso",
+      "EMAIL_DISPLAY_NAME_IN_USE": "Correo o Nombre en uso.",
+      "DISPLAYNAME_IN_USE": "Nombre en uso.",
       "DISPLAY_NAME_REQUIRED": "Debe ingresar un nombre",
       "NO_SPECIAL_CHARACTERS": "Los nombres pueden contener letras, números y _",
       "PROFANITY_ERROR": "Los nombres no pueden contener blasfemias. Por favor contacte al administrador si cree que esto es un error"

client/coral-sign-in/translations.js

@@ -19,6 +19,7 @@ export default {
       alreadyHaveAnAccount: 'Already have an account?',
       recoverPassword: 'Recover password',
       emailInUse: 'Email address already in use',
+      emailORusernameInUse: 'Email address or Username already in use',
       requiredField: 'This field is required',
       passwordsDontMatch: 'Passwords don\'t match.',
       specialCharacters: 'Display names can contain letters, numbers and _ only',
@@ -45,6 +46,7 @@ export default {
       alreadyHaveAnAccount: 'Ya tienes una cuenta?',
       recoverPassword: 'Recuperar contraseña',
       emailInUse: 'Este email se encuentra en uso',
+      emailORusernameInUse: 'Este email ó nombre se encuentran en uso',
       requiredField: 'Este campo es requerido',
       passwordsDontMatch: 'Las contraseñas no coinciden',
       specialCharacters: 'Los nombres pueden contener letras, números y _',

errors.js

@@ -54,6 +54,11 @@ const ErrEmailTaken = new APIError('Email address already in use', {
   status: 400
 });
 
+const ErrDisplayTaken = new APIError('Display name already in use', {
+  translation_key: 'DISPLAYNAME_IN_USE',
+  status: 400
+});
+
 const ErrSpecialChars = new APIError('No special characters are allowed in a display name', {
   translation_key: 'NO_SPECIAL_CHARACTERS',
   status: 400
@@ -116,6 +121,7 @@ module.exports = {
   ErrSpecialChars,
   ErrMissingDisplay,
   ErrContainsProfanity,
+  ErrDisplayTaken,
   ErrAssetCommentingClosed,
   ErrNotFound,
   ErrInvalidAssetURL,

models/comment.js

@@ -137,27 +137,18 @@ CommentSchema.statics.findByAssetId = (asset_id) => Comment.find({
 });
 
 /**
- * Finds the accepted comments by the asset_id get the comments that are
- * accepted.
- * @param {String} asset_id  identifier of the asset which owns the comments (uuid)
- * @return {Promise}
+ * findByAssetIdWithStatuses finds all the comments where the asset id matches
+ * what's provided and the status is one of the ones listed in the statuses
+ * array.
+ * @param {String} asset_id      the asset id to search by
+ * @param {Array}  [statuses=[]] the array of statuses to search by
+ * @return {Promise}             resolves to an array of comments
  */
-CommentSchema.statics.findAcceptedByAssetId = (asset_id) => Comment.find({
+CommentSchema.statics.findByAssetIdWithStatuses = (asset_id, statuses = []) => Comment.find({
   asset_id,
-  status: 'accepted'
-});
-
-/**
- * Finds the new and accepted comments by the asset_id.
- * @param {String} asset_id  identifier of the asset which owns the comments (uuid)
- * @return {Promise}
- */
-CommentSchema.statics.findAcceptedAndNewByAssetId = (asset_id) => Comment.find({
-  asset_id,
-  $or: [
-    {status: 'accepted'},
-    {status: null}
-  ]
+  status: {
+    $in: statuses
+  }
 });
 
 /**
@@ -198,19 +189,12 @@ CommentSchema.statics.findByStatus = (status = null) => {
  */
 CommentSchema.statics.moderationQueue = (status, asset_id = null) => {
 
-  /**
-   * This adds the asset_id requirement to the query if the asset_id is defined.
-   */
-  const assetIDWrap = (query) => {
-    if (asset_id) {
-      query = query.where('asset_id', asset_id);
-    }
+  // Fetch the comments with statuses.
+  let comments = Comment.findByStatus(status);
 
-    return query;
-  };
-
-  // Pre-moderation:  New comments are shown in the moderator queues immediately.
-  let comments = assetIDWrap(Comment.findByStatus(status));
+  if (asset_id) {
+    comments = comments.where('asset_id', asset_id);
+  }
 
   return comments;
 };
@@ -282,8 +266,16 @@ CommentSchema.statics.all = () => Comment.find();
  * probably to be paginated at some point in the future
  * @return {Promise} array resolves to an array of comments by that user
  */
-CommentSchema.statics.findByUserId = function (author_id) {
-  return Comment.find({author_id});
+CommentSchema.statics.findByUserId = function (author_id, admin = false) {
+
+  // do not return un-published comments for non-admins
+  let query = {author_id};
+
+  if (!admin) {
+    query.$nor = [{status: 'premod'}, {status: 'rejected'}];
+  }
+
+  return Comment.find(query);
 };
 
 // Comment model.

models/user.js

@@ -81,7 +81,11 @@ const UserSchema = new mongoose.Schema({
 
   // This is sourced from the social provider or set manually during user setup
   // and simply provides a name to display for the given user.
-  displayName: String,
+  displayName: {
+    type: String,
+    unique: true,
+    required: true
+  },
 
   // This is true when the user account is disabled, no action should be
   // acknowledged when they are disabled. Logins are also prevented.
@@ -379,6 +383,9 @@ UserService.createLocalUser = (email, password, displayName) => {
           user.save((err) => {
             if (err) {
               if (err.code === 11000) {
+                if (err.message.match('displayName')) {
+                  return reject(errors.ErrDisplayTaken);
+                }
                 return reject(errors.ErrEmailTaken);
               }
               return reject(err);

routes/api/comments/index.js

@@ -48,7 +48,7 @@ router.get('/', (req, res, next) => {
   // otherwise this will be a vulnerability if you pass user_id and something else,
   // the app will return admin-level data without the proper checks
   if (user_id) {
-    query = Comment.findByUserId(user_id);
+    query = Comment.findByUserId(user_id, authorization.has(req.user, 'admin'));
   } else if (status) {
     query = assetIDWrap(Comment.findByStatus(status === 'new' ? null : status));
   } else if (action_type) {

routes/api/stream/index.js

@@ -48,20 +48,11 @@ router.get('/', (req, res, next) => {
       settings.merge(asset.settings);
     }
 
-    // Fetch the appropriate comments stream.
-    let comments;
-
-    if (settings.moderation === 'pre') {
-      comments = Comment.findAcceptedByAssetId(asset.id);
-    } else {
-      comments = Comment.findAcceptedAndNewByAssetId(asset.id);
-    }
-
     return Promise.all([
 
       // This is the promised component... Fetch the comments based on the
       // moderation settings.
-      comments,
+      Comment.findByAssetIdWithStatuses(asset.id, [null, 'accepted']),
 
       // Send back the reference to the asset.
       asset,

tests/models/comment.js

@@ -161,28 +161,6 @@ describe('models.Comment', () => {
         expect(result[2]).to.have.property('body', 'comment 40');
       });
     });
-
-    it('should find an array of accepted comments by asset id', () => {
-      return Comment.findAcceptedByAssetId('123').then((result) => {
-        expect(result).to.have.length(1);
-        result.sort((a, b) => {
-          if (a.body < b.body) {return -1;}
-          else {return 1;}
-        });
-        expect(result[0]).to.have.property('body', 'comment 20');
-      });
-    });
-
-    it('should find an array of new and accepted comments by asset id', () => {
-      return Comment.findAcceptedAndNewByAssetId('123').then((result) => {
-        expect(result).to.have.length(2);
-        result.sort((a, b) => {
-          if (a.body < b.body) {return -1;}
-          else {return 1;}
-        });
-        expect(result[0]).to.have.property('body', 'comment 10');
-      });
-    });
   });
 
   describe('#moderationQueue()', () => {
@@ -209,6 +187,23 @@ describe('models.Comment', () => {
     });
   });
 
+  describe('#findByUserId', () => {
+    it('should return all comments if admin', () => {
+      return Comment.findByUserId('456', true)
+        .then(comments => {
+          expect(comments).to.have.length(4);
+        });
+    });
+
+    it('should not return premod and rejected comments if not admin', () => {
+      return Comment.findByUserId('456')
+        .then(comments => {
+          expect(comments).to.have.length(1);
+        });
+    });
+
+  });
+
   describe('#changeStatus', () => {
 
     it('should change the status of a comment from no status', () => {

tests/models/user.js

@@ -85,6 +85,22 @@ describe('models.User', () => {
 
   });
 
+  describe('#createLocalUser', () => {
+    it('should not create a user with duplicate display name', () => {
+      return User.createLocalUsers([{
+        email: 'otrostampi@gmail.com',
+        displayName: 'Stampi',
+        password: '1Coralito!'
+      }])
+      .then((user) => {
+        expect(user).to.be.null;
+      })
+      .catch((error) => {
+        expect(error).to.not.be.null;
+      });
+    });
+  });
+
   describe('#createEmailConfirmToken', () => {
 
     it('should create a token for a valid user', () => {

tests/routes/api/comments/index.js

@@ -83,15 +83,14 @@ describe('/api/v1/comments', () => {
       ]);
     });
 
-    it('should return only the owner’s comments if the user is not an admin', () => {
+    it('should return only the owner’s published comments if the user is not an admin', () => {
       return chai.request(app)
         .get('/api/v1/comments?user_id=456')
         .set(passport.inject({id: '456', roles: []}))
         .then(res => {
           expect(res).to.have.status(200);
-          expect(res.body.comments).to.have.length(2);
+          expect(res.body.comments).to.have.length(1);
           expect(res.body.comments[0]).to.have.property('author_id', '456');
-          expect(res.body.comments[1]).to.have.property('author_id', '456');
         });
     });
 
@@ -192,6 +191,7 @@ describe('/api/v1/comments', () => {
         .then((res) => {
           expect(res).to.have.status(201);
           expect(res.body).to.have.property('id');
+          expect(res.body).to.have.property('status', 'premod');
         });
     });
 
@@ -214,6 +214,7 @@ describe('/api/v1/comments', () => {
           expect(res).to.have.status(201);
           expect(res.body).to.have.property('id');
           expect(res.body).to.have.property('status', null);
+
           return Promise.all([
             res.body,
             Action.findByType('flag', 'comments')
@@ -252,6 +253,27 @@ describe('/api/v1/comments', () => {
         });
     });
 
+    it('should create a comment with null status if it\'s asset is has post-moderation enabled', () => {
+      return Asset
+        .findOrCreateByUrl('https://coralproject.net/article1')
+        .then((asset) => {
+          return Asset
+            .overrideSettings(asset.id, {moderation: 'post'})
+            .then(() => asset);
+        })
+        .then((asset) => {
+          return chai.request(app).post('/api/v1/comments')
+            .set(passport.inject({roles: []}))
+            .send({'body': 'Something body.', 'author_id': '123', 'asset_id': asset.id, 'parent_id': ''});
+        })
+        .then((res) => {
+          expect(res).to.have.status(201);
+          expect(res.body).to.have.property('id');
+          expect(res.body).to.have.property('asset_id');
+          expect(res.body).to.have.property('status', null);
+        });
+    });
+
     it('should create a rejected comment if the body is above the character count', () => {
       return Asset
         .findOrCreateByUrl('https://coralproject.net/article1')

tests/routes/api/stream/index.js

@@ -24,6 +24,21 @@ describe('/api/v1/stream', () => {
       }
     };
 
+    const assets = [
+      {
+        url: 'https://example.com/article/1'
+      },
+      {
+        url: 'https://example.com/article/2',
+        settings: {
+          moderation: 'pre'
+        }
+      },
+      {
+        url: 'https://example.com/article/3'
+      }
+    ];
+
     const comments = [{
       id: 'abc',
       body: 'comment 10',
@@ -58,6 +73,28 @@ describe('/api/v1/stream', () => {
       status_history: [{
         type: 'rejected'
       }]
+    }, {
+      body: 'comment 50',
+      status: 'premod',
+      status_history: [{
+        type: 'premod'
+      }]
+    }, {
+      body: 'comment 60',
+      status: 'accepted',
+      status_history: [{
+        type: 'accepted'
+      }]
+    }, {
+      body: 'comment 70',
+      status: 'rejected',
+      status_history: [{
+        type: 'rejected'
+      }]
+    }, {
+      body: 'comment 70',
+      status: null,
+      status_history: []
     }];
 
     const users = [{
@@ -79,42 +116,46 @@ describe('/api/v1/stream', () => {
     }];
 
     beforeEach(() => {
-      return Setting.init(settings).then(() => {
-        return Promise.all([
-          User.createLocalUsers(users),
-          Asset.findOrCreateByUrl('http://test.com'),
-          Asset
-            .findOrCreateByUrl('http://coralproject.net/asset2')
-            .then((asset) => {
-              return Asset
-                .overrideSettings(asset.id, {moderation: 'pre'})
-                .then(() => asset);
-            })
-        ])
-        .then(([users, asset1, asset2]) => {
+      return Setting.init(settings)
+      .then(() => Promise.all([
+        User.createLocalUsers(users),
+        Promise.all(assets.map((asset) => Asset.create(asset)))
+      ]))
+      .then(([mockUsers, mockAssets]) => {
 
-          comments[0].author_id = users[0].id;
-          comments[1].author_id = users[1].id;
-          comments[2].author_id = users[0].id;
-          comments[3].author_id = users[1].id;
-
-          comments[0].asset_id = asset1.id;
-          comments[1].asset_id = asset1.id;
-          comments[2].asset_id = asset2.id;
-          comments[3].asset_id = asset2.id;
-
-          return Promise.all([
-            Comment.create(comments),
-            Action.create(actions)
-          ]);
+        // Map the id's over.
+        mockAssets.forEach((asset, i) => {
+          assets[i].id = asset.id;
         });
+
+        mockUsers.forEach((user, i) => {
+          users[i].id = user.id;
+        });
+
+        comments.forEach((comment, i) => {
+          comments[i].author_id = users[(i % 2) === 0 ? 0 : 1].id;
+        });
+
+        comments[0].asset_id = assets[0].id;
+        comments[1].asset_id = assets[0].id;
+        comments[2].asset_id = assets[1].id;
+        comments[3].asset_id = assets[1].id;
+        comments[4].asset_id = assets[2].id;
+        comments[5].asset_id = assets[2].id;
+        comments[6].asset_id = assets[2].id;
+        comments[7].asset_id = assets[2].id;
+
+        return Promise.all([
+          Comment.create(comments),
+          Action.create(actions)
+        ]);
       });
     });
 
     it('should return a stream with comments, users and actions for an existing asset', () => {
       return chai.request(app)
         .get('/api/v1/stream')
-        .query({'asset_url': 'http://test.com'})
+        .query({asset_url: assets[0].url})
         .then(res => {
           expect(res).to.have.status(200);
           expect(res.body.assets.length).to.equal(1);
@@ -138,15 +179,47 @@ describe('/api/v1/stream', () => {
     it('should merge the settings when the asset contains settings to override it with', () => {
       return chai.request(app)
         .get('/api/v1/stream')
-        .query({'asset_url': 'http://coralproject.net/asset2'})
+        .query({asset_url: assets[1].url})
         .then((res) => {
           expect(res).to.have.status(200);
-          expect(res.body.assets.length).to.equal(1);
-          expect(res.body.comments.length).to.equal(1);
-          expect(res.body.users.length).to.equal(1);
+          expect(res.body.assets).to.have.length(1);
+          expect(res.body.comments).to.have.length(1);
+          expect(res.body.users).to.have.length(1);
           expect(res.body.settings).to.have.property('moderation', 'pre');
           expect(res.body.settings).to.not.have.property('wordlist');
         });
     });
+
+    it('should not change the previously displayed comments based on moderation state changes', () => {
+
+      let preComments, postComments;
+
+      return chai.request(app)
+        .get('/api/v1/stream')
+        .query({asset_url: assets[2].url})
+        .then((res) => {
+          expect(res).to.have.status(200);
+          expect(res.body.comments.length).to.equal(2);
+          expect(res.body.settings).to.have.property('moderation', 'post');
+
+          preComments = res.body.comments;
+
+          return Asset.overrideSettings(assets[2].id, {moderation: 'pre'});
+        })
+        .then(() => {
+          return chai.request(app)
+            .get('/api/v1/stream')
+            .query({asset_url: assets[2].url});
+        })
+        .then((res) => {
+          expect(res).to.have.status(200);
+          expect(res.body.comments.length).to.equal(2);
+          expect(res.body.settings).to.have.property('moderation', 'pre');
+
+          postComments = res.body.comments;
+
+          expect(preComments).to.deep.equal(postComments);
+        });
+    });
   });
 });

util.js

@@ -37,6 +37,8 @@ util.onshutdown = (jobs) => {
 };
 
 // Attach to the SIGTERM + SIGINT handles to ensure a clean shutdown in the
-// event that we have an external event.
-process.on('SIGTERM', () => util.shutdown());
-process.on('SIGINT',  () => util.shutdown());
+// event that we have an external event. SIGUSR2 is called when the app is asked
+// to be 'killed', same procedure here.
+process.on('SIGTERM',   () => util.shutdown());
+process.on('SIGINT',    () => util.shutdown());
+process.once('SIGUSR2', () => util.shutdown());