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

SchemaDownlader: Update to download deprecated input fields #4678

Merged
merged 2 commits into from
Feb 20, 2023

Conversation

ndwhelan
Copy link
Contributor

@ndwhelan ndwhelan commented Feb 9, 2023

Added (includeDeprecated: true) as a params to inputFields in the fragment portion of the GraphQL query to download a schema.

Current Behavior

When the downloadApolloSchema Gradle task is run, input fields with the @deprecated tag ARE NOT downloaded.

Expected Change in Behavior

When the downloadApolloSchema Gradle task is run, input fields with the @deprecated tag ARE downloaded.

Added `(includeDeprecated: true)` as a params to `inputFields` in the `fragment` portion of the GraphQL query to download a schema.

### Current Behavior

When the `downloadApolloSchema` Gradle task is run, input fields with the `@deprecated` tag **ARE NOT** downloaded.

### Expected Change in Behavior

When the `downloadApolloSchema` Gradle task is run, input fields with the `@deprecated` tag **ARE** downloaded.
@apollo-cla
Copy link

@ndwhelan: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

@netlify
Copy link

netlify bot commented Feb 9, 2023

👷 Deploy request for apollo-android-docs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit df6f9e6

@@ -199,7 +199,7 @@ object SchemaDownloader {
isDeprecated
deprecationReason
}
inputFields {
inputFields(includeDeprecated: true {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you're missing a closing parenthesis here

@ndwhelan
Copy link
Contributor Author

ndwhelan commented Feb 10, 2023 via email

@martinbonnin
Copy link
Contributor

Hi @ndwhelan how can we help here? Want us to get this through the finish line?

@ndwhelan
Copy link
Contributor Author

ndwhelan commented Feb 17, 2023 via email

@martinbonnin
Copy link
Contributor

No worries at all! Take your time, just wanted to make sure this doesn't fall through the cracks.

Fixed a missing parenthesis caught in the PR, and also request
the new fields with information in deprecation into the
`InputType` fragment.

Updated `DownloadSchemaTests` to have a schema with deprecated
fields, to validate that deprecated fields and their deprecation
reason are now returned by the query in `SchemaDownloader`.
@ndwhelan
Copy link
Contributor Author

I've updated things from the PR, and updated a test. The tests are passing, but I wasn't really convinced it wouldn't return the whole thing regardless of my query in SchemaDownloader. I want to say it was, because it would fail if I put bad syntax in, but I'm not 100% sure. Any feedback on the test or improving that would be appreciated. I should be able to resolve it promptly if necessary.

Copy link
Contributor

@martinbonnin martinbonnin left a comment

Choose a reason for hiding this comment

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

This all looks very good to me, Thanks! 🙏

Copy link
Contributor

@BoD BoD left a comment

Choose a reason for hiding this comment

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

👍

@BoD BoD merged commit 7cfc0c3 into apollographql:main Feb 20, 2023
BoD pushed a commit that referenced this pull request Mar 14, 2023
@BoD
Copy link
Contributor

BoD commented Mar 15, 2023

This has been released as part of v3.7.5

@martinbonnin martinbonnin mentioned this pull request Mar 15, 2023
@martinbonnin martinbonnin mentioned this pull request Apr 3, 2023
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.

5 participants