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

Can't validate association properties #187

Closed
dxg opened this issue Jun 5, 2013 · 2 comments
Closed

Can't validate association properties #187

dxg opened this issue Jun 5, 2013 · 2 comments

Comments

@dxg
Copy link
Collaborator

dxg commented Jun 5, 2013

Because of the change introduced by this issue it's no longer possible to validate association generated fields. There is a failing test case here.

I can see a few possible resolutions but am unsure which is best:

  1. Modify the hasOne association so that it adds first class properties to the model, rather than association_properties. Perhaps there's a good reason why they're separate.
  2. Add all_properties which would be the sum of properties and association_properties. I think this would warrant adding a method to Model such as add_property(...) else it might be hard to track everything.
  3. Check all types of properties in handleValidations although association_properties doesn't store full properties like properties does. Seems to be just a list.
  4. Modify the DDL sync so that it's possible to explicitly define the association column in the db.define call (currently, if there is overlap, the sync function will define the column twice, causing an SQL error).

What do you think?

@dresende
Copy link
Owner

dresende commented Jun 5, 2013

Extra properties are really tricky but they only happen in hasMany associations. The extra properties should be available in opts.extra in Instance.js. Perhaps we could just change the handleValidations method to look at them also.

@dxg
Copy link
Collaborator Author

dxg commented Jun 6, 2013

The issue isn't extra properties, it's association_properties created by hasOne.
They aren't available in opts.extras and whilst they could be added there and handleValidations modified to also check those, I'm not sure what value is added by maintaining two lists [properties & extras].

Having to remember to check every type of property all the time will keep introducting corner case-bugs much like this one.

I think we should have one list composed of all properties that belong to the model, much like we have one model_fields list.

hasOne would add a property to this list, and handleValidations would check this list. Driver sync would create the table definition from this one list. It should simplify the code and reduce the likelihood of related bugs.

dxg added a commit that referenced this issue Jun 6, 2013
dxg added a commit that referenced this issue Jun 6, 2013
@dresende dresende closed this as completed Jun 6, 2013
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

2 participants