aio-libs / aio-libs/aiohttp-admin

Bulk update/delete returns incomplete IDs for composite primary keys

Abierto
#1,192 1 comentario 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
Python
Estrellas
219
Forks
45
Merge medio
4 min
PR fusionados (30 d)
14

Descripción

## Summary

`updateMany` / `deleteMany` return only the **first** column of a composite primary key (e.g. `"1"`) instead of the pipe-joined ID used everywhere else (e.g. `"1|2"`).

This is a regression/gap in the compound-key support from #769, not a new feature request. Single-record endpoints already implement `"col1|col2"` correctly. Bulk endpoints do not.

## Expected

react-admin requires `updateMany` / `deleteMany` to return the same record IDs used by `getList` / `getOne` / `getMany`:

```json
{"data": ["1|2", "1|3"]}
```

That format is the contract from #769 and from `_convert_record()`:

```python
"id": "|".join(str(record[pk]) for pk in self.primary_key)
```

## Actual (reproduced on current `master`)

Against a table with PK `(id, other)` and rows `(1, 2)`, `(1, 3)`, `(4, 5)`:

| Endpoint | Status | IDs |
|---|---|---|
| `GET …/list` | 200 | `["1\|2", "1\|3", "4\|5"]` |
| `GET …/one?id=1\|2` | 200 | `"1\|2"` |
| `GET …/many?ids=["1\|2","1\|3"]` | 200 | `"1\|2"`, `"1\|3"` |
| **`PUT …/update_many?ids=["1\|2","1\|3"]`** | 200 | **`["1", "1"]`** |
| **`DELETE …?ids=["4\|5"]`** | 200 | **`["4"]`** |

The bulk **writes succeed** (incoming `"1|2"` is parsed correctly and the rows are updated/deleted). Only the **response IDs are wrong**. Two different rows both come back as `"1"`, so react-admin’s cache / selection / unselect after a bulk action will desync.

## Why this is a bug, not intentional

1. #769 defined `"first|second"` as the ID format for compound keys and was closed as implemented.
2. List/get-one/get-many already return that format; bulk is inconsistent with the same resource.
3. `_convert_ids()` is documented as converting IDs to the “correct output format”, but it does `str(i)` instead of the same `"|".join(...)` as `_convert_record()`.
4. `SAResource.update_many` / `delete_many` are typed as returning `list[tuple[Any, ...]]` (full PK tuples) but call `Result.scalars()`, which **only yields the first RETURNING column**.

## Root cause

**1. SQLAlchemy backend drops extra PK columns** (`aiohttp_admin/backends/sqlalchemy.py`):

```python
stmt = stmt.values(data).returning(*(self._table.c[pk] for pk in self.primary_key))
return list(await conn.scalars(stmt)) # first column only
```

Same pattern in `delete_many`.

**2. ID encoding does not pipe-join** (`aiohttp_admin/backends/abc.py`):

```python
def _convert_ids(self, ids: Sequence[_ID]) -> tuple[str, ...]:
return tuple(str(i) for i in ids) # "1" or "(1, 2)", never "1|2"
```

Even if (1) returned full tuples, `str((1, 2))` would be `"(1, 2)"`, not `"1|2"`.

## Test gap (no existing coverage)

| Test | What it covers | Composite PK bulk? |
|---|---|---|
| `tests/test_backends_sqlalchemy.py::test_id_nonpk` | Schema only: `primary_key == ("id", "other")` | No HTTP |
| `tests/test_backends_sqlalchemy.py::test_nonid_pk_api` | list/one/many/create/update for a **single** non-`id` PK | No |
| `tests/test_views.py::test_update_many` / `test_delete_many` | `Dummy2Model` integer PK, expects `{"data": ["1", "2"]}` | No |
| `admin-js/tests/relationships.test.js` | **Composite foreign keys**, not composite PKs | No |

That is why this slipped through: compound PK support was implemented and tested for get/create/update-one, never for bulk.

## Suggested fix

1. Return full PK tuples from `RETURNING` (`[tuple(row) for row in await conn.execute(stmt)]`) instead of `scalars()`.
2. Encode with the same helper as records: `"|".join(str(part) for part in i)`.
3. Add an API test for composite PK `update_many` / `delete_many` asserting `{"data": ["1|2", "1|3"]}`.

Single-column PKs stay `"1"` / `"2"` because `(1,)` joins to `"1"`.

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.