Instead of storing todo.userId, use publish composite to control th…

.meteor/versions

@@ -93,6 +93,7 @@ reactive-dict@1.1.3
 reactive-var@1.0.6
 reload@1.1.4
 retry@1.0.4
+reywood:publish-composite@1.4.2
 routepolicy@1.0.6
 service-configuration@1.0.5
 session@1.1.1

packages/lists/lists-tests.js

@@ -75,13 +75,12 @@ describe('lists', () => {
       function assertListAndTodoArePrivate() {
         assert.equal(Lists.findOne(listId).userId, userId);
         assert.isTrue(Lists.findOne(listId).isPrivate());
-        assert.equal(Todos.findOne(todoId).userId, userId);
         assert.isTrue(Todos.findOne(todoId).editableBy(userId));
+        assert.isFalse(Todos.findOne(todoId).editableBy(Random.id()));
       }
 
       it('makes a list private and updates the todos', () => {
         // Check initial state is public
-        assert.isUndefined(Todos.findOne(todoId).userId);
         assert.isFalse(Lists.findOne(listId).isPrivate());
 
         // Set up method arguments and context

packages/lists/lists.js

@@ -1,15 +1,6 @@
 /* global Lists:true */
 /* global SimpleSchema Factory faker Denormalizer Todos */
 
-// Not sure where the best spot to put this is
-const userIdDenormalizer = new Denormalizer({
-  source: () => Lists,
-  // TODO - can't depend as will lead to circular dep -- single collections package?
-  target: () => Package.todos.Todos,
-  field: 'userId',
-  foreignKey: 'listId'
-});
-
 class ListsCollection extends Mongo.Collection {
   insert(list, callback) {
     if (!list.name) {
@@ -40,8 +31,6 @@ Lists.deny({
   remove() { return true; },
 });
 
-Lists.userIdDenormalizer = userIdDenormalizer;
-
 Lists.schema = new SimpleSchema({
   name: { type: String },
   incompleteCount: {type: Number, defaultValue: 0},

packages/lists/methods.js

@@ -33,8 +33,6 @@ Lists.methods.makePrivate = new ValidatedMethod({
     Lists.update(listId, {
       $set: { userId: this.userId }
     });
-
-    Lists.userIdDenormalizer.set(listId, this.userId);
   }
 });
 
@@ -56,14 +54,9 @@ Lists.methods.makePublic = new ValidatedMethod({
 
     // XXX the security check above is not atomic, so in theory a race condition could
     // result in exposing private data
-    const numUpdated = Lists.update(listId, {
+    Lists.update(listId, {
       $unset: { userId: true }
     });
-
-    if (numUpdated) {
-      // Only denormalize if the list was actually updated
-      Lists.userIdDenormalizer.unset(listId);
-    }
   }
 });
 

packages/todos-lib/package.js

@@ -19,7 +19,8 @@ Package.onUse(function(api) {
     'aldeed:collection2@2.5.0',
     'dburles:collection-helpers@1.0.4',
     'denormalizer',
-    'mdg:validation-error'
+    'mdg:validation-error',
+    'reywood:publish-composite@1.4.2'
   ]);
 
   // Client-side libraries

packages/todos/methods.js

@@ -23,7 +23,6 @@ Todos.methods.insert = new ValidatedMethod({
       createdAt: new Date()
     };
 
-    Lists.userIdDenormalizer.insert(todo);
     Todos.insert(todo);
 
     // XXX should this just get the incomplete count from a query? otherwise

packages/todos/publications.js

@@ -1,28 +1,23 @@
-/* global Todos */
+/* global Todos, Lists */
 /* eslint-disable prefer-arrow-callback */
 
-Meteor.publish('list/todos', function(listId) {
+Meteor.publishComposite('list/todos', function(listId) {
   check(listId, String);
 
-  const publicTodosSelector = {
-    listId: listId,
-    userId: { $exists: false }
-  };
-
-  if (! this.userId) {
-    // If we aren't logged in, only return public todo items
-    return Todos.find(publicTodosSelector);
-  }
+  const userId = this.userId;
+  return {
+    find() {
+      const query = {
+        _id: listId,
+        $or: [{userId: {$exists: false}}, {userId}]
+      };
 
-  const privateTodosSelector = {
-    listId: listId,
-    userId: this.userId
+      return Lists.find(query);
+    },
+    children: [{
+      find(list) {
+        return Todos.find({listId: list._id});
+      }
+    }]
   };
-
-  // We need to make sure that you can only get todos that are in a public list, or in a list that
-  // belongs to the current user, so we need to use Mongo $or.
-  return Todos.find({ $or: [
-    publicTodosSelector,
-    privateTodosSelector
-  ]});
 });

packages/todos/todos.js

@@ -23,11 +23,6 @@ Todos.schema = new SimpleSchema({
     regEx: SimpleSchema.RegEx.Id,
     denyUpdate: true
   },
-  userId: {
-    type: String,
-    regEx: SimpleSchema.RegEx.Id,
-    optional: true
-  },
   text: {
     type: String,
     max: 100
@@ -54,16 +49,10 @@ Factory.define('todo', Todos, {
 });
 
 Todos.helpers({
-  getList() {
+  list() {
     return Lists.findOne(this.listId);
   },
   editableBy(userId) {
-    if (! this.userId) {
-      // This todo is in a public list
-      return true;
-    }
-
-    // This todo is in a private list, but the list is owned by the relevant user
-    return this.userId === userId;
+    return this.list().editableBy(userId);
   }
 });