slackapi / deno-slack-sdk

SDK for building Run on Slack apps using Deno
https://api.slack.com/automation
MIT License
155 stars 27 forks source link

fix(manifest): Strip protocol before feeding just the outgoing domain to the manifest API #246

Open arunsathiya opened 9 months ago

arunsathiya commented 9 months ago

Summary

Fixes https://github.com/slackapi/deno-slack-sdk/issues/245

When one or more outgoing domains in the manifest contain the protocol, the protocol is stripped off before feeding just the domain to the manifest API.

Testing

{
  "imports": {
    "deno-slack-sdk/": "/Users/arun/deno-slack-sdk/src/",
    "deno-slack-api/": "https://deno.land/x/deno_slack_api@2.1.1/"
  }
}

Special notes

N/A

Requirements

arunsathiya commented 9 months ago

I just have one question about import_map.json usage. Does the slack run command run --import-map behind the scenes? That part is a bit unclear to me because the Slack CLI is not open source.

codecov[bot] commented 9 months ago

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (ca8e418) 98.07% compared to head (b584112) 97.95%.

Files Patch % Lines
src/manifest/mod.ts 86.36% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #246 +/- ## ========================================== - Coverage 98.07% 97.95% -0.12% ========================================== Files 58 58 Lines 2280 2301 +21 Branches 137 140 +3 ========================================== + Hits 2236 2254 +18 - Misses 42 45 +3 Partials 2 2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

arunsathiya commented 9 months ago

Thanks for the review, @mwbrooks!

One suggestion would be to consider add some tests, if possible.

Definitely! I have added them in https://github.com/slackapi/deno-slack-sdk/pull/246/commits/2cff6cacd34793aed913e64704ed338270fb257c.

The slack run command starts a SocketMode connections managed by the CLI. When a function event is received, the CLI will invoke the script defined by the "start" hook in slack.json (example). That script currently calls the deno-slack-runtime/src/local-run.ts. In the source, you can see that the SDK reads the outgoing domains from the manifest and adds it to the Deno processes --allow-net.

Appreciate the explanation here, and exciting that Slack CLI will be open source in the future! I look forward to it. 😄