Skip to content

Commit 58e5421

Browse files
committed
Address PR review feedback: fix file handle leak, duplicate filters, scope consistency, SMTP parsing, ConvertDateTime return type, and doc wording
1 parent 701e100 commit 58e5421

4 files changed

Lines changed: 49 additions & 18 deletions

File tree

Calendar/CalLogHelpers/CalLogCSVFunctions.ps1

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ function MapSharedFolder {
5151
if ($ExternalMasterID -eq "NotFound") {
5252
return "Not Shared"
5353
} else {
54-
$SharedFolders[$ExternalMasterID]
54+
$script:SharedFolders[$ExternalMasterID]
5555
}
5656
}
5757

@@ -98,31 +98,31 @@ function CreateExternalMasterIDMap {
9898

9999
if ($AllFolderNames.Count -eq 0) {
100100
$unknownCount++
101-
$SharedFolders[$ExternalID] = "UnknownSharedCalendarCopy$unknownCount"
101+
$script:SharedFolders[$ExternalID] = "UnknownSharedCalendarCopy$unknownCount"
102102
Write-Host -ForegroundColor red "Found Zero folder names to map to for ExternalID [$ExternalID]."
103103
} elseif ($AllFolderNames.Count -eq 1) {
104-
$SharedFolders[$ExternalID] = $AllFolderNames[0]
104+
$script:SharedFolders[$ExternalID] = $AllFolderNames[0]
105105
Write-Verbose "Found map: [$($AllFolderNames[0])] is for $ExternalID"
106106
} else {
107107
# we still have multiple possible Folder Names, need to chose one or combine
108108
Write-Host -ForegroundColor Red "Unable to Get Exact Folder for $ExternalID"
109109
Write-Host -ForegroundColor Red "Found $($AllFolderNames.count) possible folders: $($AllFolderNames -join ', ')"
110110

111111
if ($AllFolderNames.Count -eq 2) {
112-
$SharedFolders[$ExternalID] = $AllFolderNames[0] + " + " + $AllFolderNames[1]
112+
$script:SharedFolders[$ExternalID] = $AllFolderNames[0] + " + " + $AllFolderNames[1]
113113
} else {
114114
$unknownCount++
115-
$SharedFolders[$ExternalID] = "UnknownSharedCalendarCopy$unknownCount"
115+
$script:SharedFolders[$ExternalID] = "UnknownSharedCalendarCopy$unknownCount"
116116
}
117117
}
118118
}
119119

120120
Write-Host -ForegroundColor Green "Created the following Shared Calendar Mapping:"
121-
foreach ($Key in $SharedFolders.Keys) {
122-
Write-Host -ForegroundColor Green "$Key : $($SharedFolders[$Key])"
121+
foreach ($Key in $script:SharedFolders.Keys) {
122+
Write-Host -ForegroundColor Green "$Key : $($script:SharedFolders[$Key])"
123123
}
124124
Write-Verbose "Created the following Mapping :"
125-
Write-Verbose $SharedFolders
125+
Write-Verbose $script:SharedFolders
126126
}
127127

128128
<#
@@ -254,9 +254,9 @@ function ConvertDateTime {
254254
return $Parsed
255255
}
256256

257-
# Priority 5: Return original string rather than losing data
257+
# Priority 5: Return MinValue so sorting is not broken by mixed types
258258
Write-Warning "Unable to parse date: [$DateTime]"
259-
return $DateTime
259+
return [DateTime]::MinValue
260260
}
261261

