sadpandajoe opened a new pull request, #38084:
URL: https://github.com/apache/superset/pull/38084
### SUMMARY
`PUT /api/v1/report/{id}` with `{"database": N}` on a Report-type schedule
returned **500 Internal Server Error** instead of a validation error. The POST
endpoint correctly rejects this at the schema level (400), but the PUT path had
no equivalent check.
**Root cause:** `ReportSchedulePutSchema` lacks the `@validates_schema`
cross-validation that `ReportSchedulePostSchema` has
(`validate_report_references`). For PUT with partial updates, the schema can't
reliably perform this check because `type` may be absent from the payload. The
fix adds command-level validation in `UpdateReportScheduleCommand.validate()`.
**Changes:**
- Add `ReportScheduleDatabaseNotAllowedValidationError` exception class
- Add command-level validation enforcing both invariants:
- **Reports must not have a database** — rejects database in payload or on
model
- **Alerts must have a database** — rejects null/missing database in
payload and on model
- Add `allow_none=True` on `ReportSchedulePutSchema.database` to allow
clearing via `{"database": null}` (consistent with the `dashboard` field)
- Change DB existence check from truthiness to explicit `database_id is not
None`
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
**Before:**
```
PUT /api/v1/report/23 {"database": 1} → 500 Internal Server Error
```
**After:**
```
PUT /api/v1/report/23 {"database": 1} → 422 {"message": {"database":
"Database reference is not allowed on a report"}}
```
### TESTING INSTRUCTIONS
1. Create an Alert with a database and a Report without one
2. `PUT /api/v1/report/{report_id}` with `{"database": <valid_id>}` → should
return **422**
3. `PUT /api/v1/report/{alert_id}` with `{"database": null}` → should return
**422**
4. `PUT /api/v1/report/{alert_id}` with `{"type": "Report", "database":
null}` → should return **200** (valid transition)
5. `PUT /api/v1/report/{report_id}` with `{"type": "Alert"}` (no database) →
should return **422**
6. `PUT /api/v1/report/{report_id}` with `{"type": "Alert", "database":
<valid_id>}` → should return **200**
**Automated tests:**
- 7 new integration test methods covering all type/database combinations and
transitions
- 12 new unit tests for `UpdateReportScheduleCommand.validate()` covering
the full payload × model matrix
- 2 existing tests updated for new validation constraints
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
