fix: Removes references to organisations after deletion (LLC-26) (#1511)

Author: PrinceWaune

lib/models/organisation-test.js

@@ -2,10 +2,73 @@ import chai, { expect } from 'chai';
 import chaiAsPromised from 'chai-as-promised';
 import { Error } from 'mongoose';
 import Organisation from './organisation';
+import User from './user';
+import Role from './role';
+import { createOrganisation, createUser } from './test-utils/testModelsHelper';
+import { userFixture } from './test-utils/fixtures/user.fixtures';
+import { orgFixture } from './test-utils/fixtures/organisation.fixtures';
 
 chai.use(chaiAsPromised);
 
 describe('organisation model', () => {
+  afterEach(async () => {
+    await Role.deleteMany({});
+    await Organisation.deleteMany({});
+    await User.deleteMany({});
+  });
+
+  it('should properly create organisation', async () => {
+    const user = await createUser(userFixture);
+    const organisation = await createOrganisation(orgFixture);
+
+    const userAfterOrganisationCreated = await User.findOne({
+      email: user.email
+    });
+
+    const createdUsersOrganisation = userAfterOrganisationCreated
+      .organisations
+      .find(id => id.toString() === organisation._id.toString());
+    const createdUsersOrganisationSetting = userAfterOrganisationCreated
+      .organisationSettings
+      .find(orgSetting =>
+        orgSetting.organisation.toString() === organisation._id.toString()
+      );
+    const createdUsersRoles = await Role.find({
+      organisation: organisation._id
+    });
+
+    await expect(createdUsersOrganisation).to.not.be.undefined;
+    await expect(createdUsersOrganisationSetting).to.not.be.undefined;
+    await expect(createdUsersRoles).to.be.an('array').to.have.lengthOf(2);
+  });
+
+  it('should properly delete organisation', async () => {
+    const user = await createUser(userFixture);
+    const organisation = await createOrganisation(orgFixture);
+
+    await organisation.remove();
+
+    const userAfterOrganisationDeleted = await User.findOne({
+      email: user.email
+    });
+
+    const deletedUsersOrganisation = userAfterOrganisationDeleted
+      .organisations
+      .find(id => id.toString() === organisation._id.toString());
+    const deletedUsersOrganisationSetting = userAfterOrganisationDeleted
+      .organisationSettings
+      .find(orgSetting =>
+        orgSetting.organisation.toString() === organisation._id.toString()
+      );
+    const deletedUsersRoles = await Role.find({
+      organisation: organisation._id
+    });
+
+    await expect(deletedUsersRoles).to.be.empty;
+    await expect(deletedUsersOrganisation).to.be.undefined;
+    await expect(deletedUsersOrganisationSetting).to.be.undefined;
+  });
+
   it('should not throw a ValidationError when settings.LOCKOUT_ATTEMPS is larger than 0', async () => {
     const f = Organisation.create({
       settings: {

lib/models/organisation.js

@@ -9,7 +9,6 @@ import filterByOrg from 'lib/models/plugins/filterByOrg';
 import keys from 'lodash/keys';
 import union from 'lodash/union';
 import map from 'lodash/map';
-import find from 'lodash/find';
 import isUndefined from 'lodash/isUndefined';
 import {
   VIEW_PUBLIC_DASHBOARDS,
@@ -22,8 +21,62 @@ import { update$dteTimezoneInDB } from 'lib/helpers/update$dteTimezoneInDB';
 
 let Organisation;
 
+/**
+ * @typedef {object} ExpirationNotifications
+ *  @property {string} weekBeforeNotificationSent
+ *  @property {string} expirationNotificationSent
+ */
+
+/**
+ * @typedef {object} UsageStats
+ *  @property {Date} RUN_DATETIME
+ *  @property {Boolean} HAS_CHILDREN
+ *  @property {Number} OWN_COUNT
+ *  @property {Number} TOTAL_COUNT
+ *  @property {Number} OWN_ESTIMATED_BYTES
+ *  @property {Number} TOTAL_ESTIMATED_BYTES
+ */
+
+/**
+ * @typedef {object} Settings
+ *  @property {Boolean} LOCKOUT_ENABLED
+ *  @property {Number} LOCKOUT_ATTEMPS
+ *  @property {Number} LOCKOUT_SECONDS
+ *  @property {Boolean} PASSWORD_HISTORY_CHECK
+ *  @property {Number} PASSWORD_HISTORY_TOTAL
+ *  @property {Number} PASSWORD_MIN_LENGTH
+ *  @property {Boolean} PASSWORD_REQUIRE_ALPHA
+ *  @property {Boolean} PASSWORD_REQUIRE_NUMBER
+ *  @property {Boolean} PASSWORD_USE_CUSTOM_REGEX
+ *  @property {String} PASSWORD_CUSTOM_REGEX
+ *  @property {String} PASSWORD_CUSTOM_MESSAGE
+ */
+
+/**
+ * Plain object structure without mongoose model methods
+ *
+ * @typedef {object} Organisation
+ *  @property {string} name
+ *  @property {string} logoPath
+ *  @property {UploadSchema} logo TODO: define type
+ *  @property {string} customColors
+ *  @property {Organisation} parent
+ *  @property {*} owner TODO: define type
+ *  @property {Date} expiration
+ *  @property {ExpirationNotifications} expirationNotifications
+ *  @property {UsageStats} usageStats
+ *  @property {Settings} settings
+ *  @property {string} timezone
+ */
+
+/** @typedef {module:mongoose.Model<Organisation>} OrganisationModel */
+
+export const EMAIL_NOOP = 'EMAIL_NOOP';
+export const EMAIL_PROCESSING = 'EMAIL_PROCESSING';
+export const EMAIL_SENT = 'EMAIL_SENT';
+export const EMAIL_STATUS = [EMAIL_NOOP, EMAIL_PROCESSING, EMAIL_SENT];
+
 async function validateLockoutAttempt(value) {
-  // only validate field if lockout is enabled
   if (this.settings && this.settings.LOCKOUT_ENABLED) {
     if (value < 1) {
       throw new Error('A user should be allowed at least one attempt');
@@ -33,7 +86,6 @@ async function validateLockoutAttempt(value) {
 }
 
 async function validateLockoutSeconds(value) {
-  // only validate field if lockout is enabled
   if (this.settings && this.settings.LOCKOUT_ENABLED) {
     if (value < 5) {
       throw new Error('Must be at least 5 seconds');
@@ -43,7 +95,6 @@ async function validateLockoutSeconds(value) {
 }
 
 async function validateHistoryTotal(value) {
-  // only validate field if lockout is enabled
   if (this.settings && this.settings.LOCKOUT_ENABLED) {
     if (value < 1) {
       throw new Error('At least one password must be stored and checked with this setting enabled');
@@ -71,12 +122,6 @@ async function validateMinPasswordLength(value) {
   return true;
 }
 
-export const EMAIL_NOOP = 'EMAIL_NOOP';
-export const EMAIL_PROCESSING = 'EMAIL_PROCESSING';
-export const EMAIL_SENT = 'EMAIL_SENT';
-
-export const EMAIL_STATUS = [EMAIL_NOOP, EMAIL_PROCESSING, EMAIL_SENT];
-
 const schema = new mongoose.Schema({
   name: { type: String },
   logoPath: { type: String },
@@ -105,15 +150,13 @@ const schema = new mongoose.Schema({
   },
   settings: {
     LOCKOUT_ENABLED: { type: Boolean, default: true },
-    // number of attempts before lock out
     LOCKOUT_ATTEMPS: {
       type: Number,
       default: 5,
       validate: {
         validator: validateLockoutAttempt,
       },
     },
-    // 30 minute lock out period
     LOCKOUT_SECONDS: {
       type: Number,
       default: 1800,
@@ -176,15 +219,20 @@ schema.plugin(auditRemove, {
   auditName: 'OrganisationAudit'
 });
 
+/**
+ * @param req
+ * @param res
+ * @param next
+ */
 schema.statics.readScopeChecks = function runReadScopeChecks(req, res, next) {
   // we should always be allowed to read orgs
   return next();
 };
 
 /**
  * Returns an array of org ids that the given client can view limited to the level
- * @param  {Object}   orgIds ids to start with
- * @param  {Object}   totalIds ids found so far
+ * @param  {Object}   stepIds ids to start with
+ * @param  {Object}   cumulativeIds ids found so far
  * @param  {Number}   level  How many levels deep to look for children
  *                           - default 0 (you can see your orgs)
  * @param  {Function} cb     Callback to be called with the result
@@ -210,108 +258,102 @@ schema.statics.limitIdsByOrg = function limitIdsByOrg(
   }
 };
 
-schema.post('init', function handleInit() {
-  this._original = this.toObject();
-});
-
-schema.pre('save', async function preSave(next) {
-  const User = getConnection().model('User');
+const createRoles = async (orgModel) => {
   const Role = getConnection().model('Role');
 
-  // https://github.com/Automattic/mongoose/issues/1474
-  this.wasNew = this.isNew;
+  await Role.create({
+    title: 'Observer',
+    owner_id: orgModel.owner,
+    organisation: orgModel._id,
+    scopes: [
+      VIEW_PUBLIC_DASHBOARDS,
+      VIEW_PUBLIC_VISUALISATIONS,
+      VIEW_PUBLIC_JOURNEYS
+    ]
+  });
+
+  return await Role.create({
+    title: 'Admin',
+    owner_id: orgModel.owner,
+    organisation: orgModel._id,
+    scopes: [ALL]
+  });
+};
 
-  const organisation = this;
-  const objectId = mongoose.Types.ObjectId;
+const updateOwner = async (orgModel, User, adminRole) => {
+  const owner = await User.findById(orgModel.owner);
 
-  if (!isUndefined(this._oldExpiration) && this.expiration !== this._oldExpiration) {
-    this.expirationNotifications.expirationNotificationSent = EMAIL_NOOP;
-    this.expirationNotifications.weekBeforeNotificationSent = EMAIL_NOOP;
+  if (!owner) {
+    return;
   }
 
-  // https://github.com/Automattic/mongoose/issues/1474
-  if (organisation.wasNew) {
-    const createAdminRole = await Role.create({
-      title: 'Admin',
-      owner_id: organisation.owner,
-      organisation: organisation._id,
-      scopes: [ALL]
-    });
-
-    const createObserverRole = await Role.create({
-      title: 'Observer',
-      owner_id: organisation.owner,
-      organisation: organisation._id,
-      scopes: [
-        VIEW_PUBLIC_DASHBOARDS,
-        VIEW_PUBLIC_VISUALISATIONS,
-        VIEW_PUBLIC_JOURNEYS
-      ]
-    });
+  owner.organisations = union(owner.organisations || [], [orgModel._id]);
+  owner.organisationSettings = union(
+    owner.organisationSettings || [],
+    [{
+      organisation: orgModel._id,
+      roles: [adminRole._id],
+    }]
+  );
 
-    const owner = await User.findById(objectId(organisation.owner));
-    if (owner) {
-      // add this org to the creator's allowed orgs
-      const newRoles = [createAdminRole._id];
+  await owner.save();
+};
 
-      const existingOrganisationSettings = owner.organisationSettings || [];
-      let updatedSettings = existingOrganisationSettings;
+const createDependencies = async (orgModel, User) => {
+  const adminRole = await createRoles(orgModel);
+  await updateOwner(orgModel, User, adminRole);
+};
 
-      // check if the user has settings for this org
-      const existingOS = find(existingOrganisationSettings, setting =>
-         setting.organisation.toString() === organisation._id.toString()
-      );
+const removeDependencies = async (orgModel) => {
+  const Role = getConnection().model('Role');
+  const User = getConnection().model('User');
 
-      if (existingOS) {
-        const updatedOS = existingOS;
-        // if the setting already exists, update it
-        if (updatedOS.roles) {
-          // union the roles if exist
-          updatedOS.roles = union(updatedOS.roles, newRoles);
-        } else {
-          // set the new roles if no previous roles exist
-          updatedOS.roles = newRoles;
-        }
-
-        // loop through to create the updated settings array
-        updatedSettings = map(updatedSettings, (os) => {
-          if (os.organisation.toString() === organisation._id.toString()) {
-            // return the updated settings into the array
-            return updatedOS;
-          }
-        });
-      } else {
-        // insert a new item
-        updatedSettings.push({
-          organisation: organisation._id,
-          roles: newRoles
-        });
+  await Role.deleteMany({ organisation: [orgModel._id] });
+  await User.updateMany(
+    {},
+    {
+      $pull: {
+        organisations: orgModel._id,
+        organisationSettings: { organisation: orgModel._id },
       }
+    }
+  );
+};
 
-      // update the settings on the owner
-      owner.organisations = union(owner.organisations, [organisation._id]);
-      owner.organisationSettings = updatedSettings;
 
-      await owner.save();
-    }
+schema.post('init', function handleInit() {
+  this._original = this.toObject();
+});
+
+schema.pre('save', async function preSave(next) {
+  const User = getConnection().model('User');
+
+  if (!isUndefined(this._oldExpiration) && this.expiration !== this._oldExpiration) {
+    this.expirationNotifications.expirationNotificationSent = EMAIL_NOOP;
+    this.expirationNotifications.weekBeforeNotificationSent = EMAIL_NOOP;
+  }
 
-    await Promise.all([createAdminRole, createObserverRole]);
+  if (!isUndefined(this._oldTimezone) && this.timezone !== this._oldTimezone) {
+    await update$dteTimezoneInDB(this._id, this.timezone);
   }
 
-  // Update users settings
   await User.updateMany(
-    { ownerOrganisation: organisation._id },
+    { ownerOrganisation: this._id },
     { ownerOrganisationSettings: this.settings },
     { runValidators: false },
   );
 
-  // Update timezone offset of queries
-  if (!isUndefined(this._oldTimezone) && this.timezone !== this._oldTimezone) {
-    await update$dteTimezoneInDB(organisation._id, this.timezone);
+  if (this.isNew) {
+    await createDependencies(this, User);
   }
 
   next();
 });
 
+schema.post('remove', async (orgModel, next) => {
+  await removeDependencies(orgModel);
+  next();
+});
+
 Organisation = getConnection().model('Organisation', schema, 'organisations');
 export default Organisation;