Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

TechDufus
Copy link

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
Copy link
Member

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
Copy link
Author

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

@TechDufus
Copy link
Author

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
Copy link
Member

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
Copy link
Author

TechDufus commented Dec 5, 2020

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
Copy link
Member

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
Copy link
Author

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:

  • What sections do we limit this escaping to on each card
  • Is -DisableMarkdown what we want to call this?
    • -EscapeMarkdown ?
    • -DisableFormatting ? If users don't know it's markdown or otherwise.
  • What is our character list that we want to escape?
    • Currently I have these, but this does not include \ because of how I am looping inside of Invoke-EscapeMarkdownCharacters. But perhaps we need to not loop and just have a manual replace line for each character, so we can escape a \ and the loop wouldn't escape those slashes also.
      • ` * _ { } [ ] ( ) # + - . !

@PrzemyslawKlys
Copy link
Member

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
Copy link
Author

TechDufus commented Dec 7, 2020

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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:

  • let's define chars with some sane defaults in the top-level cmdlet
  • user can control this globally
  • if someone opens up a request we can come back to this
  • alternatively, new cmdlet could be added such as Control-TeamsFormatting -Type ActivityTitle -Chars ... or something that allows one to overwrite global settings. This would only work if it's executed within ScriptBlock not the "old way".

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

-Text strings ignore '_' underscore character.
2 participants