From f276f3affa3a0fc37a6a1648247acd767819c4a0 Mon Sep 17 00:00:00 2001 From: Konrad Kleine <193408+kwk@users.noreply.github.com> Date: Mon, 17 Sep 2018 14:30:48 +0200 Subject: [PATCH] Don't have a zero-parent relationship for root iteration/area (#2283) Before a root iteration/area had a parent iteration with a zeroed UUID: ```yaml "relationships": { "parent": { "data": { "id": "00000000-0000-0000-0000-000000000000", "type": "iterations" }, ``` That is removed now. This adds a test that shows how a root and a child iteration/area are represented on JSONAPI level. Added `testfixture.SetAreaNames()` and `testfixture.AreaByName()` functions as well. This fixes the root cause for https://github.com/openshiftio/openshift.io/issues/4309 . The UI fix in https://github.com/Raunak1203/fabric8-planner/commit/e7c34a267613ae35c030041740f540abcde16849 only fixes a symptom by implementing a work around. --- controller/area.go | 2 +- controller/area_blackbox_test.go | 14 ++-- controller/iteration.go | 5 +- controller/iteration_blackbox_test.go | 64 +++++++++++++------ ..._if_modified_since.res.payload.golden.json | 18 +++--- ...ired_if_none_match.res.payload.golden.json | 18 +++--- .../show/ok-root-area.headers.golden.json | 14 ++++ ...n => ok-root-area.res.payload.golden.json} | 18 +++--- ...k-child-iteration.res.payload.golden.json} | 28 ++++---- .../ok-root-iteration.res.payload.golden.json | 42 ++++++++++++ test/testfixture/customize_entity_funcs.go | 13 ++++ test/testfixture/query_functions.go | 15 +++++ 12 files changed, 184 insertions(+), 67 deletions(-) create mode 100755 controller/test-files/area/show/ok-root-area.headers.golden.json rename controller/test-files/area/show/{ok.res.payload.golden.json => ok-root-area.res.payload.golden.json} (76%) rename controller/test-files/iteration/show/{ok.res.iteration.golden.json => ok-child-iteration.res.payload.golden.json} (69%) create mode 100755 controller/test-files/iteration/show/ok-root-iteration.res.payload.golden.json diff --git a/controller/area.go b/controller/area.go index 95d01fd72b..b19456f1b1 100644 --- a/controller/area.go +++ b/controller/area.go @@ -239,7 +239,7 @@ func ConvertArea(db application.DB, request *http.Request, ar area.Area, options // Now check the path, if the path is empty, then this is the topmost area // in a specific space. - if ar.Path.ParentPath().IsEmpty() == false { + if !ar.Path.ParentPath().IsEmpty() { parent := ar.Path.ParentID().String() i.Relationships.Parent = &app.RelationGeneric{ Data: &app.GenericData{ diff --git a/controller/area_blackbox_test.go b/controller/area_blackbox_test.go index eced741b18..2945493403 100644 --- a/controller/area_blackbox_test.go +++ b/controller/area_blackbox_test.go @@ -151,18 +151,22 @@ func (rest *TestAreaREST) TestCreateChildArea() { }) } -func (rest *TestAreaREST) TestShowArea() { +func (rest *TestAreaREST) TestShow() { rest.T().Run("Success", func(t *testing.T) { // Setup - a := tf.NewTestFixture(t, rest.DB, tf.Areas(1)).Areas[0] + fxt := tf.NewTestFixture(t, rest.DB, + tf.CreateWorkItemEnvironment(), + tf.Areas(1, tf.SetAreaNames("root")), + ) + a := fxt.AreaByName("root") svc, ctrl := rest.SecuredController() - t.Run("OK", func(t *testing.T) { + t.Run("root", func(t *testing.T) { // when res, area := test.ShowAreaOK(t, svc.Context, svc, ctrl, a.ID.String(), nil, nil) safeOverriteHeader(t, res, app.ETag, "0icd7ov5CqwDXN6Fx9z18g==") //then - compareWithGoldenAgnostic(t, filepath.Join(rest.testDir, "show", "ok.res.payload.golden.json"), area) - compareWithGoldenAgnostic(t, filepath.Join(rest.testDir, "show", "ok.res.headers.golden.json"), res.Header()) + compareWithGoldenAgnostic(t, filepath.Join(rest.testDir, "show", "ok-root-area.res.payload.golden.json"), area) + compareWithGoldenAgnostic(t, filepath.Join(rest.testDir, "show", "ok-root-area.headers.golden.json"), res.Header()) }) t.Run("Using ExpiredIfModifedSince Header", func(t *testing.T) { diff --git a/controller/iteration.go b/controller/iteration.go index ffef1fba04..22697d4e9d 100644 --- a/controller/iteration.go +++ b/controller/iteration.go @@ -3,6 +3,8 @@ package controller import ( "context" "fmt" + "net/http" + "github.com/fabric8-services/fabric8-wit/app" "github.com/fabric8-services/fabric8-wit/application" "github.com/fabric8-services/fabric8-wit/errors" @@ -14,7 +16,6 @@ import ( "github.com/fabric8-services/fabric8-wit/space" "github.com/fabric8-services/fabric8-wit/space/authz" "github.com/fabric8-services/fabric8-wit/workitem" - "net/http" "github.com/goadesign/goa" uuid "github.com/satori/go.uuid" @@ -497,7 +498,7 @@ func ConvertIteration(request *http.Request, itr iteration.Iteration, additional Related: &relatedURL, }, } - if itr.Path.IsEmpty() == false { + if !itr.Path.ParentPath().IsEmpty() { parentID := itr.Path.ParentID().String() parentRelatedURL := rest.AbsoluteURL(request, app.IterationHref(parentID)) i.Relationships.Parent = &app.RelationGeneric{ diff --git a/controller/iteration_blackbox_test.go b/controller/iteration_blackbox_test.go index f3d3c8edf9..ee49d70e9b 100644 --- a/controller/iteration_blackbox_test.go +++ b/controller/iteration_blackbox_test.go @@ -3,6 +3,13 @@ package controller_test import ( "context" "fmt" + "net/http" + "net/url" + "path/filepath" + "strings" + "testing" + "time" + token "github.com/dgrijalva/jwt-go" "github.com/fabric8-services/fabric8-wit/account" "github.com/fabric8-services/fabric8-wit/app" @@ -24,12 +31,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" - "net/http" - "net/url" - "path/filepath" - "strings" - "testing" - "time" ) type TestIterationREST struct { @@ -265,20 +266,47 @@ func (rest *TestIterationREST) TestFailValidationIterationNameStartWith() { assert.Contains(rest.T(), err.Error(), "type.name must match the regexp") } -func (rest *TestIterationREST) TestShowIterationOK() { - +func (rest *TestIterationREST) TestShow() { // given - fxt := tf.NewTestFixture(rest.T(), rest.DB, createSpaceAndRootAreaAndIterations()...) - itr := *fxt.Iterations[1] + fxt := tf.NewTestFixture(rest.T(), rest.DB, + tf.CreateWorkItemEnvironment(), + tf.Iterations(2, + tf.SetIterationNames("root", "child"), + func(fxt *tf.TestFixture, idx int) error { + switch idx { + case 1: + start, err := time.Parse(time.RFC822, "02 Jan 06 15:04 MST") + if err != nil { + return err + } + fxt.Iterations[idx].StartAt = &start + fxt.Iterations[idx].EndAt = ptr.Time(start.Add(time.Hour * 24 * 365 * 100)) + } + return nil + }), + tf.Areas(2), + ) svc, ctrl := rest.SecuredController() - // when - _, created := test.ShowIterationOK(rest.T(), svc.Context, svc, ctrl, itr.ID.String(), nil, nil) - // then - assertIterationLinking(rest.T(), created.Data) - require.NotNil(rest.T(), created.Data.Relationships.Workitems.Meta) - assert.Equal(rest.T(), 0, created.Data.Relationships.Workitems.Meta[KeyTotalWorkItems]) - assert.Equal(rest.T(), 0, created.Data.Relationships.Workitems.Meta[KeyClosedWorkItems]) - compareWithGoldenAgnostic(rest.T(), filepath.Join(rest.testDir, "show", "ok.res.iteration.golden.json"), created) + rest.T().Run("root iteration", func(t *testing.T) { + // when + _, i := test.ShowIterationOK(t, svc.Context, svc, ctrl, fxt.IterationByName("root").ID.String(), nil, nil) + // then + assertIterationLinking(t, i.Data) + require.NotNil(t, i.Data.Relationships.Workitems.Meta) + assert.Equal(t, 0, i.Data.Relationships.Workitems.Meta[KeyTotalWorkItems]) + assert.Equal(t, 0, i.Data.Relationships.Workitems.Meta[KeyClosedWorkItems]) + compareWithGoldenAgnostic(t, filepath.Join(rest.testDir, "show", "ok-root-iteration.res.payload.golden.json"), i) + }) + rest.T().Run("child iteration", func(t *testing.T) { + // when + _, i := test.ShowIterationOK(t, svc.Context, svc, ctrl, fxt.IterationByName("child").ID.String(), nil, nil) + // then + assertIterationLinking(t, i.Data) + require.NotNil(t, i.Data.Relationships.Workitems.Meta) + assert.Equal(t, 0, i.Data.Relationships.Workitems.Meta[KeyTotalWorkItems]) + assert.Equal(t, 0, i.Data.Relationships.Workitems.Meta[KeyClosedWorkItems]) + compareWithGoldenAgnostic(t, filepath.Join(rest.testDir, "show", "ok-child-iteration.res.payload.golden.json"), i) + }) } func (rest *TestIterationREST) TestShowIterationOKUsingExpiredIfModifiedSinceHeader() { diff --git a/controller/test-files/area/show/expired_if_modified_since.res.payload.golden.json b/controller/test-files/area/show/expired_if_modified_since.res.payload.golden.json index 8155ff616b..8615eef112 100755 --- a/controller/test-files/area/show/expired_if_modified_since.res.payload.golden.json +++ b/controller/test-files/area/show/expired_if_modified_since.res.payload.golden.json @@ -2,32 +2,32 @@ "data": { "attributes": { "created-at": "0001-01-01T00:00:00Z", - "name": "area 00000000-0000-0000-0000-000000000001", + "name": "root", "parent_path": "/", "parent_path_resolved": "/", "updated-at": "0001-01-01T00:00:00Z", "version": 0 }, - "id": "00000000-0000-0000-0000-000000000002", + "id": "00000000-0000-0000-0000-000000000001", "links": { - "related": "http:///api/areas/00000000-0000-0000-0000-000000000002", - "self": "http:///api/areas/00000000-0000-0000-0000-000000000002" + "related": "http:///api/areas/00000000-0000-0000-0000-000000000001", + "self": "http:///api/areas/00000000-0000-0000-0000-000000000001" }, "relationships": { "children": { "links": { - "related": "http:///api/areas/00000000-0000-0000-0000-000000000002/children", - "self": "http:///api/areas/00000000-0000-0000-0000-000000000002/children" + "related": "http:///api/areas/00000000-0000-0000-0000-000000000001/children", + "self": "http:///api/areas/00000000-0000-0000-0000-000000000001/children" } }, "space": { "data": { - "id": "00000000-0000-0000-0000-000000000003", + "id": "00000000-0000-0000-0000-000000000002", "type": "spaces" }, "links": { - "related": "http:///api/spaces/00000000-0000-0000-0000-000000000003", - "self": "http:///api/spaces/00000000-0000-0000-0000-000000000003" + "related": "http:///api/spaces/00000000-0000-0000-0000-000000000002", + "self": "http:///api/spaces/00000000-0000-0000-0000-000000000002" } } }, diff --git a/controller/test-files/area/show/expired_if_none_match.res.payload.golden.json b/controller/test-files/area/show/expired_if_none_match.res.payload.golden.json index 8155ff616b..8615eef112 100755 --- a/controller/test-files/area/show/expired_if_none_match.res.payload.golden.json +++ b/controller/test-files/area/show/expired_if_none_match.res.payload.golden.json @@ -2,32 +2,32 @@ "data": { "attributes": { "created-at": "0001-01-01T00:00:00Z", - "name": "area 00000000-0000-0000-0000-000000000001", + "name": "root", "parent_path": "/", "parent_path_resolved": "/", "updated-at": "0001-01-01T00:00:00Z", "version": 0 }, - "id": "00000000-0000-0000-0000-000000000002", + "id": "00000000-0000-0000-0000-000000000001", "links": { - "related": "http:///api/areas/00000000-0000-0000-0000-000000000002", - "self": "http:///api/areas/00000000-0000-0000-0000-000000000002" + "related": "http:///api/areas/00000000-0000-0000-0000-000000000001", + "self": "http:///api/areas/00000000-0000-0000-0000-000000000001" }, "relationships": { "children": { "links": { - "related": "http:///api/areas/00000000-0000-0000-0000-000000000002/children", - "self": "http:///api/areas/00000000-0000-0000-0000-000000000002/children" + "related": "http:///api/areas/00000000-0000-0000-0000-000000000001/children", + "self": "http:///api/areas/00000000-0000-0000-0000-000000000001/children" } }, "space": { "data": { - "id": "00000000-0000-0000-0000-000000000003", + "id": "00000000-0000-0000-0000-000000000002", "type": "spaces" }, "links": { - "related": "http:///api/spaces/00000000-0000-0000-0000-000000000003", - "self": "http:///api/spaces/00000000-0000-0000-0000-000000000003" + "related": "http:///api/spaces/00000000-0000-0000-0000-000000000002", + "self": "http:///api/spaces/00000000-0000-0000-0000-000000000002" } } }, diff --git a/controller/test-files/area/show/ok-root-area.headers.golden.json b/controller/test-files/area/show/ok-root-area.headers.golden.json new file mode 100755 index 0000000000..bc7fe93197 --- /dev/null +++ b/controller/test-files/area/show/ok-root-area.headers.golden.json @@ -0,0 +1,14 @@ +{ + "Cache-Control": [ + "private,max-age=120" + ], + "Content-Type": [ + "application/vnd.api+json" + ], + "Etag": [ + "0icd7ov5CqwDXN6Fx9z18g==" + ], + "Last-Modified": [ + "Mon, 01 Jan 0001 00:00:00 GMT" + ] +} \ No newline at end of file diff --git a/controller/test-files/area/show/ok.res.payload.golden.json b/controller/test-files/area/show/ok-root-area.res.payload.golden.json similarity index 76% rename from controller/test-files/area/show/ok.res.payload.golden.json rename to controller/test-files/area/show/ok-root-area.res.payload.golden.json index 8155ff616b..8615eef112 100755 --- a/controller/test-files/area/show/ok.res.payload.golden.json +++ b/controller/test-files/area/show/ok-root-area.res.payload.golden.json @@ -2,32 +2,32 @@ "data": { "attributes": { "created-at": "0001-01-01T00:00:00Z", - "name": "area 00000000-0000-0000-0000-000000000001", + "name": "root", "parent_path": "/", "parent_path_resolved": "/", "updated-at": "0001-01-01T00:00:00Z", "version": 0 }, - "id": "00000000-0000-0000-0000-000000000002", + "id": "00000000-0000-0000-0000-000000000001", "links": { - "related": "http:///api/areas/00000000-0000-0000-0000-000000000002", - "self": "http:///api/areas/00000000-0000-0000-0000-000000000002" + "related": "http:///api/areas/00000000-0000-0000-0000-000000000001", + "self": "http:///api/areas/00000000-0000-0000-0000-000000000001" }, "relationships": { "children": { "links": { - "related": "http:///api/areas/00000000-0000-0000-0000-000000000002/children", - "self": "http:///api/areas/00000000-0000-0000-0000-000000000002/children" + "related": "http:///api/areas/00000000-0000-0000-0000-000000000001/children", + "self": "http:///api/areas/00000000-0000-0000-0000-000000000001/children" } }, "space": { "data": { - "id": "00000000-0000-0000-0000-000000000003", + "id": "00000000-0000-0000-0000-000000000002", "type": "spaces" }, "links": { - "related": "http:///api/spaces/00000000-0000-0000-0000-000000000003", - "self": "http:///api/spaces/00000000-0000-0000-0000-000000000003" + "related": "http:///api/spaces/00000000-0000-0000-0000-000000000002", + "self": "http:///api/spaces/00000000-0000-0000-0000-000000000002" } } }, diff --git a/controller/test-files/iteration/show/ok.res.iteration.golden.json b/controller/test-files/iteration/show/ok-child-iteration.res.payload.golden.json similarity index 69% rename from controller/test-files/iteration/show/ok.res.iteration.golden.json rename to controller/test-files/iteration/show/ok-child-iteration.res.payload.golden.json index aa69aa9ea4..1124bfd88f 100755 --- a/controller/test-files/iteration/show/ok.res.iteration.golden.json +++ b/controller/test-files/iteration/show/ok-child-iteration.res.payload.golden.json @@ -3,45 +3,45 @@ "attributes": { "active_status": true, "created-at": "0001-01-01T00:00:00Z", - "description": "some description (see function github.com/fabric8-services/fabric8-wit/controller_test.(*TestIterationREST).TestShowIterationOK in controller/iteration_blackbox_test.go)", + "description": "some description (see function github.com/fabric8-services/fabric8-wit/controller_test.(*TestIterationREST).TestShow in controller/iteration_blackbox_test.go)", "endAt": "0001-01-01T00:00:00Z", - "name": "iteration 00000000-0000-0000-0000-000000000001", - "parent_path": "/00000000-0000-0000-0000-000000000002", - "resolved_parent_path": "/space 00000000-0000-0000-0000-000000000003", + "name": "child", + "parent_path": "/00000000-0000-0000-0000-000000000001", + "resolved_parent_path": "/root", "startAt": "0001-01-01T00:00:00Z", "state": "new", "updated-at": "0001-01-01T00:00:00Z", "user_active": false }, - "id": "00000000-0000-0000-0000-000000000004", + "id": "00000000-0000-0000-0000-000000000002", "links": { - "related": "http:///api/iterations/00000000-0000-0000-0000-000000000004", - "self": "http:///api/iterations/00000000-0000-0000-0000-000000000004" + "related": "http:///api/iterations/00000000-0000-0000-0000-000000000002", + "self": "http:///api/iterations/00000000-0000-0000-0000-000000000002" }, "relationships": { "parent": { "data": { - "id": "00000000-0000-0000-0000-000000000002", + "id": "00000000-0000-0000-0000-000000000001", "type": "iterations" }, "links": { - "related": "http:///api/iterations/00000000-0000-0000-0000-000000000002", - "self": "http:///api/iterations/00000000-0000-0000-0000-000000000002" + "related": "http:///api/iterations/00000000-0000-0000-0000-000000000001", + "self": "http:///api/iterations/00000000-0000-0000-0000-000000000001" } }, "space": { "data": { - "id": "00000000-0000-0000-0000-000000000005", + "id": "00000000-0000-0000-0000-000000000003", "type": "spaces" }, "links": { - "related": "http:///api/spaces/00000000-0000-0000-0000-000000000005", - "self": "http:///api/spaces/00000000-0000-0000-0000-000000000005" + "related": "http:///api/spaces/00000000-0000-0000-0000-000000000003", + "self": "http:///api/spaces/00000000-0000-0000-0000-000000000003" } }, "workitems": { "links": { - "related": "http:///api/workitems/?filter[iteration]=00000000-0000-0000-0000-000000000004" + "related": "http:///api/workitems/?filter[iteration]=00000000-0000-0000-0000-000000000002" }, "meta": { "closed": 0, diff --git a/controller/test-files/iteration/show/ok-root-iteration.res.payload.golden.json b/controller/test-files/iteration/show/ok-root-iteration.res.payload.golden.json new file mode 100755 index 0000000000..4c1238a3f4 --- /dev/null +++ b/controller/test-files/iteration/show/ok-root-iteration.res.payload.golden.json @@ -0,0 +1,42 @@ +{ + "data": { + "attributes": { + "active_status": false, + "created-at": "0001-01-01T00:00:00Z", + "description": "some description (see function github.com/fabric8-services/fabric8-wit/controller_test.(*TestIterationREST).TestShow in controller/iteration_blackbox_test.go)", + "name": "root", + "parent_path": "/", + "resolved_parent_path": "/", + "state": "new", + "updated-at": "0001-01-01T00:00:00Z", + "user_active": false + }, + "id": "00000000-0000-0000-0000-000000000001", + "links": { + "related": "http:///api/iterations/00000000-0000-0000-0000-000000000001", + "self": "http:///api/iterations/00000000-0000-0000-0000-000000000001" + }, + "relationships": { + "space": { + "data": { + "id": "00000000-0000-0000-0000-000000000002", + "type": "spaces" + }, + "links": { + "related": "http:///api/spaces/00000000-0000-0000-0000-000000000002", + "self": "http:///api/spaces/00000000-0000-0000-0000-000000000002" + } + }, + "workitems": { + "links": { + "related": "http:///api/workitems/?filter[iteration]=00000000-0000-0000-0000-000000000001" + }, + "meta": { + "closed": 0, + "total": 0 + } + } + }, + "type": "iterations" + } +} \ No newline at end of file diff --git a/test/testfixture/customize_entity_funcs.go b/test/testfixture/customize_entity_funcs.go index 6329c9f25c..ae1603e892 100644 --- a/test/testfixture/customize_entity_funcs.go +++ b/test/testfixture/customize_entity_funcs.go @@ -179,6 +179,19 @@ func SetIterationNames(names ...string) CustomizeIterationFunc { } } +// SetAreaNames takes the given names and uses them during creation of areas. +// The length of requested areas and the number of names must match or the +// NewFixture call will return an error. +func SetAreaNames(names ...string) CustomizeAreaFunc { + return func(fxt *TestFixture, idx int) error { + if len(fxt.Areas) != len(names) { + return errs.Errorf("number of names (%d) must match number of areas to create (%d)", len(names), len(fxt.Areas)) + } + fxt.Areas[idx].Name = names[idx] + return nil + } +} + // PlaceIterationUnderRootIteration when asking for more than one iteration, all // but the first one will be placed under the first iteration (aka root // iteration). diff --git a/test/testfixture/query_functions.go b/test/testfixture/query_functions.go index ad711985f2..787dc25973 100644 --- a/test/testfixture/query_functions.go +++ b/test/testfixture/query_functions.go @@ -2,6 +2,7 @@ package testfixture import ( "github.com/fabric8-services/fabric8-wit/account" + "github.com/fabric8-services/fabric8-wit/area" "github.com/fabric8-services/fabric8-wit/iteration" "github.com/fabric8-services/fabric8-wit/label" "github.com/fabric8-services/fabric8-wit/query" @@ -40,6 +41,20 @@ func (fxt *TestFixture) IterationByName(name string, spaceID ...uuid.UUID) *iter return nil } +// AreaByName returns the first area that has the given name (if any). If you +// have areas with the same name in different spaces you can also pass in one +// space ID to filter by space as well. +func (fxt *TestFixture) AreaByName(name string, spaceID ...uuid.UUID) *area.Area { + for _, a := range fxt.Areas { + if a.Name == name && len(spaceID) > 0 && a.SpaceID == spaceID[0] { + return a + } else if a.Name == name && len(spaceID) == 0 { + return a + } + } + return nil +} + // WorkItemTypeByName returns the first work item type that has the given name // (if any). If you have work item types with the same name in different space // templates you can also pass in one space template ID to filter by space