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

Allow message unfurling with auth URL #882

Merged
merged 6 commits into from Jan 18, 2021
Merged

Allow message unfurling with auth URL #882

merged 6 commits into from Jan 18, 2021

Conversation

zerok
Copy link
Contributor

@zerok zerok commented Jan 12, 2021

This PR is basically a continuation of prior work by @kautsig in #839. I've just extracted his change into a separate branch for easier merging and added some test cases.

API changes

  • Adds UnfurlMessageWithAuthURL method to the client
  • Adds UnfurlMessageWithAuthURLContext method to the client
  • Adds MsgOptionUnfurlAuthURL used within the two methods above

The Unfurl methods were added to be consistent with the UnfurlMessage method already exposed by the client.

@ds0nt
Copy link

ds0nt commented Jan 16, 2021

@ds0nt
Copy link

ds0nt commented Jan 16, 2021

"Specifying user_auth_url or user_auth_message will automatically imply user_auth_required being set to true. If both user_auth_url and user_auth_message are provided, user_auth_message takes precedence."

@zerok
Copy link
Contributor Author

zerok commented Jan 16, 2021

@ds0nt Good point. I'll add them on Monday to the PR 🙂

@zerok
Copy link
Contributor Author

zerok commented Jan 18, 2021

@ds0nt done :)

Copy link
Member

@kanata2 kanata2 left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you.

@kanata2 kanata2 merged commit 1a122fb into slack-go:master Jan 18, 2021
@ds0nt
Copy link

ds0nt commented Jan 21, 2021

Thanks! :D!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants