mirror of
https://github.com/apache/superset.git
synced 2026-04-25 19:14:27 +00:00
refactor(reports): deduplicate database check and add precedence integration test
Extract repeated effective-database logic into a single computation in UpdateReportScheduleCommand.validate(). Add integration test asserting Report + nonexistent DB returns "not allowed" (not "does not exist") to lock API contract precedence. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -100,23 +100,19 @@ class UpdateReportScheduleCommand(UpdateMixin, BaseReportScheduleCommand):
|
||||
)
|
||||
)
|
||||
|
||||
# Determine effective database state (payload overrides model)
|
||||
if "database" in self._properties:
|
||||
has_database = self._properties["database"] is not None
|
||||
else:
|
||||
has_database = self._model.database_id is not None
|
||||
|
||||
# Validate database is not allowed on Report type
|
||||
if report_type == ReportScheduleType.REPORT:
|
||||
if "database" in self._properties:
|
||||
has_database = self._properties["database"] is not None
|
||||
else:
|
||||
has_database = self._model.database_id is not None
|
||||
if has_database:
|
||||
exceptions.append(ReportScheduleDatabaseNotAllowedValidationError())
|
||||
if report_type == ReportScheduleType.REPORT and has_database:
|
||||
exceptions.append(ReportScheduleDatabaseNotAllowedValidationError())
|
||||
|
||||
# Validate Alert has a database
|
||||
if report_type == ReportScheduleType.ALERT:
|
||||
if "database" in self._properties:
|
||||
has_database = self._properties["database"] is not None
|
||||
else:
|
||||
has_database = self._model.database_id is not None
|
||||
if not has_database:
|
||||
exceptions.append(ReportScheduleAlertRequiredDatabaseValidationError())
|
||||
if report_type == ReportScheduleType.ALERT and not has_database:
|
||||
exceptions.append(ReportScheduleAlertRequiredDatabaseValidationError())
|
||||
|
||||
# Validate if DB exists (for alerts)
|
||||
if report_type == ReportScheduleType.ALERT and database_id is not None:
|
||||
|
||||
@@ -1654,6 +1654,38 @@ class TestReportSchedulesApi(SupersetTestCase):
|
||||
"message": {"database": "Database reference is not allowed on a report"}
|
||||
}
|
||||
|
||||
@pytest.mark.usefixtures("create_report_schedules")
|
||||
def test_update_report_schedule_nonexistent_database_returns_not_allowed(self):
|
||||
"""
|
||||
ReportSchedule API: Test Report + nonexistent DB returns 'not allowed',
|
||||
not 'does not exist' — type invariant takes precedence.
|
||||
"""
|
||||
self.login(ADMIN_USERNAME)
|
||||
|
||||
report_schedule = (
|
||||
db.session.query(ReportSchedule)
|
||||
.filter(ReportSchedule.name == "name1")
|
||||
.one_or_none()
|
||||
)
|
||||
uri = f"api/v1/report/{report_schedule.id}"
|
||||
|
||||
# Transition to Report type first
|
||||
rv = self.put_assert_metric(
|
||||
uri,
|
||||
{"type": ReportScheduleType.REPORT, "database": None},
|
||||
"put",
|
||||
)
|
||||
assert rv.status_code == 200
|
||||
|
||||
# Report + nonexistent DB → 422 "not allowed" (not "does not exist")
|
||||
database_max_id = db.session.query(func.max(Database.id)).scalar()
|
||||
rv = self.put_assert_metric(uri, {"database": database_max_id + 1}, "put")
|
||||
assert rv.status_code == 422
|
||||
data = json.loads(rv.data.decode("utf-8"))
|
||||
assert data == {
|
||||
"message": {"database": "Database reference is not allowed on a report"}
|
||||
}
|
||||
|
||||
@pytest.mark.usefixtures("create_report_schedules")
|
||||
def test_update_alert_schedule_database_allowed(self):
|
||||
"""
|
||||
|
||||
Reference in New Issue
Block a user