feat(np): Adds renderer for MSTeams issue alerts - #112890
GabeVillalobos wants to merge 1 commit into
Conversation
|
|
||
| @classmethod | ||
| def build_actions(cls, *, group: Group, issue_url: str) -> list[Action]: | ||
| return [OpenUrlAction(type=ActionType.OPEN_URL, title="View Issue", url=issue_url)] |
There was a problem hiding this comment.
It might be nice to take the title as a param here; so it's a little easier to modify in the future.
There was a problem hiding this comment.
Trying to figure out if this is vestigial? Seems like this is all temporary anyway, and for now there's only one action that's likely to stay "View Issue."
| ] | ||
|
|
||
| @classmethod | ||
| def get_renderer( |
There was a problem hiding this comment.
🤔 any thoughts on this being a top level registry? i'm thinking something like: renderer = notification_renderer_registry(Provider.MS_TEAMS, Category.ISSUE)
There was a problem hiding this comment.
Not opposed to it at all. I'll sync with the team on why we went with this approach to begin with vs rolling another registry.
| class IssueMSTeamsRenderer(NotificationRenderer[MSTeamsRenderable]): | ||
| provider_key = NotificationProviderKey.MSTEAMS | ||
|
|
||
| @classmethod |
There was a problem hiding this comment.
curious about the class methods on these vs having them be instance methods? is the design pattern here meant to be more generic and allow us to return classes from helper methods and invoke easily? (that's what the get_renderer method seems like at least)
There was a problem hiding this comment.
I'm not sure what the historical reason for it is, though I do like that it keeps the render classes fully stateless by design.
9380f1c to
38b1e86
Compare
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
Backend Test FailuresFailures on
|
hobzcalvin
left a comment
There was a problem hiding this comment.
#124860 is the new version of this PR
|
|
||
| @classmethod | ||
| def build_actions(cls, *, group: Group, issue_url: str) -> list[Action]: | ||
| return [OpenUrlAction(type=ActionType.OPEN_URL, title="View Issue", url=issue_url)] |
There was a problem hiding this comment.
Trying to figure out if this is vestigial? Seems like this is all temporary anyway, and for now there's only one action that's likely to stay "View Issue."
WIP MSTeams issue alert renderer
There are some key things we'll want to fix first though: