tien238lnd opened a new issue, #45174:
URL: https://github.com/apache/superset/issues/45174
### Bug description
Since #37815 the Datasets list page loads its rows from the combined `GET
/api/v1/datasource/` endpoint instead of `GET /api/v1/dataset/`, and it does so
even when `SEMANTIC_LAYERS` is off. The combined endpoint does not match the
dataset endpoint in three ways, and each one has already surfaced as a separate
bug report or PR. This issue collects them in one place.
6.1.0 is not affected: its `DatasetList` still uses
`useListViewResource('dataset')`. The change is on `master` only.
**1. Filters are dropped silently**
`_parse_filters` in `superset/commands/datasource/list.py` only understands
`source_type`, `table_name`, `sql`, `database`, `semantic_layer_uuid` and,
since #43829, `schema`. Every other filter falls through without an error, so
the list comes back unfiltered while the filter chip stays lit. Still broken on
`master`:
- **Editor** (`editors`, `rel_m_m`)
- **Modified by** (`changed_by`, `rel_o_m`)
- **Certified** (`id`, `dataset_is_certified`)
Dashboards and Charts are not affected because they call their own list
endpoints.
**2. Permission gate**
`combined_list` was protected by `@protect()` on a class with
`class_permission_name = "Datasource"`, so a role with `can_read` on `Dataset`
but not on `Datasource` got a 403, which the page shows as an empty table.
#41553 mapped the method to `read`, which covers Gamma (#39947 proposed the
same and was closed). #43887 then switched it to `@protect_read("Dataset",
"SemanticView")`, which matches what `UPDATING.md` says about the endpoint. As
far as I can tell this axis is resolved on `master`; listing it so the three
can be read together.
**3. Failure mode differs**
`GET /api/v1/dataset/` answers `400` for a filter it cannot apply (`Filter
column: <col> not allowed to filter`, `Filter operation: <opr> not allowed on
column: <col>`). The combined endpoint ignores it. That is why the first axis
went unnoticed: nothing fails, the result just looks plausible.
**Steps to reproduce**
1. Have a few datasets with different editors, schemas and last-modified
users.
2. Open the Datasets list and pick an Editor, a Schema or a Modified by
value.
3. Compare with the same filter on `GET /api/v1/dataset/`:
```
GET
/api/v1/datasource/?q=(filters:!((col:editors,opr:rel_m_m,value:<subject_id>)),page:0,page_size:25)
GET
/api/v1/dataset/?q=(filters:!((col:editors,opr:rel_m_m,value:<subject_id>)),page:0,page_size:25)
```
**Expected behavior**
The list only shows the datasets that match the filter, and both endpoints
return the same rows for the same filter.
**Actual behavior**
The combined endpoint returns the whole list (`count` is the unfiltered
total; on a deployment with about 300 datasets, filtering by one editor
returned 305 rows from `/api/v1/datasource/` and 35 from `/api/v1/dataset/`).
### Related work
- #37815 introduced the combined endpoint and switched the page to it.
- #43829 (merged) fixed the Schema filter.
- #41580 (open, needs a rebase) restores schema, owner, certified and
changed_by. It matches `col == "owners"`, but the page sends `editors` since
#38831, and it compares `Subject.user_id` where the Editor dropdown sends
`Subject.id` (and editors can also be roles or groups).
- #41553 (merged), #39947 (closed) and #43887 (merged) for the permission
axis.
### Proposed fix
Two small PRs, linked below once they are open:
1. Backend: accept `editors`, `changed_by` and `id`/`dataset_is_certified`
on the combined endpoint with the same predicates as `/api/v1/dataset/`
(sharing the certification clause with `DatasetCertifiedFilter`), and reject a
column it cannot filter on with `400`, like the dataset endpoint.
2. Frontend: when `SEMANTIC_LAYERS` is off every row is a dataset, so the
page can keep using `/api/v1/dataset/`, the way `DatabaseList` and
`DatasetSelect` already gate on the flag.
### Open questions for maintainers
- Is answering `400` for an unknown filter column on `/api/v1/datasource/`
acceptable, or should it keep ignoring them? It is a small behavior change for
API callers.
- Should the older columns (`table_name`, `sql`, `schema`) also reject a
wrong operator? #43829's tests pin the current "ignore" behavior, so I left
them alone.
- A semantic view has no schema, editors or certification, so those filters
exclude semantic views, while `changed_by` applies to both and `certified =
false` keeps semantic views. Is that the intended reading of the filters?
### Screenshots/recordings
_No response_
### Superset version
master / latest-dev
### Python version
3.11
### Node version
18 or greater
### Browser
Chrome
--
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]
