Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

Create and Read Work Item Link #322

Closed
aslakknutsen opened this issue Sep 30, 2016 · 18 comments
Closed

Create and Read Work Item Link #322

aslakknutsen opened this issue Sep 30, 2016 · 18 comments
Milestone

Comments

@aslakknutsen
Copy link
Contributor

No description provided.

@kwk
Copy link
Collaborator

kwk commented Sep 30, 2016

Collection some thoughts high level thoughts for the implementation

So in my mind a work item can be associated to more than one other work item. For instance, a work item could be blocked by more than one work item type.

From today's session with @baijum, @pranavgore09, @tsmaeder, and @aslakknutsen I remember that we don't yet put a restriction on the the types of work items that can be associated.

We have the requirement (TODO: PLEASE LINK SOURCE HERE) that we need to be able to see an association from both ends.

Implementation of REST interface for read-operation

From the above thoughts I think, we need to be able to ask the REST interface these questions:

  • Who blocks (replace with concrete association) work item with ID X?
  • Who is blocked by (replace with concrete association) work item with ID X?

Both questions can be represented as actions in the design/resource.go file. Depending on the format of the answer:

  1. collection of work items (as with the list action of the workitem resource)
  2. collection of work item IDs
  3. collection of work item IDs + Titles ( @aslakknutsen mentioned this today )

the number of follow up requests needed to present valuable information to the user can increase dramatically if we don't put enough information on a work item in the response.
In the end it is all a trade-off of having to send too much information (full work items) vs. too much follow-up queries (only send work item IDs).

If I remember correctly, @aslakknutsen mentioned today that unlike Github references, our work items shall display a title in addition to the obligatory ID. If that's what we want to achieve without forcing the UI to issue follow-up queries, we should deliver the ID as well as the title in the response. One thing to remember is that Github, does not display the title of a referenced issue but it shows it when hovering over it. See #322

@joshuawilson, @mindreeper2420 can you share your thoughts on your requirements for mobile first, please?

@aslakknutsen
Copy link
Contributor Author

aslakknutsen commented Sep 30, 2016

@kwk

I believe we said in relation to the UX for 'add association' that there is
no 'association type' at this point; 
https://github.com/almighty/almighty-core/issues/307 

https://github.com/almighty/almighty-core/issues/306 says ID, Title, and Type,
but given we're not doing Type's in this round I personally
would have liked to see State instead.

Updated: Ignore prevtextious text.. :) After some discussion and a New UX proposal 'association type' in 'add' is included: https://redhat.invisionapp.com/share/J38SPOKNA#/screens/193221470 Unknown where 'all' types are defined atm, so still an option to skip it in this sprint from Core.

I don't think we need to answer any questions like the above as a separate endpoint yet. We need to be able to 'create an association' and 'include all associations for a given work item when seeing the work item details'.

Which translates in my mind to something like:

PATCH /api/workitem/N

{  
   "links":[  
      {  
         "href":"/api/workitem/n1"
      }
   ]
}

GET /api/workitem/N

{  
   "fields":{  

   },
   "links":[  
      {  
         "self":"/api/workitem/n1",
         "id":"n1",
         "system.title":"Wee",
         "system.state":"open"
      }
   ]
}

@kwk kwk changed the title Create and Read Work Item Association Create and Read Work Item Link Oct 4, 2016
@kwk
Copy link
Collaborator

kwk commented Oct 4, 2016

@aslakknutsen See #323 (comment) for the discussion about the types.

Based on your answer and the GET /api/workitem/N response I see that we're resolving all relevant info on a linked work item in the response. That's okay.

Since this issue is titled "Create and Read Work Item Link", I assume that updating (e.g. adding a link to an existing set of links) or removing a link shall not be part of the change to fix this issue, right?

I'm asking because we may want to include the internal ID of a link in the list of links in the response. This would allow the UI to refer to a link more precisely, don't you think? This can of course be added later. I just wanted to make sure that it isn't the "id" field from your response. You "id" field is probably intended for the UI to show text somewhere.

@aslakknutsen Does, the "self" field mean that the linked item is an internal work item and not some remote one?

I'm not sure that we should include any of the system.* fields in the GET response. It may seem okay for now. But for close-to-real-time updates (if we ever want this) on the page this might cause us a lot of trouble in the long run. I have dealt with such problems in the past and used meteor to get rid of the REST APIs.

@kwk
Copy link
Collaborator

kwk commented Oct 4, 2016

