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

Allow to disable field suggestions #3537

Merged
merged 4 commits into from
Jun 10, 2024
Merged

Conversation

patrick91
Copy link
Member

@patrick91 patrick91 commented Jun 10, 2024

:)

Summary by Sourcery

This pull request introduces a new configuration option suggest_field in StrawberryConfig to allow disabling field suggestions in error responses. Documentation and tests have been updated to reflect this new feature.

  • New Features:
    • Introduced a new configuration option suggest_field in StrawberryConfig to disable field suggestions in error responses.
  • Documentation:
    • Updated the schema configurations documentation to include the new suggest_field option and other available configurations.
  • Tests:
    • Added tests to verify the default behavior of field suggestions and the ability to disable field suggestions using the new suggest_field configuration.

Copy link
Contributor

sourcery-ai bot commented Jun 10, 2024

Reviewer's Guide by Sourcery

This pull request introduces a new configuration option to disable field suggestions in Strawberry's GraphQL schema. The changes include updates to the documentation, new tests to verify the functionality, and modifications to the core schema processing logic to respect the new configuration.

File-Level Changes

Files Changes
docs/types/schema-configurations.mdx
RELEASE.md
Updated documentation to include the new 'suggest_field' configuration and added a release note.
tests/schema/test_execution_errors.py Added new tests to verify the default behavior and the ability to disable field suggestions.
strawberry/schema/base.py
strawberry/schema/schema.py
strawberry/schema/config.py
Modified core schema processing logic to respect the new 'suggest_field' configuration.

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

RELEASE.md Outdated
name: str


schema = strawberry.Schema(query=Query, config=StrawberryConfig(suggest_field=False))
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
schema = strawberry.Schema(query=Query, config=StrawberryConfig(suggest_field=False))
schema = strawberry.Schema(query=Query, config=StrawberryConfig(suggest_fields=False))

not sure about this name though

Copy link
Member

Choose a reason for hiding this comment

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

This is something that is usually referred as "command not found suggestion"

Maybe field_not_found_suggestions? But not sure either =P

Copy link
Member Author

Choose a reason for hiding this comment

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

maybe disable_field_suggestions would work better

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @patrick91 - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 5 issues found
  • 🟢 Security: all looks good
  • 🟡 Testing: 2 issues found
  • 🟢 Complexity: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

docs/types/schema-configurations.mdx Show resolved Hide resolved
strawberry/schema/base.py Show resolved Hide resolved
Comment on lines 103 to 104
if not self.config.suggest_field:
for error in errors:
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (performance): Potential performance impact in _process_errors.

Iterating over all errors to remove field suggestions could have a performance impact if the error list is large. Consider evaluating the performance implications or adding a note about this.

strawberry/schema/base.py Show resolved Hide resolved
strawberry/schema/config.py Outdated Show resolved Hide resolved
tests/schema/test_execution_errors.py Show resolved Hide resolved
tests/schema/test_execution_errors.py Show resolved Hide resolved
@botberry
Copy link
Member

botberry commented Jun 10, 2024

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


This release adds a new configuration to disable field suggestions in the error
response.

@strawberry.type
class Query:
    name: str


schema = strawberry.Schema(
    query=Query, config=StrawberryConfig(disable_field_suggestions=True)
)

Trying to query { nam } will not suggest to query name instead.

Here's the tweet text:

🆕 Release (next) is out! Thanks to @patrick91 for the PR 👏

This release allows to disable field suggestions when sending an operation with
the wrong field.

Get it here 👉 https://strawberry.rocks/release/(next)

Copy link

codecov bot commented Jun 10, 2024

Codecov Report

Attention: Patch coverage is 97.50000% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.57%. Comparing base (b7f2881) to head (ea7cbe5).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #3537    +/-   ##
========================================
  Coverage   96.56%   96.57%            
========================================
  Files         523      523            
  Lines       33485    33591   +106     
  Branches     5554     5573    +19     
========================================
+ Hits        32336    32440   +104     
- Misses        914      915     +1     
- Partials      235      236     +1     

Copy link

codspeed-hq bot commented Jun 10, 2024

CodSpeed Performance Report

Merging #3537 will not alter performance

Comparing feature/disable-field-suggestions (ea7cbe5) with main (ad1f2eb)

Summary

✅ 12 untouched benchmarks

Copy link
Member

@bellini666 bellini666 left a comment

Choose a reason for hiding this comment

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

LGTM!

Left one suggestion regarding the config name, but not sure about that either :)

RELEASE.md Outdated
name: str


schema = strawberry.Schema(query=Query, config=StrawberryConfig(suggest_field=False))
Copy link
Member

Choose a reason for hiding this comment

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

This is something that is usually referred as "command not found suggestion"

Maybe field_not_found_suggestions? But not sure either =P

@patrick91 patrick91 merged commit d615081 into main Jun 10, 2024
64 checks passed
@patrick91 patrick91 deleted the feature/disable-field-suggestions branch June 10, 2024 16:45
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.

3 participants