slack-github-action icon indicating copy to clipboard operation
slack-github-action copied to clipboard

Nested keys in JSON payloads can't be accessed in Slack workflows

Open rzumer opened this issue 1 year ago • 6 comments

Description

This repo uses flat for flattening JSON arrays, but by default nested keys are delimited with periods. Periods are not allowed in variable fields in Slack workflows, so it's impossible to reference those keys from flattened payloads.

What type of issue is this? (place an x in one of the [ ])

  • [x] bug
  • [ ] enhancement (feature request)
  • [ ] question
  • [ ] documentation related
  • [ ] example code related
  • [ ] testing related
  • [ ] discussion

Requirements (place an x in each of the [ ])

  • [x] I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • [x] I've read and agree to the Code of Conduct.
  • [x] I've searched for any related issues and avoided creating a duplicate issue.

rzumer avatar Jan 17 '24 20:01 rzumer

Hey @rzumer! 👋 Really solid callout and thanks for sending a fix in for it as well. I can confirm this is a limitation in Workflow Builder and your patch is a solid solution!

I plan to review the changes soon, but as a workaround for now I suggest individually setting these keys at the top-level of the payload. For example:

{
  "github_event_name": ${{ github.event_name }},
  "github_repository": ${{ github.repository }}
}

zimeg avatar Jan 17 '24 20:01 zimeg

Thanks for the quick response. For a few reasons I won't get into, manually mapping the nested keys would get tedious in my workflows. My plan was to build and run it directly from my branch in a custom action (I went through the steps in my test workflow to validate my PR today), but I don't have an urgent need for it, so hopefully if a fix can be released relatively quickly I can avoid that entirely.

Of course having the workflow builder accept periods in variable keys would work too. But I don't think I can send a patch there so I chose the path with the least expected amount of friction :)

rzumer avatar Jan 17 '24 22:01 rzumer

...manually mapping the nested keys would get tedious in my workflows.

Makes total sense! Checking now to see if the dot notation is something Workflow Builder can support to avoid a possible breaking change for existing workflows that might expect this as a variable input. I think there might be a few out there!

I'll keep ya updated on anything I hear with that. If there aren't plans for dot notation, I think your patch is the way to go 🚀

zimeg avatar Jan 19 '24 20:01 zimeg

I believe I'm running into this but I'm not sure I'm understanding the workaround.

Given this:

name: message-slack
description: Messages the slack channel
inputs:
  message:
    description: The message to put into the channel
    required: true
  slack-channel-id:
    description: The id for the slack channel
    required: true
  slack-team-id:
    description: The id for the slack team
    required: true
  environment:
    description: The environment we are running on
    required: true

runs:
  using: composite
  steps:
    - uses: slackapi/[email protected]
      env:
        WORKFLOW_RUN_URL: <secret>
        SLACK_WEBHOOK_URL: <secret>
        SLACK_WEBHOOK_TYPE: INCOMING_WEBHOOK        
      with:
        slack-message: ${{ inputs.message }}
        payload: |
          {
            "text": ${{ inputs.message }}
          }

I was getting an error. If I hard code the payload as:

payload: |
          {
            "text": "test"
          }

it works

SamuelCox avatar May 14 '24 16:05 SamuelCox

@SamuelCox this seems to be from a different cause to me. This issue is more about payloads with attributes that have objects as values. Things like:

      payload: |
        {
          "example": {
            "value": "12"
          }
        }

Happy to take look into workaround for your workflow in a new issue, but I'm guessing the variable message isn't being quoted properly so JSON parsing is failing. :thinking: The following might help:

      payload: |
        {
          "text": ${{ toJSON(inputs.message) }}
        }

zimeg avatar May 15 '24 04:05 zimeg

Yeah you were right, that fixed it, thank you! Sorry for commenting on an unrelated issue

SamuelCox avatar May 15 '24 08:05 SamuelCox