mikebridge commented on PR #39977: URL: https://github.com/apache/superset/pull/39977#issuecomment-4432424007
@richardfogaca thanks for the careful review — all four addressed in `c499e318fe` (or in earlier `44e1d1710b` for one of them). Per-item: **`views/filters.py:164` — broad bypass scope** — agreed, fixed. `BaseDeletedStateFilter` now applies the bypass per-query via `.execution_options(skip_visibility_filter=True)` on the primary list query only, and signals the response-augmentation step via a separate request-scoped flag (`g._augment_response_with_deleted_at`). The two concerns are decoupled. Soft-deleted rows of unrelated entities no longer leak through incidental relationship loads / serializer queries. **`commands/restore.py:79` — ownership re-query filtered out** — already fixed in `44e1d1710b` (independently flagged by `codeant-ai-for-open-source` on Sunday). `BaseRestoreCommand.validate()` now sets `g.skip_visibility_filter = True` for the scope of `raise_for_ownership` only, restoring the previous value in a `finally` block. New unit tests in `tests/unit_tests/commands/test_restore.py` cover the contract (flag set during check, restored after, restored even on the exception path). **`daos/database.py:82` — flag accepted but ignored** — agreed, fixed. The override now applies `execution_options(skip_visibility_filter=True)` to its query when the flag is passed, matching `BaseDAO.find_by_id`. **`daos/base.py:299` — positional ABI** — agreed, fixed. `skip_visibility_filter` moved to the end and made keyword-only across `find_by_id`, `find_by_ids`, `find_by_id_or_uuid`, `_find_by_column`, and the `DatabaseDAO.find_by_id` override. Existing in-tree callers all already use keyword form, so no churn there. Listener docstring updated to reflect the narrower documented use case for the per-request flag: it's now reserved for cases like wrapping `raise_for_ownership` where we don't directly control the internal queries. The list-endpoint use case has moved to the per-query option. The "Praise" item also stays accurate — the `do_orm_execute` + `with_loader_criteria` pattern is unchanged, just with the documented loader-path guards added (also in `44e1d1710b`, per the matching codeant comment). -- 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]
