fix(server): request approval should publish the exact version (#1291)

* fix(server): request approval should publish the exact version * fix test * fix test
reearth · Oct 30, 2024 · cae24f8 · cae24f8
1 parent af0bd0d
commit cae24f8
Show file tree

Hide file tree

Showing 8 changed files with 218 additions and 32 deletions.
diff --git a/server/e2e/gql_item_test.go b/server/e2e/gql_item_test.go
@@ -300,6 +300,7 @@ func updateItem(e *httpexpect.Expect, iID, version string, fields []map[string]a
 					item {
 					  id
 					  schemaId
+					  version
 					  fields {
 						value
 						type

diff --git a/server/e2e/gql_model_test.go b/server/e2e/gql_model_test.go
@@ -44,7 +44,7 @@ func createModel(e *httpexpect.Expect, pID, name, desc, key string) (string, *ht
 	return res.Path("$.data.createModel.model.id").Raw().(string), res
 }
 
-func updateModel(e *httpexpect.Expect, mId string, name, desc, key *string) *httpexpect.Value {
+func updateModel(e *httpexpect.Expect, mId string, name, desc, key *string, public bool) *httpexpect.Value {
 	requestBody := GraphQLRequest{
 		Query: `mutation UpdateModel($modelId: ID!, $name: String, $description: String, $key: String,  $public: Boolean!) {
 				  updateModel(input: {modelId: $modelId, name: $name, description: $description, key: $key, public: $public}) {
@@ -64,7 +64,7 @@ func updateModel(e *httpexpect.Expect, mId string, name, desc, key *string) *htt
 			"name":        name,
 			"description": desc,
 			"key":         key,
-			"public":      false,
+			"public":      public,
 		},
 	}
 
@@ -336,7 +336,7 @@ func TestUpdateModel(t *testing.T) {
 	pId, _ := createProject(e, wId.String(), "test", "test", "test-2")
 
 	mId, _ := createModel(e, pId, "test", "test", "test-2")
-	res := updateModel(e, mId, lo.ToPtr("updated name"), lo.ToPtr("updated desc"), lo.ToPtr("updated_key"))
+	res := updateModel(e, mId, lo.ToPtr("updated name"), lo.ToPtr("updated desc"), lo.ToPtr("updated_key"), false)
 	res.Object().
 		Value("data").Object().
 		Value("updateModel").Object().

diff --git a/server/e2e/gql_porject_test.go b/server/e2e/gql_porject_test.go
@@ -47,6 +47,47 @@ func createProject(e *httpexpect.Expect, wID, name, desc, alias string) (string,
 	return res.Path("$.data.createProject.project.id").Raw().(string), res
 }
 
+func updateProject(e *httpexpect.Expect, pID, name, desc, alias, publicationScope string, publicAssets bool) (string, *httpexpect.Value) {
+	requestBody := GraphQLRequest{
+		Query: `mutation UpdateProject($projectId: ID!, $name: String!, $description: String!, $alias: String!, $publicationScope: ProjectPublicationScope!, $publicAssets: Boolean!) {
+				  updateProject(input: {projectId: $projectId, name: $name, description: $description, alias: $alias, publication: {scope: $publicationScope, assetPublic: $publicAssets}}) {
+					project {
+					  id
+					  name
+					  description
+					  alias
+					  publication {
+						scope
+						assetPublic
+						__typename
+					  }
+					  __typename
+					}
+					__typename
+				  }
+				}`,
+		Variables: map[string]any{
+			"projectId":        pID,
+			"name":             name,
+			"description":      desc,
+			"alias":            alias,
+			"publicationScope": publicationScope,
+			"publicAssets":     publicAssets,
+		},
+	}
+
+	res := e.POST("/api/graphql").
+		WithHeader("Origin", "https://example.com").
+		WithHeader("X-Reearth-Debug-User", uId1.String()).
+		WithHeader("Content-Type", "application/json").
+		WithJSON(requestBody).
+		Expect().
+		Status(http.StatusOK).
+		JSON()
+
+	return res.Path("$.data.updateProject.project.id").Raw().(string), res
+}
+
 func TestCreateProject(t *testing.T) {
 	e := StartServer(t, &app.Config{}, true, baseSeederUser)
 

diff --git a/server/e2e/gql_request_test.go b/server/e2e/gql_request_test.go
@@ -297,3 +297,129 @@ func TestCloseRequest(t *testing.T) {
 	_, itm_closed := getItem(e, iid1)
 	itm_closed.Path("$.data.node").Object().Value("version").IsEqual(ver1)
 }
+
+func TestRequestFlow(t *testing.T) {
+	e := StartServer(t, &app.Config{}, true, baseSeederUser)
+
+	// 1- create public project
+	pId, _ := createProject(e, wId.String(), "test", "test", "test-1")
+	updateProject(e, pId, "test", "test", "test-1", "PUBLIC", true)
+
+	// 2- create public model
+	mId, _ := createModel(e, pId, "test", "test", "test-1")
+	updateModel(e, mId, lo.ToPtr("test"), lo.ToPtr("test"), lo.ToPtr("test-1"), true)
+
+	fid, _ := createField(e, mId, "text", "text", "text",
+		false, false, false, false, "Text",
+		map[string]any{
+			"text": map[string]any{},
+		})
+	sId, _, _ := getModel(e, mId)
+
+	// 3- create item with version 1
+	iid1, i1 := createItem(e, mId, sId, nil, []map[string]any{
+		{"schemaFieldId": fid, "value": "v1", "type": "Text"},
+	})
+	ver1 := i1.Path("$.data.createItem.item.version").Raw().(string)
+
+	// 4- update item to version 2
+	iid1, i1 = updateItem(e, iid1, ver1, []map[string]any{
+		{"schemaFieldId": fid, "value": "v2", "type": "Text"},
+	})
+	ver2 := i1.Path("$.data.updateItem.item.version").Raw().(string)
+
+	// check public item: should return no results
+	res := e.GET("/api/p/{project}/{model}", "test-1", "test-1").
+		Expect().
+		Status(http.StatusOK).
+		JSON()
+	res.IsEqual(map[string]any{
+		"results":    []map[string]any{},
+		"totalCount": 0,
+		"hasMore":    false,
+		"limit":      50,
+		"offset":     0,
+		"page":       1,
+	})
+
+	// 5- create request with version 2
+	res = createRequest(e, pId, "test", lo.ToPtr("test"), lo.ToPtr("WAITING"), []string{uId1.String()}, []any{map[string]any{"itemId": iid1, "version": ver2}})
+	req := res.Path("$.data.createRequest.request").Object()
+	rid := req.Value("id").String().Raw()
+
+	// 6- update item to version 3
+	iid1, i1 = updateItem(e, iid1, ver2, []map[string]any{
+		{"schemaFieldId": fid, "value": "v3", "type": "Text"},
+	})
+	ver3 := i1.Path("$.data.updateItem.item.version").Raw().(string)
+
+	// 7- approve request
+	res = approveRequest(e, rid)
+	req = res.Path("$.data.approveRequest.request").Object()
+	req.Value("id").IsEqual(rid)
+	req.Value("state").IsEqual("APPROVED")
+
+	// check item and its status
+	_, itm := getItem(e, iid1)
+	itm.Path("$.data.node").Object().Value("version").IsEqual(ver3)
+	itm.Path("$.data.node").Object().Value("status").IsEqual("PUBLIC_DRAFT")
+
+	// check public item: should return version 2
+	res = e.GET("/api/p/{project}/{model}", "test-1", "test-1").
+		Expect().
+		Status(http.StatusOK).
+		JSON()
+	res.IsEqual(map[string]any{
+		"results": []map[string]any{
+			{
+				"id":   iid1,
+				"text": "v2",
+			},
+		},
+		"totalCount": 1,
+		"hasMore":    false,
+		"limit":      50,
+		"offset":     0,
+		"page":       1,
+	})
+
+	// 8- create request with version 3
+	res = createRequest(e, pId, "test", lo.ToPtr("test"), lo.ToPtr("WAITING"), []string{uId1.String()}, []any{map[string]any{"itemId": iid1, "version": ver3}})
+	req = res.Path("$.data.createRequest.request").Object()
+	rid = req.Value("id").String().Raw()
+
+	// check item and its status
+	_, itm = getItem(e, iid1)
+	itm.Path("$.data.node").Object().Value("version").IsEqual(ver3)
+	itm.Path("$.data.node").Object().Value("status").IsEqual("PUBLIC_REVIEW")
+
+	// 9- approve request
+	res = approveRequest(e, rid)
+	req = res.Path("$.data.approveRequest.request").Object()
+	req.Value("id").IsEqual(rid)
+	req.Value("state").IsEqual("APPROVED")
+
+	// check item and its status
+	_, itm = getItem(e, iid1)
+	itm.Path("$.data.node").Object().Value("version").IsEqual(ver3)
+	itm.Path("$.data.node").Object().Value("status").IsEqual("PUBLIC")
+
+	// check public item: should return version 3
+	res = e.GET("/api/p/{project}/{model}", "test-1", "test-1").
+		Expect().
+		Status(http.StatusOK).
+		JSON()
+	res.IsEqual(map[string]any{
+		"results": []map[string]any{
+			{
+				"id":   iid1,
+				"text": "v3",
+			},
+		},
+		"totalCount": 1,
+		"hasMore":    false,
+		"limit":      50,
+		"offset":     0,
+		"page":       1,
+	})
+}
diff --git a/server/internal/usecase/interactor/item.go b/server/internal/usecase/interactor/item.go
@@ -678,6 +678,12 @@ func (i Item) Update(ctx context.Context, param interfaces.UpdateItemParam, oper
 			return nil, err
 		}
 
+		// re-fetch item so the new version is returned
+		itm, err = i.repos.Item.FindByID(ctx, param.ItemID, nil)
+		if err != nil {
+			return nil, err
+		}
+
 		if err = i.handleReferenceFields(ctx, *s, itm.Value(), oldFields); err != nil {
 			return nil, err
 		}

diff --git a/server/internal/usecase/interactor/item_test.go b/server/internal/usecase/interactor/item_test.go
@@ -991,8 +991,22 @@ func TestWorkFlow(t *testing.T) {
 	s := schema.New().NewID().Workspace(accountdomain.NewWorkspaceID()).Project(prj.ID()).MustBuild()
 	m := model.New().NewID().Project(prj.ID()).Schema(s.ID()).RandomKey().MustBuild()
 	i := item.New().NewID().Schema(s.ID()).Model(m.ID()).Project(prj.ID()).Thread(id.NewThreadID()).MustBuild()
-	ri, _ := request.NewItem(i.ID(), lo.ToPtr(version.New().String()))
 	u := user.New().Name("aaa").NewID().Email("[email protected]").Workspace(wid).MustBuild()
+
+	ctx := context.Background()
+	db := memory.New()
+	err := db.Project.Save(ctx, prj)
+	assert.NoError(t, err)
+	err = db.Schema.Save(ctx, s)
+	assert.NoError(t, err)
+	err = db.Model.Save(ctx, m)
+	assert.NoError(t, err)
+	err = db.Item.Save(ctx, i)
+	assert.NoError(t, err)
+
+	vi, err := db.Item.FindByID(ctx, i.ID(), nil)
+	assert.NoError(t, err)
+	ri, _ := request.NewItem(i.ID(), lo.ToPtr(vi.Version().String()))
 	req1 := request.New().
 		NewID().
 		Workspace(wid).
@@ -1009,17 +1023,6 @@ func TestWorkFlow(t *testing.T) {
 			OwningWorkspaces: id.WorkspaceIDList{wid},
 		},
 	}
-	ctx := context.Background()
-
-	db := memory.New()
-	err := db.Project.Save(ctx, prj)
-	assert.NoError(t, err)
-	err = db.Schema.Save(ctx, s)
-	assert.NoError(t, err)
-	err = db.Model.Save(ctx, m)
-	assert.NoError(t, err)
-	err = db.Item.Save(ctx, i)
-	assert.NoError(t, err)
 
 	itemUC := NewItem(db, nil)
 

diff --git a/server/internal/usecase/interactor/request.go b/server/internal/usecase/interactor/request.go
@@ -279,7 +279,12 @@ func (r Request) Approve(ctx context.Context, requestID id.RequestID, operator *
 		// apply changes to items (publish items)
 		for _, itm := range req.Items() {
 			// publish the approved version
-			if err := r.repos.Item.UpdateRef(ctx, itm.Item(), version.Public, version.Latest.OrVersion().Ref()); err != nil {
+			dist := itm.Pointer().Ref()
+			// this should not happen, used for backward compatibility (will set the latest version as published)
+			if dist == nil {
+				dist = version.Latest.OrVersion().Ref()
+			}
+			if err := r.repos.Item.UpdateRef(ctx, itm.Item(), version.Public, dist); err != nil {
 				return nil, err
 			}
 		}

diff --git a/server/internal/usecase/interactor/request_test.go b/server/internal/usecase/interactor/request_test.go
@@ -353,21 +353,38 @@ func TestRequest_Approve(t *testing.T) {
 	defer util.MockNow(now)()
 
 	// TODO: add error cases
+	wid := accountdomain.NewWorkspaceID()
 	prj := project.New().NewID().MustBuild()
 	s := schema.New().NewID().Workspace(accountdomain.NewWorkspaceID()).Project(prj.ID()).MustBuild()
 	m := model.New().NewID().Schema(s.ID()).RandomKey().MustBuild()
 	i := item.New().NewID().Schema(s.ID()).Model(m.ID()).Project(prj.ID()).Thread(id.NewThreadID()).MustBuild()
-	item, _ := request.NewItem(i.ID(), lo.ToPtr(version.New().String()))
-	wid := accountdomain.NewWorkspaceID()
 	u := user.New().Name("aaa").NewID().Email("[email protected]").Workspace(wid).MustBuild()
+
+	ctx := context.Background()
+	db := memory.New()
+	// if tc.mockRequestErr {
+	//	memory.SetRequestError(db.Request, tc.wantErr)
+	// }
+	err := db.Project.Save(ctx, prj)
+	assert.NoError(t, err)
+	err = db.Schema.Save(ctx, s)
+	assert.NoError(t, err)
+	err = db.Model.Save(ctx, m)
+	assert.NoError(t, err)
+	err = db.Item.Save(ctx, i)
+	assert.NoError(t, err)
+
+	vi, err := db.Item.FindByID(ctx, i.ID(), nil)
+	assert.NoError(t, err)
+	ri, _ := request.NewItem(i.ID(), lo.ToPtr(vi.Version().String()))
 	req1 := request.New().
 		NewID().
 		Workspace(wid).
 		Project(prj.ID()).
 		Reviewers(accountdomain.UserIDList{u.ID()}).
 		CreatedBy(accountdomain.NewUserID()).
 		Thread(id.NewThreadID()).
-		Items(request.ItemList{item}).
+		Items(request.ItemList{ri}).
 		Title("foo").
 		MustBuild()
 	op := &usecase.Operator{
@@ -376,22 +393,9 @@ func TestRequest_Approve(t *testing.T) {
 			OwningWorkspaces: accountdomain.WorkspaceIDList{wid},
 		},
 	}
-	ctx := context.Background()
 
-	db := memory.New()
-	// if tc.mockRequestErr {
-	//	memory.SetRequestError(db.Request, tc.wantErr)
-	// }
-	err := db.Project.Save(ctx, prj)
-	assert.NoError(t, err)
 	err = db.Request.Save(ctx, req1)
 	assert.NoError(t, err)
-	err = db.Schema.Save(ctx, s)
-	assert.NoError(t, err)
-	err = db.Model.Save(ctx, m)
-	assert.NoError(t, err)
-	err = db.Item.Save(ctx, i)
-	assert.NoError(t, err)
 
 	requestUC := NewRequest(db, nil)
 	_, err = requestUC.Approve(ctx, req1.ID(), op)
-Original file line number
+Diff line change
@@ Expand Up @@
     					item {
     					  id
     					  schemaId
+    					  version
     					  fields {
     						value
     						type
@@ Expand Down @@