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

Closed
3 tasks done
ctholho opened this issue Mar 12, 2022 · 1 comment · Fixed by #12595
Closed
3 tasks done

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

ctholho opened this issue Mar 12, 2022 · 1 comment · Fixed by #12595
Labels

Comments

@ctholho
Copy link

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

@azrikahar
Copy link
Contributor

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants