Problem/Motivation

In recent versions of the module a new field has been added to retrieve translations of a node. All works in the cases:

1. the node has no translations
2. the node has one or more publishded translations

however, if the node has one translation that is unpublished, and the user doing the query cannot access unpublished nodes, the query fails with this fatal error:

{
    "$operation": {
        "queryId": null,
        "query": "query GetNodeByPath($path: String!, $langcode: String!) {\n  route(path: $path, langcode: $langcode) {\n    __typename\n    ... on RouteInternal {\n      __typename\n      entity {\n        ...FragmentNodeUnion\n      }\n    }\n    ... on RouteRedirect {\n      __typename\n      status\n      url\n      internal\n    }\n  }\n}\n\nfragment FragmentMetaTag on MetaTagValue {\n  __typename\n  tag\n  attributes {\n    name\n    content\n  }\n}\n\nfragment FragmentTextSummary on TextSummary {\n  value\n  processed\n  format\n  summary\n}\n\nfragment FragmentImage on Image {\n  url\n  width\n  height\n  alt\n  title\n  size\n  mime\n}\n\nfragment FragmentUser on User {\n  name\n  mail\n}\n\nfragment FragmentNodeTranslation on Translation {\n  __typename\n  path\n  langcode {\n    id\n  }\n}\n\nfragment FragmentNodeArticle on NodeArticle {\n  excerpt\n  sticky\n  body {\n    ...FragmentTextSummary\n  }\n  image {\n    ...FragmentImage\n  }\n  author {\n    __typename\n    ... on User {\n      ...FragmentUser\n    }\n  }\n  translations {\n    ...FragmentNodeTranslation\n  }\n}\n\nfragment FragmentText on Text {\n  value\n  processed\n  format\n}\n\nfragment FragmentParagraphFormattedText on ParagraphFormattedText {\n  formattedTextHeading: heading\n  formattedTextText: formattedText {\n    ...FragmentText\n  }\n}\n\nfragment FragmentLink on Link {\n  __typename\n  title\n  url\n  internal\n}\n\nfragment FragmentParagraphLink on ParagraphLink {\n  links {\n    ...FragmentLink\n  }\n}\n\nfragment FragmentFile on File {\n  name\n  url\n  size\n  mime\n  description\n}\n\nfragment FragmentMediaAudio on MediaAudio {\n  name\n  mediaAudioFile {\n    ...FragmentFile\n  }\n}\n\nfragment FragmentMediaDocument on MediaDocument {\n  name\n  mediaDocumentFile: mediaDocument {\n    ...FragmentFile\n  }\n}\n\nfragment FragmentMediaImage on MediaImage {\n  name\n  mediaImage {\n    ...FragmentImage\n  }\n}\n\nfragment FragmentMediaRemoteVideo on MediaRemoteVideo {\n  name\n  mediaOembedVideo\n}\n\nfragment FragmentMediaVideo on MediaVideo {\n  name\n  mediaVideoFile {\n    ...FragmentFile\n  }\n}\n\nfragment FragmentMediaUnion on MediaInterface {\n  __typename\n  id\n  ...FragmentMediaAudio\n  ...FragmentMediaDocument\n  ...FragmentMediaImage\n  ...FragmentMediaRemoteVideo\n  ...FragmentMediaVideo\n}\n\nfragment FragmentParagraphImage on ParagraphImage {\n  image {\n    ...FragmentMediaUnion\n  }\n}\n\nfragment FragmentParagraphVideo on ParagraphVideo {\n  video {\n    ...FragmentMediaUnion\n  }\n}\n\nfragment FragmentParagraphFileAttachments on ParagraphFileAttachment {\n  fileAttachmentsParagraphHeading: heading\n  fileAttachmentsParagraphFormattedText: formattedText {\n    ...FragmentText\n  }\n  fileAttachments {\n    ...FragmentMediaUnion\n  }\n}\n\nfragment FragmentParagraphHero on ParagraphHero {\n  formattedText {\n    ...FragmentText\n  }\n  image {\n    ...FragmentMediaUnion\n  }\n  primaryLink {\n    ...FragmentLink\n  }\n  secondaryLink {\n    ...FragmentLink\n  }\n  paragraphHeroHeading: heading\n}\n\nfragment FragmentParagraphAccordionItem on ParagraphAccordionItem {\n  __typename\n  id\n  accordionItemHeading: heading\n  accordionItemFormattedText: formattedText {\n    ...FragmentText\n  }\n  contentElements {\n    ... on ParagraphInterface {\n      __typename\n      id\n      ... on ParagraphAccordionItemContentElementsUnion {\n        ...FragmentParagraphFormattedText\n        ...FragmentParagraphImage\n        ...FragmentParagraphLink\n        ...FragmentParagraphFileAttachments\n        ...FragmentParagraphVideo\n      }\n    }\n  }\n}\n\nfragment FragmentParagraphAccordion on ParagraphAccordion {\n  heading\n  accordionLayout\n  primaryLink {\n    ...FragmentLink\n  }\n  accordionFormattedText: formattedText {\n    ...FragmentText\n  }\n  accordionItems {\n    ... on ParagraphInterface {\n      __typename\n      id\n      ...FragmentParagraphAccordionItem\n    }\n  }\n}\n\nfragment FragmentParagraphListingArticle on ParagraphListingArticle {\n  __typename\n  id\n  paragraphListingArticleHeading: heading\n  limit\n}\n\nfragment FragmentArticleTeaser on NodeArticle {\n  __typename\n  id\n  image {\n    ...FragmentImage\n  }\n  path\n  title\n  sticky\n  excerpt\n  created {\n    timestamp\n  }\n  author {\n    __typename\n    ... on User {\n      ...FragmentUser\n    }\n  }\n}\n\nfragment FragmentParagraphLiftupArticle on ParagraphLiftupsArticle {\n  heading\n  articles {\n    ... on NodeArticle {\n      ...FragmentArticleTeaser\n    }\n  }\n}\n\nfragment FragmentNodeFrontpage on NodeFrontpage {\n  contentElements {\n    ... on ParagraphInterface {\n      __typename\n      id\n      ... on NodeFrontpageContentElementsUnion {\n        ...FragmentParagraphFormattedText\n        ...FragmentParagraphLink\n        ...FragmentParagraphImage\n        ...FragmentParagraphVideo\n        ...FragmentParagraphFileAttachments\n        ...FragmentParagraphHero\n        ...FragmentParagraphAccordion\n        ...FragmentParagraphListingArticle\n        ...FragmentParagraphLiftupArticle\n      }\n    }\n  }\n  translations {\n    ...FragmentNodeTranslation\n  }\n}\n\nfragment FragmentNodePage on NodePage {\n  contentElements {\n    ... on ParagraphInterface {\n      __typename\n      id\n      ... on NodePageContentElementsUnion {\n        ...FragmentParagraphFormattedText\n        ...FragmentParagraphLink\n        ...FragmentParagraphImage\n        ...FragmentParagraphVideo\n        ...FragmentParagraphFileAttachments\n        ...FragmentParagraphHero\n        ...FragmentParagraphAccordion\n        ...FragmentParagraphListingArticle\n        ...FragmentParagraphAccordion\n        ...FragmentParagraphLiftupArticle\n      }\n    }\n  }\n  translations {\n    ...FragmentNodeTranslation\n  }\n}\n\nfragment FragmentNodeUnion on NodeInterface {\n  __typename\n  id\n  title\n  status\n  path\n  langcode {\n    id\n  }\n  created {\n    timestamp\n  }\n  changed {\n    timestamp\n  }\n  metatag {\n    ...FragmentMetaTag\n  }\n  ...FragmentNodeArticle\n  ...FragmentNodeFrontpage\n  ...FragmentNodePage\n}",
        "operation": "GetNodeByPath",
        "variables": {
            "path": "\/articles\/article-finnish",
            "langcode": "fi"
        },
        "extensions": null
    },
    "$result->data": {
        "route": {
            "__typename": "RouteInternal",
            "entity": null
        }
    },
    "$result->errors": [
        {
            "message": "Cannot return null for non-nullable field \"NodeArticle.translations\".",
            "locations": [
                {
                    "line": 73,
                    "column": 3
                }
            ],
            "path": [
                "route",
                "entity",
                "translations",
                0
            ]
        }
    ],
    "$result->extensions": []
}

