API=PATCH:WorkItem.2 (follow jsonapi spec) #482

pranavgore09 · 2016-11-11T12:37:04Z

Side effect fixes #466 (How to use - please see ui#348 )

WorkItem2 :- Patch the work item. (Allow partial updates & do not change WIT)
Follows jsonapi spec to define input payload for PATCH call.


{
   "data": {
      "attributes": {
         "system.state": "new",
         "system.title": "Example story",
         "version": "1"
      },
      "id": "42",
      "relationships": {
         "assignee": {
            "data": {
               "id": "6c5610be-30b2-4880-9fec-81e4f8e4fd76",
               "type": "identities"
            }
         },
         "baseType": {
            "data": {
               "id": "system.userstory",
               "type": "workitemtypes"
            }
         }
      },
      "type": "workitems"
   }
}

Following attributes are mandatory for every update request with correct value.

data.id
data.type
data.attributes.version

This payload is constructed for updating WI.
Above structure will update work item(if exists) with id 42 and set it's assignee to given UUID if it exists in system.

aslakknutsen · 2016-11-11T16:59:35Z

I believe fields == attributes

"data": {
  "attributes": {
     "system.creator": "user-ref",
     "system.state": "new",
     "system.title": "Example story",
     "version": 5
  },
  "id": "42",
}

pranavgore09 · 2016-11-12T03:15:45Z

@aslakknutsen , That is possible, but then WIT type and Version will be on the same level of title,description etc, state, etc. That differs from model definition. I understand jsonapi spec do not say to follow model definition but...

// WorkItem represents a work item as it is stored in the database
type WorkItem struct {
    gormsupport.Lifecycle
    ID uint64 `gorm:"primary_key"`
    // Id of the type of this work item
    Type string
    // Version for optimistic concurrency control
    Version int
    // the field values
    Fields Fields `sql:"type:jsonb"`
}

codecov-io · 2016-11-12T03:31:28Z

Current coverage is 67.52% (diff: 84.29%)

Merging #482 into master will increase coverage by 0.79%

@@             master       #482   diff @@
==========================================
  Files            62         63     +1   
  Lines          3286       3403   +117   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           2193       2298   +105   
- Misses          857        862     +5   
- Partials        236        243     +7

Powered by Codecov. Last update 9bd5c82...519dbaf

aslakknutsen · 2016-11-12T04:01:51Z

@pranavgore09 Sure, Model != REST.

Version belong more as a ETag on HTTP Header level Move usage of optimistic locking 'version' out of payload #155
Do we even want Version? Decide on Concurrency Strategy #420

pranavgore09 · 2016-11-16T08:48:33Z

@aslakknutsen @tsmaeder I have addressed above comments except versioning. I have not tried etag or another similar in this commit. So the new payload looks like -

{
   "data": {
      "attributes": {
         "system.state": "new",
         "system.title": "Example story",
         "type": "system.userstory",
         "version": "1"
      },
      "id": "42",
      "relationships": {
         "assignee": {
            "data": {
               "id": "6c5610be-30b2-4880-9fec-81e4f8e4fd76",
               "type": "identities"
            }
         }
      },
      "type": "workitems"
   }
}

As of now. attributes can accept anything. But version and type are mandatory and validated via our code.

Should we rename type to wiType in REST payload ? considering jsonapi spec is already having type as reserved word kind of.

aslakknutsen · 2016-11-16T09:15:18Z

Type should be a relationship.

On Wed, Nov 16, 2016, 08:48 Pranav Gore notifications@github.com wrote:

@aslakknutsen https://github.com/aslakknutsen @tsmaeder
https://github.com/tsmaeder I have addressed above comments except
versioning. I have not tried etag or another similar in this commit. So
the new payload looks like -

{
"data": {
"attributes": {
"system.state": "new",
"system.title": "Example story",
"type": "system.userstory",
"version": "1"
},
"id": "42",
"relationships": {
"assignee": {
"data": {
"id": "6c5610be-30b2-4880-9fec-81e4f8e4fd76",
"type": "identities"
}
}
},
"type": "workitems"
}
}

As of now. attributes can accept anything. But version and type are
mandatory and validated via our code.

Should we rename type to wiType in REST payload ? considering jsonapi spec
is already having type as reserved word kind of.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#482 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAIEPvwqLVXl7GvLG2VXGiEFBygAKP9jks5q-sNhgaJpZM4Kvta0
.

