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

docs: enhance REST API OpenAPI spec with nested filter example #7366

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

LucasZapico
Copy link
Contributor

Associated issue: Enhance REST API documentation for querying on nested objects [#7365]

Context

Description: The current OpenAPI spec does not call out the preferred method for filtering by nested object properties.

Proposed Change:
Add abstract for to filter on nested object properties
Add example for filter on nested object properties

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This pull request enhances the REST API OpenAPI specification by adding documentation and examples for filtering on nested object properties.

  • Added description for filtering nested objects in computeFilterParameters function in packages/twenty-server/src/engine/core-modules/open-api/utils/parameters.utils.ts
  • Included new example simpleNested for filtering on nested properties in parameters.utils.ts
  • Updated corresponding test file packages/twenty-server/src/engine/core-modules/open-api/utils/__tests__/parameters.utils.spec.ts to include tests for new nested filter examples
  • Improves developer experience by providing clearer documentation on using nested filters in API queries

2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@FelixMalfait
Copy link
Member

Thanks, great to improve the doc!
I think we don't support proper "nest filtering" in the sense that you can't yet filter by objects like people.company = 'Google' (yet, will come) ; but you can filter on composite fields (e.g. full name, emails, links). Would you mind editing to reflect that wording?

Copy link
Contributor

@martmull martmull left a comment

Choose a reason for hiding this comment

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

Lgtm thank you

@martmull martmull merged commit 8162786 into twentyhq:main Oct 10, 2024
5 of 6 checks passed
Copy link

Thanks @LucasZapico for your contribution!
This marks your 2nd PR on the repo. You're top 18% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

Contributions

harshit078 pushed a commit to harshit078/twenty that referenced this pull request Oct 14, 2024
@LucasZapico
Copy link
Contributor Author

Hey I just want to bump this a bit. I saw your response @FelixMalfait and I wanted to do some further exploration on my end.

I want to do some discovery on the viability of setting up playwright api tests tied into the openAPi tool you all are using to create spec, test, parity in a "clean" way. This way the docs, and api are exactly what is expected. We'll see where it goes. Cheers

No promises but I just wanted to let you know I haven't forgotten.

@FelixMalfait
Copy link
Member

Great thanks @LucasZapico! I know we have been working on end-to-end tests for the server but I didn't follow this at all #6923 / #6717 ; cc @charlesBochet

Also as mentioned above I think it's misleading to talk about nested object when we can't filter by nested object @martmull

@martmull
Copy link
Contributor

martmull commented Oct 17, 2024

@FelixMalfait I merged, then I saw your comment. Indeed, we should improve the wording
To filter on composite type fields use **field.subField[COMPARATOR]:value_1
intead of
To filter on nested objects use **field.nestedField[COMPARATOR]:value_1

@martmull
Copy link
Contributor

@FelixMalfait #7783

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

Successfully merging this pull request may close these issues.

4 participants