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

feat: Add input variable to stringify result #15

Closed
wants to merge 2 commits into from

Conversation

hasier
Copy link

@hasier hasier commented Apr 16, 2024

ℹ️ About

First of all, thanks for building this very handy action! 🙇

While adding it to our workflows, I realised I couldn't fully use it correctly due to the way newlines are encoded in the output. Similar to what @bachiri reported, I am not sure just replacing the newlines as introduced in #9 does the job. There are other characters that could interfere with the JSON format, so I guess the more robust option would be to JSON.stringify() the output. This should make sure \n is encoded as necessary, as well as any other character.

Let me know your thoughts!

I've also bumped the version string in the lockfiles to 1.1.0, let me know if you'd rather have a different numbering.


  • Has 🧪 tests
  • Has 📖 documentation

Fixes #8

@hasier
Copy link
Author

hasier commented Apr 29, 2024

@JoshCoady sorry for the direct ping, I saw you merged #9 and I was wondering if you might be the right person to look into this one too 🙂

@justinwride
Copy link
Contributor

@hasier I am experiencing a problem since version 1.0.2 where I'm seeing literal \n in Slack messages when they were newlines before. I reverted back to 1.0.1 to fix this issue, but would like to see newline characters work as actual newlines in Slack.

@hasier
Copy link
Author

hasier commented Jun 10, 2024

@hasier I am experiencing a problem since version 1.0.2 where I'm seeing literal \n in Slack messages when they were newlines before. I reverted back to 1.0.1 to fix this issue, but would like to see newline characters work as actual newlines in Slack.

@justinwride that was my issue too, I had to pin back to 1.0.1 as well, then added a tiny workaround similar to the one I proposed here as our payload was slightly more complex and was breaking the encoding in some other ways.

      - name: Transform markdown into Slack format
        id: transform
        # pin to 1.0.1 as they later changed the string output format
        # https://github.com/LoveToKnow/slackify-markdown-action/pull/12/files#r1310488091
        uses: LoveToKnow/[email protected]
        with:
          text: ${{ github.event.release.body }}

      - name: Build Slack payload
        id: build
        uses: actions/github-script@v7
        env:
          BODY: ${{ steps.transform.outputs.text }}
        with:
          script: |
            // Dump the contents to a JSON string as {"text": "..."}
            core.setOutput('payload', JSON.stringify({text: process.env['BODY']}));

      - name: Send payload to Slack
        id: slack
        uses: slackapi/slack-github-action@v1
        with:
          payload: ${{ steps.build.outputs.payload }}
        env:
          SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }}
          SLACK_WEBHOOK_TYPE: INCOMING_WEBHOOK

@VincentLanglet
Copy link
Contributor

Hi, I proposed #16 to revert the breaking change on the new line. You can now use 1.1.1.

toJSON can be used manually in the actions, example

{
                                    "type": "section",
                                    "text": {
                                        "type": "mrkdwn",
                                        "text": ${{ toJSON(steps.changelog.outputs.text) }}
                                    }
                                }

but this PR is still interesting because it could add natively the JSON encoding.

You might need to rebase master @hasier

@hasier
Copy link
Author

hasier commented Jun 24, 2024

Thanks @VincentLanglet! Seems simple enough to use with toJSON(). I used the following payload instead, as I am using incoming webhooks.

{
    "text": ${{ toJSON(steps.changelog.outputs.text) }}
}

These changes feel less useful now, so I am closing this PR.

@hasier hasier closed this Jun 24, 2024
@hasier hasier deleted the jsonify-output branch June 24, 2024 12:01
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.

Convert the line breaks to Slack \n
3 participants