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

Move GraphQL types into separate modules #343

Merged
merged 5 commits into from
Apr 26, 2023
Merged

Conversation

adzialocha
Copy link
Member

@adzialocha adzialocha commented Apr 26, 2023

This PR restructures the graphql::types module.

  • Separate GraphQL Objects, Input Values and Responses into new modules
  • Convert "constructors" to simple "build" methods
  • Move resolvers into separate module, make them simple methods as well
  • Rename DocumentValue to Resolved and implement downcast method on it
  • Move PaginationData to stores::query as it is not related to GraphQL
  • Rename all_documents query to collection

📋 Checklist

  • Add tests that cover your changes
  • Add this PR to the Unreleased section in CHANGELOG.md
  • Link this PR to any issues it closes
  • New files contain a SPDX license header

@codecov
Copy link

codecov bot commented Apr 26, 2023

Codecov Report

Patch coverage: 94.85% and project coverage change: +0.01 🎉

Comparison is base (e6e8e31) 90.67% compared to head (4620908) 90.68%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #343      +/-   ##
==========================================
+ Coverage   90.67%   90.68%   +0.01%     
==========================================
  Files          74       74              
  Lines        6423     6423              
==========================================
+ Hits         5824     5825       +1     
+ Misses        599      598       -1     
Impacted Files Coverage Δ
aquadoggo/src/db/stores/query.rs 94.89% <ø> (ø)
aquadoggo/src/graphql/mutations/publish.rs 100.00% <ø> (ø)
aquadoggo/src/graphql/queries/next_args.rs 97.43% <ø> (ø)
aquadoggo/src/graphql/resolvers.rs 90.76% <90.76%> (ø)
...uadoggo/src/graphql/objects/document_collection.rs 91.48% <91.48%> (ø)
aquadoggo/src/graphql/objects/document.rs 96.96% <96.96%> (ø)
...quadoggo/src/graphql/input_values/fields_filter.rs 100.00% <100.00%> (ø)
aquadoggo/src/graphql/input_values/order.rs 100.00% <100.00%> (ø)
aquadoggo/src/graphql/objects/document_fields.rs 100.00% <100.00%> (ø)
aquadoggo/src/graphql/queries/collection.rs 100.00% <100.00%> (ø)
... and 3 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@adzialocha adzialocha marked this pull request as ready for review April 26, 2023 11:28
@adzialocha
Copy link
Member Author

adzialocha commented Apr 26, 2023

During development of the low-level query backend I stumbled over not finding the methods which resolve certain parts of the GraphQL tree, so I thought it could be cool to move it all to a resolvers module.

There are more resolvers of course (for example for the totalCount field etc.) which are not in there, but I think that might be a bit too much for now? Most of the resolvers are also "only" there to downcast values.

Copy link
Member

@sandreae sandreae left a comment

Choose a reason for hiding this comment

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

Yeh, looks good to me. Actually, my first design of these module builders/reducers was in a functional style like this, but I changed it along the way cos I thought it gave a familiar api between the dynamic and static types. I didn't really think it was a big success though. I like this 👍

@sandreae
Copy link
Member

There are more resolvers of course (for example for the totalCount field etc.) which are not in there, but I think that might be a bit too much for now? Most of the resolvers are also "only" there to downcast values.

Yeh, agree it's fine like this now.

@sandreae sandreae merged commit 860e0f8 into main Apr 26, 2023
@adzialocha adzialocha deleted the rename-graphql-types branch April 26, 2023 12:19
adzialocha added a commit that referenced this pull request Apr 26, 2023
* main:
  Move GraphQL types into separate modules (#343)
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.

None yet

2 participants