Skip to content

Commit c9977b1

Browse files
committed
feat: support SMTP, default SMTP and in-platform notification channels
`CatalogSmtp` and `CatalogDefaultSmtp` had been sitting commented out with a TODO blaming the generated client: # TODO: there is an issue with generated client which causes these two # classes to fail. type in declarative_notification_channel_destination.py # contains only WEBHOOK as valid value That was the oneOf-flattening defect fixed in the previous commit, so `destination` could only ever be a webhook. With the generated composed model now accepting every member, add the missing destinations and widen the union: * CatalogSmtp - custom SMTP server * CatalogDefaultSmtp - the platform's own mail server * CatalogInPlatform - in-platform notifications `destination` becomes `CatalogNotificationChannelDestination`, the union the commented-out code intended. Also drop the runtime monkeypatch that re-populated `NotificationChannelDestination.allowed_values[("type",)]`. It treated one symptom of the same defect from outside the generated client; the template fix covers every collapsed composed model, so patching class attributes at import time is no longer needed. Reading a channel needs an explicit `from_api` rather than cattrs: the four destination classes have no uniquely-required field to disambiguate a union on (IN_PLATFORM carries nothing but its type), so dispatch on `type` the way `_provider_config_from_api` does for LLM providers. It keeps only the fields each class declares, because the API sends some we do not model - reading a webhook returns `has_secret_key` - and dropping those matches what cattrs did before and keeps reads working when the API grows a field.
1 parent f63c411 commit c9977b1

3 files changed

Lines changed: 241 additions & 40 deletions

File tree

packages/gooddata-sdk/src/gooddata_sdk/__init__.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,10 @@
140140
)
141141
from gooddata_sdk.catalog.organization.layout.notification_channel import (
142142
CatalogDeclarativeNotificationChannel,
143+
CatalogDefaultSmtp,
144+
CatalogInPlatform,
145+
CatalogNotificationChannelDestination,
146+
CatalogSmtp,
143147
CatalogWebhook,
144148
)
145149
from gooddata_sdk.catalog.organization.service import (
Lines changed: 104 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,53 +1,25 @@
11
# (C) 2024 GoodData Corporation
2+
from __future__ import annotations
3+
24
import builtins
5+
from typing import Any, Union
36

47
from attrs import define, field
8+
from attrs import fields as attrs_fields
59
from gooddata_api_client.model.declarative_notification_channel import DeclarativeNotificationChannel
6-
from gooddata_api_client.model.notification_channel_destination import NotificationChannelDestination
10+
from gooddata_api_client.model.default_smtp import DefaultSmtp
11+
from gooddata_api_client.model.in_platform import InPlatform
12+
from gooddata_api_client.model.smtp import Smtp
713
from gooddata_api_client.model.webhook import Webhook
814

915
from gooddata_sdk.catalog.base import Base
10-
11-
# The generator collapses the `type` enums of NotificationChannelDestination's oneOf children
12-
# into the last child's single value (IN_PLATFORM), so valid destinations of the other types
13-
# fail client-side validation on both serialization and response parsing. Restore the full set.
14-
NotificationChannelDestination.allowed_values[("type",)].update(
15-
{
16-
"WEBHOOK": "WEBHOOK",
17-
"SMTP": "SMTP",
18-
"DEFAULT_SMTP": "DEFAULT_SMTP",
19-
"IN_PLATFORM": "IN_PLATFORM",
20-
}
21-
)
22-
23-
# TODO: there is an issue with generated client which causes these two classes to fail
24-
# type in gooddata_api_client/model/declarative_notification_channel_destination.py contains only WEBHOOK as valid value
25-
# @define(kw_only=True)
26-
# class CatalogDefaultSmtp(Base):
27-
# from_email: Optional[str] = None
28-
# from_email_name: Optional[str] = None
29-
#
30-
# @staticmethod
31-
# def client_class() -> Type[DefaultSmtp]:
32-
# return DefaultSmtp
33-
#
34-
#
35-
# @define(kw_only=True)
36-
# class CatalogSmtp(Base):
37-
# from_email: Optional[str] = None
38-
# from_email_name: Optional[str] = None
39-
# host: Optional[str] = None
40-
# password: Optional[str] = None
41-
# port: Optional[int] = None
42-
# username: Optional[str] = None
43-
#
44-
# @staticmethod
45-
# def client_class() -> Type[Smtp]:
46-
# return Smtp
16+
from gooddata_sdk.utils import safeget
4717

4818

4919
@define(kw_only=True)
5020
class CatalogWebhook(Base):
21+
"""Webhook destination for notifications."""
22+
5123
type: str = field(default="WEBHOOK", init=False)
5224
url: str
5325
token: str | None = field(default=None, eq=False)
@@ -58,6 +30,85 @@ def client_class() -> builtins.type[Webhook]:
5830
return Webhook
5931

6032

33+
@define(kw_only=True)
34+
class CatalogSmtp(Base):
35+
"""Custom SMTP destination for notifications.
36+
37+
`host`, `port`, `username` and `password` are required by the API on create
38+
and update, but are optional here because reads never return the password.
39+
"""
40+
41+
type: str = field(default="SMTP", init=False)
42+
from_email: str | None = None
43+
from_email_name: str | None = None
44+
host: str | None = None
45+
port: int | None = None
46+
username: str | None = None
47+
password: str | None = field(default=None, eq=False)
48+
49+
@staticmethod
50+
def client_class() -> builtins.type[Smtp]:
51+
return Smtp
52+
53+
54+
@define(kw_only=True)
55+
class CatalogDefaultSmtp(Base):
56+
"""Default SMTP destination for notifications - the platform's own mail server."""
57+
58+
type: str = field(default="DEFAULT_SMTP", init=False)
59+
from_email: str | None = None
60+
from_email_name: str | None = None
61+
62+
@staticmethod
63+
def client_class() -> builtins.type[DefaultSmtp]:
64+
return DefaultSmtp
65+
66+
67+
@define(kw_only=True)
68+
class CatalogInPlatform(Base):
69+
"""In-platform destination for notifications."""
70+
71+
type: str = field(default="IN_PLATFORM", init=False)
72+
73+
@staticmethod
74+
def client_class() -> builtins.type[InPlatform]:
75+
return InPlatform
76+
77+
78+
CatalogNotificationChannelDestination = Union[
79+
CatalogWebhook,
80+
CatalogSmtp,
81+
CatalogDefaultSmtp,
82+
CatalogInPlatform,
83+
]
84+
85+
_DESTINATION_BY_TYPE: dict[str, builtins.type[CatalogNotificationChannelDestination]] = {
86+
"WEBHOOK": CatalogWebhook,
87+
"SMTP": CatalogSmtp,
88+
"DEFAULT_SMTP": CatalogDefaultSmtp,
89+
"IN_PLATFORM": CatalogInPlatform,
90+
}
91+
92+
93+
def _destination_from_api(data: dict[str, Any]) -> CatalogNotificationChannelDestination:
94+
"""Build the right destination class from a `destination` payload.
95+
96+
Dispatched on `type` explicitly rather than left to cattrs: the four
97+
destination classes have no uniquely-required field to disambiguate a
98+
union on (`IN_PLATFORM` carries nothing but its type).
99+
"""
100+
destination_type = safeget(data, ["type"])
101+
destination_class = _DESTINATION_BY_TYPE.get(destination_type)
102+
if destination_class is None:
103+
raise ValueError(f"Unknown notification channel destination type: {destination_type}")
104+
# Keep only what the class can take: `type` is init=False (implied by the
105+
# class itself) and the API sends fields we do not model - `has_secret_key`
106+
# on a webhook, for one. Silently ignoring those matches how cattrs used to
107+
# structure this and keeps reads working when the API grows a field.
108+
accepted = {f.name for f in attrs_fields(destination_class) if f.init}
109+
return destination_class(**{k: v for k, v in data.items() if k in accepted})
110+
111+
61112
@define(kw_only=True)
62113
class CatalogDeclarativeNotificationChannel(Base):
63114
id: str
@@ -66,9 +117,22 @@ class CatalogDeclarativeNotificationChannel(Base):
66117
destination_type: str | None = None
67118
custom_dashboard_url: str | None = None
68119
allowed_recipients: str | None = None
69-
# destination: Optional[Union[CatalogDefaultSmtp, CatalogSmtp, CatalogWebhook]] = None
70-
destination: CatalogWebhook | None = None
120+
destination: CatalogNotificationChannelDestination | None = None
71121

72122
@staticmethod
73123
def client_class() -> builtins.type[DeclarativeNotificationChannel]:
74124
return DeclarativeNotificationChannel
125+
126+
@classmethod
127+
def from_api(cls, entity: dict[str, Any]) -> CatalogDeclarativeNotificationChannel:
128+
data = entity if isinstance(entity, dict) else entity.to_dict(camel_case=False)
129+
raw_destination = safeget(data, ["destination"])
130+
return cls(
131+
id=data["id"],
132+
name=safeget(data, ["name"]),
133+
description=safeget(data, ["description"]),
134+
destination_type=safeget(data, ["destination_type"]),
135+
custom_dashboard_url=safeget(data, ["custom_dashboard_url"]),
136+
allowed_recipients=safeget(data, ["allowed_recipients"]),
137+
destination=_destination_from_api(raw_destination) if raw_destination is not None else None,
138+
)
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
# (C) 2026 GoodData Corporation
2+
"""Tests for the notification channel destination union.
3+
4+
`CatalogSmtp` / `CatalogDefaultSmtp` used to be commented out in the SDK with a
5+
TODO pointing at the generated client: the composed destination model accepted a
6+
single `type` only, so `to_api()` raised `ApiValueError` for every other
7+
destination. That was the same oneOf-flattening defect covered by
8+
`test_composed_oneof_unions.py`; with it fixed in `model_utils`, all four
9+
destinations are usable and `NotificationChannelDestination.allowed_values` no
10+
longer has to be monkeypatched at import time.
11+
"""
12+
13+
from __future__ import annotations
14+
15+
import pytest
16+
from gooddata_api_client.model.json_api_notification_channel_in_attributes_destination import (
17+
JsonApiNotificationChannelInAttributesDestination,
18+
)
19+
from gooddata_api_client.model.notification_channel_destination import NotificationChannelDestination
20+
from gooddata_sdk import (
21+
CatalogDeclarativeNotificationChannel,
22+
CatalogDefaultSmtp,
23+
CatalogInPlatform,
24+
CatalogSmtp,
25+
CatalogWebhook,
26+
)
27+
28+
DESTINATIONS = {
29+
"WEBHOOK": (
30+
CatalogWebhook(url="https://webhook.site/hook", token="secret"),
31+
{"type": "WEBHOOK", "url": "https://webhook.site/hook", "token": "secret"},
32+
),
33+
"SMTP": (
34+
CatalogSmtp(
35+
from_email="sender@example.com",
36+
from_email_name="Sender",
37+
host="smtp.example.com",
38+
port=587,
39+
username="user",
40+
password="secret",
41+
),
42+
{
43+
"type": "SMTP",
44+
"from_email": "sender@example.com",
45+
"from_email_name": "Sender",
46+
"host": "smtp.example.com",
47+
"port": 587,
48+
"username": "user",
49+
"password": "secret",
50+
},
51+
),
52+
"DEFAULT_SMTP": (
53+
CatalogDefaultSmtp(from_email="sender@example.com", from_email_name="Sender"),
54+
{"type": "DEFAULT_SMTP", "from_email": "sender@example.com", "from_email_name": "Sender"},
55+
),
56+
"IN_PLATFORM": (CatalogInPlatform(), {"type": "IN_PLATFORM"}),
57+
}
58+
59+
60+
@pytest.mark.parametrize("destination_type", sorted(DESTINATIONS))
61+
def test_destination_to_api(destination_type: str) -> None:
62+
"""Every destination must survive `to_api()`, not just WEBHOOK."""
63+
destination, expected = DESTINATIONS[destination_type]
64+
65+
assert destination.to_api().to_dict() == expected
66+
67+
68+
@pytest.mark.parametrize("destination_type", sorted(DESTINATIONS))
69+
def test_notification_channel_to_api_carries_the_destination(destination_type: str) -> None:
70+
destination, expected = DESTINATIONS[destination_type]
71+
channel = CatalogDeclarativeNotificationChannel(
72+
id=f"channel-{destination_type.lower()}",
73+
name=f"Channel {destination_type}",
74+
destination=destination,
75+
allowed_recipients="CREATOR",
76+
)
77+
78+
api_object = channel.to_api()
79+
80+
assert api_object.to_dict()["destination"] == expected
81+
82+
83+
@pytest.mark.parametrize("destination_type", sorted(DESTINATIONS))
84+
def test_notification_channel_round_trips(destination_type: str) -> None:
85+
"""`from_api` must rebuild the concrete destination class, not a webhook."""
86+
destination, _ = DESTINATIONS[destination_type]
87+
channel = CatalogDeclarativeNotificationChannel(
88+
id=f"channel-{destination_type.lower()}",
89+
name=f"Channel {destination_type}",
90+
destination=destination,
91+
custom_dashboard_url="https://dashboard.site",
92+
allowed_recipients="CREATOR",
93+
)
94+
95+
restored = CatalogDeclarativeNotificationChannel.from_api(channel.to_dict(camel_case=False))
96+
97+
assert restored == channel
98+
assert type(restored.destination) is type(destination)
99+
assert restored.destination is not None
100+
assert restored.destination.type == destination_type
101+
102+
103+
def test_from_api_rejects_an_unknown_destination_type() -> None:
104+
with pytest.raises(ValueError, match="Unknown notification channel destination type: TELEPATHY"):
105+
CatalogDeclarativeNotificationChannel.from_api({"id": "channel", "destination": {"type": "TELEPATHY"}})
106+
107+
108+
@pytest.mark.parametrize(
109+
"model",
110+
[NotificationChannelDestination, JsonApiNotificationChannelInAttributesDestination],
111+
ids=["shared", "json_api"],
112+
)
113+
@pytest.mark.parametrize("destination_type", sorted(DESTINATIONS))
114+
def test_generated_destination_models_accept_every_type(model, destination_type: str) -> None:
115+
"""Pin the generated composed models too - this is where the defect lived."""
116+
_, payload = DESTINATIONS[destination_type]
117+
118+
destination = model(**payload)
119+
120+
assert destination.type == destination_type
121+
122+
123+
def test_destination_model_class_attribute_is_still_collapsed() -> None:
124+
"""Guard the reason the monkeypatch could go: the fix is at validation time.
125+
126+
The generator still writes a single-variant enum onto the composed parent -
127+
nothing regenerates that away - so this documents that the models above pass
128+
because `model_utils` unions the members, not because the class attribute
129+
got fixed.
130+
"""
131+
collapsed = NotificationChannelDestination.allowed_values.get(("type",), {})
132+
133+
assert len(collapsed) == 1

0 commit comments

Comments
 (0)