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

fix(core): Add channels to asset entities on admin api #2717

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DanielBiegler
Copy link
Contributor

Description

I found that the channel-aware asset entities obviously have a channels field but its actually not exposed in the admin api. It got added in #700 but cant be queried.

Please note: I purposefully have not included re-generated types to make this PR as concise and review-able as possible. Its better from a operational security standpoint that you create the large auto generated files yourself
so that theres no chance to sneak something unexpected in @michaelbromley 👍

For finishing this PR it should probably include this newly exposed field in the e2e-tests! Note that you'll need to look into the following when running:

mutation assign {
  assignAssetsToChannel(input: { assetIds: ["50"], channelId: "2" }) {
    name
    channels {
      id
      code
    }
  }
}

You'll see a

{
  "errors": [
    {
      "message": "Cannot return null for non-nullable field Asset.channels.",
      "locations": [
        {
          "line": 37,
          "column": 5
        }
      ],
      "path": [
        "assignAssetsToChannel",
        0,
        "channels"
      ]
    }
  ],
  "data": null
}

eventhough the mutation will go through and work!

{
  "data": {
    "asset": {
      "name": "kari-shea-398668-unsplash.jpg",
      "channels": [
        {
          "id": "1",
          "token": "pxmu1fra2nrky6xivt",
          "code": "__default_channel__"
        },
        {
          "id": "2",
          "token": "example",
          "code": "Example"
        }
      ]
    }
  }
}

Please someone feel free to take over this PR, I just wanted to create this as the starting point for fixing #2478 but theres more to do there for example maybe a removeAssetsFromChannel mutation and ui which I dont want to do because of Angular & RxJs 😄

Breaking changes

Should be none AFAIK

Screenshots

You can add screenshots here if applicable.

Checklist

📌 Always:

  • I have set a clear title
  • My PR is small and contains a single feature
  • I have checked my own PR

👍 Most of the time:

  • I have added or updated test cases
  • I have updated the README if needed

Copy link

netlify bot commented Mar 5, 2024

Deploy Preview for effervescent-donut-4977b2 ready!

Name Link
🔨 Latest commit c16283a
🔍 Latest deploy log https://app.netlify.com/sites/effervescent-donut-4977b2/deploys/65e70695207794000842a0b7
😎 Deploy Preview https://deploy-preview-2717--effervescent-donut-4977b2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

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.

None yet

1 participant