Models: Replace before.insert with autoValues

The before.insert hooks have the problem, that they are executed in a different order if called from the client or from the server. If called from the client, the before.insert hook is called before validation of the schema, but if called from the server, the validation is called first and fails.
9 years ago · a2888250f4
parent 14e2b3c15f
commit a2888250f4
6 changed files with 129 additions and 75 deletions
--- a/client/components/cards/labels.js
+++ b/client/components/cards/labels.js
@ -9,9 +9,7 @@ BlazeComponent.extendComponent({
  },

  labels() {
-    return labelColors.map((color) => {
-      return { color, name: '' };
-    });
+    return labelColors.map((color) => ({ color, name: '' }));
  },

  isSelected(color) {
--- a/models/boards.js
+++ b/models/boards.js
@ -6,25 +6,77 @@ Boards.attachSchema(new SimpleSchema({
  },
  slug: {
    type: String,
+    autoValue() { // eslint-disable-line consistent-return
+      // XXX We need to improve slug management. Only the id should be necessary
+      // to identify a board in the code.
+      // XXX If the board title is updated, the slug should also be updated.
+      // In some cases (Chinese and Japanese for instance) the `getSlug` function
+      // return an empty string. This is causes bugs in our application so we set
+      // a default slug in this case.
+      if (this.isInsert && !this.isSet) {
+        let slug = 'board';
+        const title = this.field('title');
+        if (title.isSet) {
+          slug = getSlug(title.value) || slug;
+        }
+        return slug;
+      }
+    },
  },
  archived: {
    type: Boolean,
+    autoValue() { // eslint-disable-line consistent-return
+      if (this.isInsert && !this.isSet) {
+        return false;
+      }
+    },
  },
  createdAt: {
    type: Date,
-    denyUpdate: true,
+    autoValue() { // eslint-disable-line consistent-return
+      if (this.isInsert) {
+        return new Date();
+      } else {
+        this.unset();
+      }
+    },
  },
  // XXX Inconsistent field naming
  modifiedAt: {
    type: Date,
-    denyInsert: true,
    optional: true,
+    autoValue() { // eslint-disable-line consistent-return
+      if (this.isUpdate) {
+        return new Date();
+      } else {
+        this.unset();
+      }
+    },
  },
  // De-normalized number of users that have starred this board
  stars: {
    type: Number,
+    autoValue() { // eslint-disable-line consistent-return
+      if (this.isInsert) {
+        return 0;
+      }
+    },
  },
  // De-normalized label system
+  'labels': {
+    type: [Object],
+    autoValue() { // eslint-disable-line consistent-return
+      if (this.isInsert && !this.isSet) {
+        const colors = Boards.simpleSchema()._schema['labels.$.color'].allowedValues;
+        const defaultLabelsColors = _.clone(colors).splice(0, 6);
+        return defaultLabelsColors.map((color) => ({
+          color,
+          _id: Random.id(6),
+          name: '',
+        }));
+      }
+    },
+  },
  'labels.$._id': {
    // We don't specify that this field must be unique in the board because that
    // will cause performance penalties and is not necessary since this field is
@ -47,6 +99,19 @@ Boards.attachSchema(new SimpleSchema({
  // XXX We might want to maintain more informations under the member sub-
  // documents like de-normalized meta-data (the date the member joined the
  // board, the number of contributions, etc.).
+  'members': {
+    type: [Object],
+    autoValue() { // eslint-disable-line consistent-return
+      if (this.isInsert && !this.isSet) {
+        return [{
+          userId: this.userId,
+          isAdmin: true,
+          isActive: true,
+          isInvited: false,
+        }];
+      }
+    },
+  },
  'members.$.userId': {
    type: String,
  },
@ -70,6 +135,11 @@ Boards.attachSchema(new SimpleSchema({
      'wisteria',
      'midnight',
    ],
+    autoValue() { // eslint-disable-line consistent-return
+      if (this.isInsert && !this.isSet) {
+        return Boards.simpleSchema()._schema.color.allowedValues[0];
+      }
+    },
  },
  description: {
    type: String,
@ -338,41 +408,6 @@ if (Meteor.isServer) {
  });
 }

-Boards.before.insert((userId, doc) => {
-  // XXX We need to improve slug management. Only the id should be necessary
-  // to identify a board in the code.
-  // XXX If the board title is updated, the slug should also be updated.
-  // In some cases (Chinese and Japanese for instance) the `getSlug` function
-  // return an empty string. This is causes bugs in our application so we set
-  // a default slug in this case.
-  doc.slug = doc.slug || getSlug(doc.title) || 'board';
-  doc.createdAt = new Date();
-  doc.archived = false;
-  doc.members = doc.members || [{
-    userId,
-    isAdmin: true,
-    isActive: true,
-  }];
-  doc.stars = 0;
-  doc.color = Boards.simpleSchema()._schema.color.allowedValues[0];
-
-  // Handle labels
-  const colors = Boards.simpleSchema()._schema['labels.$.color'].allowedValues;
-  const defaultLabelsColors = _.clone(colors).splice(0, 6);
-  doc.labels = defaultLabelsColors.map((color) => {
-    return {
-      color,
-      _id: Random.id(6),
-      name: '',
-    };
-  });
-});
-
-Boards.before.update((userId, doc, fieldNames, modifier) => {
-  modifier.$set = modifier.$set || {};
-  modifier.$set.modifiedAt = new Date();
-});
-
 if (Meteor.isServer) {
  // Let MongoDB ensure that a member is not included twice in the same board
  Meteor.startup(() => {
--- a/models/cardComments.js
+++ b/models/cardComments.js
@ -16,10 +16,22 @@ CardComments.attachSchema(new SimpleSchema({
  createdAt: {
    type: Date,
    denyUpdate: false,
+    autoValue() { // eslint-disable-line consistent-return
+      if (this.isInsert) {
+        return new Date();
+      } else {
+        this.unset();
+      }
+    },
  },
  // XXX Should probably be called `authorId`
  userId: {
    type: String,
+    autoValue() { // eslint-disable-line consistent-return
+      if (this.isInsert && !this.isSet) {
+        return this.userId;
+      }
+    },
  },
 }));

@ -44,11 +56,6 @@ CardComments.helpers({

 CardComments.hookOptions.after.update = { fetchPrevious: false };

-CardComments.before.insert((userId, doc) => {
-  doc.createdAt = new Date();
-  doc.userId = userId;
-});
-
 if (Meteor.isServer) {
  CardComments.after.insert((userId, doc) => {
    Activities.insert({
--- a/models/cards.js
+++ b/models/cards.js
@ -9,6 +9,11 @@ Cards.attachSchema(new SimpleSchema({
  },
  archived: {
    type: Boolean,
+    autoValue() { // eslint-disable-line consistent-return
+      if (this.isInsert && !this.isSet) {
+        return false;
+      }
+    },
  },
  listId: {
    type: String,
@ -25,10 +30,19 @@ Cards.attachSchema(new SimpleSchema({
  },
  createdAt: {
    type: Date,
-    denyUpdate: true,
+    autoValue() { // eslint-disable-line consistent-return
+      if (this.isInsert) {
+        return new Date();
+      } else {
+        this.unset();
+      }
+    },
  },
  dateLastActivity: {
    type: Date,
+    autoValue() {
+      return new Date();
+    },
  },
  description: {
    type: String,
@ -46,6 +60,11 @@ Cards.attachSchema(new SimpleSchema({
  // the `members` field?
  userId: {
    type: String,
+    autoValue() { // eslint-disable-line consistent-return
+      if (this.isInsert && !this.isSet) {
+        return this.userId;
+      }
+    },
  },
  sort: {
    type: Number,
@ -190,17 +209,6 @@ Cards.mutations({
  },
 });

-Cards.before.insert((userId, doc) => {
-  doc.createdAt = new Date();
-  doc.dateLastActivity = new Date();
-  if(!doc.hasOwnProperty('archived')){
-    doc.archived = false;
-  }
-  if (!doc.userId) {
-    doc.userId = userId;
-  }
-});
-
 if (Meteor.isServer) {
  Cards.after.insert((userId, doc) => {
    Activities.insert({
--- a/models/lists.js
+++ b/models/lists.js
@ -6,13 +6,24 @@ Lists.attachSchema(new SimpleSchema({
  },
  archived: {
    type: Boolean,
+    autoValue() { // eslint-disable-line consistent-return
+      if (this.isInsert && !this.isSet) {
+        return false;
+      }
+    },
  },
  boardId: {
    type: String,
  },
  createdAt: {
    type: Date,
-    denyUpdate: true,
+    autoValue() { // eslint-disable-line consistent-return
+      if (this.isInsert) {
+        return new Date();
+      } else {
+        this.unset();
+      }
+    },
  },
  sort: {
    type: Number,
@ -22,8 +33,14 @@ Lists.attachSchema(new SimpleSchema({
  },
  updatedAt: {
    type: Date,
-    denyInsert: true,
    optional: true,
+    autoValue() { // eslint-disable-line consistent-return
+      if (this.isUpdate) {
+        return new Date();
+      } else {
+        this.unset();
+      }
+    },
  },
 }));

@ -73,18 +90,6 @@ Lists.mutations({

 Lists.hookOptions.after.update = { fetchPrevious: false };

-Lists.before.insert((userId, doc) => {
-  doc.createdAt = new Date();
-  doc.archived = false;
-  if (!doc.userId)
-    doc.userId = userId;
-});
-
-Lists.before.update((userId, doc, fieldNames, modifier) => {
-  modifier.$set = modifier.$set || {};
-  modifier.$set.modifiedAt = new Date();
-});
-
 if (Meteor.isServer) {
  Lists.after.insert((userId, doc) => {
    Activities.insert({
--- a/models/unsavedEdits.js
+++ b/models/unsavedEdits.js
@ -14,6 +14,11 @@ UnsavedEditCollection.attachSchema(new SimpleSchema({
  },
  userId: {
    type: String,
+    autoValue() { // eslint-disable-line consistent-return
+      if (this.isInsert && !this.isSet) {
+        return this.userId;
+      }
+    },
  },
 }));

@ -28,7 +33,3 @@ if (Meteor.isServer) {
    fetch: ['userId'],
  });
 }
-
-UnsavedEditCollection.before.insert((userId, doc) => {
-  doc.userId = userId;
-});