From ebe10faf95969fc3c43d6ff98346dc1b90d84794 Mon Sep 17 00:00:00 2001 From: Luke Massa Date: Wed, 6 Sep 2023 13:02:11 -0400 Subject: [PATCH 1/4] Change message for when request is not approved --- server/events/command_requirement_handler.go | 2 +- server/events/command_requirement_handler_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/events/command_requirement_handler.go b/server/events/command_requirement_handler.go index dadb526a78..a8c29e600c 100644 --- a/server/events/command_requirement_handler.go +++ b/server/events/command_requirement_handler.go @@ -22,7 +22,7 @@ func (a *DefaultCommandRequirementHandler) ValidatePlanProject(repoDir string, c switch req { case raw.ApprovedRequirement: if !ctx.PullReqStatus.ApprovalStatus.IsApproved { - return "Pull request must be approved by at least one person other than the author before running plan.", nil + return "Pull request must be approved according to the project's approval rules before running plan.", nil } case raw.MergeableRequirement: if !ctx.PullReqStatus.Mergeable { diff --git a/server/events/command_requirement_handler_test.go b/server/events/command_requirement_handler_test.go index f76ff7254c..6ea45b7a65 100644 --- a/server/events/command_requirement_handler_test.go +++ b/server/events/command_requirement_handler_test.go @@ -58,7 +58,7 @@ func TestAggregateApplyRequirements_ValidatePlanProject(t *testing.T) { ApprovalStatus: models.ApprovalStatus{IsApproved: false}, }, }, - wantFailure: "Pull request must be approved by at least one person other than the author before running plan.", + wantFailure: "Pull request must be approved according to the project's approval rules before running plan.", wantErr: assert.NoError, }, { @@ -142,7 +142,7 @@ func TestAggregateApplyRequirements_ValidateApplyProject(t *testing.T) { ApprovalStatus: models.ApprovalStatus{IsApproved: false}, }, }, - wantFailure: "Pull request must be approved by at least one person other than the author before running apply.", + wantFailure: "Pull request must be approved according to the project's approval rules before running plan.", wantErr: assert.NoError, }, { @@ -249,7 +249,7 @@ func TestAggregateApplyRequirements_ValidateImportProject(t *testing.T) { ApprovalStatus: models.ApprovalStatus{IsApproved: false}, }, }, - wantFailure: "Pull request must be approved by at least one person other than the author before running import.", + wantFailure: "Pull request must be approved according to the project's approval rules before running plan.", wantErr: assert.NoError, }, { From 479516d6e669cc684ca9a0909de35e48c52fce43 Mon Sep 17 00:00:00 2001 From: Luke Massa Date: Wed, 6 Sep 2023 13:10:21 -0400 Subject: [PATCH 2/4] Updates --- server/events/command_requirement_handler.go | 2 +- server/events/command_requirement_handler_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/events/command_requirement_handler.go b/server/events/command_requirement_handler.go index a8c29e600c..d34314ccca 100644 --- a/server/events/command_requirement_handler.go +++ b/server/events/command_requirement_handler.go @@ -43,7 +43,7 @@ func (a *DefaultCommandRequirementHandler) ValidateApplyProject(repoDir string, switch req { case raw.ApprovedRequirement: if !ctx.PullReqStatus.ApprovalStatus.IsApproved { - return "Pull request must be approved by at least one person other than the author before running apply.", nil + return "Pull request must be approved according to the project's approval rules before running apply.", nil } // this should come before mergeability check since mergeability is a superset of this check. case valid.PoliciesPassedCommandReq: diff --git a/server/events/command_requirement_handler_test.go b/server/events/command_requirement_handler_test.go index 6ea45b7a65..760d839481 100644 --- a/server/events/command_requirement_handler_test.go +++ b/server/events/command_requirement_handler_test.go @@ -249,7 +249,7 @@ func TestAggregateApplyRequirements_ValidateImportProject(t *testing.T) { ApprovalStatus: models.ApprovalStatus{IsApproved: false}, }, }, - wantFailure: "Pull request must be approved according to the project's approval rules before running plan.", + wantFailure: "Pull request must be approved according to the project's approval rules before running apply.", wantErr: assert.NoError, }, { From 7af229e2736530dc34ad02049bb4c007f40f7797 Mon Sep 17 00:00:00 2001 From: Luke Massa Date: Wed, 6 Sep 2023 13:11:01 -0400 Subject: [PATCH 3/4] Fix --- server/events/command_requirement_handler.go | 2 +- server/events/command_requirement_handler_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/events/command_requirement_handler.go b/server/events/command_requirement_handler.go index d34314ccca..8af12bec54 100644 --- a/server/events/command_requirement_handler.go +++ b/server/events/command_requirement_handler.go @@ -70,7 +70,7 @@ func (a *DefaultCommandRequirementHandler) ValidateImportProject(repoDir string, switch req { case raw.ApprovedRequirement: if !ctx.PullReqStatus.ApprovalStatus.IsApproved { - return "Pull request must be approved by at least one person other than the author before running import.", nil + return "Pull request must be approved according to the project's approval rules before running import.", nil } case raw.MergeableRequirement: if !ctx.PullReqStatus.Mergeable { diff --git a/server/events/command_requirement_handler_test.go b/server/events/command_requirement_handler_test.go index 760d839481..7a9891b07c 100644 --- a/server/events/command_requirement_handler_test.go +++ b/server/events/command_requirement_handler_test.go @@ -142,7 +142,7 @@ func TestAggregateApplyRequirements_ValidateApplyProject(t *testing.T) { ApprovalStatus: models.ApprovalStatus{IsApproved: false}, }, }, - wantFailure: "Pull request must be approved according to the project's approval rules before running plan.", + wantFailure: "Pull request must be approved according to the project's approval rules before running apply.", wantErr: assert.NoError, }, { @@ -249,7 +249,7 @@ func TestAggregateApplyRequirements_ValidateImportProject(t *testing.T) { ApprovalStatus: models.ApprovalStatus{IsApproved: false}, }, }, - wantFailure: "Pull request must be approved according to the project's approval rules before running apply.", + wantFailure: "Pull request must be approved according to the project's approval rules before running import.", wantErr: assert.NoError, }, { From 1a418c13f9daeb91e36e8cead0cbc35b157062e9 Mon Sep 17 00:00:00 2001 From: Luke Massa Date: Wed, 6 Sep 2023 13:13:50 -0400 Subject: [PATCH 4/4] Fix tests --- server/events/project_command_runner_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/events/project_command_runner_test.go b/server/events/project_command_runner_test.go index d82a912cd9..786b67088e 100644 --- a/server/events/project_command_runner_test.go +++ b/server/events/project_command_runner_test.go @@ -272,7 +272,7 @@ func TestDefaultProjectCommandRunner_ApplyNotApproved(t *testing.T) { When(mockWorkingDir.GetWorkingDir(ctx.BaseRepo, ctx.Pull, ctx.Workspace)).ThenReturn(tmp, nil) res := runner.Apply(ctx) - Equals(t, "Pull request must be approved by at least one person other than the author before running apply.", res.Failure) + Equals(t, "Pull request must be approved according to the project's approval rules before running apply.", res.Failure) } // Test that if mergeable is required and the PR isn't mergeable we give an error. @@ -676,7 +676,7 @@ func TestDefaultProjectCommandRunner_Import(t *testing.T) { IsApproved: false, }, }, - expFailure: "Pull request must be approved by at least one person other than the author before running import.", + expFailure: "Pull request must be approved according to the project's approval rules before running import.", }, }