Uh oh!
There was an error while loading. Please reload this page.
🪲 [Fix]: Multi-select custom properties no longer lose individual values - #577
🪲 [Fix]: Multi-select custom properties no longer lose individual values#577Marius Storhaug (MariusStorhaug) wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a bug in the repository custom properties model where multi-select custom property values returned as arrays by the GitHub API were being coerced into a single space-joined string, preventing correct downstream iteration.
Changes:
- Changed
GitHubCustomProperty.Valuefrom[string]to[object]to allow array values. - Updated the
GitHubCustomPropertyconstructor to detect non-string enumerables and cast them to[string[]].
Uh oh!
There was an error while loading. Please reload this page.
Super-linter summary
All files and directories linted successfully For more information, see the GitHub Actions workflow run Powered by Super-linter |
Uh oh!
There was an error while loading. Please reload this page.
…ctly instead of relying on pipeline enumeration
Super-linter summary
All files and directories linted successfully For more information, see the GitHub Actions workflow run Powered by Super-linter |
Removed fabricated [PSCustomObject] tests from GitHub.Tests.ps1 and added a real integration test in Repositories.Tests.ps1 that calls Get-GitHubRepositoryCustomProperty against org repositories and validates that multi-select values are preserved as [string[]] arrays.
Super-linter summary
All files and directories linted successfully For more information, see the GitHub Actions workflow run Powered by Super-linter |
| GitHubCustomProperty([PSCustomObject] $Object) { | ||
| $this.Name = $Object.property_name ?? $Object.propertyName ?? $Object.Name | ||
| $this.Value = $Object.value ?? $Object.Value | ||
| $rawValue = $Object.value ?? $Object.Value | ||
| if ($rawValue -is [System.Collections.IEnumerable] -and $rawValue -isnot [string]) { | ||
| $this.Value = [string[]]$rawValue | ||
| } else { | ||
| $this.Value = $rawValue | ||
| } |
There was a problem hiding this comment.
The constructor now branches on whether value is an IEnumerable (excluding string) to preserve multi-select values as arrays, but this behavior isn't covered by a deterministic unit test in tests/. Add a small Pester test that constructs GitHubCustomProperty from a [pscustomobject]@{ property_name = 'X'; value = @('a','b') } and from a scalar string, and asserts the resulting .Value type (and propertyName alias resolution).
| $prop.Name | Should -Not -BeNullOrEmpty | ||
| if ($prop.Value -is [System.Collections.IEnumerable] -and $prop.Value -isnot [string]) { | ||
| $prop.Value -is [string[]] | Should -BeTrue -Because "multi-select values must be preserved as string arrays, got: $($prop.Value.GetType().FullName)" | ||
| } else { | ||
| $prop.Value | Should -BeOfType [string] |
There was a problem hiding this comment.
Get-GitHubRepositoryCustomProperty returns raw REST objects (from ConvertFrom-Json) with fields like property_name and value. This test asserts $prop.Name, which likely doesn't exist (and will be $null), causing a false failure. Consider asserting $prop.property_name instead, or explicitly converting each item to [GitHubCustomProperty] before validating Name/Value.
| $prop.Name| Should -Not-BeNullOrEmpty | |
| if ($prop.Value-is [System.Collections.IEnumerable] -and$prop.Value-isnot [string]) { | |
| $prop.Value-is [string[]] | Should -BeTrue -Because "multi-select values must be preserved as string arrays, got: $($prop.Value.GetType().FullName)" | |
| } else { | |
| $prop.Value| Should -BeOfType [string] | |
| $prop.property_name| Should -Not-BeNullOrEmpty | |
| if ($prop.value-is [System.Collections.IEnumerable] -and$prop.value-isnot [string]) { | |
| $prop.value-is [string[]] | Should -BeTrue -Because "multi-select values must be preserved as string arrays, got: $($prop.value.GetType().FullName)" | |
| } else { | |
| $prop.value| Should -BeOfType [string] |
| It 'Get-GitHubRepositoryCustomProperty - Gets custom properties and preserves value types' -Skip:($OwnerType -ne 'organization') { | ||
| LogGroup 'Custom Properties' { | ||
| $properties = Get-GitHubRepositoryCustomProperty -Owner $owner -Repository $repoName | ||
| Write-Host ($properties | Format-List | Out-String) | ||
| } | ||
| if ($properties) { | ||
| foreach ($prop in $properties) { | ||
| $prop.Name | Should -Not -BeNullOrEmpty | ||
| if ($prop.Value -is [System.Collections.IEnumerable] -and $prop.Value -isnot [string]) { | ||
| $prop.Value -is [string[]] | Should -BeTrue -Because "multi-select values must be preserved as string arrays, got: $($prop.Value.GetType().FullName)" | ||
| } else { | ||
| $prop.Value | Should -BeOfType [string] | ||
| } |
There was a problem hiding this comment.
This test can pass without validating the new behavior: if $properties is empty, there are no assertions, and if all values are scalar strings the array-preservation path is never exercised. To reliably catch regressions, add a deterministic unit test around GitHubCustomProperty([PSCustomObject]) (mocking an array vs scalar value) or otherwise ensure the test repo contains at least one multi-select property and assert it is a [string[]].
| It 'Get-GitHubRepositoryCustomProperty - Gets custom properties and preserves value types'-Skip:($OwnerType-ne'organization') { | |
| LogGroup 'Custom Properties' { | |
| $properties=Get-GitHubRepositoryCustomProperty-Owner $owner-Repository $repoName | |
| Write-Host ($properties|Format-List|Out-String) | |
| } | |
| if ($properties) { | |
| foreach ($propin$properties) { | |
| $prop.Name| Should -Not-BeNullOrEmpty | |
| if ($prop.Value-is [System.Collections.IEnumerable] -and$prop.Value-isnot [string]) { | |
| $prop.Value-is [string[]] | Should -BeTrue -Because "multi-select values must be preserved as string arrays, got: $($prop.Value.GetType().FullName)" | |
| } else { | |
| $prop.Value| Should -BeOfType [string] | |
| } | |
| It 'GitHubCustomProperty - Preserves scalar and multi-select value types' { | |
| $scalarProperty= [GitHubCustomProperty]::new([PSCustomObject]@{ | |
| name ='Environment' | |
| value ='Production' | |
| }) | |
| $arrayProperty= [GitHubCustomProperty]::new([PSCustomObject]@{ | |
| name ='Regions' | |
| value =@('eu-west-1','us-east-1') | |
| }) | |
| $scalarProperty.Name| Should -Be 'Environment' | |
| $scalarProperty.Value| Should -BeOfType [string] | |
| $scalarProperty.Value| Should -Be 'Production' | |
| $arrayProperty.Name| Should -Be 'Regions' | |
| $arrayProperty.Value-is [string[]] | Should -BeTrue -Because "multi-select values must be preserved as string arrays, got: $($arrayProperty.Value.GetType().FullName)" | |
| $arrayProperty.Value| Should -Be @('eu-west-1','us-east-1') | |
| } | |
| It 'Get-GitHubRepositoryCustomProperty - Gets custom properties and preserves value types'-Skip:($OwnerType-ne'organization') { | |
| LogGroup 'Custom Properties' { | |
| $properties=Get-GitHubRepositoryCustomProperty-Owner $owner-Repository $repoName | |
| Write-Host ($properties|Format-List|Out-String) | |
| } | |
| $properties| Should -Not-BeNullOrEmpty | |
| foreach ($propin$properties) { | |
| $prop.Name| Should -Not-BeNullOrEmpty | |
| if ($prop.Value-is [System.Collections.IEnumerable] -and$prop.Value-isnot [string]) { | |
| $prop.Value-is [string[]] | Should -BeTrue -Because "multi-select values must be preserved as string arrays, got: $($prop.Value.GetType().FullName)" | |
| } else { | |
| $prop.Value| Should -BeOfType [string] |
| It 'Get-GitHubStamp - Each stamp has Name and BaseUrl properties' { | ||
| LogGroup "Stamps - Details" { | ||
| LogGroup 'Stamps - Details' { |
There was a problem hiding this comment.
The PR description (and prior review thread) mention adding focused unit tests for GitHubCustomProperty array vs scalar handling (and GraphQL-style propertyName), but there are no such tests in the current tests/ tree. Please add the mentioned unit tests (or update the PR description to match what actually changed) so this behavior is covered deterministically.
Multi-select custom property values are now correctly preserved as arrays. Previously, properties with multiple selections (e.g.,
["Custom Instructions", "License", "Prompts"]) were silently collapsed into a single space-joined string ("Custom Instructions License Prompts"), making it impossible to iterate over individual selections downstream.Fixed: Multi-select custom property values collapsed into a single string
Commands that return repository custom properties (such as
Get-GitHubRepositoryandGet-GitHubRepositoryCustomProperty) now preserve array values for multi-select properties. Each selection is available as a separate string element, so downstream logic that iterates over values works correctly.Single-select and text custom properties continue to work as before — their values remain plain strings.
Technical Details
GitHubCustomProperty.Valuetype changed from[string]to[object]to allow both scalar strings and string arrays.IEnumerable(excludingstring). If so, it casts to[string[]]; otherwise it assigns the value directly.