@aslakknutsen in my mind a work item should contain a links field like you mentioned but I think it should look more like this:

...
"links": [
  {
    "title": "Foo",
    "id": "42",
    "url": "https://depending-if-internal-or-external-work-item/this/will/link/internally/or/externally",
    "workItemType": "system.bug",
    "fields": [
      {
          "system.state": "open"
      }
    ]
  }
]
....

The reason for the fields array is to be able to extend the API without breaking compatibility. If the system.state is not required or some other field may be needed, then we can extend it easily.

Since system.title is a common, yet still a custom type that is not enforced in the system, we should stick to fields that we have marked as required.

@kwk
Copy link
Collaborator

kwk commented Oct 4, 2016

To create a link, it should be possible to deduce from the given string if something is an remote issue or an internal one. Don't you think?

@aslakknutsen
Copy link
Contributor Author

The reason for the fields array is to be able to extend the API without breaking compatibility. If the system.state is not required or some other field may be needed, then we can extend it easily.

Yeah, sorry. That was me being lazy in my example. The WI part of the Link should serialize in the same way as the WI itself(even tho only partly) including the 'fields' attribute.

@aslakknutsen
Copy link
Contributor Author

To create a link, it should be possible to deduce from the given string if something is an remote issue or an internal one. Don't you think?

I don't think this endpoint should deal with anything beyond a link between two existing work items. 'Something' else should deal with discovery / auto creation of the target(if external and found as RemoteWorkItem?) / search for existing Work items. Parts of it could be here; #330

@aslakknutsen
Copy link
Contributor Author

Since this issue is titled "Create and Read Work Item Link", I assume that updating (e.g. adding a link to an existing set of links) or removing a link shall not be part of the change to fix this issue, right?

