feat: Support appending to message attachments instead of replacing #74
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nickelsen
commented
Sep 30, 2024
| ) | ||
| } | ||
|
|
||
| private suspend fun getSlackMessageById( |
There was a problem hiding this comment.
These functions are moved to SlackHttpClient.
nickelsen
commented
Sep 30, 2024
| @@ -126,81 +121,54 @@ class SlackClient( | |||
| messageId: String? = null, | |||
| previousAttachments: List<SlackMessage.Attachment>? = null, | |||
| ): SlackMessage { | |||
| val attachments = mutableMapOf<JobType, SlackMessage.Attachment>() | |||
| val attachments = if (appendAttachments) { | |||
There was a problem hiding this comment.
This is the primary change of the PR, adding support for appending attachments instead of replacing them, depending on the input property.
There was a problem hiding this comment.
I think this would break the updating of status in the notifier?
Specifically where it goes from In Progress to Success or am I wrong?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For php release flow, we would like timestamps on the events, e.g. when build started, when deploy finished. To support that, I've added a flag called
append-status-updateswhich will add new attachments with status updates incl timestamp to existing slack messages instead of replacing them with new status.I've added two tests; one of the replacing functionality and one of the appending functionality
To make the tests work, I had to inject a layer between the message builder and the http client in
SlackClient, which I callSlackHttpClient, which I then replace with a fake instance during testing, to assert on the messages and content that would be sent to slack.Disclaimer: This is my first Kotlin PR ever, so I accept a lot of feedback. :D