docs icon indicating copy to clipboard operation
docs copied to clipboard

add warning regarding subscription and mutation redaction of relational fields

Open chrisbonifacio opened this issue 1 year ago • 3 comments

Description of changes:

Add warning regarding subscription and mutation redaction of relational fields

Related GitHub issue #, if available:

https://github.com/aws-amplify/amplify-category-api/issues/2636

https://github.com/aws-amplify/amplify-category-api/issues/2609

https://github.com/aws-amplify/amplify-category-api/issues/2658

Instructions

If this PR should not be merged upon approval for any reason, please submit as a DRAFT

Which product(s) are affected by this PR (if applicable)?

  • [ ] amplify-cli
  • [ ] amplify-ui
  • [ ] amplify-studio
  • [ ] amplify-hosting
  • [ ] amplify-libraries

Which platform(s) are affected by this PR (if applicable)?

  • [ ] JS
  • [ ] Swift
  • [ ] Android
  • [ ] Flutter
  • [ ] React Native

Please add the product(s)/platform(s) affected to the PR title

Checks

  • [ ] Does this PR conform to the styleguide?

  • [ ] Does this PR include filetypes other than markdown or images? Please add or update unit tests accordingly.

  • [ ] Are any files being deleted with this PR? If so, have the needed redirects been created?

  • [ ] Are all links in MDX files using the MDX link syntax rather than HTML link syntax?
    ref: MDX: [link](https://docs.amplify.aws/) HTML: <a href="https://docs.amplify.aws/">link</a>

When this PR is ready to merge, please check the box below

  • [ ] Ready to merge

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

chrisbonifacio avatar Jun 20 '24 21:06 chrisbonifacio

Lovely to see this documented somewhere so others can be pointed to it. I wonder if we could add something to the graphql response too as a sign post to the reason the nested types are suddenly null? Was wondering if something could be done with the deprecated directive or similar, adds a message that devs can find ideally without throwing an exception but if you keep the expection at least there is a reason, link to these docs and a possible one liner explaining what has happened, a delicious DX rather than what we currently have.

Alternatively/Additionally add to the error messages explaining that the auth differs so whats why its coming back empty?

As it currently stands I went down rabbit holes trying to find what might have changed to cause it even to the dread of "have I over written live data with dev data somehow?" which is never a good feeling. An explaination (and link to these new docs) would have do wonders.

Thoughts?

PeteDuncanson avatar Jun 21 '24 12:06 PeteDuncanson

I'm curious is there a plan to update the autogenerated code for mutations so relational fields are not included? This would be really handy as otherwise the autogenerated mutations will continue to throw errors such as:

"graphQLErrors": [
        {
            "path": [
                "createModelOfSomeKind",
                "linkedModel",
                "id"
            ],
            "locations": null,
            "message": "Cannot return null for non-nullable type: 'ID' within parent 'LinkedModel' (/createModelOfSomeKind/linkedModel/id)"
        },
        ...

Or is the recommentation to create custom mutations for everything from now on?

Thanks!

pr0g avatar Jun 24 '24 17:06 pr0g

@chrisbonifacio Could you please add info about the new feature flag graphqltransformer > subscriptionsInheritPrimaryAuth.

If enabled, subscriptions will inherit the primary model authorization rules for the relational fields. If disabled, relational fields will be redacted in mutation response when there is a difference between auth rules between primary and related models.

sundersc avatar Jun 24 '24 21:06 sundersc