diff --git a/.github/workflows/superset-python-unittest.yml b/.github/workflows/superset-python-unittest.yml index 5e506c1e6b6d..de27026b2d61 100644 --- a/.github/workflows/superset-python-unittest.yml +++ b/.github/workflows/superset-python-unittest.yml @@ -65,6 +65,8 @@ jobs: uses: $/.github/actions/setup-backend/ with: python-version: ${{ matrix.python-version }} + - name: Verify Teams SDK is available for authentication and delivery tests + run: python -c "import microsoft_teams.apps" - name: SDK unit tests env: PYTEST_DISABLE_PLUGIN_AUTOLOAD: "1" diff --git a/docs/admin_docs/configuration/alerts-reports.mdx b/docs/admin_docs/configuration/alerts-reports.mdx index 3d88f92c91af..ae55df53f983 100644 --- a/docs/admin_docs/configuration/alerts-reports.mdx +++ b/docs/admin_docs/configuration/alerts-reports.mdx @@ -892,6 +892,217 @@ This information can then be retrieved from the endpoint `/api/v1/saved_query/` schedule the queries that have `schedule_info` in their JSON metadata. For schedulers other than Airflow, additional fields can be easily added to the configuration file above. +## Microsoft Teams configuration + +Teams is an optional, disabled-by-default notification destination for +attachment-free alerts in standard channels. Each deployment supports one +Microsoft tenant and bot identity. Scheduled reports, private/shared channels, +direct messages, and file attachments are not supported. + +### Install and configure + +Install the extra in both the web-server and Celery worker images using the same +Superset release as the deployment: + +```bash +pip install 'apache-superset[teams]==' +superset db upgrade +``` + +Configure both processes with the same server-side settings. Load the secret +value from a deployment secret store, rather than checking it into source control: + +```python +import os + +FEATURE_FLAGS = { + "ALERT_REPORTS": True, + "ALERT_REPORT_TEAMS": True, +} +ALERT_REPORTS_TEAMS = { + "tenant_id": os.environ["TEAMS_TENANT_ID"], + "application_id": os.environ["TEAMS_APPLICATION_ID"], + "credential": os.environ["TEAMS_CLIENT_SECRET"], + "destinations": [ + { + "key": "ops", + "label": "Operations alerts", + "team_id": os.environ["TEAMS_OPS_TEAM_ID"], + "channel_id": os.environ["TEAMS_OPS_CHANNEL_ID"], + }, + ], +} +``` + +Destination keys must be unique, stable identifiers of 1–128 letters, digits, +underscores or hyphens, starting with a letter or digit. Keep keys stable when +changing routing. Each destination requires a label, team ID, and channel ID. +Approve only standard channels in the configured tenant. Duplicate keys or +channels and unknown configuration fields are rejected. An empty mapping leaves +Teams unconfigured. + +For `team_id`, use the Teams thread ID, for example `19:example@thread.tacv2`, +not the Microsoft 365 group GUID shown as the Team ID in Graph or the Teams +admin center. In Teams, open the team's **More options (…) > Get link to team**. +Copy the value between `/l/team/` and `/conversations` and URL-decode it +(`%3A` becomes `:` and `%40` becomes `@`). Do not use the link's `groupId` parameter. +For `channel_id`, use **Get link to channel** and URL-decode the value immediately +after `/l/channel/`. See Microsoft's [Teams link formats](https://learn.microsoft.com/en-us/microsoftteams/platform/concepts/build-and-test/deep-link-teams). +The installation callback matches `team_id` exactly. A group GUID will cause +installation events to return 403 and leave the destination without an active +installation. + +Merge these feature flags into your existing flags. Restart the web server and +workers after changing configuration or rotating a secret. Apply the migration +before accepting installation events. The scheduler, broker, and worker setup +from the earlier Alerts and Reports instructions still applies; the Teams SDK +does not run another web server. Attachment-free Teams alerts do not need a +browser or screenshot worker. + +### Register the Microsoft app + +Use a Microsoft Entra application restricted to your organization and an Azure +Bot registration with the same application ID and tenant. Create a client secret +and put its **value** in `TEAMS_CLIENT_SECRET`. Enable the Microsoft Teams channel +on the bot registration. Set its messaging endpoint to +`https://superset.example/api/v1/teams/events/`, using your public HTTPS host and +any deployment URL prefix. See Microsoft's +[bot registration guidance](https://learn.microsoft.com/en-us/azure/bot-service/bot-service-quickstart-registration?view=azure-bot-service-4.0). + +Package the notification-only app for your organization's Teams app catalog. +The bot must support team scope only and have `isNotificationOnly` enabled; +see the [manifest reference](https://learn.microsoft.com/en-us/microsoftteams/platform/resources/schema/manifest-schema#bots). +This example uses manifest version 1.22, which supports the required fields: + +```json +{ + "$schema": "https://developer.microsoft.com/json-schemas/teams/v1.22/MicrosoftTeams.schema.json", + "manifestVersion": "1.22", + "version": "1.0.0", + "id": "cccccccc-cccc-4ccc-8ccc-cccccccccccc", + "developer": { + "name": "Your organization", + "websiteUrl": "https://superset.example/", + "privacyUrl": "https://superset.example/privacy", + "termsOfUseUrl": "https://superset.example/terms" + }, + "name": { "short": "Superset Alerts" }, + "description": { + "short": "Superset alert notifications", + "full": "Receive configured Superset alerts in approved standard Teams channels." + }, + "icons": { "outline": "outline.png", "color": "color.png" }, + "accentColor": "#20A7C9", + "bots": [ + { + "botId": "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb", + "scopes": ["team"], + "isNotificationOnly": true, + "supportsFiles": false + } + ] +} +``` + +Replace the package `id` with a new UUID and `botId` with your configured application +ID. Replace the example URLs with your organization's published pages. Include +`manifest.json`, a 32×32 transparent outline icon, and a 192×192 color icon at the +root of the app ZIP. Have your Teams administrator approve the package and allow +installation in the selected teams. Install it in each approved team after the +callback is reachable. Configure each standard channel's actual team and channel +identifiers on the server; display labels and Microsoft 365 group names are not +routing IDs. Do not add personal or group-chat scopes, file support, or +private/shared channel capabilities. + +Installing the app updates the installation record for its team, but does not +approve any destinations. Configure the allowlist separately. Uninstalling marks +the installation inactive; reinstalling refreshes its verified service URL and +reactivates the same record. If an initial installation happened before callback +routing was ready, remove and reinstall the app to deliver a fresh event. +Lifecycle events require a timezone-qualified source timestamp. Older events are +acknowledged without changing installation state, and removal wins timestamp ties. +Removal records are retained even when the installation event has not arrived. + +### Route the callback + +Your ingress must forward `POST /api/v1/teams/events/` (including the trailing +slash) to Flask, preserving the Authorization header and JSON body. The callback +accepts bodies up to 256 KiB, or a smaller deployment body limit, and returns 413 +for oversized requests before JSON parsing, SDK authentication, or database access. +Do not send this path through a browser login redirect or an ingress SSO challenge. Keep TLS +termination and the configured public URL consistent. Other Superset routes keep +their normal authentication and CSRF requirements; do not exempt the whole API. +The callback validates Microsoft's connector token, application, tenant, approved +team, and signed service URL before changing installations. + +Allow outbound connectivity from the web server for connector signing keys and +from workers for Entra token acquisition and the verified Bot Framework service +URL. Use the public Microsoft cloud endpoints supported by this adapter. Changing +the bot identity requires matching configuration on both web and worker processes +and a new verified installation for that identity. + +### Create a Teams alert + +1. Install the app in each approved team once the callback is reachable. +2. In **Alerts & reports**, create an alert and disable attachments. +3. Choose **Teams** as the notification method and select an approved destination + by its display label. Add another Teams notification method to notify another + channel. +4. Save the alert and check its execution logs after delivery. + +Each alert creates a separate channel post containing its name, optional +description, and an **Open in Superset** link when link inclusion is enabled. +Execution errors show a generic failure message rather than SQL diagnostics. + +You can save an alert before installing the bot, but delivery requires an active +installation. Saved destinations that are no longer configured remain visible +as unavailable. Restore the destination's stable key or select a replacement. +Credentials are managed in deployment configuration, not in the alert editor. + +### Configure a recipient through the API + +Use recipient type `Teams`, `recipient_config_json` containing only +`{"target": "ops"}`, and `report_format="NONE"`. Replace `ops` with a configured +destination key. Create, update, and delivery validation enforce the same +restrictions, including partial updates to saved alerts. + +The protected destination API returns keys, labels, and installation availability, +without credentials or routing details. + +### Troubleshooting + +| Symptom | Check | +| --- | --- | +| Teams is absent from the editor | Install the extra, enable both feature flags, provide valid configuration, choose an alert, and disable attachments. | +| Destination says “Bot installation required” | Check the bot is installed in the configured team and the callback received its installation event. Saving is allowed before installation; delivery requires an active record. | +| Saved destination says “Unavailable” | Restore the original stable key or choose an approved replacement. Configuration changes do not rewrite saved recipients. | +| Callback returns 404 | Check `ALERT_REPORT_TEAMS`, ingress path/prefix, and that the request reached this deployment. | +| Callback returns 503 | Check the optional SDK is installed and the deployment configuration is nonempty and passes validation. | +| Callback returns 401 or 403 | Check connector token authentication, bot application/tenant IDs, approved team, signed service URL, and that ingress preserved Authorization. Browser cookies cannot authenticate this route. | +| Delivery fails authorization | Check installation status, secret value/expiry, bot registration, and channel access. A second 401 after one refresh or a 403 ends delivery. | +| Save fails after a configuration change | Restore the integration/key or explicitly remove/replace Teams recipients. Partial updates validate the merged schedule, including format and schedule type. | +| Delivery times out or exhausts retries | Check Entra and Bot Framework connectivity and worker execution context. The 60-second retry budget includes token acquisition and cleanup and is capped by the execution deadline. Use a supported prefork worker with finite `working_timeout` and `ALERT_REPORTS_WORKING_TIME_OUT_KILL=True` for the outer task limit. | +| A duplicate channel post appears | A response can be lost after Teams accepted a post. Bounded retries may create another root post; delivery is not exactly-once. | + +Use the existing alert execution logs to inspect success and failure. Do not copy +access tokens, credentials, raw SDK responses, or SQL diagnostics into channel +messages or issue reports. Rotate credentials through deployment configuration; +there is no browser credential-management page. + +### Delivery timeouts and retries + +Delivery allows up to four attempts within a cooperative 60-second retry budget, +limited by the remaining report execution deadline. Requests have socket timeouts +of at most 10 seconds. Throttling honors Microsoft's retry delay, and expired +authentication can refresh once. A lost response can cause duplicate posts. + +Socket timeouts do not bound DNS resolution or an entire streaming response. For +an outer time limit, use a Celery prefork worker with +[time-limit support](https://docs.celeryq.dev/en/stable/userguide/workers.html#time-limits), +keep `ALERT_REPORTS_WORKING_TIME_OUT_KILL=True`, and set a finite schedule +`working_timeout`. These limits cover the entire alert task, including query +execution and delivery. Hard task termination cannot guarantee resource cleanup. + :::resources - [Tutorial: Automated Alerts and Reporting via Slack/Email in Superset](https://dev.to/ngtduc693/apache-superset-topic-5-automated-alerts-and-reporting-via-slackemail-in-superset-2gbe) diff --git a/docs/developer_docs/contributing/howtos.md b/docs/developer_docs/contributing/howtos.md index 65803d04375a..5022b90a148a 100644 --- a/docs/developer_docs/contributing/howtos.md +++ b/docs/developer_docs/contributing/howtos.md @@ -211,6 +211,29 @@ npm run playwright:debug tests/auth/login.spec.ts npm run playwright:report ``` +### Testing Microsoft Teams notifications + +For deployment setup, see the [Microsoft Teams configuration guide](/admin-docs/configuration/alerts-reports#microsoft-teams-configuration). + +Backend tests under `tests/unit_tests/integrations/teams/` exercise the real SDK +with fake HTTP transports. Install the `teams` extra to run authentication and +transport tests rather than skipping them. Notification contract tests at +`tests/unit_tests/reports/notifications/teams_tests.py` run without the SDK. + +The browser regression at `playwright/tests/reports/teams-alerts.spec.ts` requires a +running Superset instance, an admin test user, Reports and Teams enabled, at least +one configured destination, and a selectable test database. An unconfigured +integration skips this optional browser test. Use an isolated test deployment +with placeholder UUIDs and a dummy secret; no Microsoft app or installation is +needed for this test. It mocks destination availability, creates only inactive +alerts, blocks execution requests, verifies saved recipient keys, and deletes its +schedule afterward. It never clicks **Trigger now**. Run it from +`superset-frontend`: + +```bash +npx playwright test playwright/tests/reports/teams-alerts.spec.ts --project=chromium +``` + ### Debugging Server App For debugging the Flask backend: diff --git a/docs/static/feature-flags.json b/docs/static/feature-flags.json index e39581a4439c..e35adb3fd771 100644 --- a/docs/static/feature-flags.json +++ b/docs/static/feature-flags.json @@ -21,6 +21,12 @@ "lifecycle": "development", "description": "Enables experimental tabs UI for Alerts and Reports" }, + { + "name": "ALERT_REPORT_TEAMS", + "default": false, + "lifecycle": "development", + "description": "Enable notification-only Microsoft Teams alerts to approved standard channels." + }, { "name": "CHART_PLUGINS_EXPERIMENTAL", "default": false, diff --git a/docs/static/resources/openapi.json b/docs/static/resources/openapi.json index dfdbf9fdc984..23a956e926be 100644 --- a/docs/static/resources/openapi.json +++ b/docs/static/resources/openapi.json @@ -9448,7 +9448,8 @@ "Email", "Slack", "SlackV2", - "Webhook" + "Webhook", + "Teams" ], "type": "string" } @@ -29206,6 +29207,59 @@ ] } }, + "/api/v1/report/teams_destinations/": { + "get": { + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "properties": { + "result": { + "items": { + "properties": { + "available": { + "type": "boolean" + }, + "key": { + "type": "string" + }, + "label": { + "type": "string" + } + }, + "type": "object" + }, + "type": "array" + } + }, + "type": "object" + } + } + }, + "description": "Approved destinations and installation availability" + }, + "401": { + "$ref": "#/components/responses/401" + }, + "403": { + "$ref": "#/components/responses/403" + } + }, + "security": [ + { + "jwt": [] + }, + { + "api_key": [] + } + ], + "summary": "List approved Teams alert destinations", + "tags": [ + "Report Schedules" + ] + } + }, "/api/v1/report/{pk}": { "delete": { "parameters": [ diff --git a/pyproject.toml b/pyproject.toml index ef8495b6dd6f..1526d1b9d075 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -268,6 +268,7 @@ pinot = ["pinotdb[sqlalchemy]>=9.2.2, <10.0.0"] playwright = ["playwright>=1.64.0, <2"] postgres = ["psycopg2-binary==2.9.13"] presto = ["pyhive[presto]>=0.7.0"] +teams = ["microsoft-teams-apps==2.1.1"] trino = ["trino>=0.340.0"] prophet = [ "prophet>=1.5.0, <2", diff --git a/requirements/development.in b/requirements/development.in index d76072d3dc5a..47889f5de383 100644 --- a/requirements/development.in +++ b/requirements/development.in @@ -16,7 +16,7 @@ # specific language governing permissions and limitations # under the License. # --e .[development,bigquery,clickhouse,cockroachdb,crate,databend,druid,duckdb,elasticsearch,fastmcp,firebird,gevent,gsheets,monetdb,mongodb,mssql,mysql,oracle,postgres,presto,prophet,risingwave,starrocks,trino,thumbnails,ydb] +-e .[development,bigquery,clickhouse,cockroachdb,crate,databend,druid,duckdb,elasticsearch,fastmcp,firebird,gevent,gsheets,monetdb,mongodb,mssql,mysql,oracle,postgres,presto,prophet,risingwave,starrocks,trino,teams,thumbnails,ydb] -e ./superset-extensions-cli[test] # testcontainers-backed db_engine_specs tests (tests/testcontainers/) -- # see .github/workflows/testcontainers.yml diff --git a/requirements/development.txt b/requirements/development.txt index 4118dac0118d..3a5901e6cd7d 100644 --- a/requirements/development.txt +++ b/requirements/development.txt @@ -27,6 +27,8 @@ amqp==5.3.1 # via # -c requirements/base-constraint.txt # kombu +annotated-doc==0.0.5 + # via fastapi annotated-types==0.7.0 # via # -c requirements/base-constraint.txt @@ -207,6 +209,8 @@ cryptography==50.0.2 # authlib # google-auth # joserfc + # microsoft-teams-apps + # msal # oracledb # paramiko # pyjwt @@ -227,6 +231,8 @@ defusedxml==0.7.1 # via # -c requirements/base-constraint.txt # odfpy +dependency-injector==4.49.1 + # via microsoft-teams-apps deprecated==1.2.18 # via # -c requirements/base-constraint.txt @@ -277,6 +283,8 @@ exceptiongroup==1.3.0 # via fastmcp-slim execnet==2.1.2 # via pytest-xdist +fastapi==0.162.0 + # via microsoft-teams-apps fastmcp==4.1.2 # via apache-superset fastmcp-slim==4.1.2 @@ -468,6 +476,7 @@ httpcore2==2.15.1 httpx==0.28.1 # via # apache-superset + # microsoft-teams-common # testcontainers httpx2==2.15.1 # via @@ -490,9 +499,7 @@ idna==3.20 # url-normalize # yarl importlib-metadata==8.7.0 - # via - # keyring - # opentelemetry-api + # via keyring iniconfig==2.0.0 # via pytest isodate==0.7.2 @@ -629,10 +636,22 @@ mdurl==0.1.2 # via # -c requirements/base-constraint.txt # markdown-it-py +microsoft-teams-api==2.1.1 + # via microsoft-teams-apps +microsoft-teams-apps==2.1.1 + # via apache-superset +microsoft-teams-cards==2.1.1 + # via microsoft-teams-api +microsoft-teams-common==2.1.1 + # via + # microsoft-teams-api + # microsoft-teams-apps more-itertools==10.8.0 # via # jaraco-classes # jaraco-functools +msal==1.39.0 + # via microsoft-teams-apps msgpack==1.2.3 # via # -c requirements/base-constraint.txt @@ -685,10 +704,13 @@ openpyxl==3.1.5 # pandas opensearch-py==2.8.0 # via elasticsearch-dbapi -opentelemetry-api==1.39.1 +opentelemetry-api==1.45.1 # via + # fastapi # fastmcp-slim # mcp + # microsoft-teams-api + # microsoft-teams-apps oracledb==26.0.0 # via # apache-superset @@ -824,17 +846,21 @@ pydantic==2.14.1 # -c requirements/base-constraint.txt # apache-superset # apache-superset-core + # fastapi # fastmcp-slim # mcp # mcp-types + # microsoft-teams-api # openapi-pydantic # pydantic-settings pydantic-core==2.50.1 # via # -c requirements/base-constraint.txt # pydantic -pydantic-settings==2.10.1 - # via fastmcp-slim +pydantic-settings==2.16.1 + # via + # fastmcp-slim + # microsoft-teams-apps pydata-google-auth==1.9.0 # via pandas-gbq pydruid==0.6.9 @@ -860,6 +886,9 @@ pyjwt==2.15.1 # flask-appbuilder # flask-jwt-extended # mcp + # microsoft-teams-api + # microsoft-teams-apps + # msal pylint==3.3.7 # via apache-superset pymonetdb==1.9.1 @@ -943,6 +972,7 @@ python-dotenv==1.2.2 # -c requirements/base-constraint.txt # apache-superset # fastmcp-slim + # microsoft-teams-apps # pydantic-settings # testcontainers python-ldap==3.4.8 @@ -989,6 +1019,7 @@ requests==2.33.0 # google-api-core # google-cloud-bigquery # jsonschema-path + # msal # opensearch-py # pydruid # pyhive @@ -1123,6 +1154,7 @@ sshtunnel==0.5.1 # apache-superset starlette==1.3.1 # via + # fastapi # fastmcp-slim # mcp starrocks==1.3.4 @@ -1165,7 +1197,9 @@ typing-extensions==4.16.0 # apache-superset # apache-superset-core # cattrs + # dependency-injector # exceptiongroup + # fastapi # fastmcp-slim # grpcio # httpx2 @@ -1177,6 +1211,7 @@ typing-extensions==4.16.0 # py-key-value-aio # pydantic # pydantic-core + # pydantic-settings # pyopenssl # referencing # shillelagh @@ -1187,6 +1222,7 @@ typing-extensions==4.16.0 typing-inspection==0.4.4 # via # -c requirements/base-constraint.txt + # fastapi # mcp # pydantic # pydantic-settings @@ -1221,6 +1257,7 @@ uvicorn==0.37.0 # via # fastmcp-slim # mcp + # microsoft-teams-apps verlib2==0.3.2 # via # crate diff --git a/superset-frontend/playwright/tests/reports/teams-alerts.spec.ts b/superset-frontend/playwright/tests/reports/teams-alerts.spec.ts new file mode 100644 index 000000000000..8eaf487b1f19 --- /dev/null +++ b/superset-frontend/playwright/tests/reports/teams-alerts.spec.ts @@ -0,0 +1,197 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { test, expect } from '@playwright/test'; +import { apiDelete } from '../../helpers/api/requests'; + +interface Destination { + key: string; + label: string; + available: boolean; +} + +// Requires ALERT_REPORTS, ALERT_REPORT_TEAMS and a deployment allowlist. No bot +// installation or live credential is needed: every saved schedule stays inactive. +test('creates and edits an inactive Teams alert and preserves unavailable destinations', async ({ + page, +}) => { + test.setTimeout(90000); + const configured = await page.request.get( + 'api/v1/report/teams_destinations/', + ); + test.skip( + configured.status() === 404, + 'Reports/Teams integration is disabled', + ); + expect(configured.ok()).toBeTruthy(); + const { result: destinations }: { result: Destination[] } = + await configured.json(); + test.skip( + destinations.length === 0, + 'Configure at least one approved Teams destination', + ); + const destination = destinations[0]; + let removed = false; + let alertId: number | undefined; + let executionRequests = 0; + + await page.route('**/api/v1/report/teams_destinations/', route => + route.fulfill({ + json: { + result: removed + ? [] + : destinations.map(item => ({ ...item, available: false })), + }, + }), + ); + // Exercise real schedule persistence, but reject an accidental active save or + // execution request before it can reach the server. + await page.route(/\/api\/v1\/report\/(?:\d+\/?)?$/, async route => { + const request = route.request(); + if (['POST', 'PUT'].includes(request.method())) { + const payload = request.postDataJSON() as { + active: boolean; + report_format: string; + type: string; + }; + expect(payload.active).toBe(false); + expect(payload.report_format).toBe('NONE'); + expect(payload.type).toBe('Alert'); + } + await route.continue(); + }); + await page.route(/\/api\/v1\/report\/\d+\/execute\/?(?:\?.*)?$/, route => { + executionRequests += 1; + return route.abort(); + }); + + const name = `Teams inactive ${test.info().testId} ${Date.now()}`; + try { + await page.goto('alert/list/'); + await page + .getByRole('button', { name: 'plus Alert', exact: true }) + .first() + .click(); + const modal = page.getByTestId('alert-report-modal'); + await modal.getByPlaceholder('Enter alert name').fill(name); + await modal.getByRole('switch').first().uncheck(); + await modal.getByTestId('alert-condition-panel').click(); + await modal + .getByRole('combobox', { name: 'Database', exact: true }) + .click(); + await page + .locator('.ant-select-dropdown:visible .ant-select-item-option') + .first() + .click(); + await modal.locator('.ace_editor').click(); + await page.keyboard.insertText('SELECT 1'); + await modal + .getByRole('combobox', { name: 'Condition', exact: true }) + .click(); + await page.getByText('Not null', { exact: true }).click(); + await modal.getByTestId('contents-panel').click(); + const attachments = modal.getByRole('switch', { + name: 'Include attachment', + }); + if (await attachments.count()) await attachments.uncheck(); + await modal.getByTestId('schedule-panel').click(); + await expect( + modal.getByRole('combobox', { name: 'Timezone selector' }), + ).toBeVisible(); + await modal.getByTestId('notification-method-panel').click(); + await modal.getByRole('combobox', { name: /^Delivery method/ }).click(); + await page.getByText('Teams', { exact: true }).click(); + await modal.getByRole('combobox', { name: 'Teams destination' }).click(); + await page + .getByText(`${destination.label} (Bot installation required)`, { + exact: true, + }) + .click(); + await expect(modal.getByText(/Install the notification bot/)).toBeVisible(); + await expect( + modal.getByRole('button', { name: 'Add', exact: true }), + ).toBeEnabled(); + const created = page.waitForResponse( + response => + response.request().method() === 'POST' && + new URL(response.url()).pathname.endsWith('/api/v1/report/'), + ); + await modal.getByRole('button', { name: 'Add', exact: true }).click(); + const response = await created; + expect(response.ok()).toBeTruthy(); + ({ id: alertId } = await response.json()); + expect(alertId).toBeDefined(); + await expect(modal).not.toBeVisible(); + + const edit = async () => { + await page.reload(); + const row = page + .getByRole('row') + .filter({ has: page.getByText(name, { exact: true }) }); + await row.getByRole('button', { name: 'Edit', exact: true }).click(); + await modal.getByTestId('notification-method-panel').click(); + }; + await edit(); + await expect( + modal.getByText(`${destination.label} (Bot installation required)`, { + exact: true, + }), + ).toBeVisible(); + await modal.getByTestId('general-information-panel').click(); + await expect(modal.getByRole('switch').first()).not.toBeChecked(); + await modal + .locator('input[name="description"]') + .fill('Updated without activating delivery'); + const updated = page.waitForResponse( + result => + result.request().method() === 'PUT' && + new URL(result.url()).pathname.endsWith(`/report/${alertId}`), + ); + await modal.getByRole('button', { name: 'Save', exact: true }).click(); + expect((await updated).ok()).toBeTruthy(); + const saved = await page.request.get(`api/v1/report/${alertId}`); + const { result } = await saved.json(); + expect(result.active).toBe(false); + expect(result.report_format).toBe('NONE'); + expect(result.recipients).toHaveLength(1); + expect(result.recipients[0].type).toBe('Teams'); + expect(JSON.parse(result.recipients[0].recipient_config_json)).toEqual({ + target: destination.key, + }); + + removed = true; + await edit(); + await expect( + modal.getByText(`${destination.key} (Unavailable)`, { exact: true }), + ).toBeVisible(); + await expect( + modal.getByText(/This saved Teams destination is unavailable/), + ).toBeVisible(); + await modal.getByRole('button', { name: 'Cancel', exact: true }).click(); + const unchanged = await page.request.get(`api/v1/report/${alertId}`); + expect((await unchanged.json()).result.recipients).toEqual( + result.recipients, + ); + expect(executionRequests).toBe(0); + } finally { + if (alertId !== undefined) { + const response = await apiDelete(page, `api/v1/report/${alertId}`); + expect(response.ok()).toBeTruthy(); + } + } +}); diff --git a/superset-frontend/src/features/alerts/AlertReportModal.test.tsx b/superset-frontend/src/features/alerts/AlertReportModal.test.tsx index cec5b278d9ff..c8b35157e809 100644 --- a/superset-frontend/src/features/alerts/AlertReportModal.test.tsx +++ b/superset-frontend/src/features/alerts/AlertReportModal.test.tsx @@ -57,9 +57,11 @@ jest.mock('src/utils/getBootstrapData', () => ({ })), })); +let mockNotificationMethods = ['Email', 'Slack', 'SlackV2']; + jest.mock('src/features/databases/state.ts', () => ({ useCommonConf: () => ({ - ALERT_REPORTS_NOTIFICATION_METHODS: ['Email', 'Slack', 'SlackV2'], + ALERT_REPORTS_NOTIFICATION_METHODS: mockNotificationMethods, }), })); @@ -330,6 +332,7 @@ const restoreDefaultTabsRoute = () => { }; afterEach(() => { + mockNotificationMethods = ['Email', 'Slack', 'SlackV2']; restoreDefaultTabsRoute(); // Clear call history so stale counts don't leak between tests @@ -1168,9 +1171,9 @@ test('removes notification method on clicking trash can', async () => { /add another notification method/i, ); await userEvent.click(addNotificationMethod); - const images = screen.getAllByRole('img'); - const trash = images[images.length - 1]; - await userEvent.click(trash); + await userEvent.click( + screen.getByRole('button', { name: /remove notification method/i }), + ); expect( screen.getAllByRole('combobox', { name: /delivery method/i }).length, ).toBe(1); @@ -4662,3 +4665,434 @@ test('global attachment policy hides both the alert toggle and attachment contro ); } }); + +test('Changing the first Teams method preserves the following destination', async () => { + mockNotificationMethods = ['Email', 'Teams']; + fetchMock.get('glob:*/api/v1/report/92', { + result: { + ...generateMockPayload(true), + id: 92, + report_format: 'NONE', + recipients: [ + { id: 1, type: 'Teams', recipient_config_json: '{"target":"ops"}' }, + { id: 2, type: 'Teams', recipient_config_json: '{"target":"finance"}' }, + ], + }, + }); + fetchMock.get( + 'glob:*/api/v1/report/teams_destinations/', + { + result: [ + { key: 'ops', label: 'Operations', available: true }, + { key: 'finance', label: 'Finance', available: false }, + ], + }, + { name: 'teams-destinations' }, + ); + fetchMock.put( + 'glob:*/api/v1/report/92', + { id: 92, result: {} }, + { name: 'save-teams' }, + ); + try { + render( + , + { + useRedux: true, + initialState: { user: adminUser }, + }, + ); + await waitFor(() => + expect(screen.getByRole('button', { name: /save/i })).toBeEnabled(), + ); + await userEvent.click(screen.getByTestId('notification-method-panel')); + expect( + await screen.findByText('Finance (Bot installation required)'), + ).toBeInTheDocument(); + expect( + screen.getAllByRole('combobox', { name: 'Teams destination' }), + ).toHaveLength(2); + expect( + screen.getByText(/Add another notification method/i), + ).toBeInTheDocument(); + await userEvent.click(screen.getByTestId('contents-panel')); + const attachmentToggle = screen.getByRole('switch', { + name: 'Include attachment', + }); + await userEvent.click(attachmentToggle); + await waitFor(() => + expect(screen.getByRole('button', { name: /save/i })).toBeDisabled(), + ); + await userEvent.click(attachmentToggle); + await waitFor(() => + expect(screen.getByRole('button', { name: /save/i })).toBeEnabled(), + ); + await userEvent.click(screen.getByTestId('notification-method-panel')); + expect( + screen.getByText('Finance (Bot installation required)'), + ).toBeInTheDocument(); + await userEvent.click( + screen.getAllByRole('combobox', { name: /^Delivery method/ })[0], + ); + await userEvent.click(await screen.findByText('Email')); + await addAsyncSelectValue( + /email recipients/i, + 'owner@example.com', + reportUsersEndpoint, + ); + await userEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => + expect(fetchMock.callHistory.calls('save-teams')).toHaveLength(1), + ); + const [call] = fetchMock.callHistory.calls('save-teams'); + expect(JSON.parse(call.options.body as string)).toMatchObject({ + report_format: 'NONE', + recipients: [ + { + type: 'Email', + recipient_config_json: { + target: 'owner@example.com', + ccTarget: '', + bccTarget: '', + }, + }, + { type: 'Teams', recipient_config_json: { target: 'finance' } }, + ], + }); + expect(JSON.parse(call.options.body as string).recipients).toEqual([ + { + type: 'Email', + recipient_config_json: { + target: 'owner@example.com', + ccTarget: '', + bccTarget: '', + }, + }, + { type: 'Teams', recipient_config_json: { target: 'finance' } }, + ]); + } finally { + fetchMock.removeRoute('teams-destinations'); + fetchMock.removeRoute('save-teams'); + } +}); + +test.each([ + ['an active alert', true, false], + ['an inactive alert', false, false], + ['pausing an alert', true, true], +])('Teams unavailable integration allows %s', async (_, active, pause) => { + const id = pause ? 193 : active ? 194 : 195; + mockNotificationMethods = ['Email', 'Slack']; + fetchMock.get(`glob:*/api/v1/report/${id}`, { + result: { + ...generateMockPayload(true), + id, + active, + report_format: 'NONE', + recipients: [ + { + id: 1, + type: 'Teams', + recipient_config_json: '{"target":"removed-key"}', + }, + { id: 2, type: 'Slack', recipient_config_json: { target: 'C123' } }, + ], + }, + }); + fetchMock.get( + 'glob:*/api/v1/report/teams_destinations/', + { result: [] }, + { name: 'teams-unavailable' }, + ); + fetchMock.put( + `glob:*/api/v1/report/${id}`, + { id, result: {} }, + { + name: 'teams-unavailable-save', + }, + ); + try { + render( + , + { useRedux: true, initialState: { user: adminUser } }, + ); + await userEvent.click(screen.getByTestId('notification-method-panel')); + expect( + await screen.findByText('removed-key (Unavailable)'), + ).toBeInTheDocument(); + expect( + await screen.findByText(/Teams delivery is unavailable/), + ).toBeInTheDocument(); + await userEvent.click(screen.getByTestId('general-information-panel')); + const description = screen.getByPlaceholderText( + /include description to be sent with alert/i, + ); + await userEvent.clear(description); + await userEvent.type(description, 'Updated description'); + if (pause) await userEvent.click(screen.getByRole('switch')); + const save = screen.getByRole('button', { name: /save/i }); + await waitFor(() => expect(save).toBeEnabled()); + await userEvent.click(screen.getByTestId('notification-method-panel')); + const slackTarget = screen.getByTestId('recipients'); + fireEvent.change(slackTarget, { target: { value: 'C456' } }); + await waitFor(() => expect(save).toBeDisabled()); + fireEvent.change(slackTarget, { target: { value: 'C123' } }); + await waitFor(() => expect(save).toBeEnabled()); + await userEvent.click(save); + await waitFor(() => + expect( + fetchMock.callHistory.calls('teams-unavailable-save'), + ).toHaveLength(1), + ); + const [call] = fetchMock.callHistory.calls('teams-unavailable-save'); + expect(JSON.parse(call.options.body as string)).toMatchObject({ + active: pause ? false : active, + description: 'Updated description', + report_format: 'NONE', + recipients: [ + { type: 'Teams', recipient_config_json: { target: 'removed-key' } }, + { + type: 'Slack', + recipient_config_json: { + target: 'C123', + ccTarget: '', + bccTarget: '', + }, + }, + ], + }); + } finally { + fetchMock.removeRoute(`glob:*/api/v1/report/${id}`); + fetchMock.removeRoute('teams-unavailable'); + fetchMock.removeRoute('teams-unavailable-save'); + } +}); + +test('Teams unavailable saved keys can be replaced without losing inactive state', async () => { + mockNotificationMethods = ['Email', 'Teams']; + fetchMock.get('glob:*/api/v1/report/94', { + result: { + ...generateMockPayload(true), + id: 94, + active: false, + report_format: 'NONE', + recipients: [ + { + id: 1, + type: 'Teams', + recipient_config_json: { target: 'removed-key' }, + }, + ], + }, + }); + fetchMock.get( + 'glob:*/api/v1/report/teams_destinations/', + { + result: [{ key: 'ops', label: 'Operations', available: false }], + }, + { name: 'teams-replacement' }, + ); + fetchMock.put( + 'glob:*/api/v1/report/94', + { id: 94, result: {} }, + { name: 'teams-replacement-save' }, + ); + try { + render( + , + { + useRedux: true, + initialState: { user: adminUser }, + }, + ); + await userEvent.click(screen.getByTestId('notification-method-panel')); + expect( + await screen.findByText('removed-key (Unavailable)'), + ).toBeInTheDocument(); + await userEvent.click( + screen.getByRole('combobox', { name: 'Teams destination' }), + ); + await userEvent.click( + await screen.findByText('Operations (Bot installation required)'), + ); + expect( + screen.queryByText(/This saved Teams destination is unavailable/), + ).not.toBeInTheDocument(); + await userEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => + expect( + fetchMock.callHistory.calls('teams-replacement-save'), + ).toHaveLength(1), + ); + const [call] = fetchMock.callHistory.calls('teams-replacement-save'); + const payload = JSON.parse(call.options.body as string); + expect(payload.active).toBe(false); + expect(payload.report_format).toBe('NONE'); + expect(payload.recipients).toEqual([ + { type: 'Teams', recipient_config_json: { target: 'ops' } }, + ]); + } finally { + fetchMock.removeRoute('teams-replacement'); + fetchMock.removeRoute('teams-replacement-save'); + } +}); + +test('Teams saved recipients stay visible and replaceable when the optional SDK is unavailable', async () => { + mockNotificationMethods = ['Email']; + fetchMock.get('glob:*/api/v1/report/95', { + result: { + ...generateMockPayload(true), + id: 95, + report_format: 'NONE', + recipients: [ + { id: 1, type: 'Teams', recipient_config_json: '{"target":"ops"}' }, + ], + }, + }); + fetchMock.get( + 'glob:*/api/v1/report/teams_destinations/', + { result: [] }, + { name: 'teams-sdk-missing' }, + ); + try { + render( + , + { + useRedux: true, + initialState: { user: adminUser }, + }, + ); + await userEvent.click(screen.getByTestId('notification-method-panel')); + expect(await screen.findByText('ops (Unavailable)')).toBeInTheDocument(); + expect( + screen.getByText( + /Teams delivery is unavailable.*optional Teams dependency/, + ), + ).toBeInTheDocument(); + expect( + screen.getByRole('combobox', { name: 'Teams destination' }), + ).toBeDisabled(); + const save = screen.getByRole('button', { name: /save/i }); + await waitFor(() => expect(save).toBeEnabled()); + await userEvent.click(screen.getByTestId('general-information-panel')); + await userEvent.click(screen.getByRole('switch')); + await waitFor(() => expect(save).toBeDisabled()); + await userEvent.click(screen.getByRole('switch')); + await waitFor(() => expect(save).toBeEnabled()); + await userEvent.click(screen.getByTestId('notification-method-panel')); + await userEvent.click( + screen.getByRole('combobox', { name: /^Delivery method/ }), + ); + await userEvent.click(await screen.findByText('Email')); + expect( + screen.queryByRole('combobox', { name: 'Teams destination' }), + ).not.toBeInTheDocument(); + expect(screen.queryByText('ops (Unavailable)')).not.toBeInTheDocument(); + } finally { + fetchMock.removeRoute('teams-sdk-missing'); + } +}); + +test.each(['PNG', 'CSV'])( + 'Teams edits normalize saved %s format when attachments are globally disabled', + async reportFormat => { + mockNotificationMethods = ['Email', 'Teams']; + fetchMock.removeRoute(configurationEndpoint); + fetchMock.get( + configurationEndpoint, + { + result: { ...mockReportConfiguration, alerts_attach_reports: false }, + }, + { name: configurationEndpoint }, + ); + fetchMock.get( + 'glob:*/api/v1/report/96', + { + result: { + ...generateMockPayload(true), + id: 96, + report_format: reportFormat, + recipients: [ + { + id: 1, + type: 'Email', + recipient_config_json: { target: 'test@example.com' }, + }, + ], + }, + }, + { name: 'teams-saved-format' }, + ); + fetchMock.get( + 'glob:*/api/v1/report/teams_destinations/', + { + result: [{ key: 'ops', label: 'Operations', available: true }], + }, + { name: 'teams-hidden-attachments' }, + ); + fetchMock.put( + 'glob:*/api/v1/report/96', + { id: 96, result: {} }, + { name: 'save-teams-hidden' }, + ); + try { + render( + , + { + useRedux: true, + initialState: { user: adminUser }, + }, + ); + await userEvent.click(screen.getByTestId('notification-method-panel')); + await userEvent.click( + screen.getByRole('combobox', { name: /^Delivery method/ }), + ); + await userEvent.click(await screen.findByText('Teams')); + await userEvent.click( + screen.getByRole('combobox', { name: 'Teams destination' }), + ); + await userEvent.click(await screen.findByText('Operations')); + await waitFor(() => + expect(screen.getByRole('button', { name: /save/i })).toBeEnabled(), + ); + await userEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => + expect(fetchMock.callHistory.calls('save-teams-hidden')).toHaveLength( + 1, + ), + ); + const [call] = fetchMock.callHistory.calls('save-teams-hidden'); + expect(JSON.parse(call.options.body as string)).toMatchObject({ + report_format: 'NONE', + recipients: [ + { type: 'Teams', recipient_config_json: { target: 'ops' } }, + ], + }); + } finally { + fetchMock.removeRoute('teams-saved-format'); + fetchMock.removeRoute('teams-hidden-attachments'); + fetchMock.removeRoute('save-teams-hidden'); + fetchMock.removeRoute(configurationEndpoint); + fetchMock.get( + configurationEndpoint, + { result: mockReportConfiguration }, + { name: configurationEndpoint }, + ); + } + }, +); diff --git a/superset-frontend/src/features/alerts/AlertReportModal.tsx b/superset-frontend/src/features/alerts/AlertReportModal.tsx index 5dc406d4a6c9..11d80faa3fab 100644 --- a/superset-frontend/src/features/alerts/AlertReportModal.tsx +++ b/superset-frontend/src/features/alerts/AlertReportModal.tsx @@ -119,6 +119,10 @@ import { isUserAdmin } from 'src/dashboard/util/permissionUtils'; import NumberInput from './components/NumberInput'; import { AlertReportCronScheduler } from './components/AlertReportCronScheduler'; import { NotificationMethod } from './components/NotificationMethod'; +import { + getNotificationMethodOptions, + supportsTeamsRecipients, +} from './notificationMethods'; import { buildErrorTooltipMessage } from './buildErrorTooltipMessage'; import { useReportConfiguration } from './hooks/useReportConfiguration'; @@ -580,7 +584,7 @@ const AlertReportModal: FunctionComponent = ({ const currentUserSubjectId = getBootstrapData()?.common?.user_subject_id; // Check config for alternate notification methods setting const conf = useCommonConf(); - const allowedNotificationMethods: NotificationMethodOption[] = + const configuredNotificationMethods: NotificationMethodOption[] = conf?.ALERT_REPORTS_NOTIFICATION_METHODS || DEFAULT_NOTIFICATION_METHODS; const [disableSave, setDisableSave] = useState(true); @@ -707,6 +711,21 @@ const AlertReportModal: FunctionComponent = ({ const formatOptionEnabled = attachmentsEnabledForAlerts || isReport; const attachmentControlsVisible = isReport || (attachmentsEnabledForAlerts && reportFormat !== 'NONE'); + const teamsIntegrationAvailable = configuredNotificationMethods.includes( + NotificationMethodOption.Teams, + ); + const teamsSupported = supportsTeamsRecipients( + isReport, + attachmentsEnabledForAlerts, + reportFormat, + ); + const allowedNotificationMethods = useMemo( + () => + configuredNotificationMethods.filter( + method => method !== NotificationMethodOption.Teams || teamsSupported, + ), + [configuredNotificationMethods, teamsSupported], + ); const [previousAttachmentFormat, setPreviousAttachmentFormat] = useState( DEFAULT_NOTIFICATION_FORMAT, ); @@ -740,53 +759,27 @@ const AlertReportModal: FunctionComponent = ({ } : undefined; - const [notificationAddState, setNotificationAddState] = - useState('active'); - const [notificationSettings, setNotificationSettings] = useState< NotificationSetting[] >([]); const [emailSubject, setEmailSubject] = useState(''); const [emailError, setEmailError] = useState(false); - const allowedNotificationMethodsCount = useMemo( - () => - allowedNotificationMethods.reduce((accum: string[], setting: string) => { - if ( - accum.some(nm => nm.includes('slack')) && - setting.toLowerCase().includes('slack') - ) { - return accum; - } - return [...accum, setting.toLowerCase()]; - }, []).length, - [allowedNotificationMethods], + const availableNotificationMethods = getNotificationMethodOptions( + allowedNotificationMethods, + notificationSettings, ); + const notificationAddState: NotificationAddStatus = notificationSettings.some( + setting => !setting.method, + ) + ? 'disabled' + : 'active'; const onNotificationAdd = () => { setNotificationSettings([ ...notificationSettings, - { - recipients: '', - // options shown in the newly added notification method - options: allowedNotificationMethods.filter( - // are filtered such that - option => - // options are not included - !notificationSettings.reduce( - // when it exists in previous notificationSettings - (accum, setting) => accum || option === setting.method, - false, - ), - ), - }, + { recipients: '', options: availableNotificationMethods }, ]); - - setNotificationAddState( - notificationSettings.length === allowedNotificationMethodsCount - ? 'hidden' - : 'disabled', - ); }; const { @@ -1040,31 +1033,12 @@ const AlertReportModal: FunctionComponent = ({ const settings: NotificationSetting[] = [...notificationSettings]; settings[index] = setting; - // if you've changed notification method -> remove trailing methods - if (notificationSettings[index].method !== setting.method) { - notificationSettings[index] = setting; - - setNotificationSettings( - notificationSettings.filter((_, idx) => idx <= index), - ); - - if (notificationSettings.length - 1 > index) { - setNotificationAddState('active'); - } - - if (setting.method !== undefined && notificationAddState !== 'hidden') { - setNotificationAddState('active'); - } - } else { - setNotificationSettings(settings); - } + setNotificationSettings(settings); }; const removeNotificationSetting = (index: number) => { - const settings = notificationSettings.slice(); - - settings.splice(index, 1); - setNotificationSettings(settings); - setNotificationAddState('active'); + setNotificationSettings( + notificationSettings.filter((_, idx) => idx !== index), + ); }; const updateAnchorState = (value: any) => { @@ -1099,10 +1073,9 @@ const AlertReportModal: FunctionComponent = ({ onHide(); setNotificationSettings([]); setCurrentAlert({ ...defaultAlert }); - setNotificationAddState('active'); }; - const onSave = () => { + const getRecipients = () => { // Notification Settings const recipients: Recipient[] = []; @@ -1111,14 +1084,33 @@ const AlertReportModal: FunctionComponent = ({ recipients.push({ recipient_config_json: { target: setting.recipients, - ccTarget: setting.cc, - bccTarget: setting.bcc, + ...(setting.method === NotificationMethodOption.Teams + ? {} + : { + ccTarget: setting.cc, + bccTarget: setting.bcc, + }), }, type: setting.method, }); } }); + return recipients; + }; + + const effectiveReportFormat = + !isReport && + !attachmentControlsVisible && + (!isEditMode || + notificationSettings.some( + setting => setting.method === NotificationMethodOption.Teams, + )) + ? 'NONE' + : reportFormat || DEFAULT_NOTIFICATION_FORMAT; + + const onSave = () => { + const recipients = getRecipients(); const shouldEnableForceScreenshot = contentType === ContentType.Chart && !isReport; @@ -1171,13 +1163,10 @@ const AlertReportModal: FunctionComponent = ({ database: currentAlert?.database?.value, editors: mapSubjectPickerValuesToIds(currentAlert?.editors || []), recipients, - report_format: reportFormat || DEFAULT_NOTIFICATION_FORMAT, + report_format: effectiveReportFormat, extra: contentType === ContentType.Dashboard ? currentAlert?.extra : {}, }; - if (!isReport && !attachmentControlsVisible && !isEditMode) { - data.report_format = 'NONE'; - } if (!attachmentControlsVisible && isEditMode) { // Hidden settings stay stored; only the format controls attachment delivery. delete data.chart; @@ -1906,6 +1895,59 @@ const AlertReportModal: FunctionComponent = ({ return false; } + const hasTeamsRecipients = notificationSettings.some( + setting => setting.method === NotificationMethodOption.Teams, + ); + // Match the backend's delivery-change check so unavailable integrations do + // not prevent pausing or editing metadata on an existing alert. + const normalizeRecipients = ( + recipients: { + type: string; + recipient_config_json: string | Recipient['recipient_config_json']; + }[], + ) => + JSON.stringify( + recipients + .map(({ type, recipient_config_json: config }) => { + const parsed = + typeof config === 'string' ? JSON.parse(config) : config; + return JSON.stringify([ + type, + Object.entries(parsed) + .filter( + ([key, value]) => + value !== undefined && + !( + type !== NotificationMethodOption.Teams && + ['ccTarget', 'bccTarget'].includes(key) && + value === '' + ), + ) + .sort(([left], [right]) => left.localeCompare(right)), + ]); + }) + .sort(), + ); + const deliveryUnchanged = + hasTeamsRecipients && + isEditMode && + resource && + !(currentAlert?.active && !resource.active) && + resource.type === (isReport ? 'Report' : 'Alert') && + resource.report_format === effectiveReportFormat && + normalizeRecipients(resource.recipients || []) === + normalizeRecipients(getRecipients()); + const invalidTeamsRecipients = + hasTeamsRecipients && + ((!deliveryUnchanged && + (!teamsSupported || !teamsIntegrationAvailable)) || + notificationSettings.some( + setting => + setting.method === NotificationMethodOption.Teams && + !setting.recipients, + )); + if (invalidTeamsRecipients) return false; + let hasInfo = false; notificationSettings.forEach(setting => { @@ -2255,7 +2297,6 @@ const AlertReportModal: FunctionComponent = ({ method: NotificationMethodOption.Email, }, ]); - setNotificationAddState('active'); } }, [alert]); @@ -2282,7 +2323,7 @@ const AlertReportModal: FunctionComponent = ({ const config = typeof setting.recipient_config_json === 'string' ? JSON.parse(setting.recipient_config_json) - : {}; + : setting.recipient_config_json; return { method: setting.type, recipients: config.target || setting.recipient_config_json, @@ -2293,11 +2334,6 @@ const AlertReportModal: FunctionComponent = ({ }); setNotificationSettings(settings); - setNotificationAddState( - settings.length === allowedNotificationMethods.length - ? 'hidden' - : 'active', - ); setContentType( resource.chart ? ContentType.Chart : ContentType.Dashboard, ); @@ -2363,6 +2399,9 @@ const AlertReportModal: FunctionComponent = ({ updateEmailSubject(); }, [ currentAlertSafe.name, + currentAlertSafe.active, + resource, + isReport, currentAlertSafe.editors, currentAlertSafe.run_as, currentAlertSafe.run_as_type, @@ -2378,6 +2417,8 @@ const AlertReportModal: FunctionComponent = ({ contentType, attachmentControlsVisible, reportFormat, + teamsSupported, + teamsIntegrationAvailable, nativeFilterData, notificationSettings, conditionNotNull, @@ -3286,11 +3327,21 @@ const AlertReportModal: FunctionComponent = ({ children: ( <> {notificationSettings.map((notificationSetting, i) => ( - + = ({ /> ))} - { - // Prohibit 'add notification method' button if only one present - allowedNotificationMethodsCount > - notificationSettings.length && ( - - ) - } + {availableNotificationMethods.length > 0 && ( + + )} ), }, diff --git a/superset-frontend/src/features/alerts/components/NotificationMethod.test.tsx b/superset-frontend/src/features/alerts/components/NotificationMethod.test.tsx index 201e6a31a9da..fa35b13d9be2 100644 --- a/superset-frontend/src/features/alerts/components/NotificationMethod.test.tsx +++ b/superset-frontend/src/features/alerts/components/NotificationMethod.test.tsx @@ -954,3 +954,81 @@ describe('NotificationMethod', () => { }); }); }); + +jest.mock('./TeamsDestinationSelect', () => ({ + __esModule: true, + default: ({ + value, + disabled, + onChange, + }: { + value: string; + disabled?: boolean; + onChange: (key: string) => void; + }) => ( + onChange(event.target.value)} + /> + ), +})); + +test.each([ + { supported: true, available: true, warning: null }, + { + supported: false, + available: true, + warning: /Teams delivery requires an alert with attachments disabled/, + }, + { + supported: true, + available: false, + warning: /Teams delivery is unavailable/, + }, +])( + 'Teams retains its saved key when supported=$supported and available=$available', + ({ supported, available, warning }) => { + const onUpdate = jest.fn(); + render( + , + ); + const destination = screen.getByRole('textbox', { + name: 'Teams destination', + }); + expect(destination).toHaveValue('saved-key'); + if (warning) { + expect(screen.getByText(warning)).toBeInTheDocument(); + expect(destination).toBeDisabled(); + } else { + expect(destination).toBeEnabled(); + fireEvent.change(destination, { target: { value: 'ops' } }); + expect(onUpdate).toHaveBeenCalledWith( + 2, + expect.objectContaining({ + method: NotificationMethodOption.Teams, + recipients: 'ops', + }), + ); + } + }, +); diff --git a/superset-frontend/src/features/alerts/components/NotificationMethod.tsx b/superset-frontend/src/features/alerts/components/NotificationMethod.tsx index cb1e64b399fe..83255626bafe 100644 --- a/superset-frontend/src/features/alerts/components/NotificationMethod.tsx +++ b/superset-frontend/src/features/alerts/components/NotificationMethod.tsx @@ -28,6 +28,7 @@ import { import rison from 'rison'; import { t } from '@apache-superset/core/translation'; +import { Alert } from '@apache-superset/core/components'; import { FeatureFlag, JsonResponse, @@ -49,6 +50,7 @@ import { SlackChannel, } from '../types'; import { StyledInputContainer } from '../AlertReportModal'; +import TeamsDestinationSelect from './TeamsDestinationSelect'; const StyledNotificationMethod = styled.div` ${({ theme }) => ` @@ -143,6 +145,8 @@ const TRANSLATIONS = { }; interface NotificationMethodProps { + teamsSupported?: boolean; + teamsIntegrationAvailable?: boolean; setting?: NotificationSetting | null; index: number; onUpdate?: (index: number, updatedSetting: NotificationSetting) => void; @@ -256,6 +260,8 @@ const fetchEmailRecipientOptions = async ( }; export const NotificationMethod: FunctionComponent = ({ + teamsSupported = true, + teamsIntegrationAvailable = true, setting = null, index, onUpdate, @@ -480,7 +486,8 @@ export const NotificationMethod: FunctionComponent = ({ method === NotificationMethodOption.Slack) || (isFeatureEnabled(FeatureFlag.AlertReportWebhook) && method === NotificationMethodOption.Webhook) || - method === NotificationMethodOption.Email, + method === NotificationMethodOption.Email || + method === NotificationMethodOption.Teams, ) .map(method => ({ label: @@ -488,8 +495,11 @@ export const NotificationMethod: FunctionComponent = ({ ? NotificationMethodOption.Slack : method, value: method, + disabled: + method === NotificationMethodOption.Teams && + (!teamsSupported || !teamsIntegrationAvailable), })), - [options, useSlackV1], + [options, useSlackV1, teamsSupported, teamsIntegrationAvailable], ); if (!setting) { @@ -626,6 +636,23 @@ export const NotificationMethod: FunctionComponent = ({ ) : null} + {method === NotificationMethodOption.Teams && !teamsSupported && ( + + )} + {method === NotificationMethodOption.Teams && + !teamsIntegrationAvailable && ( + + )} {method !== NotificationMethodOption.Webhook ? (
@@ -639,7 +666,15 @@ export const NotificationMethod: FunctionComponent = ({ *
- {method === NotificationMethodOption.Slack ? ( + {method === NotificationMethodOption.Teams ? ( + + onUpdate?.(index, { ...setting, recipients: value }) + } + /> + ) : method === NotificationMethodOption.Slack ? ( <>
{ }); }); }); + +test('renders Teams with an accessible icon', () => { + const { container } = render( + , + ); + expect( + container.querySelector('[data-icon="usergroup-add"]'), + ).toBeInTheDocument(); +}); diff --git a/superset-frontend/src/features/alerts/components/RecipientIcon.tsx b/superset-frontend/src/features/alerts/components/RecipientIcon.tsx index 8e4dd26ee6de..f50edf1789ad 100644 --- a/superset-frontend/src/features/alerts/components/RecipientIcon.tsx +++ b/superset-frontend/src/features/alerts/components/RecipientIcon.tsx @@ -52,6 +52,12 @@ export default function RecipientIcon({ type }: { type: string }) { ); recipientIconConfig.label = NotificationMethodOption.Slack; break; + case NotificationMethodOption.Teams: + recipientIconConfig.icon = ( + + ); + recipientIconConfig.label = NotificationMethodOption.Teams; + break; case NotificationMethodOption.Webhook: recipientIconConfig.icon = ( diff --git a/superset-frontend/src/features/alerts/components/TeamsDestinationSelect.test.tsx b/superset-frontend/src/features/alerts/components/TeamsDestinationSelect.test.tsx new file mode 100644 index 000000000000..9a78c5b50e8c --- /dev/null +++ b/superset-frontend/src/features/alerts/components/TeamsDestinationSelect.test.tsx @@ -0,0 +1,149 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import fetchMock from 'fetch-mock'; +import { + render, + screen, + userEvent, + waitFor, +} from 'spec/helpers/testing-library'; +import TeamsDestinationSelect from './TeamsDestinationSelect'; + +const endpoint = 'glob:*/api/v1/report/teams_destinations/'; +const destinations = [ + { key: 'ops', label: 'Operations', available: true }, + { key: 'finance', label: 'Finance', available: false }, +]; + +afterEach(() => fetchMock.clearHistory().removeRoutes()); + +test('selects approved destinations by label and returns only the stable key', async () => { + fetchMock.get(endpoint, { result: destinations }); + const onChange = jest.fn(); + render(); + await userEvent.click(screen.getByRole('combobox')); + await userEvent.click(await screen.findByText('Operations')); + expect(onChange).toHaveBeenCalledWith('ops', expect.anything()); +}); + +test('explains a missing installation without clearing the saved destination', async () => { + fetchMock.get(endpoint, { result: destinations }); + const onChange = jest.fn(); + render(); + expect( + await screen.findByText( + /Install the notification bot in the approved team before delivery/, + ), + ).toBeInTheDocument(); + expect( + screen.getByText('Finance (Bot installation required)'), + ).toBeInTheDocument(); + expect(onChange).not.toHaveBeenCalled(); +}); + +test('preserves removed destination keys with a visible unavailable state', async () => { + fetchMock.get(endpoint, { result: destinations }); + const onChange = jest.fn(); + render(); + expect( + await screen.findByText(/This saved Teams destination is unavailable/), + ).toBeInTheDocument(); + expect(screen.getByText('removed-key (Unavailable)')).toBeInTheDocument(); + expect(onChange).not.toHaveBeenCalled(); +}); + +test('a loading failure keeps saved selections and displays a recoverable error', async () => { + fetchMock.get(endpoint, 500); + const onChange = jest.fn(); + render(); + expect( + await screen.findByText(/Unable to load Teams destinations/), + ).toBeInTheDocument(); + expect(screen.getByRole('combobox')).toBeDisabled(); + expect(onChange).not.toHaveBeenCalled(); +}); + +test('supports saving a configured destination before bot installation', async () => { + fetchMock.get(endpoint, { result: destinations }); + const onChange = jest.fn(); + render(); + await userEvent.click(screen.getByRole('combobox')); + await userEvent.click( + await screen.findByText('Finance (Bot installation required)'), + ); + expect(onChange).toHaveBeenCalledWith('finance', expect.anything()); +}); + +test('keeps Teams destinations disabled for incompatible schedules', async () => { + fetchMock.get(endpoint, { result: destinations }); + render(); + await waitFor(() => + expect(screen.getByText('Operations')).toBeInTheDocument(), + ); + expect(screen.getByRole('combobox')).toBeDisabled(); +}); + +test('explains an empty allowlist without inventing destinations', async () => { + fetchMock.get(endpoint, { result: [] }); + const onChange = jest.fn(); + render(); + expect( + await screen.findByText(/No approved Teams destinations are configured/), + ).toBeInTheDocument(); + expect(onChange).not.toHaveBeenCalled(); +}); + +test('allows replacing a removed saved key with an approved destination', async () => { + fetchMock.get(endpoint, { result: destinations }); + const onChange = jest.fn(); + const { rerender } = render( + , + ); + expect(await screen.findByText('removed (Unavailable)')).toBeInTheDocument(); + await userEvent.click(screen.getByRole('combobox')); + await userEvent.click(await screen.findByText('Operations')); + expect(onChange).toHaveBeenCalledWith('ops', expect.anything()); + rerender(); + expect( + screen.queryByText(/This saved Teams destination is unavailable/), + ).not.toBeInTheDocument(); + expect(screen.getAllByText('Operations').length).toBeGreaterThan(0); +}); + +test('an availability-only refresh preserves the stable saved destination key', async () => { + fetchMock.get(endpoint, { result: destinations }); + const onChange = jest.fn(); + const { unmount } = render( + , + ); + expect( + await screen.findByText('Finance (Bot installation required)'), + ).toBeInTheDocument(); + unmount(); + fetchMock.removeRoutes(); + fetchMock.get(endpoint, { + result: destinations.map(item => ({ ...item, available: true })), + }); + render(); + expect(await screen.findByText('Finance')).toBeInTheDocument(); + expect( + screen.queryByText(/Install the notification bot/), + ).not.toBeInTheDocument(); + expect(onChange).not.toHaveBeenCalled(); +}); diff --git a/superset-frontend/src/features/alerts/components/TeamsDestinationSelect.tsx b/superset-frontend/src/features/alerts/components/TeamsDestinationSelect.tsx new file mode 100644 index 000000000000..1c719ffb85e1 --- /dev/null +++ b/superset-frontend/src/features/alerts/components/TeamsDestinationSelect.tsx @@ -0,0 +1,125 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { useEffect, useState } from 'react'; +import { t } from '@apache-superset/core/translation'; +import { Alert } from '@apache-superset/core/components'; +import { SupersetClient } from '@superset-ui/core'; +import { Select } from '@superset-ui/core/components'; +import { TeamsDestination } from '../types'; + +interface TeamsDestinationSelectProps { + value: string; + onChange: (value: string) => void; + disabled?: boolean; +} + +export default function TeamsDestinationSelect({ + value, + onChange, + disabled = false, +}: TeamsDestinationSelectProps) { + const [destinations, setDestinations] = useState([]); + const [loading, setLoading] = useState(true); + const [loadFailed, setLoadFailed] = useState(false); + + useEffect(() => { + let cancelled = false; + SupersetClient.get({ endpoint: '/api/v1/report/teams_destinations/' }) + .then(({ json }) => { + if (!cancelled) setDestinations(json.result); + }) + .catch(() => { + if (!cancelled) setLoadFailed(true); + }) + .finally(() => { + if (!cancelled) setLoading(false); + }); + return () => { + cancelled = true; + }; + }, []); + + const selectedDestination = destinations.find(item => item.key === value); + const savedDestinationUnavailable = + Boolean(value) && !loading && !loadFailed && !selectedDestination; + const installationRequired = + selectedDestination !== undefined && !selectedDestination.available; + const options = destinations.map(destination => ({ + value: destination.key, + label: destination.available + ? destination.label + : t('%s (Bot installation required)', destination.label), + })); + if (value && !selectedDestination) { + options.push({ + value, + label: loading || loadFailed ? value : t('%s (Unavailable)', value), + }); + } + + return ( + <> +