bocoup / nest-weekly-review

An application for managing billing data for consulting projects
http://weekly-review.bocoup.com/
1 stars 2 forks source link

prefix project names with organization #85

Closed tkellen closed 9 years ago

tkellen commented 9 years ago

closes gh-41

tkellen commented 9 years ago

I haven't addressed testing here (yet), but I'm curious about this: https://github.com/bocoup/nest-weekly-review/pull/85/files#diff-d7c4ae8e76e1a05593dd50e4c713093bR37

Is it possible for this to even work?

tkellen commented 9 years ago

Please disregard this until I have time to make a more considered attempt. I was mostly just tinkering here.

jugglinmike commented 9 years ago

@tkellen When I request a project phase resource with include=project.organization, e.g.

GET http://api.loc/v1/project-phases?after=2015-04-19&before=2015-04-26&include%5B0%5D=reviews&include%5B1%5D=project&include%5B2%5D=employees&include%5B3%5D=project.organization

The response includes the organizations as expected, but the links from the project phases are referenced using dot notation:

{
  "links": {
    // ...
    "organizations": [
      {
        "id": 38,
        "name": "Chrome River",
        "inactive": false,
        "pipedrive_id": 373,
        "github_user": null
      },
      // ...
    ]
  },
  "project-phases": [
    {
      "id": 33,
      "name": "Phase-1",
      "project_id": 119,
      "first_day": "2015-03-30T04:00:00.000Z",
      "last_day": "2015-06-12T04:00:00.000Z",
      "bill_method_id": 1,
      "rate": 2000,
      "project_sow_id": 3022,
      "links": {
        "project": {
          "type": "projects",
          "id": 119
        },
        "sow": {
          "type": "project-sows",
          "id": 3022,
          "href": "/project-sows/3022"
        },
        "billMethod": {
          "type": "bill-methods",
          "id": 1,
          "href": "/bill-methods/1"
        },
        "reviews": {
          "type": "project-phase-reviews",
          "ids": [
            60,
            62,
            73,
            82,
            93
          ]
        },
        "employees": {
          "type": "employees",
          "ids": [
            26,
            23
          ]
        },
        "project.organization": {
          "type": "organizations",
          "id": 38
        }
      }
    },
    // ...
  ]
}

In order to expand this correctly, I'll have to update the parser. Before I do, I'd just like to verify: is this specified by the JSONAPI draft? I can see details on how to request related resources, but I'm having trouble finding evidence for the shape of the response. I ask because I could see this being done in a few different ways (see below), and I want to be sure before updating the parser.

{
  "links": {
    // ...
    "organizations": [
      {
        "id": 38,
        "name": "Chrome River",
        "inactive": false,
        "pipedrive_id": 373,
        "github_user": null
      },
      // ...
    ]
  },
  "project-phases": [
    {
      "id": 33,
      "name": "Phase-1",
      "project_id": 119,
      "first_day": "2015-03-30T04:00:00.000Z",
      "last_day": "2015-06-12T04:00:00.000Z",
      "bill_method_id": 1,
      "rate": 2000,
      "project_sow_id": 3022,
      "links": {
        "project": {
          "type": "projects",
-         "id": 119
+         "id": 119,
+         "links": {
+           "organization": {
+             "type": "organizations",
+             "id": 38,
+             "href": "/organizations/38"
+           }
          }
        },
        "sow": {
          "type": "project-sows",
          "id": 3022,
          "href": "/project-sows/3022"
        },
        "billMethod": {
          "type": "bill-methods",
          "id": 1,
          "href": "/bill-methods/1"
        },
        "reviews": {
          "type": "project-phase-reviews",
          "ids": [
            60,
            62,
            73,
            82,
            93
          ]
        },
        "employees": {
          "type": "employees",
          "ids": [
            26,
            23
          ]
+       }
-       },
-       "project.organization": {
-         "type": "organizations",
-         "id": 38
-       }
      }
    },
    // ...
  ]
}

...or:

{
  "links": {
    // ...
    "organizations": [
      {
        "id": 38,
        "name": "Chrome River",
        "inactive": false,
        "pipedrive_id": 373,
-       "github_user": null
+       "github_user": null,
+       "links": {
+         "organization": {
+           "type": "organizations",
+           "id": 38,
+           "href": "/organizations/38"
+         }
      },
      // ...
    ]
  },
  "project-phases": [
    {
      "id": 33,
      "name": "Phase-1",
      "project_id": 119,
      "first_day": "2015-03-30T04:00:00.000Z",
      "last_day": "2015-06-12T04:00:00.000Z",
      "bill_method_id": 1,
      "rate": 2000,
      "project_sow_id": 3022,
      "links": {
        "project": {
          "type": "projects",
          "id": 119
        },
        "sow": {
          "type": "project-sows",
          "id": 3022,
          "href": "/project-sows/3022"
        },
        "billMethod": {
          "type": "bill-methods",
          "id": 1,
          "href": "/bill-methods/1"
        },
        "reviews": {
          "type": "project-phase-reviews",
          "ids": [
            60,
            62,
            73,
            82,
            93
          ]
        },
        "employees": {
          "type": "employees",
          "ids": [
            26,
            23
          ]
+       }
-       },
-       "project.organization": {
-         "type": "organizations",
-         "id": 38
-       }
      }
    },
    // ...
  ]
}
tkellen commented 9 years ago

@jugglinmike The response structure from v1 of the API is quite a bit different than what json-api specifies. The current version of endpoints (and v2 of the API) is tracking the current draft of the spec (which will be marked 1.0 on May 21st, and will henceforth be taking a very hard line about not breaking backwards compatibility). So, no, you won't find justification for v1 response shapes in the JSON-API spec. The v1 structure is locked at the point in time it was created.

...that being said, I am actually in the middle of nailing down the semantics around nested relations and have been asking these same questions:

From https://github.com/endpoints/endpoints/issues/118

For example, given a blog post with comments who have authors, if you want to find everyone who participated in the conversation, you could do /blog/1?include=comments.authors. This breaks down for a number of reasons if you try to represent that relationship in a client-side identity map.

Without the intermediate relationships (the comments) there is no way to associate commentors directly to a blog post without adding a has-many relation to them from the blog model.

If you keyed that relation as comments.authors, it would be pretty annoying to reference in any tool that uses dot notation already.

Instead, for this case, there should be a relation called commentors on the blog post which maps to comments.authors internally.

Note that there is a subtle difference here... when using "normalized" relationship names (e.g. commentors) instead of "relationship paths" (e.g. comments.authors), if you request /blog/1?include=comments,commentors it becomes a little less clear that each comment should include linkage to its author. Perhaps this is a non issue, but...

JSON-API currently makes a recommendation that if a relationship path is used, only the last member of the path should be included. After dealing with the actual implementation of this, I think this is a poor recommendation. I believe a relationship path should normatively be required to include all members of the path. If a client wants to skip the intermediate records (e.g. get all commentors, but not their comments), it should only be possible if the API provides a "normalized" commentors link. If the path comments.authors is used, all intermediate records should be included.

I'm working to get community consensus around this in JSON-API, as that will ultimately control our implementation: https://github.com/json-api/json-api/issues/497

Soooo, long answer short, I believe you should make the client work with the current request shape with the expectation v2 of the API will be much more sensible about this. I hope that's an acceptable answer!

jugglinmike commented 9 years ago

I've followed up on this in gh-104.