codeant-ai-for-open-source[bot] commented on code in PR #38715:
URL: https://github.com/apache/superset/pull/38715#discussion_r2959328125


##########
superset/commands/report/base.py:
##########
@@ -80,6 +83,145 @@ def validate_chart_dashboard(
         elif not update:
             exceptions.append(ReportScheduleEitherChartOrDashboardError())
 
+    def _validate_report_extra(self, exceptions: list[ValidationError]) -> 
None:
+        extra: Optional[ReportScheduleExtra] = self._properties.get("extra")
+        dashboard = self._properties.get("dashboard")
+
+        # On PUT requests, dashboard may not be in the payload — fall back to 
the model
+        if dashboard is None:
+            model = getattr(self, "_model", None)
+            dashboard = getattr(model, "dashboard", None)
+
+        if extra is None or dashboard is None:
+            return
+
+        dashboard_state = extra.get("dashboard")
+        if not dashboard_state:
+            return
+
+        if not isinstance(dashboard_state, dict):
+            exceptions.append(
+                ValidationError(
+                    _("extra.dashboard must be an object"),
+                    "extra",
+                )
+            )
+            return
+
+        position_data = json.loads(dashboard.position_json or "{}")
+        active_tabs = dashboard_state.get("activeTabs") or []
+        invalid_tab_ids = set(active_tabs) - set(position_data.keys())
+
+        if anchor := dashboard_state.get("anchor"):
+            try:
+                anchor_list: list[str] = json.loads(anchor)
+                if _invalid_tab_ids := set(anchor_list) - 
set(position_data.keys()):
+                    invalid_tab_ids.update(_invalid_tab_ids)
+            except json.JSONDecodeError:
+                if anchor not in position_data:
+                    invalid_tab_ids.add(anchor)
+
+        if invalid_tab_ids:
+            exceptions.append(
+                ValidationError(
+                    _("Invalid tab ids: %(tab_ids)s", 
tab_ids=str(invalid_tab_ids)),
+                    "extra",
+                )
+            )
+
+        self._validate_native_filters(dashboard, dashboard_state, exceptions)
+
+    def _validate_native_filters(
+        self,
+        dashboard: Any,
+        dashboard_state: Any,
+        exceptions: list[ValidationError],
+    ) -> None:
+        native_filters = dashboard_state.get("nativeFilters")
+        if not native_filters:
+            return
+
+        if not isinstance(native_filters, list):
+            exceptions.append(
+                ValidationError(
+                    _("nativeFilters must be a list"),
+                    "extra",
+                )
+            )
+            return
+
+        required_keys = {"nativeFilterId", "filterType", "columnName", 
"filterValues"}
+        valid_filter_ids: set[str] | None = None
+
+        for idx, native_filter in enumerate(native_filters):
+            if not isinstance(native_filter, dict):
+                exceptions.append(
+                    ValidationError(
+                        _("nativeFilters[%(idx)s] must be an object", idx=idx),
+                        "extra",
+                    )
+                )
+                continue
+
+            missing_keys = required_keys - set(native_filter.keys())
+            if missing_keys:
+                exceptions.append(
+                    ValidationError(
+                        _(
+                            "nativeFilters[%(idx)s] missing required keys: 
%(keys)s",
+                            idx=idx,
+                            keys=", ".join(sorted(missing_keys)),
+                        ),
+                        "extra",
+                    )
+                )
+                continue
+
+            if not isinstance(native_filter["filterValues"], list):
+                exceptions.append(
+                    ValidationError(
+                        _(
+                            "nativeFilters[%(idx)s].filterValues must be a 
list",
+                            idx=idx,
+                        ),
+                        "extra",
+                    )
+                )
+                continue
+
+            filter_id = native_filter["nativeFilterId"]
+            if not isinstance(filter_id, str) or not filter_id:
+                exceptions.append(
+                    ValidationError(
+                        _(
+                            "nativeFilters[%(idx)s].nativeFilterId"
+                            " must be a non-empty string",
+                            idx=idx,
+                        ),
+                        "extra",
+                    )
+                )
+                continue
+            if valid_filter_ids is None:
+                json_metadata = json.loads(dashboard.json_metadata or "{}")
+                valid_filter_ids = {
+                    f["id"]
+                    for f in json_metadata.get("native_filter_configuration", 
[])
+                    if "id" in f

Review Comment:
   **Suggestion:** `native_filter_configuration` is allowed to be `null` in 
dashboard metadata, but iterating directly over 
`json_metadata.get("native_filter_configuration", [])` will raise `TypeError` 
when it is `None`. Coerce it to an empty list before iterating. [possible bug]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Report create/update can fail with server exception.
   - ⚠️ Native filter validation breaks on nullable dashboard metadata.
   ```
   </details>
   
   ```suggestion
                       for f in 
(json_metadata.get("native_filter_configuration") or [])
                       if isinstance(f, dict) and "id" in f
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Store dashboard metadata with `"native_filter_configuration": null`; this 
is
   schema-valid because `DashboardJSONMetadataSchema` sets `allow_none=True`
   (`superset/dashboards/schemas.py:142`).
   
   2. Create or update report via `/api/v1/report/` or `/api/v1/report/<id>`
   (`superset/reports/api.py`), with `extra.dashboard.nativeFilters` present.
   
   3. Validation enters `_validate_native_filters()`
   (`superset/commands/report/base.py:134-224`) from create/update validate 
flows
   (`create.py:140-141`, `update.py:133-134`).
   
   4. Set comprehension iterates 
`json_metadata.get("native_filter_configuration", [])`
   (`base.py:209`); when value is `None`, iteration raises `TypeError`, causing 
request
   failure instead of clean 422 validation.
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset/commands/report/base.py
   **Line:** 209:210
   **Comment:**
        *Possible Bug: `native_filter_configuration` is allowed to be `null` in 
dashboard metadata, but iterating directly over 
`json_metadata.get("native_filter_configuration", [])` will raise `TypeError` 
when it is `None`. Coerce it to an empty list before iterating.
   
   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%2F38715&comment_hash=dc9439161fbb8d4970d5815556f65dadf38ab03a989fcc3933649f7b153ec205&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38715&comment_hash=dc9439161fbb8d4970d5815556f65dadf38ab03a989fcc3933649f7b153ec205&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]

Reply via email to