From 8c044f7e38b440bd2a9375d69f167a947d4d24eb Mon Sep 17 00:00:00 2001 From: hmmwhatsthisdo <2093321+hmmwhatsthisdo@users.noreply.github.com> Date: Wed, 7 Aug 2024 19:24:35 -0700 Subject: [PATCH 1/8] Refactor Set-JiraIssue to only call Get-JiraField once --- JiraPS/Public/Set-JiraIssue.ps1 | 46 ++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/JiraPS/Public/Set-JiraIssue.ps1 b/JiraPS/Public/Set-JiraIssue.ps1 index 33d6f633..31db5d8b 100644 --- a/JiraPS/Public/Set-JiraIssue.ps1 +++ b/JiraPS/Public/Set-JiraIssue.ps1 @@ -156,17 +156,51 @@ function Set-JiraIssue { ) } + if ($Fields) { + + # Fetch all available fields ahead-of-time to avoid repeated API calls in the upcoming loop. + # Eventually, this may be better to extract from EditMeta. + $AvailableFields = Get-JiraField -Credential $Credential -ErrorAction Stop + + $AvailableFieldsById = $AvailableFields | Group-Object -Property Id -AsHashTable -AsString + $AvailableFieldsByName = $AvailableFields | Group-Object -Property Name -AsHashTable -AsString + Write-Debug "[$($MyInvocation.MyCommand.Name)] Resolving `$Fields" foreach ($_key in $Fields.Keys) { - $name = $_key - $value = $Fields.$_key - $field = Get-JiraField -Field $name -Credential $Credential -ErrorAction Stop + # The Fields hashtable supports both name- and ID-based lookup for custom fields, so we have to search both. + if ($AvailableFieldsById.ContainsKey($_key)) { + $field = $AvailableFieldsById[$_key][0] + Write-Debug "[$($MyInvocation.MyCommand.Name)] $_key appears to be a field ID" + } elseif ($AvailableFieldsById.ContainsKey("customfield_$_key")) { + $field = $AvailableFieldsById["customfield_$_key"][0] + Write-Debug "[$($MyInvocation.MyCommand.Name)] $_key appears to be a numerical field ID (customfield_$_key)" + } elseif ($AvailableFieldsByName.ContainsKey($_key) -and $AvailableFieldsByName[$_key].Count -eq 1) { + $field = $AvailableFieldsByName[$_key][0] + Write-Debug "[$($MyInvocation.MyCommand.Name)] $_key appears to be a human-readable field name ($($field.ID))" + } elseif ($AvailableFieldsByName.ContainsKey($_key)) { + # Jira does not prevent multiple custom fields with the same name, so we have to ensure + # any name references are unambiguous. + + # More than one value in $AvailableFieldsByName (i.e. .Count -gt 1) indicates two duplicate custom fields. + $exception = ([System.ArgumentException]"Ambiguously Referenced Parameter") + $errorId = 'ParameterValue.AmbiguousParameter' + $errorCategory = 'InvalidArgument' + $errorTarget = $Fields + $errorItem = New-Object -TypeName System.Management.Automation.ErrorRecord $exception, $errorId, $errorCategory, $errorTarget + $errorItem.ErrorDetails = "Field name [$name] in -Fields hashtable ambiguously refers to more than one field. Use Get-JiraField for more information, or specify the custom field by its ID." + $PSCmdlet.ThrowTerminatingError($errorItem) + } else { + $exception = ([System.ArgumentException]"Invalid value for Parameter") + $errorId = 'ParameterValue.InvalidFields' + $errorCategory = 'InvalidArgument' + $errorTarget = $Fields + $errorItem = New-Object -TypeName System.Management.Automation.ErrorRecord $exception, $errorId, $errorCategory, $errorTarget + $errorItem.ErrorDetails = "Unable to identify field [$name] from -Fields hashtable. Use Get-JiraField for more information." + $PSCmdlet.ThrowTerminatingError($errorItem) + } - # For some reason, this was coming through as a hashtable instead of a String, - # which was causing ConvertTo-Json to crash later. - # Not sure why, but this forces $id to be a String and not a hashtable. $id = [string]$field.Id $issueProps.update[$id] = @(@{ 'set' = $value }) } From 32c51a9ec93800c55d520508bf9efc0f9b6c4005 Mon Sep 17 00:00:00 2001 From: hmmwhatsthisdo <2093321+hmmwhatsthisdo@users.noreply.github.com> Date: Wed, 7 Aug 2024 19:33:23 -0700 Subject: [PATCH 2/8] Reduce log-spam and add explicit debug message before calling Get-JiraField --- JiraPS/Public/Set-JiraIssue.ps1 | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/JiraPS/Public/Set-JiraIssue.ps1 b/JiraPS/Public/Set-JiraIssue.ps1 index 31db5d8b..70d15667 100644 --- a/JiraPS/Public/Set-JiraIssue.ps1 +++ b/JiraPS/Public/Set-JiraIssue.ps1 @@ -159,9 +159,11 @@ function Set-JiraIssue { if ($Fields) { + Write-Debug "[$($MyInvocation.MyCommand.Name)] Enumerating fields defined on the server" + # Fetch all available fields ahead-of-time to avoid repeated API calls in the upcoming loop. # Eventually, this may be better to extract from EditMeta. - $AvailableFields = Get-JiraField -Credential $Credential -ErrorAction Stop + $AvailableFields = Get-JiraField -Credential $Credential -ErrorAction Stop -Debug:$false $AvailableFieldsById = $AvailableFields | Group-Object -Property Id -AsHashTable -AsString $AvailableFieldsByName = $AvailableFields | Group-Object -Property Name -AsHashTable -AsString From cdfb7496346d7019bc2e4ce2aabed2b9ce31e56a Mon Sep 17 00:00:00 2001 From: hmmwhatsthisdo <2093321+hmmwhatsthisdo@users.noreply.github.com> Date: Wed, 7 Aug 2024 19:34:41 -0700 Subject: [PATCH 3/8] Refactor New-JiraIssue to only call Get-JiraField once --- JiraPS/Public/New-JiraIssue.ps1 | 69 ++++++++++++++++++++++++--------- 1 file changed, 51 insertions(+), 18 deletions(-) diff --git a/JiraPS/Public/New-JiraIssue.ps1 b/JiraPS/Public/New-JiraIssue.ps1 index d2d83784..286eca51 100644 --- a/JiraPS/Public/New-JiraIssue.ps1 +++ b/JiraPS/Public/New-JiraIssue.ps1 @@ -114,29 +114,62 @@ function New-JiraIssue { } } - Write-Debug "[$($MyInvocation.MyCommand.Name)] Resolving `$Fields" - foreach ($_key in $Fields.Keys) { - $name = $_key - $value = $Fields.$_key - - if ($field = Get-JiraField -Field $name -Credential $Credential -Debug:$false) { - # For some reason, this was coming through as a hashtable instead of a String, - # which was causing ConvertTo-Json to crash later. - # Not sure why, but this forces $id to be a String and not a hashtable. + If ($Fields) { + + Write-Debug "[$($MyInvocation.MyCommand.Name)] Enumerating fields for server" + + # Fetch all available fields ahead-of-time to avoid repeated API calls in the upcoming loop. + # Eventually, this may be better to extract from CreateMeta. + $AvailableFields = Get-JiraField -Credential $Credential -ErrorAction Stop -Debug:$false + + $AvailableFieldsById = $AvailableFields | Group-Object -Property Id -AsHashTable -AsString + $AvailableFieldsByName = $AvailableFields | Group-Object -Property Name -AsHashTable -AsString + + + Write-Debug "[$($MyInvocation.MyCommand.Name)] Resolving `$Fields" + + foreach ($_key in $Fields.Keys) { + + $name = $_key + $value = $Fields.$_key + # The Fields hashtable supports both name- and ID-based lookup for custom fields, so we have to search both. + if ($AvailableFieldsById.ContainsKey($name)) { + $field = $AvailableFieldsById[$name][0] + Write-Debug "[$($MyInvocation.MyCommand.Name)] $name appears to be a field ID" + } elseif ($AvailableFieldsById.ContainsKey("customfield_$name")) { + $field = $AvailableFieldsById["customfield_$name"][0] + Write-Debug "[$($MyInvocation.MyCommand.Name)] $name appears to be a numerical field ID (customfield_$name)" + } elseif ($AvailableFieldsByName.ContainsKey($name) -and $AvailableFieldsByName[$name].Count -eq 1) { + $field = $AvailableFieldsByName[$name][0] + Write-Debug "[$($MyInvocation.MyCommand.Name)] $name appears to be a human-readable field name ($($field.ID))" + } elseif ($AvailableFieldsByName.ContainsKey($name)) { + # Jira does not prevent multiple custom fields with the same name, so we have to ensure + # any name references are unambiguous. + + # More than one value in $AvailableFieldsByName (i.e. .Count -gt 1) indicates two duplicate custom fields. + $exception = ([System.ArgumentException]"Ambiguously Referenced Parameter") + $errorId = 'ParameterValue.AmbiguousParameter' + $errorCategory = 'InvalidArgument' + $errorTarget = $Fields + $errorItem = New-Object -TypeName System.Management.Automation.ErrorRecord $exception, $errorId, $errorCategory, $errorTarget + $errorItem.ErrorDetails = "Field name [$name] in -Fields hashtable ambiguously refers to more than one field. Use Get-JiraField for more information, or specify the custom field by its ID." + $PSCmdlet.ThrowTerminatingError($errorItem) + } else { + $exception = ([System.ArgumentException]"Invalid value for Parameter") + $errorId = 'ParameterValue.InvalidFields' + $errorCategory = 'InvalidArgument' + $errorTarget = $Fields + $errorItem = New-Object -TypeName System.Management.Automation.ErrorRecord $exception, $errorId, $errorCategory, $errorTarget + $errorItem.ErrorDetails = "Unable to identify field [$name] from -Fields hashtable. Use Get-JiraField for more information." + $PSCmdlet.ThrowTerminatingError($errorItem) + } + $id = $field.Id $requestBody["$id"] = $value } - else { - $exception = ([System.ArgumentException]"Invalid value for Parameter") - $errorId = 'ParameterValue.InvalidFields' - $errorCategory = 'InvalidArgument' - $errorTarget = $Fields - $errorItem = New-Object -TypeName System.Management.Automation.ErrorRecord $exception, $errorId, $errorCategory, $errorTarget - $errorItem.ErrorDetails = "Unable to identify field [$name] from -Fields hashtable. Use Get-JiraField for more information." - $PSCmdlet.ThrowTerminatingError($errorItem) - } } + Write-Verbose "[$($MyInvocation.MyCommand.Name)] Validating fields with metadata" foreach ($c in $createmeta) { Write-Debug "[$($MyInvocation.MyCommand.Name)] Checking metadata for `$c [$c]" From 7b5e29a7e6aded4d2dc765172ec87e3fb54b6466 Mon Sep 17 00:00:00 2001 From: hmmwhatsthisdo <2093321+hmmwhatsthisdo@users.noreply.github.com> Date: Wed, 7 Aug 2024 19:41:01 -0700 Subject: [PATCH 4/8] Re-add missing variable declarations --- JiraPS/Public/Set-JiraIssue.ps1 | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/JiraPS/Public/Set-JiraIssue.ps1 b/JiraPS/Public/Set-JiraIssue.ps1 index 70d15667..bd6f4b93 100644 --- a/JiraPS/Public/Set-JiraIssue.ps1 +++ b/JiraPS/Public/Set-JiraIssue.ps1 @@ -171,17 +171,20 @@ function Set-JiraIssue { Write-Debug "[$($MyInvocation.MyCommand.Name)] Resolving `$Fields" foreach ($_key in $Fields.Keys) { + $name = $_key + $value = $Fields.$_key + # The Fields hashtable supports both name- and ID-based lookup for custom fields, so we have to search both. - if ($AvailableFieldsById.ContainsKey($_key)) { - $field = $AvailableFieldsById[$_key][0] - Write-Debug "[$($MyInvocation.MyCommand.Name)] $_key appears to be a field ID" - } elseif ($AvailableFieldsById.ContainsKey("customfield_$_key")) { - $field = $AvailableFieldsById["customfield_$_key"][0] - Write-Debug "[$($MyInvocation.MyCommand.Name)] $_key appears to be a numerical field ID (customfield_$_key)" - } elseif ($AvailableFieldsByName.ContainsKey($_key) -and $AvailableFieldsByName[$_key].Count -eq 1) { - $field = $AvailableFieldsByName[$_key][0] - Write-Debug "[$($MyInvocation.MyCommand.Name)] $_key appears to be a human-readable field name ($($field.ID))" - } elseif ($AvailableFieldsByName.ContainsKey($_key)) { + if ($AvailableFieldsById.ContainsKey($name)) { + $field = $AvailableFieldsById[$name][0] + Write-Debug "[$($MyInvocation.MyCommand.Name)] [$name] appears to be a field ID" + } elseif ($AvailableFieldsById.ContainsKey("customfield_$name")) { + $field = $AvailableFieldsById["customfield_$name"][0] + Write-Debug "[$($MyInvocation.MyCommand.Name)] [$name] appears to be a numerical field ID (customfield_$name)" + } elseif ($AvailableFieldsByName.ContainsKey($name) -and $AvailableFieldsByName[$name].Count -eq 1) { + $field = $AvailableFieldsByName[$name][0] + Write-Debug "[$($MyInvocation.MyCommand.Name)] [$name] appears to be a human-readable field name ($($field.ID))" + } elseif ($AvailableFieldsByName.ContainsKey($name)) { # Jira does not prevent multiple custom fields with the same name, so we have to ensure # any name references are unambiguous. From 7fed26fc569970d5bc8c7302ae633e629a2f607b Mon Sep 17 00:00:00 2001 From: hmmwhatsthisdo <2093321+hmmwhatsthisdo@users.noreply.github.com> Date: Wed, 7 Aug 2024 19:41:23 -0700 Subject: [PATCH 5/8] Reunify parameter formatting in logs --- JiraPS/Public/New-JiraIssue.ps1 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/JiraPS/Public/New-JiraIssue.ps1 b/JiraPS/Public/New-JiraIssue.ps1 index 286eca51..52a4c893 100644 --- a/JiraPS/Public/New-JiraIssue.ps1 +++ b/JiraPS/Public/New-JiraIssue.ps1 @@ -135,13 +135,13 @@ function New-JiraIssue { # The Fields hashtable supports both name- and ID-based lookup for custom fields, so we have to search both. if ($AvailableFieldsById.ContainsKey($name)) { $field = $AvailableFieldsById[$name][0] - Write-Debug "[$($MyInvocation.MyCommand.Name)] $name appears to be a field ID" + Write-Debug "[$($MyInvocation.MyCommand.Name)] [$name] appears to be a field ID" } elseif ($AvailableFieldsById.ContainsKey("customfield_$name")) { $field = $AvailableFieldsById["customfield_$name"][0] - Write-Debug "[$($MyInvocation.MyCommand.Name)] $name appears to be a numerical field ID (customfield_$name)" + Write-Debug "[$($MyInvocation.MyCommand.Name)] [$name] appears to be a numerical field ID (customfield_$name)" } elseif ($AvailableFieldsByName.ContainsKey($name) -and $AvailableFieldsByName[$name].Count -eq 1) { $field = $AvailableFieldsByName[$name][0] - Write-Debug "[$($MyInvocation.MyCommand.Name)] $name appears to be a human-readable field name ($($field.ID))" + Write-Debug "[$($MyInvocation.MyCommand.Name)] [$name] appears to be a human-readable field name ($($field.ID))" } elseif ($AvailableFieldsByName.ContainsKey($name)) { # Jira does not prevent multiple custom fields with the same name, so we have to ensure # any name references are unambiguous. From 0815b91dfcf77fa8460dd38c6e652223a2c515f6 Mon Sep 17 00:00:00 2001 From: hmmwhatsthisdo <2093321+hmmwhatsthisdo@users.noreply.github.com> Date: Tue, 13 Aug 2024 07:53:01 -0700 Subject: [PATCH 6/8] Fix New-JiraIssue unit tests --- Tests/Functions/New-JiraIssue.Unit.Tests.ps1 | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Tests/Functions/New-JiraIssue.Unit.Tests.ps1 b/Tests/Functions/New-JiraIssue.Unit.Tests.ps1 index 6b76ae53..3b580c79 100644 --- a/Tests/Functions/New-JiraIssue.Unit.Tests.ps1 +++ b/Tests/Functions/New-JiraIssue.Unit.Tests.ps1 @@ -81,7 +81,20 @@ Describe "New-JiraIssue" -Tag 'Unit' { # This one needs to be able to output multiple objects Mock Get-JiraField { - $Field | % { + + $(If ($null -eq $Field) { + @( + 'Project' + 'IssueType' + 'Priority' + 'Summary' + 'Description' + 'Reporter' + 'CustomField' + ) + } Else { + $Field + }) | % { $object = [PSCustomObject] @{ 'Id' = $_ } From 141c4668ae5bed2d67f99456bb87cbcdd3770d6c Mon Sep 17 00:00:00 2001 From: hmmwhatsthisdo <2093321+hmmwhatsthisdo@users.noreply.github.com> Date: Tue, 13 Aug 2024 08:36:45 -0700 Subject: [PATCH 7/8] Add mock for Get-JiraField --- Tests/Functions/Set-JiraIssue.Unit.Tests.ps1 | 28 +++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/Tests/Functions/Set-JiraIssue.Unit.Tests.ps1 b/Tests/Functions/Set-JiraIssue.Unit.Tests.ps1 index 1dd2cd00..af29aa52 100644 --- a/Tests/Functions/Set-JiraIssue.Unit.Tests.ps1 +++ b/Tests/Functions/Set-JiraIssue.Unit.Tests.ps1 @@ -58,6 +58,32 @@ Describe "Set-JiraIssue" -Tag 'Unit' { return $object } + Mock Get-JiraField { + + $(If ($null -eq $Field) { + @( + 'Project' + 'IssueType' + 'Priority' + 'Summary' + 'Description' + 'Reporter' + 'CustomField' + 'customfield_12345' + 'customfield_67890' + 'customfield_111222' + ) + } Else { + $Field + }) | % { + $object = [PSCustomObject] @{ + 'Id' = $_ + } + $object.PSObject.TypeNames.Insert(0, 'JiraPS.Field') + $object + } + } + Mock Resolve-JiraIssueObject -ModuleName JiraPS { Get-JiraIssue -Key $Issue } @@ -73,7 +99,7 @@ Describe "Set-JiraIssue" -Tag 'Unit' { # actually try to query a JIRA instance Mock Invoke-JiraMethod { ShowMockInfo 'Invoke-JiraMethod' 'Method', 'Uri' - throw "Unidentified call to Invoke-JiraMethod" + throw "Unidentified call ($Method $Uri) to Invoke-JiraMethod" } Context "Sanity checking" { From 5a9b2d6b04fdf891efe0354556241976b071648f Mon Sep 17 00:00:00 2001 From: hmmwhatsthisdo <2093321+hmmwhatsthisdo@users.noreply.github.com> Date: Tue, 13 Aug 2024 08:40:15 -0700 Subject: [PATCH 8/8] Remove unnecessary mock --- Tests/Functions/Set-JiraIssue.Unit.Tests.ps1 | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Tests/Functions/Set-JiraIssue.Unit.Tests.ps1 b/Tests/Functions/Set-JiraIssue.Unit.Tests.ps1 index af29aa52..c94b3268 100644 --- a/Tests/Functions/Set-JiraIssue.Unit.Tests.ps1 +++ b/Tests/Functions/Set-JiraIssue.Unit.Tests.ps1 @@ -172,12 +172,6 @@ Describe "Set-JiraIssue" -Tag 'Unit' { } It "Updates custom fields if provided to the -Fields parameter" { - Mock Get-JiraField { - [PSCustomObject] @{ - 'Name' = $Field - 'ID' = $Field - } - } { Set-JiraIssue -Issue TEST-001 -Fields @{'customfield_12345' = 'foo'; 'customfield_67890' = 'bar'; 'customfield_111222' = @(@{'value' = 'foobar'})} } | Should Not Throw Assert-MockCalled -CommandName Invoke-JiraMethod -ModuleName JiraPS -Times 1 -Scope It -ParameterFilter { $Method -eq 'Put' -and $URI -like "$jiraServer/rest/api/*/issue/12345" -and $Body -like '*customfield_12345*set*foo*' } Assert-MockCalled -CommandName Invoke-JiraMethod -ModuleName JiraPS -Times 1 -Scope It -ParameterFilter { $Method -eq 'Put' -and $URI -like "$jiraServer/rest/api/*/issue/12345" -and $Body -like '*customfield_67890*set*bar*' }