pranavgore09 · 2016-11-16T15:32:26Z

type is taken out of fields and made it as a relationship

{
   "data": {
      "attributes": {
         "system.state": "new",
         "system.title": "Example story",
         "version": "1"
      },
      "id": "42",
      "relationships": {
         "assignee": {
            "data": {
               "id": "6c5610be-30b2-4880-9fec-81e4f8e4fd76",
               "type": "identities"
            }
         },
         "baseType": {
            "data": {
               "id": "system.userstory",
               "type": "workitemtypes"
            }
         }
      },
      "type": "workitems"
   }
}

ToDo:

add tests all around patch API

pranavgore09 · 2016-11-17T02:58:53Z

@aslakknutsen @tsmaeder : If I go with etag for versions, it need to have GET call, so I am keeping version as it is for now in Attributes, and this PR will do update of WI only. Else it will become big/clumsy. Once this is in master I will start off GET WI using workitem.2 jsonapi

Currently our path URL is - http://localhost:8080/api/workitems.2/1 in which I am concerned about id field in URL.
jsonapi spec says that ID should be in data along with type.
So, we can remove id in URL, but then item exists or not check will be done in our code and not from goadesign ,any suggestions on this ?
--- Just learnt that goadesign is not performing any check, so I am updating code to use payload.data..id only. So, ID will be present in URL but will not be consumed as of now.

sbose78 · 2016-11-17T04:28:01Z

design/user_types.go

