Skip to content
This repository has been archived by the owner on Jul 14, 2021. It is now read-only.

Add single-loaded(from server) relations declaration using $relations property #26

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ajcastro
Copy link

@ajcastro ajcastro commented Jun 23, 2016

Please see the Defining Relations section in the README.md of my fork. Thanks! :)

PS:
This has no automated test but I tested it manually.

@ajcastro
Copy link
Author

Woah, why did the build break?

@bfanger
Copy link
Owner

bfanger commented Jun 23, 2016

Previous Travis-CI build was 8 months ago, I've updated the configs and dependencies.
if you'd git rebase it probably fixes this build too.

@ajcastro
Copy link
Author

ajcastro commented Jun 27, 2016

Im sorry but Im not sure how to do that git rebase. I didn't get how to solve the problem.

@bfanger
Copy link
Owner

bfanger commented Jun 28, 2016

git remote add upstream https://github.com/bfanger/angular-activerecord.git
git pull --rebase upstream master
# check if the patch still works after the rebase
git push -f origin master

For more detailed information: stackoverflow.com/How do I clean up my Github fork so I can make clean pull requests?

@bfanger
Copy link
Owner

bfanger commented Jun 28, 2016

I like the simplicity of your implementation of relations (just mapping the objects to classes), but managing relations is a feature that can become complex really fast.

Could you write the relation feature as a separate optional js file? like a plugin?

var initialize = ActiveRecord.prototype.$initialize;
ActiveRecord.prototype.$initialize = function $initialize(properties, options) {
    initialize.apply(this, arguments);
    ... relation code here ...
}

That way ActiveRecord matches Backbone.Model and people can opt-in to your implementation of use their own.

@ajcastro
Copy link
Author

ajcastro commented Jul 7, 2016

Sorry for the late reply, I was busy this past few days. Btw, thanks for the compliment and instructions. I will do it shortly.

@ajcastro
Copy link
Author

ajcastro commented Jul 7, 2016

I am puzzled how to do put it in a separate optional js file. Maybe you can help me. Currently I'm still googling how to do that.

@ajcastro
Copy link
Author

ajcastro commented Jul 7, 2016

Aha! I think I had to create another factory, which of course has a different name e.g. ActiveRecord.Slr, and it has a dependency on ActiveRecord. Slr means single-loaded-relations.

@ajcastro
Copy link
Author

ajcastro commented Jul 7, 2016

I need help on this, I don't know yet how to fix this. I am still a newbie in karma/angular js unit testing.
https://travis-ci.org/ajcastro/angular-activerecord/builds/142970284

I already pushed my code in branch activerecord-slr in my fork. Thanks :)

@bfanger
Copy link
Owner

bfanger commented Jul 7, 2016

Thanks I'll try to merge it when I've got some free time

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

Successfully merging this pull request may close these issues.

2 participants