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

RESP-4311 Mostly update forked slack go library #34

Conversation

kelseymills
Copy link

@kelseymills kelseymills commented Nov 7, 2024

Syncing our branch of slack-go with upstream 🐘 (mostly)

This is only a mostly update because rich text caused me a world of problems so I haven't synced that with the latest.
The full changes are here #33 but I have ticketed them up to be done separately in a way that actually works in core

The places I did have to deal with conflicting changes:

  • We had a custom implementation of call block that's easy to replace with theirs
  • Bookmarks - the API methods take different args now, I've replaced our implementation with theirs but it's all the same under the hood

Note: these changes won't come into effect in core until the PR where I bump the version there

For ease of reviewing I commented on the files I had to deal with conflicts, everything else is just their updates

KouWakai and others added 30 commits December 13, 2022 09:42
…nel to close when it is finished, and consumer to see the close
…outer goroutine did not wait on all inner goroutines that had a chance to set it, also make sure to check error for context.Canceled appropriately
…l via a method that has an escape hatch; unable to change public Events field without breaking api, though
…ntexts for channel ops, though they are very similar now
Co-authored-by: Naoki Kanatani <k12naoki@gmail.com>
…erError

Co-authored-by: Naoki Kanatani <k12naoki@gmail.com>
…modehandler will react on context cancellation
This is something that Slack returns by default on API response.
Signed-off-by: Ivan Milchev <ivan@mondoo.com>
add support for user_profile_changed callback event
Aryakoste and others added 25 commits August 15, 2024 10:11
* events added with test

* added events with tests

* added all events and changes done
The code changes in this commit add support for parsing AppRateLimited events in the `ParseEvent` function. This allows the application to handle rate-limited events from the Slack API.
* Add Properties.Canvas to Channel

* Trigger GitHub Actions

---------

Co-authored-by: Lorenzo Aiello <lorenzo.aiello@slack-corp.com>
* fix: create multipart form when multipart request

* call createFormFields in go func()

del coment

* Trigger GitHub Actions

---------

Co-authored-by: Lorenzo Aiello <lorenzo.aiello@slack-corp.com>
Support publishing a messge to a specific thread
https://api.slack.com/interactivity/handling#publishing_in_thread

From slack interactivity documentation:
Publishing responses in thread 
If you want to publish a message to a specific thread, you'll need to
include an attribute response_type and set its value to in_channel.
Then, to specify the thread, include a thread_ts.

Also, be sure to set replace_original to false or you'll overwrite the
message you're wanting to respond to!
)

