Skip to content

Commit 586dd4d

Browse files
🚀 [Breaking pre-release]: Refactor GitHubPermission and improve initialization and comparison logic (#524)
## Description This pull request refactors how GitHub permissions are represented and handled throughout the codebase, consolidating the `GitHubPermissionDefinition` and `GitHubPermission` classes and updating related logic for consistency and correctness. The changes also improve the handling of permissions and events in context and installation classes, as well as streamline status calculation for installations. ### Refactoring and Consolidation of Permissions * Merged `GitHubPermissionDefinition` into a single `GitHubPermission` class, adding constructors and properties to better represent both definitions and assigned values. All static permission lists and related logic now use `GitHubPermission` instead of the previous definition class. * Updated all permission list instantiations to use the new `GitHubPermission` class, ensuring consistent construction and type usage. ### Improvements to Installation and Context Classes * Updated constructors in `GitHubAppInstallation.ps1` to use a simpler parameter set, replaced `$Context.HostName` with direct `$HostName` usage, and removed a redundant constructor. Also changed the status update method name from `UpdateStatus` to `SetStatus` and adjusted its usage. * Improved event and permission handling in `GitHubAppContext.ps1` and `GitHubAppInstallationContext.ps1` by ensuring events are always stored as arrays and permissions are only set when present. * Ensured `FilePaths` and `Events` properties are always arrays to prevent type inconsistencies. ### Streamlining Installation Status Calculation * Refactored the logic for comparing permissions in `SetStatus`, replacing manual dictionary comparison with a simpler `Compare-Object` approach for determining if permissions match between app and installation. * Added a check to ensure that installation permissions exist before attempting status calculation, improving robustness. ## Type of change <!-- Use the check-boxes [x] on the options that are relevant. --> - [ ] 📖 [Docs] - [ ] 🪲 [Fix] - [ ] 🩹 [Patch] - [ ] ⚠️ [Security fix] - [x] 🚀 [Feature] - [ ] 🌟 [Breaking change] ## Checklist <!-- Use the check-boxes [x] on the options that are relevant. --> - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas
1 parent 5bde93c commit 586dd4d

21 files changed

+1411
-1603
lines changed

src/classes/public/App/GitHubAppInstallation.ps1

Lines changed: 12 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@
5959
$this.RepositorySelection = $Object.repository_selection
6060
$this.Permissions = [GitHubPermission]::NewPermissionList($Object.permissions, $this.Type)
6161
$this.Events = , ($Object.events)
62-
$this.FilePaths = $Object.single_file_paths
62+
$this.FilePaths = , ($Object.single_file_paths)
6363
$this.CreatedAt = $Object.created_at
6464
$this.UpdatedAt = $Object.updated_at
6565
$this.SuspendedAt = $Object.suspended_at
@@ -76,16 +76,16 @@
7676
$this.RepositorySelection = $Object.repository_selection
7777
$this.Permissions = [GitHubPermission]::NewPermissionList($Object.permissions, $this.Type)
7878
$this.Events = , ($Object.events)
79-
$this.FilePaths = $Object.single_file_paths
79+
$this.FilePaths = , ($Object.single_file_paths)
8080
$this.CreatedAt = $Object.created_at
8181
$this.UpdatedAt = $Object.updated_at
8282
$this.SuspendedAt = $Object.suspended_at
8383
$this.SuspendedBy = [GitHubUser]::new($Object.suspended_by)
8484
$this.Url = $Object.html_url
85-
$this.UpdateStatus()
85+
$this.SetStatus()
8686
}
8787

