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

feat: using slack apps for slack-bridge #30630

Merged
merged 10 commits into from Oct 25, 2023

Conversation

abhinavkrin
Copy link
Member

Proposed changes (including videos or screenshots)

I have used slack apps for the slack bridge feature since the legacy bot integration has been deprecated. I have kept support for the legacy integration also. Users can switch to new slack app integration from the slack bridge setting.

Using slack app for slack bridge

bridge-using-slack-app.mp4

keeping the support for legacy bot integration

legacy-bot-test.mp4

Issue(s)

Fixes #30594

Steps to test or reproduce

  1. Admin needs to create a Slack app with the following manifest and install it to the slack workspace.
display_information:
  name: RocketChatBridge
features:
  app_home:
    home_tab_enabled: true
    messages_tab_enabled: false
    messages_tab_read_only_enabled: true
  bot_user:
    display_name: RocketChatBridge
    always_online: false
oauth_config:
  scopes:
    bot:
      - channels:history
      - channels:read
      - chat:write
      - chat:write.customize
      - groups:history
      - groups:read
      - mpim:history
      - mpim:read
      - pins:read
      - reactions:read
      - reactions:write
      - users:read
      - im:read
      - im:history
settings:
  event_subscriptions:
    bot_events:
      - app_home_opened
      - channel_left
      - member_joined_channel
      - message.channels
      - message.groups
      - reaction_added
      - reaction_removed
  interactivity:
    is_enabled: true
  org_deploy_enabled: false
  socket_mode_enabled: true
  token_rotation_enabled: false
  1. After installing the app to slack workspace. Get the app level token, bot token, and signing secret from the app management page. app token and signing secret can be found on the Basic Information page, and the bot token could be found in OAauth and Permissions page as Bot User OAuth Token.
  2. Go to workspace > settings > slack bridge. Enable slack bridge, disable Use Legacy Api Token, and add the above tokens in the respective fields. Save.

Further comments

  1. I have used Slack's recommended @slack/bolt (docs) which implements its events API using socket mode.
  2. Refactored some parts of the slack bridge to work according to the current slack API and rc lib functions.

Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
@abhinavkrin abhinavkrin requested a review from a team as a code owner October 12, 2023 17:33
@changeset-bot
Copy link

changeset-bot bot commented Oct 12, 2023

🦋 Changeset detected

Latest commit: 66e4db5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 32 packages
Name Type
@rocket.chat/meteor Minor
@rocket.chat/i18n Patch
@rocket.chat/mock-providers Patch
@rocket.chat/web-ui-registration Major
@rocket.chat/ui-client Major
@rocket.chat/gazzodown Major
@rocket.chat/fuselage-ui-kit Major
@rocket.chat/livechat Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/core-typings Minor
@rocket.chat/rest-typings Minor
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-contexts Major
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/api-client Patch
@rocket.chat/license Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/ddp-client Patch
@rocket.chat/models Patch
@rocket.chat/ui-video-conf Major
@rocket.chat/instance-status Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
apps/meteor/server/settings/slackbridge.ts Outdated Show resolved Hide resolved
apps/meteor/server/settings/slackbridge.ts Outdated Show resolved Hide resolved
apps/meteor/server/settings/slackbridge.ts Outdated Show resolved Hide resolved
apps/meteor/server/settings/slackbridge.ts Outdated Show resolved Hide resolved
.changeset/late-pants-switch.md Outdated Show resolved Hide resolved
apps/meteor/app/slackbridge/server/SlackAdapter.js Outdated Show resolved Hide resolved
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
@abhinavkrin
Copy link
Member Author

Hey @pierre-lehnen-rc Thanks for your review. I have made all the changes you suggested. Additionally, I took some time to test the changes and fixed the bugs I encountered.

@codecov
Copy link

codecov bot commented Oct 23, 2023

Codecov Report

Merging #30630 (66e4db5) into develop (6c79953) will decrease coverage by 0.17%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #30630      +/-   ##
===========================================
- Coverage    51.22%   51.05%   -0.17%     
===========================================
  Files          811      809       -2     
  Lines        15115    15220     +105     
  Branches      2759     2827      +68     
===========================================
+ Hits          7743     7771      +28     
- Misses        6961     7007      +46     
- Partials       411      442      +31     
Flag Coverage Δ
e2e 48.33% <ø> (-0.15%) ⬇️
unit 63.94% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@casalsgh
Copy link
Contributor

@abhinavkrin FYI - item is failing on our CI with message "Error: @rocket.chat/meteor#translation-check: command (/home/runner/work/Rocket.Chat/Rocket.Chat/apps/meteor) yarn run translation-check exited (1)"
@pierre-lehnen-rc @debdutdeb when possible, please your comments on this one

Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
@abhinavkrin
Copy link
Member Author

Hey @casalsgh The translation check failed. The line 39 in file apps/meteor/.scripts/check-i18n.js matches for double quoted keys at new lines.
In my code
fake-new-line
I didn't start SlackBridge_BotToken with the new line. Probably because I thought it started with a new line due to wrapping 🤦. So the regex match failed for this key.
I have fixed this.

@casalsgh casalsgh added stat: ready to merge PR tested and approved waiting for merge stat: QA skipped communityPR and removed stat: needs QA labels Oct 25, 2023
@kodiakhq kodiakhq bot merged commit 747ec6c into RocketChat:develop Oct 25, 2023
44 checks passed
@casalsgh
Copy link
Contributor

Thanks for the PR @abhinavkrin =) it will be on 6.5.0 release

debdutdeb pushed a commit that referenced this pull request Oct 26, 2023
Co-authored-by: Pierre Lehnen <55164754+pierre-lehnen-rc@users.noreply.github.com>
hugocostadev pushed a commit that referenced this pull request Oct 26, 2023
Co-authored-by: Pierre Lehnen <55164754+pierre-lehnen-rc@users.noreply.github.com>
@casalsgh casalsgh added this to the 6.5.0 milestone Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
communityPR stat: QA skipped stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slack legacy custom integration (bots) is getting deprecated which in turn would affect Slackbridge
3 participants