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

Is the Sails team willing to support beforeValidation? #6841

Closed
cristianszwarc opened this issue Aug 15, 2019 · 8 comments
Closed

Is the Sails team willing to support beforeValidation? #6841

cristianszwarc opened this issue Aug 15, 2019 · 8 comments

Comments

@cristianszwarc
Copy link

History
there is mention of a beforeValidate lifecycle callback but I don't see it being implemented:
https://github.com/balderdashy/waterline/blob/1d5a3dc6e65fe69c061a72249019adfcf3c52cb6/lib/waterline/utils/system/lifecycle-callback-builder.js#L28

the migration to 1.0 docs states: beforeValidate and afterValidate lifecycle callbacks no longer exist" https://next.sailsjs.com/documentation/upgrading/to-v-1-0

Other missing lifecycle callbacks
I have been working on enabling other listed but not available lifecycle callbacks (afterFind/afterFindOne) #6839 .

Need for beforeValidate
as others, I found that if the record being created/updated contains extra a field not defined in the model, it would remove/normalise it before the beforeCreate or beforeUpdate callbacks are executed.

I don't see where in the model I could add validations that are not related to individual fields, it feels that beforeValidate would be a great place for actions like validations across different fields

example. you might not want to allow a newMember.receiveCasinoPromos to be true while newMember.dob shows is a minor.

(In our edge situation we have extra dynamic fields that should affect the real fields before being discarded)

related: #5625

Question
Is the Sails team willing to support beforeValidation? If so, I'm happy to put some hours on it instead of creating a custom solution.

cheers!

@sailsbot
Copy link

@cristianszwarc Thanks for posting! We'll take a look as soon as possible.

In the mean time, there are a few ways you can help speed things along:

  • look for a workaround. (Even if it's just temporary, sharing your solution can save someone else a lot of time and effort.)
  • tell us why this issue is important to you and your team. What are you trying to accomplish? (Submissions with a little bit of human context tend to be easier to understand and faster to resolve.)
  • make sure you've provided clear instructions on how to reproduce the bug from a clean install.
  • double-check that you've provided all of the requested version and dependency information. (Some of this info might seem irrelevant at first, like which database adapter you're using, but we ask that you include it anyway. Oftentimes an issue is caused by a confluence of unexpected factors, and it can save everybody a ton of time to know all the details up front.)
  • read the code of conduct.
  • if appropriate, ask your business to sponsor your issue. (Open source is our passion, and our core maintainers volunteer many of their nights and weekends working on Sails. But you only get so many nights and weekends in life, and stuff gets done a lot faster when you can work on it during normal daylight hours.)
  • let us know if you are using a 3rd party plugin; whether that's a database adapter, a non-standard view engine, or any other dependency maintained by someone other than our core team. (Besides the name of the 3rd party package, it helps to include the exact version you're using. If you're unsure, check out this list of all the core packages we maintain.)

Please remember: never post in a public forum if you believe you've found a genuine security vulnerability. Instead, disclose it responsibly.

For help with questions about Sails, click here.

@johnabrams7
Copy link
Contributor

johnabrams7 commented Aug 21, 2019

@cristianszwarc Thanks for the rational and proposal to help with beforeValidate - I'll bring this up with the team for further consideration and get back with you on where to go from here.

@johnabrams7
Copy link
Contributor

@cristianszwarc Just discussed this one with the team and we're curious of your usage case / need for beforeValidate. Have you tried a workaround by coercing the data in a backend action before calling create or update related functionality? An action can possibly provide an effective workaround.

@cristianszwarc
Copy link
Author

thanks @johnabrams7

We have a base model that is used across clients, each client can configure custom fields that are then dynamically supported.

When the project was moved from Mongodb to Postgres, the list of available fields got frozen in time, custom fields can't be provided anymore to clients without altering the db.

Indeed there are multiple ways to deal with this scenario. But we have some constrains:

  • the values for these extra fields must be persisted within the same record (the table is audited and adding relations would require rethinking the audit procedure), a JSONB is our answer.
  • all already working entry/output points, actions, services and frontend apps are luckily already agnostic of the model definition but don't support nested fields such as MyModel.customFields.someField.

our solution:

  • MyModel.customFields is a JSONB where the custom fields values are to be stored.
  • afterFind lifecycle callbacks are used to spread customFields content into calculated properties record.someField
  • beforeValidate would work the other way around, any create/update payload would be pre digested so all custom fields (defined by the client config) would be moved into the right place ex MyModel.customFields.someField

@johnabrams7
Copy link
Contributor

@cristianszwarc Thanks for the explanation! Given your use case, it sounds like using blueprints may be more of a liability than a benefit. We recommend writing a custom action.

@cristianszwarc
Copy link
Author

cristianszwarc commented Aug 26, 2019

While waiting for an answer we started to use a custom method in the critical places were we need to move on, if beforeValidation will not be supported then we will implement this custom method across the code.

Should we close the issue and call the day?
cheers

@johnabrams7
Copy link
Contributor

@cristianszwarc Awesome, glad y'all found a solution. 👍

@danielsharvey
Copy link
Contributor

@cristianszwarc I've facing a similar issue. I've been looking at intercepting the process in parseBlueprintOptions(). This allows you to act before field validation. Not certain this applies for you, but thought I would mention.

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

No branches or pull requests

4 participants