Any interest in extending the MessageCard type?

[atc0005]

Overview

I'm working on several small projects where I'm looking to use this package for submitting notifications to Microsoft Teams. As part of that work, I plan on including JSON data and some verbose client details.

To that end I'm looking at using sections with multiple facts entries, but I'm not sure yet of the final payload details.

Example JSON

Example, stripped down copy of the JSON content that I'm toying with:

{
    "@type": "MessageCard",
    "@context": "http://schema.org/extensions",
    "themeColor": "0076D7",
    "summary": "Summary text goes here",
    "text": "Text example",
    "sections": [
        {
            "facts": [
                {
                    "name": "Assigned to",
                    "value": "Unassigned"
                },
                {
                    "name": "Due date",
                    "value": "Mon May 01 2017 17:07:18 GMT-0700 (Pacific Daylight Time)"
                },
                {
                    "name": "Status",
                    "value": "Not started"
                }
            ],
            "markdown": true
        }
    ]
}

Required fields

FWIW, the reference guide says that these two fields are required, but from testing it appears that Teams is pretty lenient about leaving them out:

{
    "@type": "MessageCard",
    "@context": "https://schema.org/extensions"
}

Draft 1 changes

type MessageCard struct {
	// Required; must be set to "MessageCard"
	Type string `json:"@type"`
	// Required; must be set to "https://schema.org/extensions"
	Context string `json:"@context"`

	// Summary appears to only be used when there are sections defined
	Summary    string `json:"summary,omitempty"`
	Title      string `json:"title"`
	Text       string `json:"text"`
	ThemeColor string `json:"themeColor,omitempty"`
	Sections   []struct {
		Title    string `json:"title"`
		Text     string `json:"text"`
		Markdown bool   `json:"markdown"`
		Facts    []struct {
			Name  string `json:"name"`
			Value string `json:"value"`
		} `json:"facts,omitempty"`
	} `json:"sections,omitempty"`
	PotentialAction []struct {
		Target          []string    `json:"target"`
		Context         string      `json:"@context"`
		Type            string      `json:"@type"`
		ID              interface{} `json:"@id"`
		Name            string      `json:"name"`
		IsPrimaryAction bool        `json:"isPrimaryAction"`
	} `json:"potentialAction,omitempty"`
}

The struct changes were generated by running the sample JSON through https://mholt.github.io/json-to-go/ and then tweaking the output to add additional omitifempty qualifiers. I have only just begun testing using the changes, so I can't speak to the accuracy of the results.

// NewMessageCard - create new empty message card
func NewMessageCard() MessageCard {

	// define expected values to meet Office 365 Connector card requirements
	// https://docs.microsoft.com/en-us/outlook/actionable-messages/message-card-reference#card-fields
	msgCard := MessageCard{
		Type:    "MessageCard",
		Context: "https://schema.org/extensions",
	}

	return msgCard
}

I don't know if this is the right way to do it, but I modified the constructor function to handle presetting the required fields since they're static values. While Teams is happy to accept payloads without those fields, since their docs note they're required it seems like good future-proofing to go ahead and include them.

Draft 2 changes

type MessageCardSectionFact struct {
	Name  string `json:"name"`
	Value string `json:"value"`
}

type MessageCardSection struct {
	Title      string                   `json:"title,omitempty"`
	Text       string                   `json:"text,omitempty"`
	Markdown   bool                     `json:"markdown,omitempty"`
	Facts      []MessageCardSectionFact `json:"facts,omitempty"`
	StartGroup bool                     `json:"startGroup,omitempty"`
}

type MessageCard struct {
	Type       string               `json:"@type"`
	Context    string               `json:"@context"`
	Summary    string               `json:"summary,omitempty"`
	Title      string               `json:"title,omitempty"`
	Text       string               `json:"text"`
	ThemeColor string               `json:"themeColor,omitempty"`
	Sections   []MessageCardSection `json:"sections,omitempty"`
}

Stripped out all comments in the above to save space. There may be other fields to add/remove to suit more general needs. No changes to the NewMessageCard() constructor since Draft 1.

References

• go-teams-notify/send.go

  Lines 64 to 69 in feea812

  	// MessageCard - struct of message card
  	type MessageCard struct {
  	Title string `json:"title"`
  	Text string `json:"text"`
  	ThemeColor string `json:"themeColor,omitempty"`
  	}

