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.

Sign up for GitHub

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

Jump to bottom

First ephemeral #1957

Closed
wants to merge 3 commits into from
Closed

First ephemeral #1957

wants to merge 3 commits into from

Conversation

sfrunza13
Copy link

@sfrunza13 sfrunza13 commented Nov 30, 2022 •
edited

Description

The first ephemeral I got to send. Added another field to be configured in the configuration.yaml that will allow ephemerals to be sent instead of regular messages to the user that triggers the bot.
Fixes #1941

Status

UNDER DEVELOPMENT

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • So far I have set up an App I called mybot on slack, I used my local opsdroid exposing 8080 through ngrok.
    image

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@sfrunza13
Copy link
Author

sfrunza13 commented Nov 30, 2022 •
edited

I am interested to know how maintainers envision this feature being added. Is this idea of having another line in the yaml a good idea? Should the ephemeral be an option for things other than the regular message? Should both regular messages and ephemerals be a possibility for the same bot, not just one or the other? @jacobtomlinson

@codecov
Copy link

codecov bot commented Dec 4, 2022 •
edited

Codecov Report

Base: 99.47% // Head: 99.47% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (7564b1a) compared to base (73da764).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #1957   +/-   ##
=======================================
  Coverage   99.47%   99.47%           
=======================================
  Files          80       80           
  Lines        5129     5136    +7     
=======================================
+ Hits         5102     5109    +7     
  Misses         27       27
Impacted Files Coverage Δ
opsdroid/connector/slack/connector.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sfrunza13 sfrunza13 marked this pull request as ready for review December 4, 2022 23:12
FabioRosado
FabioRosado approved these changes Feb 1, 2023
View reviewed changes
Copy link
Member

@FabioRosado FabioRosado left a comment

Choose a reason for hiding this comment

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

Hello @sfrunza13 thank you for working on this, apologies for the delay in reviewing it must have slipped by us.

Regarding your question, I think it would be good to add a configuration to the yaml file, but also allowing users to generate a Message with the ephemeral type so if a user triggers a specific skill, the bot could reply directly to that particular user.

So allowing both would be good an give more flexibility

@ryanm101
Copy link
Contributor

I actually have a use case for this..

User wants to query a DB
Bot executes query sent by user
Bot gets DB results and encrypts them into a file
Bot responds to public chat with confirmation results as attachment
Bot sends key/passphrase to user via ephemeral chat (i know i could use private chat as well) but all comms on a single channel is nice. and I dont really want users to interact with BOT in private chat for audibility.

@chunyang-wen chunyang-wen mentioned this pull request Jul 7, 2023
Open
@jacobtomlinson
Copy link
Member

I'm going through and closing out stale PRs here. Sorry we never got this over the line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FabioRosado FabioRosado FabioRosado approved these changes

Assignees
No one assigned
Labels
connector/slack connector
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature Request: Support to respond with Ephemeral message for slack connector
4 participants
@sfrunza13 @ryanm101 @jacobtomlinson @FabioRosado