From d223905125dadbba1f85880ebd2fe2f957cea4e7 Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Tue, 9 Mar 2021 18:32:08 -0800 Subject: [PATCH 01/11] actually pass the _content_ of the reviewers or teamreviewers variable --- eng/common/pipelines/templates/steps/create-pull-request.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/eng/common/pipelines/templates/steps/create-pull-request.yml b/eng/common/pipelines/templates/steps/create-pull-request.yml index bd76c296fabf..2098cc0d6895 100644 --- a/eng/common/pipelines/templates/steps/create-pull-request.yml +++ b/eng/common/pipelines/templates/steps/create-pull-request.yml @@ -84,8 +84,8 @@ steps: -PRTitle "${{ parameters.PRTitle }}" -PRBody "${{ coalesce(parameters.PRBody, parameters.CommitMsg, parameters.PRTitle) }}" -PRLabels "${{ parameters.PRLabels }}" - -UserReviewers "${{ parameters.GHReviewersVariable }}" - -TeamReviewers "${{ parameters.GHTeamReviewersVariable }}" + -UserReviewers "$(${{ parameters.GHReviewersVariable }})" + -TeamReviewers "$(${{ parameters.GHTeamReviewersVariable }})" -Assignees "${{ parameters.GHAssignessVariable }}" -CloseAfterOpenForTesting $${{ coalesce(parameters.CloseAfterOpenForTesting, 'false') }} -OpenAsDraft $${{ parameters.OpenAsDraft }} From 3d1529210f4853c8b422c16db051b6078436b3cf Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Mon, 15 Mar 2021 14:23:13 -0700 Subject: [PATCH 02/11] clean the users being passed into Submit-PullRequest.ps1 --- eng/common/scripts/Submit-PullRequest.ps1 | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/eng/common/scripts/Submit-PullRequest.ps1 b/eng/common/scripts/Submit-PullRequest.ps1 index 40a3b5ac6370..3a2a3de34af0 100644 --- a/eng/common/scripts/Submit-PullRequest.ps1 +++ b/eng/common/scripts/Submit-PullRequest.ps1 @@ -108,12 +108,18 @@ else { $resp | Write-Verbose LogDebug "Pull request created https://github.com/$RepoOwner/$RepoName/pull/$($resp.number)" + $prOwnerUser = $resp.user.login + # setting variable to reference the pull request by number Write-Host "##vso[task.setvariable variable=Submitted.PullRequest.Number]$($resp.number)" + # ensure that the user that was used to create the PR is not attempted to add as a reviewer + $usersCleaned = ($UserReviewers -split "," | ? { $_ -ne $prOwnerUser }) -join "," + $teamReviewersCleaned = ($TeamReviewers -split "," | ? { $_ -ne $prOwnerUser}) -join "," + if ($UserReviewers -or $TeamReviewers) { Add-GitHubPullRequestReviewers -RepoOwner $RepoOwner -RepoName $RepoName -PrNumber $resp.number ` - -Users $UserReviewers -Teams $TeamReviewers -AuthToken $AuthToken + -Users $usersCleaned -Teams $teamReviewersCleaned -AuthToken $AuthToken } if ($CloseAfterOpenForTesting) { From 2d7edc2b90802472b84d99bf83ae4f8508844adf Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Mon, 15 Mar 2021 14:26:10 -0700 Subject: [PATCH 03/11] add a trim to ensure that the split up arrays remain clean --- eng/common/scripts/Submit-PullRequest.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/eng/common/scripts/Submit-PullRequest.ps1 b/eng/common/scripts/Submit-PullRequest.ps1 index 3a2a3de34af0..33d0b5d84022 100644 --- a/eng/common/scripts/Submit-PullRequest.ps1 +++ b/eng/common/scripts/Submit-PullRequest.ps1 @@ -114,8 +114,8 @@ else { Write-Host "##vso[task.setvariable variable=Submitted.PullRequest.Number]$($resp.number)" # ensure that the user that was used to create the PR is not attempted to add as a reviewer - $usersCleaned = ($UserReviewers -split "," | ? { $_ -ne $prOwnerUser }) -join "," - $teamReviewersCleaned = ($TeamReviewers -split "," | ? { $_ -ne $prOwnerUser}) -join "," + $usersCleaned = ($UserReviewers -split "," | % { $_.Trim() } | ? { $_ -ne $prOwnerUser }) -join "," + $teamReviewersCleaned = ($TeamReviewers -split "," | % { $_.Trim() } | ? { $_ -ne $prOwnerUser}) -join "," if ($UserReviewers -or $TeamReviewers) { Add-GitHubPullRequestReviewers -RepoOwner $RepoOwner -RepoName $RepoName -PrNumber $resp.number ` From 6abb97a735d50cafccaf468f41da6143bd3ebbbb Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Mon, 15 Mar 2021 17:37:07 -0700 Subject: [PATCH 04/11] update to share common logic --- eng/common/scripts/Invoke-GitHubAPI.ps1 | 26 +++++++++++++++-------- eng/common/scripts/Submit-PullRequest.ps1 | 8 +++---- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/eng/common/scripts/Invoke-GitHubAPI.ps1 b/eng/common/scripts/Invoke-GitHubAPI.ps1 index 46a76251c554..a25ed27858c5 100644 --- a/eng/common/scripts/Invoke-GitHubAPI.ps1 +++ b/eng/common/scripts/Invoke-GitHubAPI.ps1 @@ -9,17 +9,25 @@ function Get-GitHubApiHeaders ($token) { return $headers } +function SplitParameterArray($members) { + if ($null -ne $members) { + if ($members -is [array]) + { + return $members + } + else { + return (@($members.Split(",") | % { $_.Trim() } | ? { return $_ })) + } + } + return $members +} + function Set-GitHubAPIParameters ($members, $parameterName, $parameters, $allowEmptyMembers=$false) { if ($null -ne $members) { - if ($members -is [array]) - { - $parameters[$parameterName] = $members - } - else { - $memberAdditions = @($members.Split(",") | % { $_.Trim() } | ? { return $_ }) - if (($memberAdditions.Count -gt 0) -or $allowEmptyMembers) { - $parameters[$parameterName] = $memberAdditions - } + $memberAdditions = SplitParameterArray -members $members + + if (($memberAdditions.Count -gt 0) -or $allowEmptyMembers) { + $parameters[$parameterName] = $memberAdditions } } diff --git a/eng/common/scripts/Submit-PullRequest.ps1 b/eng/common/scripts/Submit-PullRequest.ps1 index 33d0b5d84022..739a3c2a9374 100644 --- a/eng/common/scripts/Submit-PullRequest.ps1 +++ b/eng/common/scripts/Submit-PullRequest.ps1 @@ -114,12 +114,12 @@ else { Write-Host "##vso[task.setvariable variable=Submitted.PullRequest.Number]$($resp.number)" # ensure that the user that was used to create the PR is not attempted to add as a reviewer - $usersCleaned = ($UserReviewers -split "," | % { $_.Trim() } | ? { $_ -ne $prOwnerUser }) -join "," - $teamReviewersCleaned = ($TeamReviewers -split "," | % { $_.Trim() } | ? { $_ -ne $prOwnerUser}) -join "," + $cleanedUsers = (SplitParameterArray -members $TeamReviewers) | ? { $_ -ne $prOwnerUser } + $cleanedTeamReviewers = (SplitParameterArray -members $UserReviewers) | ? { $_ -ne $prOwnerUser } - if ($UserReviewers -or $TeamReviewers) { + if ($cleanedUsers -or $cleanedTeamReviewers) { Add-GitHubPullRequestReviewers -RepoOwner $RepoOwner -RepoName $RepoName -PrNumber $resp.number ` - -Users $usersCleaned -Teams $teamReviewersCleaned -AuthToken $AuthToken + -Users $cleanedUsers -Teams $cleanedTeamReviewers -AuthToken $AuthToken } if ($CloseAfterOpenForTesting) { From 88e367f2541384648da5935f5a349d6a62b171fe Mon Sep 17 00:00:00 2001 From: Scott Beddall <45376673+scbedd@users.noreply.github.com> Date: Tue, 16 Mar 2021 11:48:12 -0700 Subject: [PATCH 05/11] Update eng/common/scripts/Submit-PullRequest.ps1 Co-authored-by: Wes Haggard --- eng/common/scripts/Submit-PullRequest.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/common/scripts/Submit-PullRequest.ps1 b/eng/common/scripts/Submit-PullRequest.ps1 index 739a3c2a9374..c16e4199a397 100644 --- a/eng/common/scripts/Submit-PullRequest.ps1 +++ b/eng/common/scripts/Submit-PullRequest.ps1 @@ -114,7 +114,7 @@ else { Write-Host "##vso[task.setvariable variable=Submitted.PullRequest.Number]$($resp.number)" # ensure that the user that was used to create the PR is not attempted to add as a reviewer - $cleanedUsers = (SplitParameterArray -members $TeamReviewers) | ? { $_ -ne $prOwnerUser } + $cleanedUsers = (SplitParameterArray -members $UserReviewers) | ? { $_ -ne $prOwnerUser } $cleanedTeamReviewers = (SplitParameterArray -members $UserReviewers) | ? { $_ -ne $prOwnerUser } if ($cleanedUsers -or $cleanedTeamReviewers) { From 917de412053c783e48b5012de327ae16ded88ad0 Mon Sep 17 00:00:00 2001 From: Scott Beddall <45376673+scbedd@users.noreply.github.com> Date: Tue, 16 Mar 2021 11:48:19 -0700 Subject: [PATCH 06/11] Update eng/common/scripts/Submit-PullRequest.ps1 Co-authored-by: Wes Haggard --- eng/common/scripts/Submit-PullRequest.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/common/scripts/Submit-PullRequest.ps1 b/eng/common/scripts/Submit-PullRequest.ps1 index c16e4199a397..d835ced6175f 100644 --- a/eng/common/scripts/Submit-PullRequest.ps1 +++ b/eng/common/scripts/Submit-PullRequest.ps1 @@ -115,7 +115,7 @@ else { # ensure that the user that was used to create the PR is not attempted to add as a reviewer $cleanedUsers = (SplitParameterArray -members $UserReviewers) | ? { $_ -ne $prOwnerUser } - $cleanedTeamReviewers = (SplitParameterArray -members $UserReviewers) | ? { $_ -ne $prOwnerUser } + $cleanedTeamReviewers = (SplitParameterArray -members $TeamReviewers) | ? { $_ -ne $prOwnerUser } if ($cleanedUsers -or $cleanedTeamReviewers) { Add-GitHubPullRequestReviewers -RepoOwner $RepoOwner -RepoName $RepoName -PrNumber $resp.number ` From 2351b5d590917ac9701efb87aa0997e8d359eb91 Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Tue, 16 Mar 2021 16:15:47 -0700 Subject: [PATCH 07/11] cast to array explicitly to handle array length of 1 NOT being treated as an array. null coalescing operator used to default the value to empty string to avoid nil --- eng/common/scripts/Invoke-GitHubAPI.ps1 | 2 +- eng/common/scripts/Submit-PullRequest.ps1 | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/eng/common/scripts/Invoke-GitHubAPI.ps1 b/eng/common/scripts/Invoke-GitHubAPI.ps1 index a25ed27858c5..808692651a1e 100644 --- a/eng/common/scripts/Invoke-GitHubAPI.ps1 +++ b/eng/common/scripts/Invoke-GitHubAPI.ps1 @@ -24,7 +24,7 @@ function SplitParameterArray($members) { function Set-GitHubAPIParameters ($members, $parameterName, $parameters, $allowEmptyMembers=$false) { if ($null -ne $members) { - $memberAdditions = SplitParameterArray -members $members + [array]$memberAdditions = SplitParameterArray -members $members if (($memberAdditions.Count -gt 0) -or $allowEmptyMembers) { $parameters[$parameterName] = $memberAdditions diff --git a/eng/common/scripts/Submit-PullRequest.ps1 b/eng/common/scripts/Submit-PullRequest.ps1 index d835ced6175f..53333a183062 100644 --- a/eng/common/scripts/Submit-PullRequest.ps1 +++ b/eng/common/scripts/Submit-PullRequest.ps1 @@ -114,12 +114,13 @@ else { Write-Host "##vso[task.setvariable variable=Submitted.PullRequest.Number]$($resp.number)" # ensure that the user that was used to create the PR is not attempted to add as a reviewer - $cleanedUsers = (SplitParameterArray -members $UserReviewers) | ? { $_ -ne $prOwnerUser } - $cleanedTeamReviewers = (SplitParameterArray -members $TeamReviewers) | ? { $_ -ne $prOwnerUser } + # we cast to an array to ensure that length-1 arrays actually stay as array values + [array]$cleanedUsers = @(SplitParameterArray -members $UserReviewers) | ? { $_ -ne $prOwnerUser } + [array]$cleanedTeamReviewers = @(SplitParameterArray -members $TeamReviewers) | ? { $_ -ne $prOwnerUser } if ($cleanedUsers -or $cleanedTeamReviewers) { Add-GitHubPullRequestReviewers -RepoOwner $RepoOwner -RepoName $RepoName -PrNumber $resp.number ` - -Users $cleanedUsers -Teams $cleanedTeamReviewers -AuthToken $AuthToken + -Users $cleanedUsers ?? "" -Teams $cleanedTeamReviewers ?? "" -AuthToken $AuthToken } if ($CloseAfterOpenForTesting) { From 581c38025fc0dc80cfa8ee255701935abc06ee0b Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Thu, 18 Mar 2021 12:01:44 -0700 Subject: [PATCH 08/11] remove null resolution handlers --- eng/common/scripts/Submit-PullRequest.ps1 | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/eng/common/scripts/Submit-PullRequest.ps1 b/eng/common/scripts/Submit-PullRequest.ps1 index 53333a183062..4ad3263b76bc 100644 --- a/eng/common/scripts/Submit-PullRequest.ps1 +++ b/eng/common/scripts/Submit-PullRequest.ps1 @@ -118,9 +118,12 @@ else { [array]$cleanedUsers = @(SplitParameterArray -members $UserReviewers) | ? { $_ -ne $prOwnerUser } [array]$cleanedTeamReviewers = @(SplitParameterArray -members $TeamReviewers) | ? { $_ -ne $prOwnerUser } + if ($null -eq $cleanedUsers){ $cleanedUsers = "" } + if ($null -eq $cleanedTeamReviewers){ $cleanedUsers = "" } + if ($cleanedUsers -or $cleanedTeamReviewers) { Add-GitHubPullRequestReviewers -RepoOwner $RepoOwner -RepoName $RepoName -PrNumber $resp.number ` - -Users $cleanedUsers ?? "" -Teams $cleanedTeamReviewers ?? "" -AuthToken $AuthToken + -Users $cleanedUsers -Teams $cleanedTeamReviewers -AuthToken $AuthToken } if ($CloseAfterOpenForTesting) { From 75aeebe3c3f0481fd28ebfb971398fb1408ef33c Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Thu, 18 Mar 2021 14:31:06 -0700 Subject: [PATCH 09/11] update to remove return to avoid adding a null to an empty array --- eng/common/scripts/Invoke-GitHubAPI.ps1 | 1 - eng/common/scripts/Submit-PullRequest.ps1 | 7 ++----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/eng/common/scripts/Invoke-GitHubAPI.ps1 b/eng/common/scripts/Invoke-GitHubAPI.ps1 index 808692651a1e..126583200223 100644 --- a/eng/common/scripts/Invoke-GitHubAPI.ps1 +++ b/eng/common/scripts/Invoke-GitHubAPI.ps1 @@ -19,7 +19,6 @@ function SplitParameterArray($members) { return (@($members.Split(",") | % { $_.Trim() } | ? { return $_ })) } } - return $members } function Set-GitHubAPIParameters ($members, $parameterName, $parameters, $allowEmptyMembers=$false) { diff --git a/eng/common/scripts/Submit-PullRequest.ps1 b/eng/common/scripts/Submit-PullRequest.ps1 index 4ad3263b76bc..890590fee6d4 100644 --- a/eng/common/scripts/Submit-PullRequest.ps1 +++ b/eng/common/scripts/Submit-PullRequest.ps1 @@ -115,11 +115,8 @@ else { # ensure that the user that was used to create the PR is not attempted to add as a reviewer # we cast to an array to ensure that length-1 arrays actually stay as array values - [array]$cleanedUsers = @(SplitParameterArray -members $UserReviewers) | ? { $_ -ne $prOwnerUser } - [array]$cleanedTeamReviewers = @(SplitParameterArray -members $TeamReviewers) | ? { $_ -ne $prOwnerUser } - - if ($null -eq $cleanedUsers){ $cleanedUsers = "" } - if ($null -eq $cleanedTeamReviewers){ $cleanedUsers = "" } + $cleanedUsers = @(SplitParameterArray -members $UserReviewers) | ? { $_ -ne $prOwnerUser -and $null -ne $_ } + $cleanedTeamReviewers = @(SplitParameterArray -members $TeamReviewers) | ? { $_ -ne $prOwnerUser -and $null -ne $_ } if ($cleanedUsers -or $cleanedTeamReviewers) { Add-GitHubPullRequestReviewers -RepoOwner $RepoOwner -RepoName $RepoName -PrNumber $resp.number ` From 88420df9a84bda9a942e21eec8aca1b0be532338 Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Wed, 24 Mar 2021 16:04:44 -0700 Subject: [PATCH 10/11] resolve wonky problems w/ failure to submit to to nill property --- .../pipelines/templates/steps/create-pull-request.yml | 2 +- eng/common/scripts/Invoke-GitHubAPI.ps1 | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/eng/common/pipelines/templates/steps/create-pull-request.yml b/eng/common/pipelines/templates/steps/create-pull-request.yml index 2098cc0d6895..ecce673e31a4 100644 --- a/eng/common/pipelines/templates/steps/create-pull-request.yml +++ b/eng/common/pipelines/templates/steps/create-pull-request.yml @@ -86,6 +86,6 @@ steps: -PRLabels "${{ parameters.PRLabels }}" -UserReviewers "$(${{ parameters.GHReviewersVariable }})" -TeamReviewers "$(${{ parameters.GHTeamReviewersVariable }})" - -Assignees "${{ parameters.GHAssignessVariable }}" + -Assignees "$(${{ parameters.GHAssignessVariable }})" -CloseAfterOpenForTesting $${{ coalesce(parameters.CloseAfterOpenForTesting, 'false') }} -OpenAsDraft $${{ parameters.OpenAsDraft }} diff --git a/eng/common/scripts/Invoke-GitHubAPI.ps1 b/eng/common/scripts/Invoke-GitHubAPI.ps1 index 126583200223..aff08798436f 100644 --- a/eng/common/scripts/Invoke-GitHubAPI.ps1 +++ b/eng/common/scripts/Invoke-GitHubAPI.ps1 @@ -21,11 +21,11 @@ function SplitParameterArray($members) { } } -function Set-GitHubAPIParameters ($members, $parameterName, $parameters, $allowEmptyMembers=$false) { +function Set-GitHubAPIParameters ($members, $parameterName, $parameters) { if ($null -ne $members) { [array]$memberAdditions = SplitParameterArray -members $members - if (($memberAdditions.Count -gt 0) -or $allowEmptyMembers) { + if ($memberAdditions.Count -gt 0) { $parameters[$parameterName] = $memberAdditions } } @@ -318,10 +318,10 @@ function Update-GitHubIssue { if ($Milestone) { $parameters["milestone"] = $Milestone } $parameters = Set-GitHubAPIParameters -members $Labels -parameterName "labels" ` - -parameters $parameters -allowEmptyMembers $true + -parameters $parameters $parameters = Set-GitHubAPIParameters -members $Assignees -parameterName "assignees" ` - -parameters $parameters -allowEmptyMembers $true + -parameters $parameters return Invoke-RestMethod ` -Method PATCH ` From 5fe7c9f5837edb81b116b8be2bb5f049411041d1 Mon Sep 17 00:00:00 2001 From: scbedd <45376673+scbedd@users.noreply.github.com> Date: Thu, 25 Mar 2021 12:31:52 -0700 Subject: [PATCH 11/11] restored original functionality on presence of members --- eng/common/scripts/Invoke-GitHubAPI.ps1 | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/eng/common/scripts/Invoke-GitHubAPI.ps1 b/eng/common/scripts/Invoke-GitHubAPI.ps1 index aff08798436f..5d2981f01bb8 100644 --- a/eng/common/scripts/Invoke-GitHubAPI.ps1 +++ b/eng/common/scripts/Invoke-GitHubAPI.ps1 @@ -21,11 +21,13 @@ function SplitParameterArray($members) { } } -function Set-GitHubAPIParameters ($members, $parameterName, $parameters) { +function Set-GitHubAPIParameters ($members, $parameterName, $parameters, $allowEmptyMembers = $false) { if ($null -ne $members) { [array]$memberAdditions = SplitParameterArray -members $members - if ($memberAdditions.Count -gt 0) { + if ($null -eq $memberAdditions -and $allowEmptyMembers){ $memberAdditions = @() } + + if ($memberAdditions.Count -gt 0 -or $allowEmptyMembers) { $parameters[$parameterName] = $memberAdditions } } @@ -318,10 +320,10 @@ function Update-GitHubIssue { if ($Milestone) { $parameters["milestone"] = $Milestone } $parameters = Set-GitHubAPIParameters -members $Labels -parameterName "labels" ` - -parameters $parameters + -parameters $parameters -allowEmptyMembers $true $parameters = Set-GitHubAPIParameters -members $Assignees -parameterName "assignees" ` - -parameters $parameters + -parameters $parameters -allowEmptyMembers $true return Invoke-RestMethod ` -Method PATCH `