As per [block kit
docs](https://api.slack.com/reference/block-kit/blocks#date-element-type),
the date element requires a format string to be included. Currently,
submitting a block kit with this element results in a slack API error.

Also added the two optional fields `url` and `fallback` for posterity.

##### PR preparation
Run `make pr-prep` from the root of the repository to run formatting,
linting and tests.

##### Should this be an issue instead
- [ ] is it a convenience method? (no new functionality, streamlines
some use case)
- [ ] exposes a previously private type, const, method, etc.
- [ ] is it application specific (caching, retry logic, rate limiting,
etc)
- [ ] is it performance related.

##### API changes

Since API changes have to be maintained they undergo a more detailed
review and are more likely to require changes.

- no tests, if you're adding to the API include at least a single test
of the happy case.
- If you can accomplish your goal without changing the API, then do so.
- dependency changes. updates are okay. adding/removing need
justification.

###### Examples of API changes that do not meet guidelines:
- in library cache for users. caches are use case specific.
- Convenience methods for Sending Messages, update, post, ephemeral,
etc. consider opening an issue instead.
…go#1320)

##### Pull Request Guidelines

These are recommendations for pull requests.
They are strictly guidelines to help manage expectations.

##### PR preparation
Run `make pr-prep` from the root of the repository to run formatting,
linting and tests.

##### Should this be an issue instead
- [x] is it a convenience method? (no new functionality, streamlines
some use case)
- [ ] exposes a previously private type, const, method, etc.
- [ ] is it application specific (caching, retry logic, rate limiting,
etc)
- [ ] is it performance related.

Fix for [issue slack-go#1276](slack-go#1276)
Updated the datatype of RichTextInputBlockElement InitialValue from
string to *RichTextBlock
…ocks (slack-go#1319)

This PR adds support for the `unicode` parameter to the
`RichTextSectionEmojiElement` struct for rich text blocks. While this
parameter is not officially documented in Slack's API, it is present in
the JSON payload of actual Slack messages and represents the Unicode
code point of the emoji.
https://api.slack.com/reference/block-kit/blocks#emoji-element-type

For example, a rich text block with an emoji can include the unicode
field like this:

```json
"blocks": [
    {
      "type": "rich_text",
      "block_id": "xxxxx",
      "elements": [
        {
          "type": "rich_text_section",
          "elements": [
            {
              "type": "emoji",
              "name": "+1",
              "unicode": "1f44d"
            }
          ]
        }
      ]
    }
  ]
```

The unicode parameter behaves similarly to the skin-tone parameter,
which is also undocumented but has already been included in the
structure. This PR aligns the handling of unicode in the same way to
ensure emojis are fully supported in Slack message payloads.

Please review, and feel free to provide feedback if any adjustments are
needed. Thank you!



##### Pull Request Guidelines

These are recommendations for pull requests.
They are strictly guidelines to help manage expectations.

##### PR preparation
Run `make pr-prep` from the root of the repository to run formatting,
linting and tests.

##### Should this be an issue instead
- [ ] is it a convenience method? (no new functionality, streamlines
some use case)
- [ ] exposes a previously private type, const, method, etc.
- [ ] is it application specific (caching, retry logic, rate limiting,
etc)
- [ ] is it performance related.

##### API changes

Since API changes have to be maintained they undergo a more detailed
review and are more likely to require changes.

- no tests, if you're adding to the API include at least a single test
of the happy case.
- If you can accomplish your goal without changing the API, then do so.
- dependency changes. updates are okay. adding/removing need
justification.

###### Examples of API changes that do not meet guidelines:
- in library cache for users. caches are use case specific.
- Convenience methods for Sending Messages, update, post, ephemeral,
etc. consider opening an issue instead.
…go#1190)

Implement the API methods for the Calls API in Slack
https://api.slack.com/apis/calls

Implemented methods
- `calls.add` - Indicate a new call has been started
- `calls.end` - Indicate to slack that the call has ended
- `calls.info` - Get information about an ongoing slack call object
- `calls.update` - update call information
- `calls.participants.add`
- `calls.participants.remove`

Additionally, I've added the minimal version of `Block{Type: "call",
CallID: string}` which slack recommends/requires be posted back to the
channel https://api.slack.com/apis/calls#post_to_channel.

All implemented functionality is publicly documented. There appear to be
additional attributes on the `type: call` block, however those appear to
be internal values for slack's rendering, so I have left them out. See
this gist for specific responses
https://gist.github.com/winston-stripe/0cac608bd63b42d73a352be53577f7fd

##### Pull Request Guidelines

These are recommendations for pull requests.
They are strictly guidelines to help manage expectations.

##### PR preparation
Run `make pr-prep` from the root of the repository to run formatting,
linting and tests.

##### Should this be an issue instead
- [ ] is it a convenience method? (no new functionality, streamlines
some use case)
- [ ] exposes a previously private type, const, method, etc.
- [ ] is it application specific (caching, retry logic, rate limiting,
etc)
- [ ] is it performance related.

##### API changes

Since API changes have to be maintained they undergo a more detailed
review and are more likely to require changes.

- no tests, if you're adding to the API include at least a single test
of the happy case.
- If you can accomplish your goal without changing the API, then do so.
- dependency changes. updates are okay. adding/removing need
justification.

###### Examples of API changes that do not meet guidelines:
- in library cache for users. caches are use case specific.
- Convenience methods for Sending Messages, update, post, ephemeral,
etc. consider opening an issue instead.

---------

Co-authored-by: Winston Durand <me@winstondurand.com>
Adds some convenience methods to block elements to easily add
functionality

---------

Co-authored-by: Lorenzo Aiello <lorenzo.aiello@slack-corp.com>
…k-go#1328)

Completion of slack-go#1301

- Adds the new complete functions for the Function Execution Event
- Adds the context version of those methods

---
> this PR to handle event
[function_executed](https://api.slack.com/events/function_executed) and
response the function with
[functions.completeSuccess](https://api.slack.com/methods/functions.completeSuccess)
and
[functions.completeError](https://api.slack.com/methods/functions.completeError)

---------

Co-authored-by: Yoga Setiawan <yogainformatika@gmail.com>
…go#1330)

Expose the ability to override the [external_limited
option](https://api.slack.com/methods/conversations.inviteShared#arg_external_limited)
for inviteShared.

Adding the param to all the InviteSharedEmailsToConversation, etc.
methods would be a breaking change to those callers, so I opted instead
to expose the underlying helper (renamed to InviteSharedToConversation).

I feel like the convenience methods
(InviteSharedEmailsToConversation/InviteSharedUserIDsToConversation) are
not actually that much more convenient than just using the helper, and I
think we can eventually remove them in favor of having people call
InviteSharedToConversation directly. But that's a future thing.

Although it's slightly inconvenient for the caller to use *bool for
ExternalLimited, the two alternatives I considered are, I think worse:
- Include ExternalLimited as a bool in the InviteSharedParams. I dislike
this way because it gives the SDK user of InviteSharedToConversation a
different default behavior from inviteShared, since the default value in
the API is true.
- Add a bool like NonExternalLimited to InviteSharedParams. This way the
defaulting is consistent with the API if it's not specified; however,
the InviteSharedParams no longer mirror the API args, which I think is
confusing.
This PR introduces new functionalities for managing canvases and
creating channel-specific canvases.

- CreateCanvas
- DeleteCanvas
- EditCanvas
- SetCanvasAccess
- DeleteCanvasAccess
- LookupCanvasSections
- CreateChannelCanvas

Closes slack-go#1333
@kelseymills kelseymills marked this pull request as ready for review November 7, 2024 13:47
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this had conflicts but the tests & linter run and pass so not much danger here

block.go Outdated
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this had some minor conflicts because we had implemented our own call block, but I've replaced it entirely with theirs

block_call.go Outdated
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this had some minor conflicts because we had implemented our own call block, but I've replaced it entirely with theirs

block_conv.go Outdated
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this had some minor conflicts because we had implemented our own call block, but I've replaced it entirely with theirs

block_element.go Outdated
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file had conflicts, we had introduced the following inputs before the upstream did:

  • Email
  • URL
  • Number
    I've brought ours in line with theirs, but it's just a naming change

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file had conflicts but it was just moving tests around

bookmarks.go Outdated
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file had conflicts because we introduced these bookmark methods first. I've brought ours in line with theirs where possible, but they were still missing a few fields on the params so I added them back in.

conversation.go Outdated
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file had conflicts but only in the comments / new content added by them

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file had conflicts because UserChangeEvent is slightly different, have changed to their version

Copy link

@rliddler rliddler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -0,0 +1,25 @@
name: 'Close stale issues and PRs'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✂️

@kelseymills kelseymills merged commit 6f9db6c into master Nov 7, 2024
7 checks passed
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.