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

Reverting nested models for create/update #932

Closed
wants to merge 7 commits into from

Conversation

aaronpowell
Copy link
Contributor

@aaronpowell aaronpowell commented Nov 1, 2022

Why make this change?

What is this change?

  • Reverts the change introduced in Handling nested inner objects for Cosmos Mutations #531
    • Supporting nested @model on create/update poses a complex problem in the runtime, as the runtime will need to extract the sub-objects to modify the corresponding object within the database
    • It makes sense to support nested objects that aren't @model types, as they are ones that aren't stored in a separate database/collection in Cosmos
  • Added Snapshooter as a dependency for the test project
    • Snapshooter is used for "snapshot testing", in which an object is serialized and then future tests tested against the serialized value - failing if the properties/object structure changes (without the snapshot being updated)
      - Snapshooter doesn't have MS Test support shipping on NuGet (I wrote it while addressing this bug). It's been merged and is just waiting for a NuGet package, so presently the code is embedded in the DAB codebase
    • Added Snapshooter.MSTest package (which I contributed to them)
    • Snapshooter is used by HotChocolate for its own testing

How was this tested?

  • Unit Tests that cover a schema with a single type, multiple types (with no relationships), one-way relationships, two-way relationships (@model <-> @model), and nested object types (@model <-> non-@model)

- Nested @model definitions shouldn't be supported, otherwise the engine will need to find some way to extract that out
- Added Snapshooter to make testing a generated GraphQL schema a lot easier
  - Note - Snapshooter is used by HotChocolate extensively in its tests
- Snapshooter hasn't shipped a NuGet package for MSTest support, so code is embedded (I wrote the MSTest support so I know it works!)
- Added tests and their snapshots for the GraphQL schema structures that need to be covered
@mbhaskar
Copy link
Member

Supporting nested @model on create/update poses a complex problem in the runtime, as the runtime will need to extract the sub-objects to modify the corresponding object within the database

This makes sense. Let us support nested mutations only for representative objects and not objects annotated with @model. @aaronpowell Isnt the existing implementation good without if we just make sure that the object is not annotated with @model and annotate it?

@seantleonard
Copy link
Contributor

Closing for now since this is old, can reopen if revisited with merge conflicts resolved to accommodate the changes that have been added since this was opened.

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.

3 participants