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

Model hooks / default attributes #45

Open
benswinburne opened this issue Mar 9, 2020 · 5 comments
Open

Model hooks / default attributes #45

benswinburne opened this issue Mar 9, 2020 · 5 comments

Comments

@benswinburne
Copy link

Is it possible to have timestamps automatically created/touched on models when created/updated?

I saw it can be done with factories when seeding and then explicitly writing handlers for all the routes, but if I want to use the nice shorthand route handlers then it'd be nice to have some hooks i can set up on the model itself. which always run when created/touched perhaps?

Something like the below maybe?

Model.extend({
  beforeCreate: model => {
    model.createdAt = new Date;
	return this;
  }
})

The following hooks may be useful.

beforeCreate
afterCreate
beforeUpdate
afterUpdate
beforeSave (both create/update)
afterSave (both create/update)
beforeDelete
afterDelete
@samselikoff samselikoff transferred this issue from miragejs/miragejs Mar 9, 2020
@samselikoff samselikoff changed the title Automatic Timestamp Columns on Models Model hooks / default attributes Mar 9, 2020
@samselikoff
Copy link
Contributor

Was surprised that I didn't find this existing as it's been asked several times before + is definitely a legitimate feature request.

It's also confusing as folks use factory attrs as "defaults" then wonder how to use them in route handlers. (But factory attrs serve a different purpose).

It's definitely a gap in Mirage atm – best solution today is to just make some sort of "createUser" function that has the default logic, and can be shared in route handlers and/or factories.

Hooks are a good idea. As for our priority right now, it's getting Mirage working in some different environments that it doesn't right now (e.g. Next.js) and shipping a 1.0. Then, before we add a feature like this, we need to get ES6 classes in. I think that's our rough roadmap so while I think this is an important feature it's not yet being worked on.

@benswinburne
Copy link
Author

benswinburne commented Mar 10, 2020

Thanks for the response, that makes sense.

With regards to your point about the factory attributes as defaults- I guess you mean something like this?

factories: {
  creative: Factory.extend({
    createdAt: faker.date.past()
  }),
},

routes() {
  this.namespace = '/';

  this.post('/creatives', function(schema, request) {
    const formData = request...;
    schema.server.create('creative', formData)
  })
},

I haven't tried the above because I ended up extending the save() method (detailed below). I didn't find the concept of factory attributes confusing but I was thrown by the fact the fixtures don't use the factories or even the models to create the data, they're just copied into the database verbatim. I'd set up some fixtures with some static data like IDs which I always wanted to know they're there then was hoping to fill in lots of arbitrary data in using factories or models. It meant eventually i had to drop the use of fixtures and just create everything with server.create().

In the mean time, I've found the below sufficient, it simply adds some attributes on.

function save() {
  const utc = new Date().toUTCString();
  const ts = new Date(utc).toISOString();

  // This allows you to seed models with a created and or updated time as there
  // is no concept of dirty/clean attributes this is the best i can get
  if (this.isNew()) {
    this.attrs.updatedAt = this.attrs.updatedAt || this.attrs.createdAt || ts;
  } else {
    this.attrs.updatedAt = ts;
  }

  this.attrs.createdAt = this.attrs.createdAt || ts;

  if (this.fks.includes('ownerId')) {
    this.attrs.ownerId = loggedInUser.id;
  }

  return Model.prototype.save.call(this);
}

new Server({
  models: {
    creative: Model.extend({
      campaigns: hasMany(),
      owner: belongsTo('user'),
      save,
    }),
  }
});

@samselikoff
Copy link
Contributor

Sorry about the delayed response! I'm behind on issues a bit due to some consulting/training work.

With regards to your point about the factory attributes as defaults

I wasn't clear – my point was that some folks reach for using factory attributes as model defaults, but that's incorrect usage and should not be done. I was just validating that your need for model defaults is valid + a gap in Mirage today.

It meant eventually i had to drop the use of fixtures and just create everything with server.create()

I personally do this too and think it's the best approach, since using server.create lets Mirage's ORM take care of some more bookkeeping for you. As far as static IDs you always want to exist, I think there's usually better ways to achieve this, so if you're running into pain points here I'd love to hear them and possibly explore alternatives.

In the mean time, I've found the below sufficient..

Model save methods were never "meant" to be extended in userland so you might run into some unexpected behavior... you should also try to stay away from private properties like this.fks because they could change out from underneath you. (Even though you can access this.fks it's not public since it's not documented on the site. Instead you should refactor to use this.associations and then the Association API to replace your logic.)

Hopefully that helps! Let me know if you have any more questions.

@cwagner22
Copy link

Any update on the recommended way to have default attributes?

@samselikoff
Copy link
Contributor

Unfortunately not, I think functions are the best bet today. Would gladly accept PRs in this direction if someone were so inclined!

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

3 participants