Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Each documents of collection can attach some default values while insert or update #15

Closed
crapthings opened this issue Oct 6, 2013 · 13 comments

Comments

@crapthings
Copy link

i always add createdAt, createdBy, timestamp and such keys on each 'collection.before.insert' like

Projects.before.insert (userId, project) ->
    project.createdAt = new Date()
    project.timestamp = Date.now()
    project.creatorId = Meteor.userId()
    project.createdBy = Meteor.user().username

Tasks.before.insert (userId, task) ->
    task.createdAt = new Date()
    task.timestamp = Date.now()
    task.creatorId = Meteor.userId()
    task.createdBy = Meteor.user().username

but some collections do not need attaching

Could we add some feature like we can allow some collections do attach default values ?

maybe something like

[Projects, Tasks].before.insert (userId, doc) ->
    doc.createdAt = new Date()

[Projects, Tasks].before.update (userId, doc) ->
    doc.updatedAt = new Date()
@mizzao
Copy link
Contributor

mizzao commented Oct 6, 2013

I'm not sure what your issue is. However, I have some suggestions for you:

  • new Date() and Date.now() are essentially the same thing. (In fact, +new Date(), casting to a number, creates a timestamp.)
  • You should use the userId given to you in the hook rather than Meteor.user() or Meteor.userId(). Also, why store both? That seems unnecessary and will make the username value non-reactive if you use it for rendering. However, this doesn't matter because...
  • You should always attach timestamps on the server side rather than the client side, because client-generated timestamps are unreliable.

@crapthings
Copy link
Author

new Date() to save a time object, and i always use this with yarp timeago to display timeago feature
but before this i have a handlebars helper {{timeago createdAt}} so that yarp timeago can work
timeago helper is built with moment(@createdat).format()
Mon Oct 07 2013 11:32:30 GMT+0800 (CST)

Date.now() to sort
1381116719711

if it is safe to use userId instead Meteor.userId() inside hook i'll change

and if you don't insert a username how do u know who created it ? most time user can't change their login account name.

@matb33
Copy link
Collaborator

matb33 commented Oct 7, 2013

@crapthings, first, for the various dates and username stuff, I recommend instead that you only store the bare minimum (one datestamp, the creatorId) and then rely on a transform to augment your document with extra properties. Username can theoretically change -- the only thing that can't (easily) is the _id.

Second, on having some way to have collections always have some default properties added, this would be the job of an intermediary layer, something like a Collection Manager.

Currently, you are probably approaching collection creation the same way the Meteor examples do, i.e. creating a global variable: Projects = new Meteor.Collection("projects");. When you start to grow your app, you need to start thinking a bit differently. You'll need to create a manager at this point (either create a "collection-manager.js" file and put in a shared area or a "collection-manager" smart package), that would offer a simple API to create and manage collections.

The end result would allow for something like this:

// Create collection
var projects = CollectionManager.create("projects");

// Retrieve collection (if already created like above)
var projects = CollectionManager.get("projects");
projects.insert(...);
projects.findOne(...);

// Opt-in to defaults system:
CollectionManager.includeDefaults("projects");

What includeDefaults would do is add "projects" to an internal list of collections that should have the before.update etc hooks with those default fields you want added.

Let me know if you have any questions

@matb33 matb33 closed this as completed Oct 7, 2013
@matb33
Copy link
Collaborator

matb33 commented Oct 7, 2013

I should mention that to achieve a transform on Meteor.users, you'll need to do something like this:

var transform = function (doc) { return new YourUserConstructorHere(doc); };

var find = Meteor.users.find;
var findOne = Meteor.users.findOne;

Meteor.users.find = function (selector, options) {
  selector = selector || {};
  options = options || {};
  return find.call(this, selector, _.extend({transform: transform}, options));
};

Meteor.users.findOne = function (selector, options) {
  selector = selector || {};
  options = options || {};
  return findOne.call(this, selector, _.extend({transform: transform}, options));
};

IMPORTANT: Collection hooks' this.transform() won't work with this workaround! Instead, fetch the transformed user with something like var user = Meteor.users.findOne({_id: doc._id});

EDIT: Fixed zero-length arguments issue: #22 (comment)

@crapthings
Copy link
Author

rely on a transform to augment your document with extra properties.

Is this only work with client side ?

Projects._transform = (project) ->
    project.percent = ((project.stats.completed / (project.stats.uncompleted + project.stats.completed)) * 100).round() or 0
    return project

when use transform inside public with find, and it doesn't work.

i've tried this

Projects.find { creatorId: @userId },
    sort:
        timestamp: -1
    transform: (project) ->
        _.extend project, Users.findOne project.creatorId

@matb33
Copy link
Collaborator

matb33 commented Oct 8, 2013

Transform works anywhere. Although I'm not fluent in CoffeeScript, it looks like when you extend project, you may be overwriting some of its fields with those from users (like _id). Perhaps don't extend, instead add a property like getUser which would be a function that performs the findOne. This would maintain reactivity in case the user changes some of his properties

Also consider applying the transform at the collection level instead.

@crapthings
Copy link
Author

transform doesn't work this way.

if (Meteor.isServer) {
  Meteor.publish('', function() {
    return System.find({
      config: true
    }, {
      transform: function(system) {
        system.item = Items.findOne(); // i can't set property here like do it with System._transform(function(){ ### })
        return system;
      }
    });
  });
}

@matb33
Copy link
Collaborator

matb33 commented Oct 8, 2013

Almost. Now that I have a keyboard handy, I'll give you an example instead of little hints! Try this:

Items = new Meteor.Collection("items", {
    transform: function (doc) {
        ...
        return doc;
    }
});

System = new Meteor.Collection("system", {
    transform: function (doc) {
        return _.extend(doc, {
            getItem: function () {
                return Items.findOne({_id: doc.itemId});
            }
        });
    }
});

The key takeaway is that a transform defined at collection-level affects all find, findOne, allow and deny for this collection. You can override this transform by specifying it as an option in find/findOne/allow/deny of course, but when defined at the collection level, it's effectively the base transform.

HTH

EDIT: I'm not totally sure if the reactivity chain will be maintained with the getItem function. I'd have to test it out -- but I get this feeling it won't make a difference. So if you prefer, you can replace getItem with: getItem: Items.findOne({_id: doc.itemId})

@crapthings
Copy link
Author

After tried, collection transform doesn't work this way.

Items = new Meteor.Collection("items", {
    transform: function (doc) {
        return doc;
    }
});

but this can work with client.

Items._transform = (item) ->
    return item;

@matb33
Copy link
Collaborator

matb33 commented Oct 9, 2013

This sounds like an issue in your code. Transform absolutely works in both client and server. Try in a new project and test it out. Using _transform is not necessary

@scottsimock
Copy link

@matb33 Do you have any sample CollectionManager code to share? I think this is exactly what I'm looking for and would be interested to see how you implemented.

@matb33
Copy link
Collaborator

matb33 commented Feb 22, 2015

@scottsimock unfortunately I wrote that only as an example, but it shouldn't be too hard to write. That being said, there are a ton more options around these days. There may be a smart package that does this by now

@scottsimock
Copy link

@matb33 Thanks for the quick reply. Believe I'm overthinking the CollectionManager. You're just suggesting its code that calls new Mongo.Collection plus other collection setup code like collection-hooks. I was thinking it was a superclass above Mongo.Collection. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants