codeant-ai-for-open-source[bot] commented on code in PR #38847:
URL: https://github.com/apache/superset/pull/38847#discussion_r3005476817
##########
superset/reports/schemas.py:
##########
@@ -305,6 +305,19 @@ def validate_report_references( # pylint:
disable=unused-argument
)
+class ReportScheduleSubscribeSchema(ReportSchedulePostSchema):
+ """Schema for creating a chart/dashboard subscription.
+
+ ``recipients`` and ``creation_method`` are excluded — both are set
+ server-side: recipients are locked to the authenticated user's email,
+ and creation_method is derived from the presence of ``chart`` or
+ ``dashboard`` in the payload.
+ """
+
+ class Meta:
+ exclude = ("recipients", "creation_method")
Review Comment:
**Suggestion:** As currently written, the subscription schema excludes
`recipients` and `creation_method` but leaves the default `unknown=RAISE`, so
if a client includes these fields (as the testing instructions explicitly
allow), marshmallow will treat them as unknown and raise a validation error
instead of silently discarding them and letting server-side logic override the
values. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ POST /api/v1/report/subscribe 400s if recipients included.
- ⚠️ Clients reusing /report payloads break on subscribe.
- ⚠️ Behavior contradicts subscribe endpoint description comments.
```
</details>
```suggestion
unknown = EXCLUDE
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start a Superset instance with this PR code, ensuring ALERT_REPORTS
feature is enabled
so the `ReportScheduleRestApi` is registered (see
`superset/reports/api.py:67-82`).
2. Send an HTTP `POST` request to `/api/v1/report/subscribe` with JSON body
that includes
standard required fields plus a `recipients` field, for example:
`{"type": "Report", "chart": 123, "name": "Test", "crontab": "0 0 * * *",
"timezone":
"UTC", "recipients": [{"type": "Email", "recipient_config_json":
{"target":
"[email protected]"}}]}`.
3. The request reaches `ReportScheduleRestApi.subscribe` in
`superset/reports/api.py:59-105`, which calls
`self.subscribe_schema.load(request.json)`
at `superset/reports/api.py:102`.
4. `self.subscribe_schema` is an instance of `ReportScheduleSubscribeSchema`
defined in
`superset/reports/api.py:195-197` and implemented in
`superset/reports/schemas.py:308-318`; its `Meta` only sets `exclude =
("recipients",
"creation_method")` and does not set `unknown`, so Marshmallow (default
`unknown=RAISE`)
treats incoming `recipients` and `creation_method` keys as unknown fields
(because they
are excluded), raising a `ValidationError` that is caught in `subscribe` and
returned as a
`400` response (`superset/reports/api.py:103-105`) instead of silently
discarding those
fields and proceeding to set recipients and `creation_method` server-side as
described in
the endpoint docstring (`superset/reports/api.py:65-73`).
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/reports/schemas.py
**Line:** 318:318
**Comment:**
*Logic Error: As currently written, the subscription schema excludes
`recipients` and `creation_method` but leaves the default `unknown=RAISE`, so
if a client includes these fields (as the testing instructions explicitly
allow), marshmallow will treat them as unknown and raise a validation error
instead of silently discarding them and letting server-side logic override the
values.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38847&comment_hash=7ad771cc33fda703f2d224b7e474afdc7ab90881374e5e5e7e8ecad2b3e29f37&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38847&comment_hash=7ad771cc33fda703f2d224b7e474afdc7ab90881374e5e5e7e8ecad2b3e29f37&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
