Allow a user to change their own username

models/action.js

@@ -22,8 +22,8 @@ const ActionSchema = new Schema(
     item_id: String,
     user_id: String,
 
-    // The element that summaries will additionally group on in addtion to their action_type, item_type, and
-    // item_id.
+    // The element that summaries will additionally group on in addition to
+    // their action_type, item_type, and item_id.
     group_id: String,
 
     // Additional metadata stored on the field.

perms/reducers/mutation.js

@@ -1,4 +1,5 @@
-const { isString } = require('lodash');
+const { get, isString } = require('lodash');
+const moment = require('moment');
 const { check } = require('../utils');
 const types = require('../constants');
 
@@ -12,8 +13,22 @@ module.exports = (user, perm) => {
         isString(user.password) &&
         user.password.length > 0
       );
-    case types.CHANGE_USERNAME:
-      return user.status.username.status === 'REJECTED';
+
+    case types.CHANGE_USERNAME: {
+      // Only users who have their usernames rejected or those users who
+      // not changed their usernames within 14 days can change their usernames.
+      const now = moment();
+      return (
+        user.status.username.status === 'REJECTED' ||
+        get(user, 'status.username.history', [])
+          .filter(({ status }) => status === 'CHANGED')
+          .every(({ created_at }) =>
+            moment(created_at)
+              .add(14, 'days')
+              .isAfter(now)
+          )
+      );
+    }
 
     case types.SET_USERNAME:
       return user.status.username.status === 'UNSET';

services/users.js

@@ -1,4 +1,5 @@
 const uuid = require('uuid');
+const moment = require('moment');
 const bcrypt = require('bcryptjs');
 const {
   ErrMaxRateLimit,
@@ -234,57 +235,64 @@ class Users {
     return user;
   }
 
-  static async _setUsername(
-    id,
-    username,
-    fromStatus,
-    toStatus,
-    assignedBy,
-    resetAllowed = false
-  ) {
+  static async setUsername(id, username, assignedBy) {
     try {
+      const oldestEditTime = moment().subtract(14, 'days');
+
+      // A username can be set if:
+      //
+      // - The previous status was 'UNSET'
+      // - The username has not been changed within the last 14 days.
       const query = {
         id,
-        'status.username.status': fromStatus,
-      };
-      if (!resetAllowed) {
-        query.username = { $ne: username };
-      }
-
-      let user = await User.findOneAndUpdate(
-        query,
-        {
-          $set: {
-            username,
-            lowercaseUsername: username.toLowerCase(),
-            'status.username.status': toStatus,
+        $or: [
+          {
+            'status.username.status': 'UNSET',
           },
-          $push: {
-            'status.username.history': {
-              status: toStatus,
-              assigned_by: assignedBy,
-              created_at: Date.now(),
+          {
+            'status.username.status': { $in: ['APPROVED', 'SET'] },
+            'status.username.history.created_at': {
+              $not: {
+                $gte: oldestEditTime,
+              },
             },
           },
+        ],
+      };
+
+      const update = {
+        $set: {
+          username,
+          lowercaseUsername: username.toLowerCase(),
+          'status.username.status': 'SET',
         },
-        {
-          new: true,
-        }
-      );
+        $push: {
+          'status.username.history': {
+            status: 'SET',
+            assigned_by: assignedBy,
+            created_at: Date.now(),
+          },
+        },
+      };
+
+      let user = await User.findOneAndUpdate(query, update, {
+        new: true,
+      });
       if (!user) {
         user = await Users.findById(id);
         if (user === null) {
           throw new ErrNotFound();
         }
 
-        if (user.status.username.status !== fromStatus) {
+        if (
+          !['UNSET', 'APPROVED', 'SET'].includes(user.status.username.status) ||
+          !user.status.username.history.every(({ created_at }) =>
+            oldestEditTime.isAfter(created_at)
+          )
+        ) {
           throw new ErrPermissionUpdateUsername();
         }
 
-        if (!resetAllowed && user.username === username) {
-          throw new ErrSameUsernameProvided();
-        }
-
         throw new Error('edit username failed for an unexpected reason');
       }
 
@@ -298,12 +306,57 @@ class Users {
     }
   }
 
-  static async setUsername(id, username, assignedBy) {
-    return Users._setUsername(id, username, 'UNSET', 'SET', assignedBy, true);
-  }
-
   static async changeUsername(id, username, assignedBy) {
-    return Users._setUsername(id, username, 'REJECTED', 'CHANGED', assignedBy);
+    try {
+      const query = {
+        id,
+        username: { $ne: username },
+        'status.username.status': 'REJECTED',
+      };
+
+      const update = {
+        $set: {
+          username,
+          lowercaseUsername: username.toLowerCase(),
+          'status.username.status': 'CHANGED',
+        },
+        $push: {
+          'status.username.history': {
+            status: 'CHANGED',
+            assigned_by: assignedBy,
+            created_at: Date.now(),
+          },
+        },
+      };
+
+      let user = await User.findOneAndUpdate(query, update, {
+        new: true,
+      });
+      if (!user) {
+        user = await Users.findById(id);
+        if (user === null) {
+          throw new ErrNotFound();
+        }
+
+        if (user.status.username.status !== 'REJECTED') {
+          throw new ErrPermissionUpdateUsername();
+        }
+
+        if (user.username === username) {
+          throw new ErrSameUsernameProvided();
+        }
+
+        throw new Error('edit username failed for an unexpected reason');
+      }
+
+      return user;
+    } catch (err) {
+      if (err.code === 11000) {
+        throw new ErrUsernameTaken();
+      }
+
+      throw err;
+    }
   }
 
   /**

test/server/graph/mutations/changeUsername.js

@@ -89,7 +89,7 @@ describe('graph.mutations.changeUsername', () => {
       expect(res.data.changeUsername.errors).to.have.length(1);
       expect(res.data.changeUsername.errors[0]).to.have.property(
         'translation_key',
-        'NOT_AUTHORIZED'
+        'EDIT_USERNAME_NOT_AUTHORIZED'
       );
 
       // Set the user to the desired status.

test/server/services/users.js

@@ -2,6 +2,8 @@ const UsersService = require('../../../services/users');
 const SettingsService = require('../../../services/settings');
 const mailer = require('../../../services/mailer');
 const Context = require('../../../graph/context');
+const timekeeper = require('timekeeper');
+const moment = require('moment');
 
 const chai = require('chai');
 chai.use(require('chai-as-promised'));
@@ -303,6 +305,62 @@ describe('services.UsersService', () => {
         }
       });
     });
+
+    if (func === 'setUsername') {
+      it('should let a user set their username from UNSET', async () => {
+        const user = mockUsers[0];
+
+        // Set the user to the desired status.
+        await UsersService.setUsernameStatus(user.id, 'UNSET');
+        await UsersService.setUsername(user.id, 'new_username', null);
+      });
+
+      describe('time based', () => {
+        afterEach(() => {
+          timekeeper.reset();
+        });
+
+        ['SET', 'APPROVED'].forEach(status => {
+          it(`should not allow users to change their username if it was changed within 14 of today from ${status}`, async () => {
+            const user = mockUsers[0];
+
+            // Set the user to the desired status.
+            await UsersService.setUsernameStatus(user.id, status);
+
+            timekeeper.travel(
+              moment()
+                .add(5, 'days')
+                .toDate()
+            );
+
+            try {
+              await UsersService.setUsername(user.id, 'new_username', null);
+              throw new Error('edit was processed successfully');
+            } catch (err) {
+              expect(err).have.property(
+                'translation_key',
+                'EDIT_USERNAME_NOT_AUTHORIZED'
+              );
+            }
+          });
+
+          it(`allows users to change their username if it was changed 14 days before today from ${status}`, async () => {
+            const user = mockUsers[0];
+
+            // Set the user to the desired status.
+            await UsersService.setUsernameStatus(user.id, status);
+
+            timekeeper.travel(
+              moment()
+                .add(15, 'days')
+                .toDate()
+            );
+
+            await UsersService.setUsername(user.id, 'new_username', null);
+          });
+        });
+      });
+    }
   });
 
   describe('#isValidUsername', () => {