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

addIncludedArray() replacing payload data with included #253

Merged

Conversation

cristinawithout
Copy link
Contributor

I have a model 'department', which is hierarchal. Each department can have a parent. So the department model has a relationship with its own model type 'department'.

In my tests, I have a trait called 'parent' and one called 'primary'.

FactoryGuy.define('department', {
    traits: {
        primary: {
            name: 'Primary Department',
            parent: FactoryGuy.belongsTo('department', 'parent'),
        },
        parent: {
            name: 'Parent Department',
            parent: null
        }
    }
//...
});

When I use buildList() and include the 'primary' trait, the result I get back contains only the 'parent' model.

Stepping through, I found this is because addIncludedArray() in rest-fixture-converter is setting payload[key] = this.included[key];. Since the key 'department' is present in 'included', it replaces everything that in the payload when I want it to add it to the payload.

I wasn't sure where the best place to add this to tests would be, but I can put one together if you let me know.

@danielspaniel
Copy link
Collaborator

Mmm ... let's see.. seems like this ( if it only applies to rest adapter ( and I think it does ) .. would go in the rest adapter custom buildList tests ( I know there is custom build so if there is no custom buildList then make that module )
I would greatly appreciate the tests too so thanks for asking about that

@cristinawithout
Copy link
Contributor Author

Thanks. I'll try to get to the test within the next few days.

@danielspaniel
Copy link
Collaborator

@cristinawithout .. if you want some help testing this let me know

@cristinawithout
Copy link
Contributor Author

Sorry. I haven't had time to get to it yet. It should be a fairly easy test. If you want to take care of it, I wouldn't mind at all.

@danielspaniel danielspaniel merged commit ac5fec3 into adopted-ember-addons:master Nov 16, 2016
@danielspaniel
Copy link
Collaborator

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

Successfully merging this pull request may close these issues.

2 participants