From 69266d3aefb3c668229e270ee2d124887896248e Mon Sep 17 00:00:00 2001 From: Tomer Heber Date: Mon, 5 Aug 2024 06:53:49 -0500 Subject: [PATCH 1/3] Fix: Organization Policy can't be updated if max_ttl and default_ttl are nil --- client/organization.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/client/organization.go b/client/organization.go index 628f7c9b..a714d747 100644 --- a/client/organization.go +++ b/client/organization.go @@ -72,6 +72,14 @@ func (client *ApiClient) OrganizationPolicyUpdate(payload OrganizationPolicyUpda return nil, err } + if payload.DefaultTtl != nil && *payload.DefaultTtl == "" { + payload.DefaultTtl = nil + } + + if payload.MaxTtl != nil && *payload.MaxTtl == "" { + payload.MaxTtl = nil + } + var result Organization if err := client.http.Post("/organizations/"+id+"/policies", payload, &result); err != nil { return nil, err From 6e2aaaa96f23dc051617f6521ff306e776afb3b6 Mon Sep 17 00:00:00 2001 From: Tomer Heber Date: Mon, 5 Aug 2024 07:20:43 -0500 Subject: [PATCH 2/3] added a unit test --- client/organization_test.go | 52 +++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/client/organization_test.go b/client/organization_test.go index 8affa26e..b7fb1b28 100644 --- a/client/organization_test.go +++ b/client/organization_test.go @@ -139,6 +139,58 @@ var _ = Describe("Organization", func() { }) }) + Describe("OrganizationPolicyUpdate", func() { + hour12 := "12-h" + t := true + updatedMockOrganization := mockOrganization + updatedMockOrganization.DoNotConsiderMergeCommitsForPrPlans = true + updatedMockOrganization.EnableOidc = true + updatedMockOrganization.EnforcePrCommenterPermissions = true + updatedMockOrganization.DefaultTtl = &hour12 + + var updatedOrganization *Organization + var err error + + emptyString := "" + + Describe("Emtpy string is passed as null", func() { + BeforeEach(func() { + mockOrganizationIdCall(organizationId) + originalUpdatePayload := OrganizationPolicyUpdatePayload{ + DefaultTtl: &emptyString, + MaxTtl: &emptyString, + DoNotConsiderMergeCommitsForPrPlans: &t, + EnableOidc: &t, + EnforcePrCommenterPermissions: &t, + } + + sentUpdatePayload := OrganizationPolicyUpdatePayload{ + DefaultTtl: nil, + MaxTtl: nil, + DoNotConsiderMergeCommitsForPrPlans: &t, + EnableOidc: &t, + EnforcePrCommenterPermissions: &t, + } + + httpCall = mockHttpClient.EXPECT(). + Post("/organizations/"+organizationId+"/policies", sentUpdatePayload, gomock.Any()). + Do(func(path string, request interface{}, response *Organization) { + *response = updatedMockOrganization + }).Times(1) + + updatedOrganization, err = apiClient.OrganizationPolicyUpdate(originalUpdatePayload) + }) + + It("Should not return an error", func() { + Expect(err).To(BeNil()) + }) + + It("Should return organization received from API", func() { + Expect(*updatedOrganization).To(Equal(updatedMockOrganization)) + }) + }) + }) + Describe("OrganizationUserUpdateRole", func() { userId := "userId" roleId := "roleId" From da6ac65d9f8827b6822cae4cb4a2fa8260690ad2 Mon Sep 17 00:00:00 2001 From: Tomer Heber Date: Mon, 5 Aug 2024 07:32:02 -0500 Subject: [PATCH 3/3] updated test --- client/organization_test.go | 68 +++++++++++++++---------------------- 1 file changed, 28 insertions(+), 40 deletions(-) diff --git a/client/organization_test.go b/client/organization_test.go index b7fb1b28..7e14abe2 100644 --- a/client/organization_test.go +++ b/client/organization_test.go @@ -139,55 +139,43 @@ var _ = Describe("Organization", func() { }) }) - Describe("OrganizationPolicyUpdate", func() { - hour12 := "12-h" - t := true + Describe("Emtpy string is passed as null", func() { updatedMockOrganization := mockOrganization - updatedMockOrganization.DoNotConsiderMergeCommitsForPrPlans = true - updatedMockOrganization.EnableOidc = true - updatedMockOrganization.EnforcePrCommenterPermissions = true - updatedMockOrganization.DefaultTtl = &hour12 + updatedMockOrganization.DefaultTtl = nil + updatedMockOrganization.MaxTtl = nil var updatedOrganization *Organization var err error emptyString := "" - Describe("Emtpy string is passed as null", func() { - BeforeEach(func() { - mockOrganizationIdCall(organizationId) - originalUpdatePayload := OrganizationPolicyUpdatePayload{ - DefaultTtl: &emptyString, - MaxTtl: &emptyString, - DoNotConsiderMergeCommitsForPrPlans: &t, - EnableOidc: &t, - EnforcePrCommenterPermissions: &t, - } - - sentUpdatePayload := OrganizationPolicyUpdatePayload{ - DefaultTtl: nil, - MaxTtl: nil, - DoNotConsiderMergeCommitsForPrPlans: &t, - EnableOidc: &t, - EnforcePrCommenterPermissions: &t, - } - - httpCall = mockHttpClient.EXPECT(). - Post("/organizations/"+organizationId+"/policies", sentUpdatePayload, gomock.Any()). - Do(func(path string, request interface{}, response *Organization) { - *response = updatedMockOrganization - }).Times(1) - - updatedOrganization, err = apiClient.OrganizationPolicyUpdate(originalUpdatePayload) - }) + BeforeEach(func() { + mockOrganizationIdCall(organizationId) + originalUpdatePayload := OrganizationPolicyUpdatePayload{ + DefaultTtl: &emptyString, + MaxTtl: &emptyString, + } + + sentUpdatePayload := OrganizationPolicyUpdatePayload{ + DefaultTtl: nil, + MaxTtl: nil, + } + + httpCall = mockHttpClient.EXPECT(). + Post("/organizations/"+organizationId+"/policies", sentUpdatePayload, gomock.Any()). + Do(func(path string, request interface{}, response *Organization) { + *response = updatedMockOrganization + }).Times(1) + + updatedOrganization, err = apiClient.OrganizationPolicyUpdate(originalUpdatePayload) + }) - It("Should not return an error", func() { - Expect(err).To(BeNil()) - }) + It("Should not return an error", func() { + Expect(err).To(BeNil()) + }) - It("Should return organization received from API", func() { - Expect(*updatedOrganization).To(Equal(updatedMockOrganization)) - }) + It("Should return organization received from API", func() { + Expect(*updatedOrganization).To(Equal(updatedMockOrganization)) }) })