feat(growth): adds set up slack button to issue alert page #28546
Conversation
| }; | ||
|
|
||
| return ( | ||
| <Tooltip title={t('Send Alerts to Slack. Install the integration now.')}> |
There was a problem hiding this comment.
Can you at least leave a TODO above this about the tooltip problem
| from sentry.notifications.utils import has_alert_integration | ||
|
|
||
|
|
||
| class ProjectHasAlertIntegrationInstalled(ProjectEndpoint): |
There was a problem hiding this comment.
Can we add hasAlertIntegrationInstalled as an extend parameter to the ProjectSerializer?
There was a problem hiding this comment.
hmmm...let me think about it
There was a problem hiding this comment.
I don't think we have anything for expand yet for the ProjectSerializer but I can go ahead and add it for that endpoint
There was a problem hiding this comment.
I guess we do expand with kwargs like this for a project:
There was a problem hiding this comment.
@mgaeta Optional parameters to project serialization is kind of a mess here. I see we are using include for optional params on the project details endpoint so I just added it to there and added a TODO.
There was a problem hiding this comment.
Cool we can refactor the endpoint when we add expand to the project serializer. Can you put your variable in an expand now so that it won't be a breaking change later?
expand = set(filter(bool, request.GET.get("expand", "").split(",")))
if "hasAlertIntegration" in expand:
...There was a problem hiding this comment.
@mgaeta You mean so we can use expand or include? Right now we are using include for the same purpose
There was a problem hiding this comment.
include is deprecated so your new variable should be in expand. In this case, both query parameters would be used.
There was a problem hiding this comment.
@mgaeta got it, updated so only the new param uses expand
| organization={organization} | ||
| /> | ||
| ); | ||
| await tick(); |
There was a problem hiding this comment.
ticks are from enzyme. if it isn't passing without it wrap the expect with await waitFor
https://testing-library.com/docs/dom-testing-library/api-async/#waitfor
There was a problem hiding this comment.
or try using getByText instead
This PR adds a button that says "Set Up Slack" on the issue alert creation page if the organization doesn't have any alerting integrations set up for that project. We do this because some folks may not realize Slack is an available integration. For SaaS, we send the user to the integration page for Slack. For on-prem, we send them to docs. This is similar to what we've done for the issue alert emails (#26085).