• https://docs.microsoft.com/en-us/outlook/actionable-messages/message-card-reference#card-fields
• https://docs.microsoft.com/en-us/microsoftteams/platform/webhooks-and-connectors/how-to/connectors-using
• https://mholt.github.io/json-to-go/

[dasrick]

@atc0005 I tried to compare draft 1 and 2. some thoughts:

• draft 2 is much better structured/readable
• is there a reason why PotentialAction is not part of draft 2
• for me it looks compatible with the current implementation

Would you like to start with a branch? (It looks better for me when you are the contributor ...)

[atc0005]

@dasrick: I tried to compare draft 1 and 2. some thoughts:

• draft 2 is much better structured/readable

Thanks, I was hoping so. The first draft was nearly 1:1 from the output generated by https://mholt.github.io/json-to-go/, which was a great starting point (IMHO) since I'm not yet familiar enough to craft a Go struct equivalent from JSON objects. I also tried to trim back fields that didn't appear strictly necessary with the thoughts that they could be added in a later revision if needed.

• is there a reason why PotentialAction is not part of draft 2

Mostly scope. Before I checked with you to see if you had interest in incorporating the changes here, I decided I would press forward either way with implementing changes that I need for another project (which uses this package).

I knew that my Go skills are still very limited (I consider myself still very new to the language), so I didn't want to try and implement too much, too early if I couldn't find away to do it properly.

• for me it looks compatible with the current implementation

From my initial, limited testing I came to the same conclusion. The additional fields allowed for further expansion of functionality without breaking any existing code that only used the initial three fields.

[atc0005]

@dasrick: Would you like to start with a branch? (It looks better for me when you are the contributor ...)

I can. How quickly are you looking to move forward with this? I've started hacking together some large changes to test a few ideas on my fork and was planning on cleaning things up before presenting here.

A few changes from the fork worth sharing now:

• Methods to create sections, facts and add facts to said sections

Example methods (and just signature for the last one):

func (mc *MessageCard) AddSection(section ...MessageCardSection) {
	mc.Sections = append(mc.Sections, section...)
}

// AddFact adds one or many additional MessageCardSectionFact values to a
// MessageCardSection
func (mcs *MessageCardSection) AddFact(fact ...MessageCardSectionFact) {
	mcs.Facts = append(mcs.Facts, fact...)
}

// AddFactFromKeyValue accepts a key and slice of values and converts them to
// MessageCardSectionFact values
func (mcs *MessageCardSection) AddFactFromKeyValue(key string, values ...string) error {
...
}

• Functions to convert strings to Markdown code snippets or code blocks

  • intended to allow formatting fact key/value (where formatting would be applied to value) pairs
  • intended to allow formatting text fields in sections
    • e.g., for providing JSON received via API endpoints for review

