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

fix: DataType.prototype.getByVersionId could return wrong type #963

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

EvanHahn
Copy link
Contributor

We have many DataTypes backed by the same DataStore. For example, observations and tracks are both in the "data" store.

DataType.prototype.getByVersionId would pull documents from the store without checking the type, which could cause issues. Simple code example:

const track = await trackDataType.create({ /* ... */ })

await observationDataType.getByVersionId(track.versionId)
// => the track, unexpectedly!

Now, it throws a NotFoundError.

Base automatically changed from getByVersionId-test to main November 14, 2024 19:00
@EvanHahn EvanHahn force-pushed the fix-getByVersionId-returning-from-same-store branch from cb87765 to b5a797a Compare November 14, 2024 19:00
@EvanHahn EvanHahn requested a review from gmaclennan November 14, 2024 19:43
We have many `DataType`s backed by the same `DataStore`. For example,
observations and tracks are both in the "data" store.

`DataType.prototype.getByVersionId` would pull documents from the store
without checking the type, which could cause issues. Simple code
example:

    const track = await trackDataType.create({ /* ... */ })

    await observationDataType.getByVersionId(track.versionId)
    // => the track, unexpectedly!

Now, it throws a `NotFoundError`.
@EvanHahn EvanHahn force-pushed the fix-getByVersionId-returning-from-same-store branch from 3b35196 to 1c7b7cd Compare December 12, 2024 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant