From a2888250f4ab3615c1590ab3bbf90302963a7c57 Mon Sep 17 00:00:00 2001 From: Alexander Sulfrian Date: Fri, 22 Apr 2016 00:59:05 +0200 Subject: 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. --- client/components/cards/labels.js | 4 +- models/boards.js | 109 +++++++++++++++++++++++++------------- models/cardComments.js | 17 ++++-- models/cards.js | 32 ++++++----- models/lists.js | 33 +++++++----- models/unsavedEdits.js | 9 ++-- 6 files changed, 129 insertions(+), 75 deletions(-) diff --git a/client/components/cards/labels.js b/client/components/cards/labels.js index 20d95bc6..cdd5a700 100644 --- 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) { diff --git a/models/boards.js b/models/boards.js index 52272cce..715bb838 100644 --- 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(() => { diff --git a/models/cardComments.js b/models/cardComments.js index 224deb03..ce6edf3c 100644 --- 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({ diff --git a/models/cards.js b/models/cards.js index 09c86191..aa19a64a 100644 --- 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({ diff --git a/models/lists.js b/models/lists.js index 4e4a1134..a4938f67 100644 --- 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({ diff --git a/models/unsavedEdits.js b/models/unsavedEdits.js index 87a70e22..25952fb5 100644 --- 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; -}); -- cgit v1.2.3-1-g7c22