Support for more general json:api documents (links, data, meta) #42

therealcljohn · 2016-10-20T13:44:01Z

This is a proposal to implement #26. I followed the idea to use generics (mentioned in the issue) and did test:

query
findRecord
save
delete
manually in my example project located at https://git.nordwest.freifunk.net/netmon-sc/web-client

If there are more changes needed, please let me know :)

…nd meta attributes. Therefore refactor query and findRecord methods to return an instance of DocumentModel. Signed-off-by: Clemens John <clemens.john@floh1111.de>

Signed-off-by: Clemens John <clemens.john@floh1111.de>

ghidoz

Thanks! I haven't tested it yet, but I just added some comments about style, so that it's more consistent with mine.

ghidoz · 2016-10-20T17:34:09Z

src/models/document.model.ts

+
+export class DocumentModel<T> {
+  _links: LinksModel = new LinksModel;
+  _data: T;


Maybe you can set these two as private

ghidoz · 2016-10-20T17:36:04Z

src/models/document.model.ts

+    return this._links.links(name);
+  }
+
+  public data(): T {


this can be a getter:
get data(): T {...}

ghidoz · 2016-10-20T17:36:44Z

src/models/document.model.ts

+    return this._data;
+  }
+
+  public setData(data: any) {


this can be a setter:
set data(data: any) {...}

ghidoz · 2016-10-20T17:38:04Z

src/models/json-api.model.ts


  id: string;
+  _links: LinksModel = new LinksModel;
+  _document: DocumentModel<JsonApiModel>;


Maybe you can set these two as private

ghidoz · 2016-10-20T17:39:37Z

src/models/json-api.model.ts

  [key: string]: any;

-  constructor(private _datastore: JsonApiDatastore, data?: any) {
+  constructor(private _datastore: JsonApiDatastore, data?: any, document: DocumentModel<JsonApiModel> = null) {


If you use the privatehere, you can remove the line 14 and it's cleaner and consistent ;)

Moreover, instead of document: DocumentModel<JsonApiModel> = null you can write: document?: DocumentModel<JsonApiModel>

ghidoz · 2016-10-20T17:43:35Z

src/models/json-api.model.ts

      return peek;
    }
-    let newObject: T = new modelType(this._datastore, data);
+    let newObject: T = new modelType(this._datastore, data, null);


You can just not passing a third argument and it's automatically null

ghidoz · 2016-10-20T17:44:14Z

src/models/link.model.ts

+    this._name = name;
+  }
+
+  name() {


Use a getter and a return type

ghidoz · 2016-10-20T17:44:24Z

src/models/link.model.ts

+    return this._name;
+  }
+
+  href() {


Use a getter and a return type

ghidoz · 2016-10-20T17:46:20Z

src/models/links.model.ts

+    });
+  }
+
+  public links(name: string = null) {


Use name?: string instead of name: string = null

ghidoz · 2016-10-20T17:47:21Z

src/services/json-api-datastore.service.ts


  createRecord<T extends JsonApiModel>(modelType: ModelType<T>, data?: any): T {
-    return new modelType(this, {attributes: data});
+    return new modelType(this, {attributes: data}, null);


No need to pass null

ghidoz · 2016-10-20T17:52:55Z

Moreover, you should also fix the few tests I did, since now are failing of course. Thanks man!

HennerM · 2016-10-21T19:02:39Z

I for myself think that the JsonApiModel should not be extended, it doesn't really makes sense to include this property in every instance of JsonApiModel. Like @reva2 stated in #34 a JsonApiModel should just be a POJO.

therealcljohn · 2016-10-21T20:08:17Z

Hi @ghidoz, thanks for your comments! I'll look at them next week when I also have time to look at the tests.

For the point that @HennerM made, I'm too much a newby in the Javascript world to discuss this topic so I leave this open to you.

HennerM · 2016-10-22T16:40:40Z

I like the approach they took in this Java JSON-API library better, they allow defining meta and links attributes on model class level, for example:

@Type("book")
public class Book {
  @Id
  private String isbn;

...

  @Meta
  private MyCustomMetaClass meta;
}

This leaves the option to use meta and link attributes to the user.

therealcljohn · 2016-10-23T08:57:38Z

@HennerM As far as I understand this is for accessing these attributes if they are defined inside the data attribute arent they? But I need to access these attributes if they are defined at both levels: top level of the document and also on model level.

For model level one can take the approach shown in the mentioned lib but to fully implementiert the spec we will need some Kind of DocumentModel returned by the request methods to access top level attributes (i.e. in case of an empty collection).

HennerM · 2016-10-23T13:02:58Z

@floh1111 sorry for the misunderstanding, of course I agree on the need for the DocumentModel class for accessing the top level metadata. But for model-level it would be better to define them with decorators in the model classes, to reduce the dependencies in JsonApiModel

… and ModelType because I could not get it working with generics. The data property of DocumentModel can be T or T[] and I think ModelType needs more refactoring to return T and initialize DocumentModel<T[]> and DocumentModel<T> at the same time. Signed-off-by: Clemens John <clemens.john@floh1111.de>

Signed-off-by: Clemens John <clemens.john@floh1111.de>

… /home/floh1111/angular2-jsonapi/node_modules/@types/lodash/index.d.ts:245:25: Cannot find name 'Partial'. Signed-off-by: Clemens John <clemens.john@floh1111.de>

rnikunen · 2017-02-06T19:44:12Z

Is this going to be added to master anytime soon? I'm in need of accessing meta data!

therealcljohn · 2017-02-07T00:50:22Z

Hi @rnikunen I sadly had a project to finish in the past weeks but I have some more time now and I'm planning to take a look at this at the end of the week again.

rnikunen · 2017-02-07T16:14:35Z

Thanks for the heads. I think it needs to be said....I appreciate that we all have jobs and these projects are a contribution to the community of angular devs. I have forked the project and am taking the approach @ghidoz mentioned back in October in the meantime.

rnikunen · 2017-02-27T18:19:26Z

Just checkin in. We really need access to metadata for pagination and links.

gabskoro · 2017-02-28T13:47:17Z

Same here :)

RooTooZ · 2017-03-09T13:10:35Z

@HennerM i'll create new merge request
Now we can make custom metadata
See in #64

PS
@gabskoro Can you see my merge?

HennerM · 2017-07-05T22:09:52Z

Closing in favor of #64

Clemens John added 3 commits October 20, 2016 15:24

Add support for more generic JsonApi document including links, data a…

7234b62

…nd meta attributes. Therefore refactor query and findRecord methods to return an instance of DocumentModel. Signed-off-by: Clemens John <clemens.john@floh1111.de>

Fix indentation

2f09e59

Signed-off-by: Clemens John <clemens.john@floh1111.de>

Remove debug output

df244bd

Signed-off-by: Clemens John <clemens.john@floh1111.de>

ghidoz reviewed Oct 20, 2016

View reviewed changes

Clemens John added 6 commits October 24, 2016 18:27

Merge branch 'master' into jsonapi-documents

5591e52

Merge branch 'master' into jsonapi-documents

ae34f3b

Make models code style compatible

c3b7c2d

Signed-off-by: Clemens John <clemens.john@floh1111.de>

Make save and saveRecord return Observable<DocumentModel<T>>

1401814

Signed-off-by: Clemens John <clemens.john@floh1111.de>

Fix the following error that occurs on 'npm install': Error: Error at…

c8c10dc

… /home/floh1111/angular2-jsonapi/node_modules/@types/lodash/index.d.ts:245:25: Cannot find name 'Partial'. Signed-off-by: Clemens John <clemens.john@floh1111.de>

HennerM closed this Jul 5, 2017

Support for more general json:api documents (links, data, meta) #42

Support for more general json:api documents (links, data, meta) #42

Uh oh!

Conversation

therealcljohn commented Oct 20, 2016

Uh oh!

ghidoz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghidoz commented Oct 20, 2016

Uh oh!

HennerM commented Oct 21, 2016

Uh oh!

therealcljohn commented Oct 21, 2016

Uh oh!

HennerM commented Oct 22, 2016

Uh oh!

therealcljohn commented Oct 23, 2016

Uh oh!

HennerM commented Oct 23, 2016

Uh oh!

rnikunen commented Feb 6, 2017

Uh oh!

therealcljohn commented Feb 7, 2017

Uh oh!

rnikunen commented Feb 7, 2017

Uh oh!

rnikunen commented Feb 27, 2017

Uh oh!

gabskoro commented Feb 28, 2017

Uh oh!

RooTooZ commented Mar 9, 2017

Uh oh!

HennerM commented Jul 5, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants