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

[Feature Request] callback afterFind #5471

Closed
jurajpetrik opened this issue Apr 1, 2015 · 17 comments · May be fixed by balderdashy/waterline#1610
Closed

[Feature Request] callback afterFind #5471

jurajpetrik opened this issue Apr 1, 2015 · 17 comments · May be fixed by balderdashy/waterline#1610

Comments

@jurajpetrik
Copy link

Would it be possible to implement a callback afterFind? E.g. I have an event with a date specified and in afterFind I could insert an attribute hasExpired..

@dmarcelino
Copy link
Member

This has also been discussed in #525 (comment).

@aclave1, would you be interested in taking this?

I think this is a good fit for 0.11.

cc: @randallmeeker

@randallmeeker
Copy link

+1 for v0.11

@anasyb
Copy link

anasyb commented Apr 1, 2015

One minute I think this should go into sails another min I think it should go into waterline. But definitely it should go somewhere!!
I was experimenting with Model Policies (not to be confused with route policies that sails provide us with) which would also have access to the req as well as the returned records.. and run between the model action and the controller. @jurajpetrik checkout the discussion and gist here #2760
Its a simple hook you drop into api/hooks directory... would love to hear your feedback! Gist here https://gist.github.com/anasyb/c2197a0d47ef29ee7418

@anasyb
Copy link

anasyb commented Apr 1, 2015

@jurajpetrik for your particular use case have you considered an attribute function, something along the lines of:

//in some model
attributes: {
  date:...
  //...
  hasExpired: function (){ return this.date < new Date();}
}

Or is it that you want it to be sent over res?

@aclave1
Copy link
Contributor

aclave1 commented Apr 1, 2015

@dmarcelino I can take this. When should I get this done by?

@dmarcelino
Copy link
Member

How about 0.11? There is no scheduled date yet... And thanks!

@jurajpetrik
Copy link
Author

@anasyb Thanks I didn't know about attribute functions, I do want it to be sent over res though.

@anasyb
Copy link

anasyb commented Apr 3, 2015

Then maybe you have two options:
option 1. move that to the controller or the blueprint, or even a policy (depending on your case). You can keep the attr function if it has some use elsewhere.

//api/controllers/EventController.js
module.exports = {
  findOne: function( req, res ) {
    var eventId;// = bla bla.. get it from req or from wherever you're getting it!
    Event.findOne(eventId).exec(function(err, eve){
      if ( err ) return res.serverError( err );
      if ( !eve ) return res.notFound( 'No event found with ID ' + eventId );

      var eveObj = eve.toObject();
      //temp workaround for https://github.com/balderdashy/waterline/issues/891#issuecomment-88361748
      eveObj.toJSON = undefined;
      eveObj.hasExpired = eve.hasExpired();//assuming you've implemented hasExpired as an attr func

      res.ok(eveObj);//bye bye darling
    });
  }
};

option 2. override toJSON in your model to add all that fancy stuff. This is a good option if you always want to add that attr to the json of your event. You can find an example of overriding toJSON in this comment https://github.com/balderdashy/waterline/issues/317#issuecomment-38376374

Good luck!

@mikermcneil
Copy link
Member

Thanks for posting, @jurajpetrik. I'm a repo bot-- nice to meet you!

It has been 60 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message. On the other hand, if you are still waiting on a patch, please:

  • review our contribution guide to make sure this submission meets our criteria (only verified bugs with documented features, please; no questions, commentary, or bug reports about undocumented features or unofficial plugins)
  • create a new issue with the latest information, including updated version details with error messages, failing tests, and a link back to the original issue. This allows GitHub to automatically create a back-reference for future visitors arriving from search engines.

Thanks so much for your help!

@devinivy
Copy link

Reopening since this is a feature request in a current milestone.

@devinivy devinivy reopened this Sep 23, 2015
@LunaPg
Copy link

LunaPg commented Sep 29, 2015

+1 👍

@sailsbot
Copy link

Thanks for posting, @jurajpetrik. I'm a repo bot-- nice to meet you!

It has been 30 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message. On the other hand, if you are still waiting on a patch, please:

  • review our contribution guide to make sure this submission meets our criteria (only verified bugs with documented features, please; no questions, commentary, or bug reports about undocumented features or unofficial plugins)
  • create a new issue with the latest information, including updated version details with error messages, failing tests, and a link back to the original issue. This allows GitHub to automatically create a back-reference for future visitors arriving from search engines.

Thanks so much for your help!

@tyronedougherty
Copy link

Hi all, is there any update on the status of this feature? An afterFind callback would be really useful for me.

Thanks!

@Seafnox
Copy link

Seafnox commented Apr 18, 2017

An afterFind lifecycle callback would be really useful for me too!

@hitokun-s
Copy link

+1

1 similar comment
@NotBikappa
Copy link

+1

@cristianszwarc
Copy link

cristianszwarc commented Aug 13, 2019

@mikermcneil @raqem PR created for this balderdashy/waterline#1610

cheers

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

Successfully merging a pull request may close this issue.