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

Graphql naming conventions #1528

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rodyvansambeek
Copy link
Contributor

Why make this change?

As discussed in #692 and Discussion #1479 we want to be able to force the GraphQL to use the GQL best practices, in stead of 1 on 1 from the source.

As I am new to this codebase, I created a Pull-Request draft to be able to gather feedback and fix certain issues before creating the actual PR.

What is this change?

I've added the option force-naming-style on 2 different levels in the configuration:

  1. Global GraphQL Settings, which will force all naming styles for all available entities
  2. Per entity on the GraphQL EntitySetting. Which will only force the naming styles for the selected entity

I'm unsure about the force-naming-style parameter name though, please advise for any better name.

  • Summary of how your changes work to give reviewers context of your intent.
    • Links to relevant documentation / StackOverflow, if applicable.

TODO

  1. Update the documentation on:
    https://github.com/MicrosoftDocs/data-api-builder-docs/blob/main/data-api-builder/configuration-file.md
  2. Update dab.draft.schema.json

But I'd like to do that when the changes are final.

How was this tested?

  • [v] Configuration Test -> Added TestForceNamingStyleQuery in ConfigurationTests.cs
  • [v] Unit Tests -> Added test cases in EntityNameBecomesObjectName and ColumnNameBecomesFieldName

For the configuration test I have added a new table "Movies" to the DatabaseSchema-MsSql.sql and dab-config.MsSql.json in the Service.Tests folder.

Sample Request(s)

Not available.

@Aniruddh25
Copy link
Contributor

Thanks @rodyvansambeek for these contributions! It will be reviewed shortly.

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.

2 participants