Skip to content

Conversation

@zimeg
Copy link
Member

@zimeg zimeg commented Jan 4, 2024

Summary

This PR makes a few changes to the OAuth examples in documentation to remove some confusion:

  • Specifies that the /slack/oauth is added to the server URL for *_PATH environment variables to avoid unknowingly creating paths like /slack/oauth/slack/oauth/start.
  • Updates the map entries when creating a server to make the map mutable to avoid a compilation error.

Category

  • bolt (Bolt for Java)
  • bolt-{sub modules} (Bolt for Java - optional modules)
  • slack-api-client (Slack API Clients)
  • slack-api-model (Slack API Data Models)
  • slack-api-*-kotlin-extension (Kotlin Extensions for Slack API Clients)
  • slack-app-backend (The primitive layer of Bolt for Java)
  • docs (Documentation)

Requirements

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you agree to those rules.

@zimeg zimeg added the docs M-T: Documentation work only label Jan 4, 2024
@zimeg zimeg added this to the 1.37.0 milestone Jan 4, 2024
@zimeg zimeg requested review from filmaj and seratch January 4, 2024 22:13
@zimeg zimeg self-assigned this Jan 4, 2024
Copy link
Member Author

@zimeg zimeg left a comment

Choose a reason for hiding this comment

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

📝 Some notes for the reviewers!

Comment on lines -62 to -65
SlackAppServer server = new SlackAppServer(Map.of(
SlackAppServer server = new SlackAppServer(new HashMap<>(Map.ofEntries(
entry("/slack/events", apiApp), // POST /slack/events (incoming API requests from the Slack Platform)
entry("/slack/oauth", oauthApp) // GET /slack/oauth/start, /slack/oauth/callback (user access)
));
Copy link
Member Author

Choose a reason for hiding this comment

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

This raises the following error for me on openJDK 19:

[main] INFO org.eclipse.jetty.util.log - Logging initialized @2068ms to org.eclipse.jetty.util.log.Slf4jLog
Exception in thread "main" java.lang.UnsupportedOperationException
        at java.base/java.util.ImmutableCollections.uoe(ImmutableCollections.java:142)
        at java.base/java.util.ImmutableCollections$AbstractImmutableMap.putAll(ImmutableCollections.java:1080)
        at com.slack.api.bolt.jetty.SlackAppServer.<init>(SlackAppServer.java:121)
        at com.slack.api.bolt.jetty.SlackAppServer.<init>(SlackAppServer.java:70)
        at NitroApp.main(ExampleApp.java:130)

Comment on lines +85 to +86
|**SLACK_INSTALL_PATH**|**Starting point of OAuth flow**: This endpoint redirects users to the Slack Authorize endpoint with required query parameters such as `client_id`, `scope`, `user_scope` (only for v2), and `state`.<br><br>The suggested path is `/slack/oauth/start` but you can go with any path. Note that the example above automatically prepends `/slack/oauth` to this variable.|
|**SLACK_REDIRECT_URI_PATH**|**Path for OAuth Redirect URI**: This endpoint handles callback requests after the Slack's OAuth confirmation. The path must be consistent with **SLACK_REDIRECT_URI** value.<br><br>The suggested path is `/slack/oauth/callback` but you can go with any path. Note that the example above automatically prepends `/slack/oauth` to this variable.|
Copy link
Member Author

Choose a reason for hiding this comment

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

This displays like so:

docs

@codecov
Copy link

codecov bot commented Jan 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (60fc261) 74.31% compared to head (2f2a08b) 74.31%.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #1262   +/-   ##
=========================================
  Coverage     74.31%   74.31%           
  Complexity     4120     4120           
=========================================
  Files           443      443           
  Lines         13078    13078           
  Branches       1322     1322           
=========================================
  Hits           9719     9719           
  Misses         2587     2587           
  Partials        772      772           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@filmaj filmaj left a comment

Choose a reason for hiding this comment

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

I'm not really sure the implication of changing the mapping class used here and how that affects which Java runtime will have compatibility (or not), so I would prefer to wait for Kaz to provide his opinion.

Copy link
Contributor

@seratch seratch left a comment

Choose a reason for hiding this comment

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

Thank you!

@seratch seratch merged commit 91169e5 into slackapi:main Jan 8, 2024
@zimeg zimeg deleted the docs-oauth-notes branch January 8, 2024 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs M-T: Documentation work only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants