EvotecIT / PSTeams

PSTeams is a PowerShell Module working on Windows / Linux and Mac. It allows sending notifications to Microsoft Teams via WebHook Notifications. It's pretty flexible and provides a bunch of options. Initially, it only supported one sort of Team Cards but since version 2.X.X it supports Adaptive Cards, Hero Cards, List Cards, and Thumbnail Cards. All those new cards have their own cmdlets and the old version of creating Teams Cards stays as-is for compatibility reasons.
MIT License
409 stars 41 forks source link

FEATURE: Adding -DisableTextMarkdown switch to Send-TeamsMessage. #31

Open TechDufus opened 3 years ago

TechDufus commented 3 years ago

Closes #30

I've added a -DisableTextMarkdown switch to Send-TeamsMessage . My intent for this switch is to only escape markdown characters for any message section with 'text' in the title. This would escape ActivityText but not escape ActivityTitle as an example, allowing you to continue to use Markdown to format titles and subtitles, but any section labeled text would be disabled and any markdown-specific characters would be escaped.

Again, this is tied to the -DisableTextMarkdown switch, so this will only take effect if specifically called. I have not added any help documentation for this, as I have never seen help formatted other than typical comment-based help. A tour of your help method would be a fantastic opportunity for me to learn. 😊

With this PR the following is now possible with the below result...

$PoolName = "App_Pool_01"
$SectionParams = @{
    ActivityTitle    = "**Application Pool Recycle**"
    ActivitySubtitle = "_@$env:USERNAME`_ - $(Get-Date)"
    ActivityText     = "Recycled Pool: $PoolName"
    SectionInput     = [scriptblock] {
        New-TeamsButton -Name 'View Logs' -Type OpenUri -Link 'https://some.link.com'
    }
    Text             = "An app pool recycle of $PoolName was initiated by $env:USERNAME"
}
Send-TeamsMessage -DisableTextMarkdown {
    New-TeamsSection @SectionParams
} -Uri $Uri -Color DarkSeaGreen -MessageSummary 'Pool Recycle'

image

Notice the title is still bold, and I even used the '_' underscore character on my @$env:USERNAME section to prove italics still work outside of sections specifically labeled text.

PrzemyslawKlys commented 3 years ago

Documentation is autogenerated from comment-based help using Manage-Module.ps1 file. image

The builder also creates PSD1 and makes sure all functions/aliases are as supposed, merges stuff and so on. So comment-based help is all you need. It uses platyPS module in the background to generate docs.

As for your PR, I don't believe it makes sense to convert to json, convert it back and do all sort of parsing. It would be easier to add new parameter to Add-TeamsBody and simply pass parameter from Send-TeamsMessage to Add-TeamsBody and do it there. Alternatively - what I do in other modules that do this kind of "magic" is I create $Script:Teams hashtable, set it to like

$Script:Teams = @{}
$Script:Teams['MarkdownDisabled'] = $MarkdownDisabled

And then within "Facts/Texts" and so on have a logic that checks for that $Script variable and act according to that. It will have less impact on performance because you won't have to loop thru things and you can add local parameters to each command that overwrite global parameter.

TechDufus commented 3 years ago

Let me make a few changes.. Thanks for the feedback!

TechDufus commented 3 years ago

ce72816 takes care of the excess Json conversions, but it doesn't resolve the extra looping issue. Would your script hashtable method need to edit every file that takes a text field as input? I was hoping that my resolution for this would touch as few files as possible, so I decided to loop through the final $Body object just before it's shipped off..

PrzemyslawKlys commented 3 years ago

Yes it would require to touch every single function you wish to provide this functionality to. I don't think it's really a problem tho. I mean the functionality is needed. What if one wants to disable Markdown only for single Fact, but not for all texts?

TechDufus commented 3 years ago

How would we specify which regions on each section were needing to be escaped? I can't imagine that making a switch for each section would be a good idea.

ie -DisableTitleMarkdown, -DisableSubtitleMarkdown, -DisableActivityTextMarkdown, etc...