262262
function GetAttendeeCount {

Calendar/CalLogHelpers/ExportToExcelFunctions.ps1

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,11 @@ function LogScriptInfo {
5555
$script:RunNumber = [int]$Matches[1]
5656
}
5757
}
58-
} else {
59-
if ($null -ne $pkg) { $pkg.Dispose() }
6058
}
6159
} catch {
6260
Write-Verbose "Unable to read existing run count from Excel: $_"
61+
} finally {
62+
if ($null -ne $pkg) { $pkg.Dispose() }
6363
}
6464
}
6565
$script:RunNumber++
@@ -172,13 +172,33 @@ function LogScriptInfo {
172172
# Update snapshot so next identity only captures new errors
173173
$script:PreRunErrorCount = $Error.Count
174174

175-
# Append to the existing Script Info tab
175+
# Append to the existing Script Info tab (skip -Append on first write when the tab doesn't exist yet)
176176
$savedEAP = $ErrorActionPreference
177177
$ErrorActionPreference = 'SilentlyContinue'
178-
$infoExcel = $RunInfo | Export-Excel -Path $FileName -WorksheetName "Script Info" -Append -PassThru 3>$null
178+
$appendToSheet = $false
179+
if (Test-Path $FileName) {
180+
try {
181+
$testPkg = Open-ExcelPackage -Path $FileName -ErrorAction SilentlyContinue
182+
if ($null -ne $testPkg -and $null -ne $testPkg.Workbook.Worksheets["Script Info"]) {
183+
$appendToSheet = $true
184+
}
185+
if ($null -ne $testPkg) { $testPkg.Dispose() }
186+
} catch {
187+
Write-Verbose "Unable to check for existing Script Info tab: $_"
188+
}
189+
}
190+
191+
if ($appendToSheet) {
192+
$infoExcel = $RunInfo | Export-Excel -Path $FileName -WorksheetName "Script Info" -Append -PassThru 3>$null
193+
} else {
194+
$infoExcel = $RunInfo | Export-Excel -Path $FileName -WorksheetName "Script Info" -PassThru 3>$null
195+
}
179196
$ErrorActionPreference = $savedEAP
180-
$infoExcel.Workbook.Worksheets["Script Info"].TabColor = [System.Drawing.Color]::Gray
181-
Export-Excel -ExcelPackage $infoExcel -WorksheetName "Script Info"
197+
198+
if ($null -ne $infoExcel) {
199+
$infoExcel.Workbook.Worksheets["Script Info"].TabColor = [System.Drawing.Color]::Gray
200+
Export-Excel -ExcelPackage $infoExcel -WorksheetName "Script Info"
201+
}
182202
}
183203

184204
function Export-TimelineExcel {
@@ -440,6 +460,12 @@ function SetLogRowTypeFilter {
440460
$tableStartCol = $tbl.Address.Start.Column
441461
$filterColId = $logRowCol - $tableStartCol
442462

463+
# Remove any existing filterColumn for this colId to avoid duplicates on reruns
464+
$existingFilterCol = $autoFilter.SelectSingleNode("t:filterColumn[@colId='$filterColId']", $nsm)
465+
if ($null -ne $existingFilterCol) {
466+
$null = $autoFilter.RemoveChild($existingFilterCol)
467+
}
468+
443469
$filterCol = $tbl.TableXml.CreateElement("filterColumn", $ns)
444470
$filterCol.SetAttribute("colId", $filterColId.ToString())
445471
$filters = $tbl.TableXml.CreateElement("filters", $ns)

Calendar/CalLogHelpers/Invoke-GetMailbox.ps1

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,12 @@ function GetSMTPAddress {
253253
} elseif ($PassedCN -like "*<*@*>") {
254254
# This is a new format that we are seeing in the Calendar Logs.
255255
# Example: '"Jon Doe" <Jon.Doe@Contoso.com>'
256-
$result = $($PassedCN -split ("<")[-1] -split (">")[0])[1].Trim()
256+
$smtpMatch = [regex]::Match($PassedCN, '<([^>]+)>')
257+
if ($smtpMatch.Success) {
258+
$result = $smtpMatch.Groups[1].Value.Trim()
259+
} else {
260+
$result = $PassedCN
261+
}
257262
Write-Verbose "GetSMTPAddress: Using <SMTPAddress> format of [$PassedCN] as [$result]"
258263
} elseif ($PassedCN -match '</O=') {
259264
#Matching "Users Name" </O=...>

docs/Calendar/Get-CalendarDiagnosticObjectsSummary.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ Get-CalendarDiagnosticObjectsSummary.ps1 -Identity $Users -MeetingID $MeetingID
124124

125125
Before analyzing, verify:
126126

127-
- [ ] All four worksheets exist (Enhanced, Script Info, Raw, Timeline) for each participant
127+
- [ ] Participant worksheets exist for each participant (Enhanced, Raw, Timeline), and the shared Script Info worksheet is present
128128
- [ ] All key participants are included (organizer, affected attendees, delegates, room mailboxes)
129129
- [ ] LogTimestamp range covers the incident timeframe
130130
- [ ] Response rows exist (if investigating response issues — collected by default, use `-NoTrackingLogs` to skip)

0 commit comments

Comments
 (0)