fix: parse provided payloads before replacing templated variables #449
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #449 +/- ##
=======================================
Coverage 99.85% 99.86%
=======================================
Files 7 7
Lines 709 722 +13
=======================================
+ Hits 708 721 +13
Misses 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
📣 A proof of concept for this fix is available in the following commits: https://github.com/zimeg/slack-sandbox/tree/f2af0c9944c3802ffce73e270332c3d02c80a012 🔍 Test this as curious with a multline commit too perhaps: - name: Run the release candidate
uses: zimeg/slack-github-action@91855720aaee2e23bf81dae3512204a371ca1609 # v2.1.1-rc.1
env:
CHANNEL_ID: ${{ secrets.SLACK_CHANNEL_ID }}
with:
errors: true
method: files.uploadV2
token: ${{ secrets.SLACK_BOT_TOKEN }}
payload-file-path: ./uploads.json # Example below!
payload-templated: true{
"channel_id": "${{ env.CHANNEL_ID }}",
"initial_comment": "🚀 *New build*\n*Commit message:*\n${{ github.payload.head_commit.message }}\n",
"file": "./README.md",
"filename": "docs-${{ github.sha }}.md"
}$ git commit
feat: attempt the odd after pushing this multiline commit
<3 |
|
📣 This link performs the same run but with debug logs, for the kind reviewer:
|
| * @param {unknown} input - The initial value of the content. | ||
| * @returns {unknown} Content with templatized variables replaced. |
There was a problem hiding this comment.
Sweet 💯 I like the simplified arguments, makes it easier to test 🚀
There was a problem hiding this comment.
@WilliamBergamin Haha I started writing all possible types but also found that testing the actual behavior is more useful here 😉
| */ | ||
| const out = {}; | ||
| for (const [k, v] of Object.entries(input)) { | ||
| out[k] = this.templatize(v); |
There was a problem hiding this comment.
Valid use of recursion 🥇
| @@ -257,14 +295,61 @@ describe("content", () => { | |||
| mocks.fs.readFileSync | |||
There was a problem hiding this comment.
These tests are pretty sweet 💯 should we also add a case with array options?
But this might be out of scope for this project because these elements mainly appear in interactive components like static multi-selects
Example
[
{
"type": "section",
"block_id": "section678",
"text": {
"type": "mrkdwn",
"text": "Pick items from the list"
},
"accessory": {
"action_id": "text1234",
"type": "multi_static_select",
"placeholder": {
"type": "plain_text",
"text": "Select items"
},
"options": [
{
"text": {
"type": "plain_text",
"text": "*this is plain_text text*"
},
"value": "value-0"
},
{
"text": {
"type": "plain_text",
"text": "*this is plain_text text*"
},
"value": "value-2"
}
]
}
}
]There was a problem hiding this comment.
@WilliamBergamin This is a fascinating suggestion! 🧠 ✨
Right now we have a blocks array but that's at the top-level and doesn't promise we recurse as expected 🤓
I hadn't thought about sending interactive blocks from a GitHub Action but I'm now so curious about how this might connect with the same app listening for events in the workspace otherwise!
There was a problem hiding this comment.
A similar example is added in 5c48fcf to confirm the templated replacement behavior 🚀
Co-authored-by: William Bergamin <william.bergamin.coen@gmail.com>
|
@WilliamBergamin @mwbrooks Thanks both for the most helpful reviews! I'll merge this now to close the related issue and will look into release related changes next 🚢 💨 |
Summary
This PR parses the provided payload before replacing templated variables to avoid erroneous parsings from perhaps variables with linebreaks - fixes #427.
Requirements