Make payload delimiter configurable

[rzumer]

Fixes #280.

Summary

The default key delimiter when flattening payloads is a period, which is not allowed in Slack workflow variable keys. This makes the slack-send action unusable with Slack workflows/incoming webhooks if the payload contains nested keys as there is no way to access those keys (as far as I can tell).

For example if I want to include workflow metadata from GitHub in my payload with ${{ toJson(github) }}, all nested keys are inaccessible, and if I want to compose this with other custom variables by nesting the serialized github values like "github": ${{ toJson(github) }}, "some-key": "some-value", then all of the GitHub metadata is inaccessible because it's all prefixed with github. and can't be entered as a variable key in a Slack workflow. Underscores and hyphens are both valid, so changing to an underscore makes all those nested keys accessible. See flat docs

Tested on a private workflow by building and calling the action directly from my branch.

This leaves the default as a period but allows users to override it by setting the payload-delimiter option alongside the payload sent to the action.

Requirements (place an x in each [ ])

• I've read and understood the Contributing Guidelines and have done my best effort to follow them.
• I've read and agree to the Code of Conduct.

[salesforce-cla]

Thanks for the contribution! Before we can merge this, we need @rzumer to sign the Salesforce Inc. Contributor License Agreement.

[zimeg]

👋 Following up from #280 - I haven't heard of any immediate plans to support dot notation from Workflow Builder so I'm thinking this change is the right way to handle nested payloads going forward.

Since this can change key values of webhook inputs I believe this is a breaking change and will plan to include it in the next major version. Really appreciate the patience on introducing this change so far. We'll take a look at other possible changes we might want to include in a major version before merging this, but please know it's not forgotten!

[rzumer]

Thanks for the follow-up!

[zimeg]

👋 Hey @rzumer! Been thinking about this a bit and I'm hoping to avoid making a breaking change here since it's uncertain whether dot notion might eventually be supported by Workflow Builder. Also not wanting to keep this held up since flattening payloads can be super useful, so I was wondering if instead of a replacement, the delimiter can be added as an optional input argument?

I was thinking of something like the following, but let me know what you think and if this sounds good for you!

  - uses: slackapi/slack-github-action@v1.26.0
    with:
      payload: |
        {
          "example": {
            "value": "12"
          }
        }
+     payload-delimiter: '_'
    env:
      SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }}

[rzumer]

@zimeg Sorry for the delay, I've made that change and reverted the default to a period. I only tested the syntax in a JavaScript interpreter on its own this time since I don't have the build/deployment system set up for testing anymore, but it's a pretty simple change so hopefully that's fine with you.

[zimeg]

@rzumer No worries- thanks for making the update! The code changes look solid but I think we'll also need to add this to action.yml inputs with a bit or blurb of documentation in the technique 1 section for the new option 📚 After that I think this is set to merge! 🚀

Edit: It'd be nice to include a test case or two to confirm that these changes don't break later too 🙏

[rzumer]

I updated the commit to document the new field and mention it in the readme.

As for test cases, the development environment I'd set up for initial testing is gone, there is no existing test case that tests the flattened payload for reference on how to access it, and there are no testing instructions, so I just don't have the time to figure this all out right now, sorry for the trouble.

By the way, this is tagged with semver:major, but now that the default delimiter matches the current behavior, it should be minor.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.91%. Comparing base (c50e848) to head (9008c33).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #281      +/-   ##
==========================================
+ Coverage   96.84%   97.91%   +1.07%     
==========================================
  Files           2        2              
  Lines          95       96       +1     
==========================================
+ Hits           92       94       +2     
+ Misses          3        2       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[zimeg]

@rzumer Apologies for the above force pushes- something strange happened with my remotes, but I did add a few tests to catch possible regressions! You were right to call out that this section wasn't being covered yet 😅

The docs additions you made are appreciated! 🙏 Even though we aren't changing the default delimiter to use an underscore as the default value, I do think it's worth encouraging it as a default when nested payloads are used. It's not the most ideal, but this seems like an alright workaround for these cases!

I have also tested this with a local workflow and am finding that configurations are working just as expected! 🚀 With that, I think it's time we can merge this for the next minor version - a good call here. We might need to update a few of our dependencies before this, but I'm hoping it'll be soon 📦

Thank you again for calling out the strangeness of how we were flattening these nested payloads ❤️