The Wayback Machine - https://web.archive.org/web/20220325073938/https://github.com/directus/directus/issues/12128
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDK items.readOne(id) behaves like items.readMany() when id is empty string #12128

Open
3 tasks done
ctholho opened this issue Mar 12, 2022 · 1 comment
Open
3 tasks done
Labels
Bug Good First Issue Package: sdk

Comments

@ctholho
Copy link

@ctholho ctholho commented Mar 12, 2022

Preflight Checklist

Describe the Bug

If you call directus.items('my_collection').readOne(id) and id happens to be an empty string, the request will return an array of objects matching directus.items('my_collection').readMany().

Either the typing of readOne is not correct or calling readOne with an empty string should fail.

interface IItems<T extends Item> {
    ...
    readOne(id: ID, query?: QueryOne<T>): Promise<OneItem<T>>;
    readMany(query?: QueryMany<T>): Promise<ManyItems<T>>;

If you ask me, I'd like readOne to always behave the same and not fall back to readMany behavior. But that's a breaking change and would require a guard in readOne which is a high cost just for the sake of consistency. If we were to rely on typescript we could extend IItems to have the appropriate return types for readOne.

At the very least, I propose to create a PR for the docs at https://docs.directus.io/reference/sdk/#read-single-item. This behavior should be documented somewhere. But I'm waiting for your judgment on how readOne should behave.

To Reproduce

Use the directus sdk and call directus.items('my_collection').readOne(''). The http request will go to <directus-location>/items/rh_members/ and return an array.

Errors Shown

No response

What version of Directus are you using?

9.6.0

What version of Node.js are you using?

16.14.0

What database are you using?

Postgres 14

What browser are you using?

chrome

What operating system are you using?

macos

How are you deploying Directus?

docker

@ctholho ctholho added the Bug (Potential) label Mar 12, 2022
@azrikahar
Copy link
Contributor

@azrikahar azrikahar commented Mar 21, 2022

I believe this is due to the way the URL is constructed here:

async readOne(id: ID, query?: QueryOne<T>): Promise<OneItem<T>> {
const response = await this.transport.get<T>(`${this.endpoint}/${encodeURI(id as string)}`, {
params: query,
});

Since you are passing an empty string, it is indeed doing /items/my_collection/ + '', which became the readMany endpoint /items/my_collection in the end. Thanks for bringing this to our attention! Just to answer your question, this is most definitely not an intended behaviour.

@azrikahar azrikahar added Package: sdk Good First Issue and removed Bug (Potential) labels Mar 21, 2022
@rijkvanzanten rijkvanzanten added Improvement Bug and removed Improvement labels Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Good First Issue Package: sdk
3 participants