88-
GitHubAppInstallation([PSCustomObject] $Object, [string] $Target, [string] $Type, [GitHubContext] $Context) {
88+
GitHubAppInstallation([PSCustomObject] $Object, [string] $Target, [string] $Type, [string] $HostName) {
8989
$this.ID = $Object.id
9090
$this.App = [GitHubApp]::new(
9191
[PSCustomObject]@{
@@ -96,7 +96,7 @@
9696
$this.Target = [GitHubOwner]@{
9797
Name = $Target
9898
Type = $Type
99-
Url = "https://$($Context.HostName)/$Target"
99+
Url = "https://$HostName/$Target"
100100
}
101101
$this.Type = $Type
102102
$this.RepositorySelection = $Object.repository_selection
@@ -107,38 +107,21 @@
107107
$this.UpdatedAt = $Object.updated_at
108108
$this.SuspendedAt = $Object.suspended_at
109109
$this.SuspendedBy = [GitHubUser]::new($Object.suspended_by)
110-
$this.Url = "https://$($Context.HostName)/$($Type.ToLower())s/$Target/settings/installations/$($Object.id)"
110+
$this.Url = "https://$HostName/$($Type.ToLower())s/$Target/settings/installations/$($Object.id)"
111111
$this.Status = 'Unknown'
112112
}
113113

114-
GitHubAppInstallation([PSCustomObject] $Object, [string] $Target, [string] $Type, [GitHubContext] $Context, [GitHubApp] $App) {
115-
$this.ID = $Object.id
116-
$this.App = $App
117-
$this.Target = [GitHubOwner]@{
118-
Name = $Target
119-
Type = $Type
120-
Url = "https://$($Context.HostName)/$Target"
121-
}
122-
$this.Type = $Type
123-
$this.RepositorySelection = $Object.repository_selection
124-
$this.Permissions = [GitHubPermission]::NewPermissionList($Object.permissions, $this.Type)
125-
$this.Events = , ($Object.events)
126-
$this.FilePaths = $Object.single_file_paths
127-
$this.CreatedAt = $Object.created_at
128-
$this.UpdatedAt = $Object.updated_at
129-
$this.SuspendedAt = $Object.suspended_at
130-
$this.SuspendedBy = [GitHubUser]::new($Object.suspended_by)
131-
$this.Url = "https://$($Context.HostName)/$($Type.ToLower())s/$Target/settings/installations/$($Object.id)"
132-
$this.UpdateStatus()
133-
}
134-
135114
# Updates the Status property by comparing installation permissions with app permissions
136115
# filtered by the appropriate scope based on installation type
137-
[void] UpdateStatus() {
116+
[void] SetStatus() {
138117
if (-not $this.App -or -not $this.App.Permissions) {
139118
$this.Status = 'Unknown'
140119
return
141120
}
121+
if (-not $this.Permissions) {
122+
$this.Status = 'Unknown'
123+
return
124+
}
142125

143126
# Get app permissions filtered by installation type scope
144127
$appPermissionsFiltered = switch ($this.Type) {
@@ -156,38 +139,7 @@
156139
}
157140
}
158141

159-
# Compare permissions by creating lookup dictionaries
160-
$appPermissionLookup = @{}
161-
foreach ($perm in $appPermissionsFiltered) {
162-
$appPermissionLookup[$perm.Name] = $perm.Value
163-
}
164-
165-
$installationPermissionLookup = @{}
166-
foreach ($perm in $this.Permissions) {
167-
$installationPermissionLookup[$perm.Name] = $perm.Value
168-
}
169-
170-
# Check if permissions match
171-
$permissionsMatch = $true
172-
173-
# Check if all app permissions exist in installation with same values
174-
foreach ($name in $appPermissionLookup.Keys) {
175-
if (-not $installationPermissionLookup.ContainsKey($name) -or
176-
$installationPermissionLookup[$name] -ne $appPermissionLookup[$name]) {
177-
$permissionsMatch = $false
178-
break
179-
}
180-
}
181-
182-
# Check if installation has any extra permissions not in the app
183-
if ($permissionsMatch) {
184-
foreach ($name in $installationPermissionLookup.Keys) {
185-
if (-not $appPermissionLookup.ContainsKey($name)) {
186-
$permissionsMatch = $false
187-
break
188-
}
189-
}
190-
}
142+
$permissionsMatch = Compare-Object -ReferenceObject $appPermissionsFiltered -DifferenceObject $this.Permissions | Measure-Object
191143

192144
$this.Status = $permissionsMatch ? 'Ok' : 'Outdated'
193145
}

src/classes/public/Context/GitHubContext/GitHubAppContext.ps1

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@
4747
$this.KeyVaultKeyReference = $Object.KeyVaultKeyReference
4848
$this.OwnerName = $Object.OwnerName
4949
$this.OwnerType = $Object.OwnerType
50-
$this.Permissions = [GitHubPermission]::NewPermissionList($Object.Permissions)
51-
$this.Events = $Object.Events
50+
if ($Object.Permissions) {
51+
$this.Permissions = [GitHubPermission]::NewPermissionList($Object.Permissions)
52+
}
53+
$this.Events = , ($Object.Events)
5254
}
5355
}

src/classes/public/Context/GitHubContext/GitHubAppInstallationContext.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
$this.ClientID = $Object.ClientID
4343
$this.InstallationID = $Object.InstallationID
4444
$this.Permissions = [GitHubPermission]::NewPermissionList($Object.Permissions, $Object.InstallationType)
45-
$this.Events = $Object.Events
45+
$this.Events = , ($Object.Events)
4646
$this.InstallationType = $Object.InstallationType
4747
$this.InstallationName = $Object.InstallationName
4848
}

0 commit comments

Comments
 (0)