Steps to reproduce

1. Create a node
2. add a translation
3. unpublish the translation
4. query for translations of that node
5. observe the error

Proposed resolution

When retrieving the list of translations, we should handle the case where the user does not have access to the translation like we do when the translation does not actually exist.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

vermario created an issue. See original summary.

vermario’s picture

So I think the problem is that in this module we are saying that the list we return cannot be null:

https://git.drupalcode.org/project/graphql_compose/-/blob/2.2.x/src/Plug...

return [
              'translations' => [
                'type' => Type::nonNull(Type::listOf(Type::nonNull(static::type($this->getPluginId())))),
                'description' => (string) $this->t('Available translations for content.'),
              ],
            ];

But the "data producer" in the graphql module can in fact return null if you don't have access to the translation:

https://git.drupalcode.org/project/graphql/-/blob/8.x-4.x/src/Plugin/Gra...

if ($access) {
          /** @var \Drupal\Core\Access\AccessResultInterface $accessResult */
          $accessResult = $entity->access($accessOperation, $accessUser, TRUE);
          $context->addCacheableDependency($accessResult);
          if (!$accessResult->isAllowed()) {
            return NULL;
          }
        }

So we should allow the list to contain also NULL elements (or we can filter them out competely if they are null?) (not sure :-) )

almunnings’s picture

Yeah good pickup! To avoid a breaking change, we could filter it as you suggest. I'll pop in another Fork/MR

almunnings’s picture

Oh, looks like you edited the 2.2.x branch and not the issue branch, i'll need to fix that up.

almunnings’s picture

Ok
Bit of a mess, because the git branches were garbled.

https://git.drupalcode.org/project/graphql_compose/-/merge_requests/93 is my commit.

Let me know how that goes for you, added bonus of some tests on it.

almunnings’s picture

Status: Active » Needs review
vermario’s picture

I tried this in my project and it works nicely. Great that we have a test. LGTM!

vermario’s picture

Status: Needs review » Reviewed & tested by the community

  • almunnings committed c7a9f059 on 2.2.x
    Issue #3464624: Ensure translations return an array if unpublished.
    

almunnings’s picture

Status: Reviewed & tested by the community » Fixed

Releasing in 2.2.1

almunnings’s picture

Status: Fixed » Closed (fixed)