-
Notifications
You must be signed in to change notification settings - Fork 440
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] reference dynamic attributes from dynamic attributes. #269
[Feature] reference dynamic attributes from dynamic attributes. #269
Conversation
We are using my feature branch in our production system and just found one bug when referencing multiple dynamic properties from a property. I fixed it and wrote a regression spec. I also refactored the factory code to not use exception anymore. Other than that this feature works really nice for us. Any thoughts/concerns on the PR @samselikoff ? |
I like it! Awesome API, I need to look at the code more (been busy lately). There's another issue related to this, originally the api was going to be |
Im glad to hear that. If you have any questions about the code don't hesitate to reach me on slack |
FYI, here a nice feature of that PR that i just discovered: Imagine you have a conference with 10 talks that could overlap in time but are guaranteed to start and end within the duration of the conference itself. This is now super convenient. server.createList('talks', 10, {
conference_id: conference.id,
start_date: ()=> dateBetween(conference.start_date, conference.end_date),
end_date() { // no arrow function here to get the correct scope for `this`
return dateBetween(this.start_date, conference.end_date);
}
}); Just sharing this with you cause you might wanna add something alike in the docs. |
Nice! I haven't forgotten about this, btw. |
Getting closer to wrapping up serializers. Factories come next. |
d0e0212
to
82ac477
Compare
rebased this one btw |
awesome, thank you. Factories are top priority after 0.2.x |
I tried to pull this branch directly into my project (using
Any thoughts as to what that might be? |
@JakeDluhy, you've done everything right, thats how i import my branch into our application aswell. It seems that ember-cli can't find the lodash package in your project. maybe a If not let me know and i'll look into it again |
As 0.2.0 is dragging out, let's go ahead and get this merged in. Would you mind rebasing + squashing your commits? After that, I'm happy to pull the trigger. Again, thank you so much for this, awesome work on this PR. |
awesome |
@lazybensch @samselikoff When are guys planning to merge this? |
@lazybensch if you could rebase I'm happy to get this merged in, as it's completely backwards compatible. |
ping @lazybensch |
Would love to see this, too! |
i hate to leave a 'me too' but I've been watching this PR for a bit and hoping it lands |
2299e27
to
37cf4e3
Compare
@samselikoff rebased the branch again. Sorry that it took me so long guys but, well it says 'lazy' right there in my pseudonym 😃 |
[Feature] reference dynamic attributes from dynamic attributes.
Haha - merged! Thanks again dude! |
Yes! This made my day! |
This PR lets you reference dynamic attributes from different dynamic attributes.
Currently you are able to reference static attributes from dynamic ones like so:
but I also want to be able to reference dynamic attributes from dynamic attributes like so:
That is now possible. I also made sure it does not matter in what order you specify the properties, so it also works for:
What (obviously) does not work is referencing
propA
frompropB
andpropB
frompropA
. If the user tries to attempt such a mad thing, he will get a nice and helpful.Cyclic dependency in properties ["propA", "propB"]'
error thrown.I also wanted to update the docs but they don't seem to be open source, is this correct?