The story(#307) only talks about Creating a link. Nothing more as far as I can tell.

e.g. adding a link to an existing set of links

Not sure what you define as a 'set' of links here, but having more than 1 link per work item sounds reasonable within the scope. Not sure more than 1 link of the same type between the same work items make any sense tho. (really really blocked by this work item?)

@aslakknutsen
Copy link
Contributor Author

I'm asking because we may want to include the internal ID of a link in the list of links in the response. This would allow the UI to refer to a link more precisely, don't you think? This can of course be added later. I just wanted to make sure that it isn't the "id" field from your response. You "id" field is probably intended for the UI to show text somewhere.

You are correct. The 'link' should be a resource in of itself, so my self link in the example is a bit miss leading. There are actually two links, one to manipulate the relationship and one to the target resource..

The JSONAPI spec defines this as:

"relationships": {
      "blocked_by": {
        "links": {
          "self": "/workitems/1/relationships/2421",
          "related": "/workitems/2"
        },
        "fields": { ... }
      }
}

@aslakknutsen
Copy link
Contributor Author

I'm not sure that we should include any of the system.* fields in the GET response. It may seem okay for now. But for close-to-real-time updates (if we ever want this) on the page this might cause us a lot of trouble in the long run. I have dealt with such problems in the past and used meteor to get rid of the REST APIs.

Not following here. You mean don't include 'fields' as part of the link? You want to call it 'title' on the link instead, and that is different why? What problems did you get into?

@kwk
Copy link
Collaborator

kwk commented Oct 5, 2016

@aslakknutsen

Not following here. You mean don't include 'fields' as part of the link? You want to call it 'title' on the link instead, and that is different why? What problems did you get into?

Don't worry, I was thinking about web sockets. Forget what I wrote.

@kwk
Copy link
Collaborator

kwk commented Oct 10, 2016

*Updated after discussion with @michaelkleinhenz. Now this includes a full CRUD API idea. *

@michaelkleinhenz here're some thoughts on the API design I have started to implement. If they will change, I'll let you know.

CRUD work item link

Summary

Create: POST api/workitems/:id/links
Read all: GET api/workitems/:id/links
Read single: GET api/workitems/:id/links/:linkid
Update: PUT api/workitems/:id/links/:linkid
Delete: DELETE api/workitems/:id/links/:linkid

Create

HTTP method: POST
endpoint: api/workitems/:id/links
Request payload example:

{
  "type": "The type of the newly created work item link (e.g. blocker)",
  "linkedWorkItem": "ID of the linked work item"
}

Response payload example (success):

{
    "linkId": "ID for the link",
    "workItemId": "ID of the linked workitem",
    "workItemType": "Name of the type of the linked work item",
    "url": "URL of the linked workitem (is this redundant)",
    "fields": [
        {"system.title": "Title of the linked work item"},
        {"system.state": "open"},
    ]
}

The fields array of a work item link can contain arbitrary key-value information for the linked work item.
This is most likely defined by the customer.

Read

In principle, for now (!!), the work item show action (GET api/workitems/:id) contains a JSON-field array-field called links. This will contain all the links for this work item (for now).

List all work item links

To list all links for a work item without the overhead of all additional work item information (as fetched by GET api/workitems/:id).

HTTP method: GET
endpoint: api/workitems/:id/links
Response payload example (success): An array of work item link objects.

[
  {
      "linkId": "ID for the link",
      "workItemId": "ID of the linked workitem",
      "workItemType": "Name of the type of the linked work item",
      "url": "URL of the linked workitem (is this redundant)",
      "fields": [
          {"system.title": "Title of the linked work item"},
          {"system.state": "open"},
      ]
  }
]

List one work item link

To fetch a specific work item link object given by a linkid.

HTTP method: GET
endpoint: api/workitems/:id/links/:linkid
Request payload example (success): A work item link object.

{
    "linkId": "ID for the link",
    "workItemId": "ID of the linked workitem",
    "workItemType": "Name of the type of the linked work item",
    "url": "URL of the linked workitem (is this redundant)",
    "fields": [
        {"system.title": "Title of the linked work item"},
        {"system.state": "open"},
    ]
}

Update work item

HTTP method: PUT
endpoint: api/workitems/:id/links/:linkid
Request payload example:

{
  "type": "The type of the newly created work item link (e.g. blocker)",
  "linkedWorkItem": "ID of the linked work item"
}

Response payload example (success):

{
    "linkId": "ID for the link",
    "workItemId": "ID of the linked workitem",
    "workItemType": "Name of the type of the linked work item",
    "url": "URL of the linked workitem (is this redundant)",
    "fields": [
        {"system.title": "Title of the linked work item"},
        {"system.state": "open"},
    ]
}

Delete work item

HTTP method: DELETE
endpoint: api/workitems/:id/links/:linkid

@aslakknutsen
Copy link
Contributor Author

@kwk Missing a level on the PATCH call? Looks like "type" there would overwrite the WorkItem.Type.

@kwk Missing type from Read? Why title as oppose to fields."system.title"? url = self? workItemType is called Type in the main object representation.

@kwk
Copy link
Collaborator

kwk commented Oct 10, 2016

@aslakknutsen I've updated the comments. Now we have a deeper level in the HTTP endpoint: api/workitems/:id/links.

It is not finished but the endpoints and HTTP methods seem to work good this way.

@michaelkleinhenz
Copy link
Collaborator

@kwk suggestion: "linkedWorkItem" should be named "linkedWorkItemId" as it is the id, not the actual work item.

@michaelkleinhenz
Copy link
Collaborator

@kwk the "fields" should be an object, not an array of objects I think. That would match the "field" in WIs.

@kwk
Copy link
Collaborator

kwk commented Oct 28, 2016

Please see my design proposal: fabric8-services/fabric8-devdoc#86

@kwk kwk mentioned this issue Nov 2, 2016
@aslakknutsen aslakknutsen modified the milestones: Sprint #123, Sprint #122 Nov 9, 2016
kwk added a commit that referenced this issue Nov 28, 2016
This change adds the first incarnation of work item links.

## Glossary

### work item link
A link describes a _bidrectional_ relationship between two work items. That
means there's a defined *source* work item and a *target* work item in a link.
Their work item type is relevant. This allows us to create a parent-child
relationship among work items, like the one between an _epic_ and a
_user-story_, like we've explained in the above paragraph.

To realize this concept of a relationship between a source and a target in the
underlying storage, we will have to define a *link type*.

### work item link category
A *link type* can have a category like `"system"`, `"extension"`, or `"user"`.

**IMPORTANT:** A user can only create, update, or delete, links in the `"user"`
link-category. Needless to say that a user can only create, update, or delete
link types with the link category `"user"`. This is a security mechanism to
prevent the user from breaking the system.

### work item link type
A link type defines what work item types can be linked together and how their
relationship can be described.

#### topology
The topology determines the restrictions placed on the usage of each work item
link type. Currently only the topology `"network"` is allowed.

**NOTE:** The core currently doesn't apply the restrictions derived from a
certain topology. This will have to be added later.

#### source
The source defines where the relationship between two work items starts. It
given as a work item ID. The type of work item that can be used as a source is
defined by the source type.

##### source type
The source type specifies the type of work item that can be used as a source.
Any sub-type of the source type is also allowed.

#### target
The target defines where the relationship between two work items ends. It is
given as a work item ID. The type of work item that can be used as a target is
defined by the target-type.

##### target type
The target type specifies the type of work item that can be used as a target.
Any sub-type of the target type is also allowed.

### forward name
The forward oriented path from source _to_ target is described with the _forward
name_. For example, if a bug _blocks_ a user story, the forward name is
`"blocks"`. See also *reverse name*.

### reverse name
The backwards oriented path from target _to_ source is described with the
_reverse name_. For example, if a bug _blocks_ a user story, the reverse name
name is `"blocked by"` as in: a user story is _blocked by_ a bug. See also
*forward name*.

### link comment (Currently not implemented)
A link comment is an optional string field on a link that allows the creator of
a link to further specify the relationship. For example, think of a fictional
`'test'` subsystem that wants to create a link between one of its `test-result`
work items and a `pull-request` work item. For convenience, it makes sense to
specify a string like: `"The code from pull request X passed the tests in test
Y."`.

## Defaults

By default, these work item link categories and types are created for the UI to
start with while there's not UI to create link types and link categories. We can
add more any time.

* link categories:
  * name: **`"system"`**
    * description: `"The system category is reserved for link types that are to
      be manipulated by the system only."`
  * name: **`"user"`**
    * description: `"The user category is reserved for link types that can to be
      manipulated by the user."`
* link types
  * name: **`"Bug blocker"`**
    * description: `"One bug blocks a planner item."`
    * source type: `"system.bug"`
    * target type: `"system.bug"`
    * forward name: `"blocks"`
    * reverse name: `"blocked by"`
    * topology: `"network"`
    * link category: `"system"`
  * name: **`"Related planner item"`**
    * description: `"One planner item or a subtype of it relates to another
      one."`
    * source type: `"system.planneritem"`
    * target type: `"system.planneritem"`
    * forward name: `"relates to"`
    * reverse name: `"relates to"`
    * topology: `"network"`
    * link category: `"system.system"`
 

## New resource actions

These actions (full CRUD) can be performed on each of the newly added resources:

* create,
* read (single),
* update (single),
* delete (single),
* list (multiple)

All actions are covered with tests.

## JSONAPI

This change also introduces [JSONAPI](http://jsonapi.org/format/) as a format
for request and response payloads. The newly added resources consume and produce
only JSONAPI, wheras all other resources now produce JSONAPI **errors** whenever
an error occurred. This change was required because I had to replace the normal
error handler with a JSONAPI specific one.

So far we implement only the minimal basic JSONAPI. That means we follow the
specification where it says that the server **MUST** do certain things.
Everything that is optional is probably not covered in this change.

## Other changes

I've unified the unauthorized CUD tests to various endpoints to avoid code
duplication in tests.

## Open topics for upcoming changes

### Inclusion of relationships

Currently you have to do multiple queries to get all information about a link.
To get the category for example, you have to query the type first in order to
get the key for the category.

In a next step we can add the [inclusion of
resources](http://jsonapi.org/format/#fetching-includes). This should greatly
simplify things for the UI.

**UPDATE:** The create, update, show, and list action of the work item link type
actually **do include** the link category/ies in the `"included"` top-level
array of the JSONAPI response.

### Pagination

Currently the list action for work item links returns all links and we might
have to add pagination sooner than later.

### Better link objects

Currently there are no `link` objects in the JSONAPI responses. These can be
spread all over the place to get more hyperlinking.

### Linking against work item subtypes

Currently you cannot create a `"related"` link type that allows you to link any
work item type. You'd have to create a link type for every combination of work
item types you want to create links for. We can extend the current
implementation with additional attributes to get started with this.

### Show links for a work item

We need to extend the workitem endpoint to include the work item links for that
particular work item.

--

This relates to #322 and #323.

To see what was planned to be covered in this change, see [this
PR](fabric8-services/fabric8-devdoc#86) . Some endpoints might
have changed but where the manual hand-written documentation is wrong, the
[swagger definition](swagger.goa.design/?url=almighty%2Falmighty-core%2Fdesign)
can help out.
@kwk
Copy link
Collaborator

kwk commented Nov 28, 2016

Closed by #421

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants