chore: change ListProjects API to request with multiple organizations #810

kakcy · 2024-02-08T05:37:30Z

This pull request primarily focuses on modifying the way organization IDs are handled in the project listing functionality. The changes allow for handling multiple organization IDs instead of a single one. This is achieved by replacing the organization_id field with organization_ids in various files and functions. The changes also include updates to related tests to accommodate this new functionality.

Here are the key changes:

Changes to Organization ID Handling:

pkg/account/api/api.go: The listProjectsByOrganizationID method in the AccountService struct now accepts a slice of organization IDs instead of a single one.
pkg/environment/api/project.go: The ListProjects method in the EnvironmentService struct has been updated to handle a slice of organization IDs.
proto/environment/service.proto: The ListProjectsRequest message now includes a repeated field organization_ids instead of a single organization_id.

Updates to Tests:

pkg/environment/api/project_test.go: The TestListProjectsMySQL function now tests with multiple organization IDs.
test/e2e/environment/project_test.go: A new test function TestListProjectsRequestOrganizations has been added to test the listing of projects for multiple organizations.

Changes in UI Code:

ui/web-v2/apps/admin/src/proto/environment/service_pb.js: Various changes were made to accommodate the new organization_ids field in the ListProjectsRequest message. These changes include updates to serialization, deserialization, and object conversion methods. [1] [2] [3] [4] [5] [6]

kentakozuka · 2024-02-08T05:53:39Z

proto/environment/service.proto