+	a.Attribute("type", d.String, func() {
+		a.Enum("workitems")
+	})
+	a.Attribute("id", d.String, "ID of the work item which is being updated", func() {


Just like in https://github.com/almighty/almighty-core/blob/master/design/user_types.go#L25
we do not include the ID in the payload because we do it allow updating the ID, and that it could be retrieved from the REST URL, I am wondering if we should include it here.

yes @sbose78 , you are right, I have mentioned that here
But ID should be in payload as per specifications of jsonapi.org mentioned here

Got it, Thanks. If the spec needs it, we would need to keep it.
Could we ensure that the workitem doesn't get updated with whatever ID is present in the payload? Or, are we validating that the same ID is present ( as that in the rest url ) ?

good point, I checked goadesign is not performing any db check for existence of item, we are doing that in repository methods. So no need to validate if they are same, we will use only from payload.
this spec tells us that ID should be present in url as well as payload.
Thanks, I will update same. https://github.com/almighty/almighty-core/pull/482/files#diff-d2f956a730e7c36b29bc0dceedd367bdR173 should be updated.

sbose78 · 2016-11-17T04:33:36Z

models/workitem2_repository.go

+		uuidStr := assigneeData.ID
+		assigneeUUID, err := uuid.FromString(uuidStr)
+		if err != nil {
+			return nil, NewBadParameterError("data.relationships.assignee.data.id should be UUID", uuidStr)


The first argument of the NewBadParameterError should be the paramater name.
The error message would be constructed internally.

thanks, done in dbc7935

sbose78 · 2016-11-17T04:33:41Z

models/workitem2_repository.go

+		}
+		_, err = identityRepo.Load(ctx, assigneeUUID)
+		if err != nil {
+			return nil, NewBadParameterError("data.relationships.assignee.data.id not found", uuidStr)


The first argument of the NewBadParameterError should be the paramater name.
The error message would be constructed internally.

thanks, done in dbc7935

pranavgore09 · 2016-11-17T11:01:44Z

@aslakknutsen , should we allow user to update type of WI in update API ?
Use case is change WI from bug to userstory.
If yes, while updating we need to check for fields only, or any other part should get updated ?

In current scenario of PATCH, I have made type as a relationship. And it is required as of now, which I will be changing.
If provided in payload then we will update type else we will use existing type of the WI and check for fields if they are present or not as described by new type.

pranavgore09 · 2016-11-21T11:18:04Z

Response for PATCH WI -

{
    "data": {
        "attributes": {
            "system.creator": "d8958274-f741-4558-b117-37cb2ed50d09",
            "system.description": null,
            "system.remote_item_id": null,
            "system.state": "open",
            "system.title": "cool title",
            "version": 8
        },
        "id": "1",
        "relationships": {
            "assignee": {
                "data": {
                    "id": "75743e50-641c-4ca5-a10d-9ae824f752f6",
                    "type": "identities"
                }
            },
            "baseType": {
                "data": {
                    "id": "system.userstory",
                    "type": "workitemtypes"
                }
            }
        },
        "type": "workitems"
    },
    "links": {
        "self": "http://localhost:8080/api/workitems.2/1"
    }
}

aslakknutsen · 2016-11-21T11:37:15Z

Doesn't have to be part of this, but system.creator is a relationship as well. And technically the relationships should have a link to the target (we don't have a single User service atm, but we do have a Type endpoint)

aslakknutsen · 2016-11-21T22:05:35Z

@pranavgore09 Have you looked at this part of the spec? http://jsonapi.org/format/#crud-updating-relationships

pranavgore09 · 2016-11-22T04:12:34Z

@aslakknutsen yes and that introduces new endpoint of CRUD API for assignee relation. And while doing that we have version attribute which crucial for update. I have added this comment where I mentioned that not doing etag as part of this set assignee.
We decided to use patch for modifying assignee and earlier it was part of Attributes then moved to relationship. now this PR makes update api according to jsonapi spec which is not needed to set assignee.
Should I revert all code above ?

pranavgore09 · 2016-11-22T04:28:56Z

Just now I have pushed the code for removing assignee because it was already done.
But I can revert to include new relationship APIs instead of patch call altogether.

pranavgore09 · 2016-11-22T05:45:44Z

@aslakknutsen IMO, as this API is working and integrated here. I keep this codebase as it is for updating WI.
And will add relationships UPDATE and DELETE(means patch with data=nil).

May be I am missing some point, but how can I manage version while update relationship assignee.
-- I can put version in Meta top level object for PATCH.

baijum · 2016-11-23T08:15:03Z

design/user_types.go

@@ -38,6 +38,71 @@ var UpdateWorkItemPayload = a.Type("UpdateWorkItemPayload", func() {
 	a.Required("type", "fields", "version")
 })

+// UpdateWorkItemJSONAPIPayload defines top level structure from jsonapi specs
+// visit : http://jsonapi.org/format/#document-top-level
+var UpdateWorkItemJSONAPIPayload = a.Type("UpdateWorkItemJSONAPIPayload", func() {


Do we need to export names?

thanks, done in 22a20be

baijum · 2016-11-23T08:15:12Z

design/user_types.go

+})
+
+// WorkItemDataForUpdate defines how an update payload will look like
+var WorkItemDataForUpdate = a.Type("WorkItemDataForUpdate", func() {


Do we need to export names?

thanks, done in 22a20be

baijum · 2016-11-23T08:16:03Z

design/user_types.go

+})
+
+// WorkItemRelationships defines only `assignee` as of now. To be updated
+var WorkItemRelationships = a.Type("WorkItemRelationships", func() {


Do we need to export names?

thanks, done in 22a20be

pranavgore09 · 2016-11-24T05:22:05Z

@aslakknutsen @baijum

{"data": {"attributes": {"version": "2", "unkown_attribute": "some value"},"id": "2","type": "workitems"}}

For input like this, when unknown attributes are added in PATCH, that unknown attribute will not be added to DB or anywhere else, but version get plus one.

Currently attributes is map[string]interface{} so no control over it, no code verifies attribute names are valid or not.

Any suggestions ?

tsmaeder · 2016-11-24T14:24:38Z

Mmhh...WokItems! Fixed title ;-)

pranavgore09 · 2016-11-24T17:00:32Z

Oops, it took me a while to get the difference :P thanks @tsmaeder

…s.2 api

Now the code is not considering ID from URL to perform any action. But we need to keep ID in URL as per specification by - http://jsonapi.org/format/#crud-updating

"relationship.BaseType" is not manadatory Do not allow to update Type of WI as of now Use existing type of WI to verify fields remove testing functions of creating user update affected test case

Added "links" in top level of response Added new method ConvertWorkItemToJSONAPI for WIcontroller Tests updated to absorb jsonapi response ToDo: Remove assignee

pass null ID for assignee relationship to remove existing assignment

Set/Unset Assignee Invalid UUID for assignee Invalid ID of WI Update only title Update only description

Use require instead of assert where needed.

pranavgore09 · 2016-11-30T09:12:53Z

@tsmaeder hmm yes, okay but UI is already using new List method.
If this work is added to workitem as a patch action then workitem resource will have mixture of non-jsonpai and jsonapi actions.
Hence IMHO I feel we should keep all jsonapi based apis in workitem.2 and we will rename it to workitem once UI completely shifts to new APIs. I understand UI needs to re-work on URLs but I think it should be easily configurable. Also putting jsonapi based changes in workitem.2 makes the resource consistent. Also we will have create/delete actions based on jsonapi spec which will go in workitem.2 only.

kwk

Tests are looking much better and cleaned up now. Please address the other minor issues.

kwk · 2016-11-30T10:13:02Z

design/user_types.go

@@ -457,3 +522,10 @@ var RelationWorkItemData = a.Type("RelationWorkItemData", func() {
 	})
 	a.Required("type", "id")
 })
+
+// WorkItemLinks has `self` as of now according to http://jsonapi.org/format/#fetching-resources
+var WorkItemLinks = a.Type("WorkItemLinks", func() {


This name might be misleading because work item links are the once that I implemented in PR #421

agreed, will WorkItemResourceLinksForJSONAPI sound okay ?
done in 519dbaf#diff-aa2cec7dc60fd15e23552b59d51fdbdcR527

kwk · 2016-11-30T10:14:14Z

models/workitem2_repository.go

+			return nil, NewBadParameterError("version", version)
+		}
+	} else {
+		return nil, VersionConflictError{simpleError{"version is mandatory"}}


Please use NewVersionConflictError

thanks done in 519dbaf#diff-d778d1bb58fa049a00282e997d150670R54

kwk · 2016-11-30T10:14:23Z

models/workitem2_repository.go

+		return nil, VersionConflictError{simpleError{"version is mandatory"}}
+	}
+	if res.Version != version {
+		return nil, VersionConflictError{simpleError{"version conflict"}}


Please use NewVersionConflictError

thanks done in 519dbaf#diff-d778d1bb58fa049a00282e997d150670R57

kwk · 2016-11-30T10:15:13Z

models/workitem2_repository.go

+
+	rel := wi.Relationships
+	// TODO
+	// if rel.Assignee.Data == nil then remove the relationship for WI


So removing an assignee isn't possible at the moment?

oops, need to remove this comment in code.
assignee is removed when ID=nil 519dbaf#diff-d778d1bb58fa049a00282e997d150670R74

kwk · 2016-11-30T10:15:51Z

models/workitem2_repository.go

+
+	if err := tx.Save(&newWi).Error; err != nil {
+		log.Print(err.Error())
+		return nil, InternalError{simpleError{err.Error()}}


Please use NewInternalError

done in 519dbaf#diff-d778d1bb58fa049a00282e997d150670R112

kwk · 2016-11-30T10:34:07Z

workitem_blackbox_test.go

+	assert.Equal(s.T(), updatedWI.Data.Attributes[models.SystemDescription], modifiedDescription)
+}
+
+func (s *WorkItem2Suite) TestWI2UpdateMultipleScenarios() {


Why not put each scenario in a distinct test method?

Each scenario is already covered in a separate method above.
This is just series of actions taken back to back by updating version as needed.

kwk · 2016-11-30T10:34:47Z

design/resources.go

+			a.Media(workItem2)
+		})
+		//ToDo: Error responses need modifications as per https://github.com/almighty/almighty-core/pull/421
+		a.Response(d.BadRequest, func() {


Use a.Response(d.BadRequest, JSONAPIErrors) instead.

thanks done in 519dbaf#diff-5147d84da6b10c041a0bafd5935f9f4fR128

kwk · 2016-11-30T10:35:02Z

design/resources.go

+		a.Response(d.BadRequest, func() {
+			a.Media(d.ErrorMedia)
+		})
+		a.Response(d.InternalServerError)


Use a.Response(d.InternalServerError, JSONAPIErrors) instead.

thanks done in 519dbaf#diff-5147d84da6b10c041a0bafd5935f9f4fR129

kwk · 2016-11-30T10:35:20Z

design/resources.go

+			a.Media(d.ErrorMedia)
+		})
+		a.Response(d.InternalServerError)
+		a.Response(d.NotFound)


Use a.Response(d.NotFound, JSONAPIErrors) instead.

thanks done in 519dbaf#diff-5147d84da6b10c041a0bafd5935f9f4fR130

kwk · 2016-11-30T10:35:32Z

design/resources.go

+		})
+		a.Response(d.InternalServerError)
+		a.Response(d.NotFound)
+		a.Response(d.Unauthorized)


