Skip to content

Fix/id type support #55

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

Merged
merged 17 commits into from
Mar 15, 2017
Merged

Fix/id type support #55

merged 17 commits into from
Mar 15, 2017

Conversation

jaredcnance
Copy link
Contributor

Closes #53

@jaredcnance
Copy link
Contributor Author

@jfhs this should fix the issues. I will be writing tests to validate it in the morning. Let me know what you think

@jfhs
Copy link
Contributor

jfhs commented Mar 15, 2017

@jaredcnance Looks good. However I checked specification, and it says that server may accept client generated ids which is explicitly forbidden in current implementation. That is not the same issue you are solving by this PR, but it requires change in same place, so maybe you can do it all at once.

I would suggest adding a flag in controller for disabling this verification, something like bool AllowClientGeneratedId which will be false by default, and can be re-enabled by users. Although that's not required for my current use-case, I think in general that would be good to have.

@jaredcnance
Copy link
Contributor Author

You're right in that a server MAY support client generated ids. I'll include a test this morning that checks whether or not it will work without much modification. I suspect this may not be so simple, but I'll dig into it.

}

return entity;
}

private static object _setHasManyRelationship(object entity,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Closes #52

@jaredcnance jaredcnance merged commit f7e3e1b into develop Mar 15, 2017
@jaredcnance jaredcnance deleted the fix/id-type-support branch March 15, 2017 22:18
@jaredcnance
Copy link
Contributor Author

You can get these updates on MyGet (version 1.1.1-alpha1-0069)

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

Successfully merging this pull request may close these issues.

2 participants