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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Support for PotentialActions #102

Closed
nmaupu opened this issue Apr 20, 2021 · 7 comments · Fixed by #103 or #110
Closed

Support for PotentialActions #102

nmaupu opened this issue Apr 20, 2021 · 7 comments · Fixed by #103 or #110
Labels
api documentation Improvements or additions to documentation enhancement New feature or request
Milestone
v2.6.0

Comments

@nmaupu
Copy link

nmaupu commented Apr 20, 2021

Hi !

Is there any plan to support potential actions in MessageCards as per https://docs.microsoft.com/en-us/microsoftteams/platform/webhooks-and-connectors/how-to/connectors-using ?
@atc0005
Copy link
Owner

atc0005 commented Apr 22, 2021

In short, it would be a useful feature to include, but I don't have anything lined up at present. I've got some obligations in front of me that would prevent me from working on this directly, but Pull Requests are welcome.

I took a stab at this around March 2020, but ran into a few walls when doing so. It's been long enough now that I only have a vague sense of the issues. I dug back through some prototype branches and I see where I stubbed out the beginnings of the work, but I didn't get far:

https://github.com/atc0005/go-teams-notify/blob/test-extended-messagecard-type/messagecard.go

This is back before I worked to cleanup the prototype code and get it ready for inclusion in the dasrick/go-teams-notify repo.

@nmaupu
Copy link
Author

nmaupu commented Apr 26, 2021

Here you go: #103

@atc0005
Copy link
Owner

atc0005 commented Apr 27, 2021

@nmaupu Thanks. I'm looking over it. It also looks like GitHub has recently changed how workflows are handled for forks. Instead of running automatically as before, I had to "approve" them first.

@atc0005 atc0005 mentioned this issue Jun 16, 2021
Closed
@atc0005 atc0005 added this to the Next Release milestone Jul 2, 2021
@atc0005 atc0005 added documentation Improvements or additions to documentation enhancement New feature or request labels Jul 2, 2021
@atc0005
Copy link
Owner

atc0005 commented Jul 2, 2021

Current thinking:

  1. Added structs to supports potential actions #103 is the first big effort at this, completing most of the work needed to add the support. I'll plan to merge soon as-is.
  2. I'll create a new PR to fix linting errors and and merge
  3. I'll create a new PR to add some additional support that I've (temporarily) bundled locally in a project of mine
  4. I'll work on documentation updates, including a new example that showcases the extended functionality
  5. Update CHANGELOG
  6. Tag & publish new release

I estimate releasing the new version within a week, hopefully within the next few days.

@atc0005 atc0005 added the api label Jul 2, 2021
@atc0005
Copy link
Owner

atc0005 commented Jul 2, 2021

3. I'll create a new PR to add some additional support that I've (temporarily) bundled locally in a project of mine

TODO (note to self, pulled from #103 (comment)):

  • make sure that this includes validation for the allowed number of potentialAction property entries
    • this applies to adding the potentialAction property entries to the main card and to sections
  • confirm (I don't recall either way) that the OpenUri action targets field values are validated
    • currently only four values are supported, others should be rejected
  • review other actions and determine where validation can be applied

@atc0005 atc0005 linked a pull request Jul 2, 2021 that will close this issue
Added structs to supports potential actions #103
Merged
atc0005 added a commit that referenced this issue Jul 2, 2021
@atc0005
Fix linting errors, adjust formatting
518af3d 
- Add doc comments where needed
- Reorder struct fields to resolve `maligned` linter complaints
- Formatting tweaks
  - intended to match existing code formatting (e.g., whitespace,
    line wrapping, etc)

refs GH-102
@atc0005 atc0005 mentioned this issue Jul 2, 2021
Merged
atc0005 added a commit that referenced this issue Jul 2, 2021
@atc0005
Add MessageCardSection.AddPotentialAction() method
e848d24 
- Add new method for `MessageCardSection` to compliment
  recently added method specific to the `MessageCard`.
- Add doc comment for `MessageCard.PotentialActions`
  - was intended for last PR, but was overlooked

refs GH-102
@atc0005 atc0005 mentioned this issue Jul 2, 2021
Merged
@atc0005
Copy link
Owner

atc0005 commented Jul 2, 2021 •
edited

Validation work appears to be the next step.

Unordered thoughts:

  • The NewMessageCardPotentialAction function is one potential hook to validate provided potentialActionType values provided (only 4 are supported)
  • MessageCardSection.AddPotentialAction()
    • is one potential hook to validate the total number of provided actions
      • 4 per collection are allowed
    • instead of with NewMessageCardPotentialAction, we could apply the validation here for the potentialActionType values provided
  • MessageCard.AddPotentialAction() ...
    • same validations here (total count, potentialActionType values)

@atc0005
Copy link
Owner

atc0005 commented Jul 7, 2021

  • The NewMessageCardPotentialAction function is one potential hook to validate provided potentialActionType values provided (only 4 are supported)

Currently the function definition does not return an error, so we'd have to modify that. Since a tagged release hasn't been made yet, I feel that we can do so without violating semantic versioning.

atc0005 added a commit that referenced this issue Jul 7, 2021
@atc0005
Extend MessageCardPotentialAction validation
d318d02 
Add two helper functions to handle:

- validation of `*MessageCardPotentialAction` values
- appending `*MessageCardPotentialAction` values to the
  `PotentialAction` field for both `MessageCard` and
  `MessageCardSection` values

The `NewMessageCardPotentialAction` factory function has been
modified to return an `error`. This provides validation at both
entry points:

- creating a new value
- adding a value to an existing collection

**NOTE:**

This change is considered to be compatible with semantic
versioning because the support for `PotentialActions` has not
yet landed in a stable public release, leaving the final form
subject to change until the next public release.

refs GH-102
@atc0005 atc0005 mentioned this issue Jul 7, 2021
Merged
atc0005 added a commit that referenced this issue Jul 8, 2021
@atc0005
Extend MessageCardPotentialAction validation
13d1824 
Add two helper functions to handle:

- validation of `*MessageCardPotentialAction` values
- appending `*MessageCardPotentialAction` values to the
  `PotentialAction` field for both `MessageCard` and
  `MessageCardSection` values

The `NewMessageCardPotentialAction` factory function has been
modified to return an `error`. This provides validation at both
entry points:

- creating a new value
- adding a value to an existing collection

**NOTE:**

This change is considered to be compatible with semantic
versioning because the support for `PotentialActions` has not
yet landed in a stable public release, leaving the final form
subject to change until the next public release.

refs GH-102
@atc0005 atc0005 mentioned this issue Jul 8, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
api documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Milestone
v2.6.0
2 participants
@nmaupu @atc0005