Use a.Response(d.Unauthorized, JSONAPIErrors) instead.

thanks done in 519dbaf#diff-5147d84da6b10c041a0bafd5935f9f4fR131

pranavgore09

Thanks @kwk for another detailed review, please update me something is not as expected.
Added comments.
Please check my responses in here

pranavgore09 · 2016-11-30T15:46:39Z

design/resources.go

+			a.Media(workItem2)
+		})
+		//ToDo: Error responses need modifications as per https://github.com/almighty/almighty-core/pull/421
+		a.Response(d.BadRequest, func() {


thanks done in 519dbaf#diff-5147d84da6b10c041a0bafd5935f9f4fR128

pranavgore09 · 2016-11-30T15:46:48Z

design/resources.go

+		a.Response(d.BadRequest, func() {
+			a.Media(d.ErrorMedia)
+		})
+		a.Response(d.InternalServerError)


thanks done in 519dbaf#diff-5147d84da6b10c041a0bafd5935f9f4fR129

pranavgore09 · 2016-11-30T15:46:51Z

design/resources.go

+			a.Media(d.ErrorMedia)
+		})
+		a.Response(d.InternalServerError)
+		a.Response(d.NotFound)


thanks done in 519dbaf#diff-5147d84da6b10c041a0bafd5935f9f4fR130