@@ -118,7 +118,7 @@ message ListProjectsRequest {
  OrderDirection order_direction = 4;
  string search_keyword = 5;
  google.protobuf.BoolValue disabled = 6;
-  string organization_id = 7;
+  repeated string organization_ids = 7;


This is a breaking change. Is this API not used by any other services? If it is, we need to add a new field instead of modify it.

I have already checked with him.
He is aware of breaking change, but since there is no place on the console that uses this property, there is no need to add a new one.

He is aware of breaking change, but since there is no place on the console that uses this property, there is no need to add a new one.

Thank you for review.
Same understanding as above.

cre8ivejp · 2024-02-08T06:29:43Z

test/e2e/environment/project_test.go

+	defer cancel()
+	c := newEnvironmentClient(t)
+	defer c.Close()
+	pageSize := int64(1)


Because we want to test two or more organizations, we must set the pageSize above 2.

Fixed to get two organization IDs.
ada5e82

cre8ivejp · 2024-02-08T06:33:51Z

test/e2e/environment/project_test.go

+	if err != nil {
+		t.Fatal(err)
+	}
+	length := int(math.Min(float64(len(orgResp.Organizations)), 2))


Suggested change

length := int(math.Min(float64(len(orgResp.Organizations)), 2))

length := int(math.Min(float64(len(orgResp.Organizations)), pageSize))

cre8ivejp · 2024-02-08T06:35:29Z

proto/environment/service.proto

@@ -118,7 +118,7 @@ message ListProjectsRequest {
  OrderDirection order_direction = 4;
  string search_keyword = 5;
  google.protobuf.BoolValue disabled = 6;
-  string organization_id = 7;
+  repeated string organization_ids = 7;


I have already checked with him.
He is aware of breaking change, but since there is no place on the console that uses this property, there is no need to add a new one.

cre8ivejp · 2024-02-08T06:37:18Z

pkg/environment/api/project.go

@@ -120,8 +120,12 @@ func (s *EnvironmentService) ListProjects(
 		return nil, err
 	}
 	whereParts := []mysql.WherePart{}
-	if req.OrganizationId != "" {
-		whereParts = append(whereParts, mysql.NewFilter("organization_id", "=", req.OrganizationId))
+	if req.OrganizationIds != nil && len(req.OrganizationIds) > 0 {


The len also checks for nil.

Suggested change

if req.OrganizationIds != nil && len(req.OrganizationIds) > 0 {

if len(req.OrganizationIds) > 0 {

cre8ivejp · 2024-02-08T06:40:02Z

pkg/environment/api/project.go

-	if req.OrganizationId != "" {
-		whereParts = append(whereParts, mysql.NewFilter("organization_id", "=", req.OrganizationId))
+	if req.OrganizationIds != nil && len(req.OrganizationIds) > 0 {
+		oIDs := make([]interface{}, 0, len(req.OrganizationIds))


Please refactor this by creating a new function, convToInterfaceSlice.

E.g.

bucketeer/pkg/feature/api/feature.go

Lines 1770 to 1778 in 0685cc5

func convToInterfaceSlice(

slice []string,

) []interface{} {

result := make([]interface{}, 0, len(slice))

for _, element := range slice {

result = append(result, element)

}

return result

}

Added convToInterfaceSlice()and modified it to convert Organization IDs using that function.
b462fb9

cre8ivejp · 2024-02-08T06:46:20Z

test/e2e/environment/project_test.go

+			}
+		}
+		if !found {
+			t.Fatalf("received a project of a not request organization expected: %v, actual: %v", orgIds, project.OrganizationId)


%s for string.

Suggested change

t.Fatalf("received a project of a not request organization expected: %v, actual: %v", orgIds, project.OrganizationId)

t.Fatalf("received a project of a not request organization expected: %v, actual: %s", orgIds, project.OrganizationId)

Changed a specifier for variable strings.
bcb4b3a

kakcy · 2024-02-09T01:51:41Z

Thank you for your review!
I have corrected the points you pointed out, so please review them.

cre8ivejp

Thank you!

When you merge, please use the following commit message.

chore: change ListProjects API to request with multiple organizations

kakcy · 2024-02-09T04:55:30Z

Thank you for reviews!

kakcy added 2 commits February 7, 2024 18:39

refactor: change ListProjects API to request with multiple organizations

93fd609

test: add a e2e test to specity Organizations with ListProjects API

3e18c88

kakcy requested review from cre8ivejp and kentakozuka as code owners February 8, 2024 05:37

kentakozuka reviewed Feb 8, 2024

View reviewed changes

cre8ivejp reviewed Feb 8, 2024

View reviewed changes

kakcy added 3 commits February 9, 2024 10:30

refactor: Change the organization ID acquisition part

b462fb9

fix: fix to get two Organization IDs

ada5e82

refactor: change a specifier for variable strings

bcb4b3a

kakcy requested review from cre8ivejp and kentakozuka February 9, 2024 01:51

cre8ivejp changed the title ~~refactor: change ListProjects API to request with multiple organizations~~ chore: change ListProjects API to request with multiple organizations Feb 9, 2024

cre8ivejp approved these changes Feb 9, 2024

View reviewed changes

kakcy merged commit 9a6f9fc into main Feb 9, 2024
16 checks passed

kakcy deleted the change-list-projects-api branch February 9, 2024 04:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: change ListProjects API to request with multiple organizations #810

chore: change ListProjects API to request with multiple organizations #810

kakcy commented Feb 8, 2024

kentakozuka Feb 8, 2024

cre8ivejp Feb 8, 2024

kakcy Feb 8, 2024

cre8ivejp Feb 8, 2024

kakcy Feb 9, 2024

cre8ivejp Feb 8, 2024

cre8ivejp Feb 8, 2024

cre8ivejp Feb 8, 2024

cre8ivejp Feb 8, 2024 •

edited

Loading

kakcy Feb 9, 2024

cre8ivejp Feb 8, 2024

kakcy Feb 9, 2024

kakcy commented Feb 9, 2024

cre8ivejp left a comment

kakcy commented Feb 9, 2024

	length := int(math.Min(float64(len(orgResp.Organizations)), 2))
	length := int(math.Min(float64(len(orgResp.Organizations)), pageSize))

	if req.OrganizationIds != nil && len(req.OrganizationIds) > 0 {
	if len(req.OrganizationIds) > 0 {

	func convToInterfaceSlice(
	slice []string,
	) []interface{} {
	result := make([]interface{}, 0, len(slice))
	for _, element := range slice {
	result = append(result, element)
	}
	return result
	}

	t.Fatalf("received a project of a not request organization expected: %v, actual: %v", orgIds, project.OrganizationId)
	t.Fatalf("received a project of a not request organization expected: %v, actual: %s", orgIds, project.OrganizationId)

chore: change ListProjects API to request with multiple organizations #810

chore: change ListProjects API to request with multiple organizations #810

Conversation

kakcy commented Feb 8, 2024

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

cre8ivejp Feb 8, 2024 • edited Loading

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

kakcy commented Feb 9, 2024

cre8ivejp left a comment

Choose a reason for hiding this comment

kakcy commented Feb 9, 2024

cre8ivejp Feb 8, 2024 •

edited

Loading