PrzemyslawKlys commented 3 years ago

No no :-) You would add one switch that is global -DisableMakrdown on Send-Teams..., then on each New-TeamsFact, New-TeamsActivityText you would have separate -DisableMarkdown. Now if someone chooses -DisableMarkdown on Send-Teams it means he wants it global. If someone does it for a fact , only fact is affected.

This is what I was thinking about - the problem with this - is that I expect everyone creating sections/facts/titles within Send-TeamsMessage

Send-TeamsMessage {
   ... 
}

And that is not always the case, shit.

function Send-TeamsMessage {
    [alias('TeamsMessage')]
    [CmdletBinding()]
    Param (
        [scriptblock] $SectionsInput,
        [alias("TeamsID", 'Url')][Parameter(Mandatory)][string]$Uri,
        [string]$MessageTitle,
        [string]$MessageText,
        [string]$MessageSummary,
        [string]$Color,
        [switch]$HideOriginalBody,
        [System.Collections.IDictionary[]]$Sections,
        [alias('Supress')][bool] $Suppress = $true,
        [switch] $DisableMarkdown
    )

    $Script:Tracker = @{
        DisableMarkdown = $DisableMarkdown.IsPresent
    }

And then for each one you would have:

function New-TeamsActivityTitle {
    [CmdletBinding()]
    [alias('ActivityTitle', 'TeamsActivityTitle')]
    param(
        [string] $Title,
        [switch] $DisableMarkdown
    )

    if (-not $DisableMarkdown) {
        if ($Script:Tracker -and $Script:Tracker.MarkdownDisabled) {
            $DisableMarkdown = $true
        }
    }

    @{
        ActivityTitle    = $Title
        Type             = 'ActivityTitle'
        MarkdownDisabled = $DisableMarkdown
    }
}

And then somewhere before sending out you would do proper action. The problem is global tracker won't work if someone creates their section/team facts and so on outside of Send-Teams, as $Script:Tracker won't exists at that point.

So we can't really check for DisableMarkdown the way I proposed but instead let users set disable markdown per fact/title/activity title, and if someone sets it on Send-Teams, overwrite that value

function New-TeamsActivityTitle {
    [CmdletBinding()]
    [alias('ActivityTitle', 'TeamsActivityTitle')]
    param(
        [string] $Title,
        [switch] $DisableMarkdown
    )
    @{
        ActivityTitle    = $Title
        Type             = 'ActivityTitle'
        MarkdownDisabled = $DisableMarkdown
    }
}

And then in global

function Send-TeamsMessage {
    [alias('TeamsMessage')]
    [CmdletBinding()]
    Param (
        [scriptblock] $SectionsInput,
        [alias("TeamsID", 'Url')][Parameter(Mandatory)][string]$Uri,
        [string]$MessageTitle,
        [string]$MessageText,
        [string]$MessageSummary,
        [string]$Color,
        [switch]$HideOriginalBody,
        [System.Collections.IDictionary[]]$Sections,
        [alias('Supress')][bool] $Suppress = $true,
        [switch] $DisableMarkdown
    )

    if ($SectionsInput) {
        $Output = & $SectionsInput
    } else {
        $Output = $Sections
    }

    if ($Color -or $Color -ne 'None') {
        try {
            $ThemeColor = ConvertFrom-Color -Color $Color
        } catch {
            $ErrorMessage = $_.Exception.Message -replace "`n", " " -replace "`r", " "
            Write-Warning "Send-TeamsMessage - Color conversion for $Color failed. Error message: $ErrorMessage"
            $ThemeColor = $null
        }
    }

    if ($DisableMarkdown) {
        $MessageTitle = Disable-TextMarkdown -InputObject $MessageTitle
        $MessageText = Disable-TextMarkdown -InputObject $MessageText
        # annything other from the global that needs fixes
    }

    $Body = Add-TeamsBody -MessageTitle $MessageTitle `
        -MessageText $MessageText `
        -ThemeColor $ThemeColor `
        -Sections $Output `
        -MessageSummary $MessageSummary `
        -HideOriginalBody:$HideOriginalBody.IsPresent
    -DisableMarkdown $DisableMarkdown.IsPresent

And then in Add-TeamsBody if DIsableMarkdown is set globally you would need to do what you did before, scanning section and going thru each object type and overwritting whatever value was set individually.

Alternatively I'm thinking that user may want to disable everything and enable markdown in certain situations only. Not sure if we should support that.

What do you think?

TechDufus commented 3 years ago

Here's a thought. On each cmdlet that takes text input, we create a [System.String[]] -DisableMarkdown with a [ValidateSet()] containing the non-Uri parameters for that cmdlet. We then only filter the catagories provided on that card, for the values provided. Here is a short example.

$PoolName = "App_Pool_01"
$SectionParams = @{
    ActivityTitle    = "**Application Pool Recycle**"
    ActivitySubtitle = "_@$env:USERNAME`_ - $(Get-Date)"
    ActivityText     = "Recycled Pool: $PoolName"
    SectionInput     = [scriptblock] {
        New-TeamsButton -Name 'View Logs' -Type OpenUri -Link 'https://some.link.com'
    }
    Text             = "An app pool recycle of $PoolName was initiated by $env:USERNAME"
    DisableMarkdown = @('Text','ActivityText')
}
Send-TeamsMessage {
    New-TeamsSection @SectionParams
} -Uri $Uri -Color DarkSeaGreen -MessageSummary 'Pool Recycle'

This would involve work on every file (which I can do once we figure out how we want to do this. 😂), but this would allow users to create the cards outside of the Send-TeamsMessage block, and also specify specific sections on specific cards that need to be escaped.

Something like this would need to be added to each command:

[ValidateSet('Title','ActivityTitle','ActivitySubtitle','ActivityText','Text','ActivityDetails','ALL')]
[System.String[]] $DisableFormatting

Several things we still need to decide on this:

PrzemyslawKlys commented 3 years ago

What about facts?

$Fact1 = New-TeamsFact -Name 'PS Version' -Value "**$($PSVersionTable.PSVersion)**"
$Fact2 = New-TeamsFact -Name 'PS Edition' -Value "**$($PSVersionTable.PSEdition)**"
$Fact3 = New-TeamsFact -Name 'OS' -Value "**$($PSVersionTable.OS)**" -DisableMarkdown

One disabled, rest enabled.

TechDufus commented 3 years ago

If it's only a cmdlet that takes in one value, then I think we would add our -DisableFormatting parameter as a switch instead. Each implimentation of this parameter on each cmdlet is going to have to be unique to that cmdlet, like the [ValidateSet()] for the specific parameters on each cmdlet.

In your example I say we create a switch and only escape the -Value that is given.

-- OR -- We keep the same format and add the following to New-TeamsFact specifically:

[ValidateSet('Name','Value','ALL')]
[System.String[]] $DisableFormatting

I see the Name and Value being the same as ActivityTitle and ActivityText in other cmdlets.

PrzemyslawKlys commented 3 years ago

Ye, I guess this would also need to apply to New-TeamsList, ListItem, New-TeamsActivityTitle and so on. Additionally I would like to have this covered for other card types. As soon as one thinks about it, things get a bit more complicated.

TechDufus commented 3 years ago

Yeah, this would be a very manual, "Open each cmd, see how text entries are handled, and add support for escaping those entries if specified"...

Additionally, what if someone only wanted to escape a specific character? Would we need to add a -EscapeCharacter * and only escape that one? Just thinking out loud on how far we want to take this feature, if it's worth implementing at all..

PrzemyslawKlys commented 3 years ago

All are good questions. I am thinking that 90% of people will find out hard way - like you did that their variable or word isn't what it's supposed to. Usually, this will come from variables such as account names which can't have certain chars anyways.

My approach would be:

Of course we could just add 2 parameters on each. In most cases it will be copy/paste. Not sure if it's a problem.