pranavgore09 · 2016-11-30T15:46:54Z

design/resources.go

+		})
+		a.Response(d.InternalServerError)
+		a.Response(d.NotFound)
+		a.Response(d.Unauthorized)


thanks done in 519dbaf#diff-5147d84da6b10c041a0bafd5935f9f4fR131

pranavgore09 · 2016-11-30T15:47:00Z

design/user_types.go

@@ -457,3 +522,10 @@ var RelationWorkItemData = a.Type("RelationWorkItemData", func() {
 	})
 	a.Required("type", "id")
 })
+
+// WorkItemLinks has `self` as of now according to http://jsonapi.org/format/#fetching-resources
+var WorkItemLinks = a.Type("WorkItemLinks", func() {


agreed, will WorkItemResourceLinksForJSONAPI sound okay ?
done in 519dbaf#diff-aa2cec7dc60fd15e23552b59d51fdbdcR527

pranavgore09 · 2016-11-30T15:47:35Z

workitem.2.go

+				BaseType: &app.RelationshipBaseType{
+					Data: &app.BaseTypeData{
+						ID:   wi.Type,
+						Type: "workitemtypes",


Added this constant and now using it in 519dbaf#diff-d2f956a730e7c36b29bc0dceedd367bdR192

pranavgore09 · 2016-11-30T15:47:38Z

workitem.2.go

+				op.Data.Relationships.Assignee = &app.RelationAssignee{
+					Data: &app.AssigneeData{
+						ID:   &valStr,
+						Type: "identities",


done in 519dbaf#diff-d778d1bb58fa049a00282e997d150670R18

pranavgore09 · 2016-11-30T15:47:42Z

workitem.2.go

+		}
+		wi, err := appl.WorkItems2().Save(ctx, toSave)
+		if err != nil {
+			switch err := err.(type) {


big help, thanks. will consume that.
Done that in 519dbaf#diff-d2f956a730e7c36b29bc0dceedd367bdR232

pranavgore09 · 2016-11-30T15:47:50Z

workitem_blackbox_test.go

+	assert.Equal(s.T(), updatedWI.Data.Attributes[models.SystemDescription], modifiedDescription)
+}
+
+func (s *WorkItem2Suite) TestWI2UpdateMultipleScenarios() {


Each scenario is already covered in a separate method above.
This is just series of actions taken back to back by updating version as needed.

pranavgore09 · 2016-11-30T16:06:58Z

models/workitem2_repository.go

+				return nil, NewBadParameterError("data.relationships.assignee.data.id", uuidStr)
+			}
+			wi.Attributes[SystemAssignee] = *uuidStr
+			//  ToDO : make it a list and append


Yes I understand that,
But this will extend the scope of task to much extent, because it needs change in type of assignee in WI, then change in input, change in assignment logic, and while reading the WorkItem. Plus adding tests for the same. And then review on PR.
So most likely I will end up not doing it complete 100% (merge in master) by tomorrow.
And it is not blocker so....can that be done out of this PR ?
any suggestion @aslakknutsen ?

Using constant got "workitemtypes" Using NewInternalError instead of InternalError Using NewVersionConflictError

kwk · 2016-12-01T08:40:49Z

design/user_types.go

@@ -524,7 +524,7 @@ var RelationWorkItemData = a.Type("RelationWorkItemData", func() {
 })

 // WorkItemLinks has `self` as of now according to http://jsonapi.org/format/#fetching-resources


The comment doesn't reflect the new name for the type.

Concerns addressed

pranavgore09 force-pushed the iss-466 branch from 0adee00 to 293ddd7 Compare November 12, 2016 03:18

sbose78 reviewed Nov 17, 2016

View reviewed changes

pranavgore09 force-pushed the iss-466 branch from f6b9613 to a95cf7c Compare November 18, 2016 18:01

pranavgore09 force-pushed the iss-466 branch from c73c00d to d09c26a Compare November 22, 2016 04:26

pranavgore09 force-pushed the iss-466 branch from d09c26a to 83a87c5 Compare November 22, 2016 17:04

baijum previously requested changes Nov 23, 2016

View reviewed changes

DhritiShikhar mentioned this pull request Nov 23, 2016

WorkItem REST Api should support PATCH for partial fields/content update #48

Closed

pranavgore09 mentioned this pull request Nov 24, 2016

Use new WI-Update api ie. PATCH=/api/workitems/WI_ID fabric8-ui/fabric8-planner#348

Closed

pranavgore09 force-pushed the iss-466 branch from d252dd5 to cd65348 Compare November 24, 2016 11:29

tsmaeder changed the title ~~WIP :- Set assignee while patching WokItem (follow jsonapi spec)~~ WIP :- Set assignee while patching WorkItem (follow jsonapi spec) Nov 24, 2016

pranavgore09 changed the title ~~WIP :- Set assignee while patching WorkItem (follow jsonapi spec)~~ API=PATCH:WorkItem.2 (follow jsonapi spec) Nov 25, 2016

pranavgore09 added the jsonapi label Nov 25, 2016

pranavgore09 added 16 commits November 30, 2016 14:33

Include parameter name and not he message.

8716de8

Modify Security related test to absorb test inputs for PATCH workitem…

1ce6f78

…s.2 api

Use ID from payload instead of URL.

e479772

Now the code is not considering ID from URL to perform any action. But we need to keep ID in URL as per specification by - http://jsonapi.org/format/#crud-updating

add test for non-uuid strings in assignee relationship

a6aea7e

make attributes a map of string to interface

30351b8

"relationship.BaseType" is not manadatory Do not allow to update Type of WI as of now Use existing type of WI to verify fields remove testing functions of creating user update affected test case

Response type for PATCH WI is now as per jsonapi specs

955a86f

Added "links" in top level of response Added new method ConvertWorkItemToJSONAPI for WIcontroller Tests updated to absorb jsonapi response ToDo: Remove assignee

Allow to remove assignee of the WI.

6c676cc

pass null ID for assignee relationship to remove existing assignment

Added Tests around update workitems.2 api

27c4ac9

Set/Unset Assignee Invalid UUID for assignee Invalid ID of WI Update only title Update only description

No need of exporting user types.

3f449d9

Add test to update sinlge attribute - State

bd29e79

Small fixes on documentation/grammer.

d28d752

Use constants from http module instead of hard code values.

6723d7a

Use constants for "types" required in jsonapi

12420c2

Upstream master merge and related changes.

5e701aa

WIP :- Using suite to test the WI-update API.

377aefb

Use "Suite" for testing work item new UPDATE action.

e56bd7e

Use require instead of assert where needed.

pranavgore09 force-pushed the iss-466 branch from 754bf4e to e56bd7e Compare November 30, 2016 09:03

kwk suggested changes Nov 30, 2016

View reviewed changes

pranavgore09 force-pushed the iss-466 branch from de28334 to 5854216 Compare November 30, 2016 15:37

pranavgore09 commented Nov 30, 2016

View reviewed changes

Using JSONAPI_Errors.

519dbaf

Using constant got "workitemtypes" Using NewInternalError instead of InternalError Using NewVersionConflictError

pranavgore09 force-pushed the iss-466 branch from 5854216 to 519dbaf Compare November 30, 2016 16:35

This was referenced Nov 30, 2016

Core should support multi assignee #534

Closed

New dedicated API for Relationship Update (jsonapi-relationship-assignee) #513

Closed

kwk reviewed Dec 1, 2016

View reviewed changes

kwk approved these changes Dec 1, 2016

View reviewed changes

Comment fixed.

a381705

aslakknutsen merged commit 319ff70 into fabric8-services:master Dec 1, 2016

		@@ -524,7 +524,7 @@ var RelationWorkItemData = a.Type("RelationWorkItemData", func() {
		})

		// WorkItemLinks has `self` as of now according to http://jsonapi.org/format/#fetching-resources

API=PATCH:WorkItem.2 (follow jsonapi spec) #482

API=PATCH:WorkItem.2 (follow jsonapi spec) #482

Conversation

pranavgore09 commented Nov 11, 2016 • edited

aslakknutsen commented Nov 11, 2016 • edited

pranavgore09 commented Nov 12, 2016

codecov-io commented Nov 12, 2016 • edited

Current coverage is 67.52% (diff: 84.29%)

aslakknutsen commented Nov 12, 2016

pranavgore09 commented Nov 16, 2016

aslakknutsen commented Nov 16, 2016

pranavgore09 commented Nov 16, 2016

pranavgore09 commented Nov 17, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranavgore09 commented Nov 17, 2016

pranavgore09 commented Nov 21, 2016

aslakknutsen commented Nov 21, 2016

aslakknutsen commented Nov 21, 2016

pranavgore09 commented Nov 22, 2016 • edited

pranavgore09 commented Nov 22, 2016

pranavgore09 commented Nov 22, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranavgore09 commented Nov 24, 2016

tsmaeder commented Nov 24, 2016

pranavgore09 commented Nov 24, 2016

pranavgore09 commented Nov 30, 2016 • edited

kwk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranavgore09 Nov 30, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranavgore09 Nov 30, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranavgore09 Nov 30, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranavgore09 Nov 30, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranavgore09 Nov 30, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranavgore09 Nov 30, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranavgore09 Nov 30, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranavgore09 Nov 30, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranavgore09 Nov 30, 2016 • edited

Choose a reason for hiding this comment

pranavgore09 left a comment • edited

Choose a reason for hiding this comment

pranavgore09 Nov 30, 2016 • edited

Choose a reason for hiding this comment

pranavgore09 Nov 30, 2016 • edited

Choose a reason for hiding this comment

pranavgore09 Nov 30, 2016 • edited

Choose a reason for hiding this comment

pranavgore09 Nov 30, 2016 • edited

Choose a reason for hiding this comment

pranavgore09 Nov 30, 2016 • edited

pranavgore09 commented Nov 11, 2016 •

edited

aslakknutsen commented Nov 11, 2016 •

edited

codecov-io commented Nov 12, 2016 •

edited

pranavgore09 commented Nov 17, 2016 •

edited

pranavgore09 commented Nov 22, 2016 •

edited

pranavgore09 commented Nov 22, 2016 •

edited

pranavgore09 commented Nov 30, 2016 •

edited

pranavgore09 Nov 30, 2016 •

edited

pranavgore09 Nov 30, 2016 •

edited

pranavgore09 Nov 30, 2016 •

edited

pranavgore09 Nov 30, 2016 •

edited

pranavgore09 Nov 30, 2016 •

edited

pranavgore09 Nov 30, 2016 •

edited

pranavgore09 Nov 30, 2016 •

edited

pranavgore09 Nov 30, 2016 •

edited

pranavgore09 Nov 30, 2016 •

edited

pranavgore09 left a comment •

edited

pranavgore09 Nov 30, 2016 •

edited

pranavgore09 Nov 30, 2016 •

edited

pranavgore09 Nov 30, 2016 •

edited

pranavgore09 Nov 30, 2016 •

edited

pranavgore09 Nov 30, 2016 •

edited

pranavgore09 Nov 30, 2016 •

edited

pranavgore09 Nov 30, 2016 •

edited

pranavgore09 Nov 30, 2016 •

edited