• Package level logger (as noted on What are your thoughts on a package-level logger that is disabled by default? #10)

• Expanding error messages from message submission attempts to include the response body text

  • e.g., this proved useful when submitting messages missing both text and summary fields as the API returned "400 Bad Request" as the status code and "Summary or Text is required." as the response body

• Constructor function for new message card section

• Export isValidWebhookURL

• Add new exported IsValidMessageCard validation function to check for a combination of missing text and summary fields

  • this is a known failure condition, so it seemed like a good idea to cover the scenario and generate an error message from the library before the remote API is forced to reject with an error

This may be some other changes, but this is what I spotted when doing a quick review to gather notes for posting here.

[atc0005]

In case my uses cases are too vague, here is an example of the output generated by a small webhook payload testing app:

https://github.com/atc0005/bounce#local-submit-json-payload-using-curl-to-json-specific-endpoint-get-formatted-response

and here is a screenshot showing the received message generated by a small CLI app I wrote (and am heavily refactoring) which uses a fork of this project:

That fork contains the changes I mentioned in my last post. I'm hoping to cleanup those changes and break into specific change sets. I'd then use those sets with Pull Requests for potential inclusion here.

[atc0005]

Back to your notes here:

• is there a reason why PotentialAction is not part of draft 2
• for me it looks compatible with the current implementation

If you want, I can spin off a branch to include the type changes noted in the OP (going with a draft 3 that brings back PotentialAction) and we can discuss each of the other items separately (or in groups of changes that make sense / seem acceptable).

Which branch should I file a Pull Request against? I assume v2, but just want to be sure. I don't yet have a full understanding of the v2 Go modules setup; I've read the docs, but have only glossed over them as I'm not ready for even a v1 across my own projects.

[atc0005]

• Methods to create sections, facts and add facts to said sections

Follow-up: I still feel that the idea fits, but the current implementation has a bug (just in case you look at them) that I'm working through.

---

EDIT: It was a bug elsewhere, the methods appear to work as intended.

[atc0005]

Worth noting (scratch notes from this morning's pre-work dev time):

The PotentialAction field that was removed in Draft 2 can be defined on both Sections and the main message card itself. The reference docs also mention 5 different types as supported, but only list 4. The example JSON file I used for testing via curl had a ViewAction type set on it which is not listed in the 4 types in the reference doc. Perhaps it is deprecated.

[atc0005]

The more I dig into this, the more I keep returning to the idea that support for PotentialAction should be added as a further enhancement and not as an initial "feature" when extending the MessageCard type.

https://docs.microsoft.com/en-us/outlook/actionable-messages/message-card-reference#actions

As noted previously, 5 different types are supported (with one of them presumably deprecated since it's not listed) and each type of potentialAction entry has different sets of fields (some common, some not). This suggests that we'll need different Go types/structs for each of the possible potentialAction types along with constructors to create them with required fields predefined (presumably there are required fields for these types, but I've not dug far enough into it yet).

[atc0005]

The PotentialAction field that was removed in Draft 2 can be defined on both Sections and the main message card itself. The reference docs also mention 5 different types as supported, but only list 4. The example JSON file I used for testing via curl had a ViewAction type set on it which is not listed in the 4 types in the reference doc. Perhaps it is deprecated.

It is indeed deprecated:

Here are a couple of suggestions:

• You should include the summary property which is required, even though the documentation doesn't say it. We'll fix that.
• Try replacing your ViewAction with an OpenUri action. ViewAction is deprecated. Although ViewAction is still supported, we want to encourage everyone to use OpenUri instead.

refs https://stackoverflow.com/a/44061405

---

EDIT: Also mentioned here:

https://docs.microsoft.com/en-us/microsoftteams/platform/webhooks-and-connectors/how-to/connectors-using

(A fourth action, ViewAction, is still supported but no longer needed; use OpenUri instead.)

That note is relevant, but potentially confusing though, as there are five types with only four being relevant to Microsoft Teams:

1. ViewAction
2. OpenUri
3. HttpPOST
4. ActionCard
5. InvokeAddInCommand
   • Outlook specific based on what I read

[dasrick]

Which branch should I file a Pull Request against? I assume v2, but just want to be sure. I don't yet have a full understanding of the v2 Go modules setup; I've read the docs, but have only glossed over them as I'm not ready for even a v1 across my own projects.

The branch "v2" will be removed soon. The branch master represent now "v2..". This should be the branch to setup a pull request against.

The more I dig into this, the more I keep returning to the idea that support for PotentialAction should be added as a further enhancement and not as an initial "feature" when extending the MessageCard type.

Agree. It's always a better idea to keep the changes small.
So, i would prefer to start without PotentialAction. We will include it in a further release.

[atc0005]

The branch "v2" will be removed soon. The branch master represent now "v2..". This should be the branch to setup a pull request against.

Acknowledged. I'll try to get something together over the next day or so.

As mentioned previously I've made a number of changes as part of testing integration with a few projects of mine. How small would you like the Pull Requests?

I can go with just the type changes or type changes + associated methods.

There are other changes that I'm not even sure are a solid fit, so I wouldn't want to include those just yet, though they would likely show up in a draft PR not long behind to aid in further discussions.

Agree. It's always a better idea to keep the changes small.
So, i would prefer to start without PotentialAction. We will include it in a further release.

Sounds good to me. I fought with it for a while before I realized I wasn't ready yet to take it on. I'm still giving it some thought, so maybe after the other changes land and stabilize we can look further into it.

[atc0005]

I added some review comments on #12; noting that here in case those remarks get lost in other repo notification traffic.

On that PR I asked about the scopelint linter warning that my changes generated. I'll go ahead and submit another PR (referencing this issue) to make it easier to discuss that specifically.