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 <ReferenceArrayInput> should forward queryOptions to getMany #9954

Merged
merged 4 commits into from
Jun 27, 2024

Conversation

bendenoz
Copy link

fix for #9953

@bendenoz bendenoz marked this pull request as ready for review June 25, 2024 15:00
@bendenoz bendenoz changed the title [WIP] Forward query options to getMany [RFR] Forward query options to getMany Jun 25, 2024
@slax57 slax57 added RFR Ready For Review v4 labels Jun 25, 2024
@slax57 slax57 added this to the 4.16.20 milestone Jun 25, 2024
@slax57 slax57 changed the title [RFR] Forward query options to getMany Fix <ReferenceArrayInput> should forward queryOptions to getMany Jun 25, 2024
Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR 🙂

Unfortunately this seems to throw a TS error at build time. Can you check?

@bendenoz
Copy link
Author

I rebased on 4.x, is it better?

@slax57
Copy link
Contributor

slax57 commented Jun 27, 2024

Thanks for the rebase.
Unfortunately the issue remains.
This is due to the queryOptions being typed as options for the getList query, which is not compatible with the getManyAggregate query.

It seems I do not have push permissions on your branch, so I published the required change here: d7ed98e
This commit makes it so that the queryOptions are typed for getList query or getManyAggregate query.

Can you apply it to your branch?

Thanks.

@bendenoz
Copy link
Author

cherry-picked and pushed

@slax57
Copy link
Contributor

slax57 commented Jun 27, 2024

Thanks! 🙂

I'm sorry I didn't realize this before, but it would be nice to also update the documentation to specify that the queryOptions are passed to both getList and getMany now.

Can you update it? 🙏

@bendenoz
Copy link
Author

ok :) see latest commit, hopefully it' only in one place?

Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I think we're good 🙂 (apart from this last suggestion)

docs/ReferenceArrayInput.md Outdated Show resolved Hide resolved
Co-authored-by: Jean-Baptiste Kaiser <[email protected]>
Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@slax57 slax57 merged commit 4b3dff0 into marmelab:4.x Jun 27, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review v4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants