-
Notifications
You must be signed in to change notification settings - Fork 122
simple accessor to get meta data passed into models #182
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
Conversation
src/models/json-api.model.ts
Outdated
| constructor(private _datastore: JsonApiDatastore, data?: any) { | ||
| if (data) { | ||
| this.id = data.id; | ||
| this.meta = data.meta; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a getter would be a bit nicer solution.
get meta(): object {
return this.data.meta;
}
It would be maybe even nicer to do the same for id property.
@jacobbullock What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@safo6m I think I agree with you on this, but currently the model object isn't persisting the entire data object. It just dumps the attributes into the object. Should we refactor the model object?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no need for any refactoring (at least not regarding this topic).
Making data in the constructor private should be enough. Then you can access data as this.data.
...
constructor(private _datastore: JsonApiDatastore, private data?: any)
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yah, we can do it that way. Do you want me to update this PR to include that edit for id you mention above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. Thanks.
|
I'd like to add a second thumbs up but since this is not possible I just add this lame comment :) |
|
@safo6m do i need to write tests for this? |
|
What is still blocking this feature from getting into master? |
|
@jacobbullock can you please resolve the conflicts? |
|
@safo6m Sorry,been distracted with other things. i'll get to it this weekend. |
|
@jacobbullock Hi! Any chance that this is going to be merged? |
|
I second the request to get this merged. |
|
I'd also like this merged :) |
Fixes issue #155 by adding the meta data for a model accessible without any requirement of adding that meta data to the datastore.