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

Add Option to Force Replies in Thread for Google Chat Bot #488

Merged
merged 2 commits into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions models/bot.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type Bot struct {
GoogleChatProjectID string `mapstructure:"google_chat_project_id"`
GoogleChatSubscriptionID string `mapstructure:"google_chat_subscription_id"`
GoogleChatCredentials string `mapstructure:"google_chat_credentials"`
GoogleChatForceReplyToThread bool `mapstructure:"google_chat_force_reply_to_thread"`
TelegramToken string `mapstructure:"telegram_token"`
Users map[string]string `mapstructure:"slack_users"`
UserGroups map[string]string `mapstructure:"slack_usergroups"`
Expand Down
22 changes: 11 additions & 11 deletions remote/gchat/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ type DomainEvent struct {
// HandleOutput handles input messages for this remote.
func HandleRemoteInput(inputMsgs chan<- models.Message, rules map[string]models.Rule, bot *models.Bot) {
c := &Client{
Credentials: bot.GoogleChatCredentials,
ProjectID: bot.GoogleChatProjectID,
SubscriptionID: bot.GoogleChatSubscriptionID,
Credentials: bot.GoogleChatCredentials,
ProjectID: bot.GoogleChatProjectID,
SubscriptionID: bot.GoogleChatSubscriptionID,
ForceReplyToThread: bot.GoogleChatForceReplyToThread,
}

// Read messages from Google Chat
Expand All @@ -41,9 +42,10 @@ func HandleRemoteInput(inputMsgs chan<- models.Message, rules map[string]models.
// HandleRemoteOutput handles output messages for this remote.
func HandleRemoteOutput(message models.Message, bot *models.Bot) {
c := &Client{
Credentials: bot.GoogleChatCredentials,
ProjectID: bot.GoogleChatProjectID,
SubscriptionID: bot.GoogleChatSubscriptionID,
Credentials: bot.GoogleChatCredentials,
ProjectID: bot.GoogleChatProjectID,
SubscriptionID: bot.GoogleChatSubscriptionID,
ForceReplyToThread: bot.GoogleChatForceReplyToThread,
}

// Send messages to Google Chat
Expand Down Expand Up @@ -77,11 +79,8 @@ func toMessage(m *pubsub.Message) (models.Message, error) {
message.ChannelID = event.Space.Name
message.BotMentioned = true // Google Chat only supports @bot mentions
message.DirectMessageOnly = event.Space.SingleUserBotDm

if event.Space.Threaded {
message.ThreadID = event.Message.Thread.Name
message.ThreadTimestamp = event.EventTime
}
message.ThreadID = event.Message.Thread.Name
message.ThreadTimestamp = event.EventTime

// make channel variables available
message.Vars["_channel.name"] = message.ChannelName // will be empty if it came via DM
Expand All @@ -95,6 +94,7 @@ func toMessage(m *pubsub.Message) (models.Message, error) {
if event.User != nil {
message.Vars["_user.name"] = event.User.DisplayName
message.Vars["_user.id"] = event.User.Name
message.Vars["_user.internal_id"] = event.User.Name
Copy link
Member

Choose a reason for hiding this comment

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

curious about this, since it's already available as _user.id?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

_user.id is overwritten in the following lines by the email, if it exists.

message.Vars["_user.id"] = domainEvent.User.Email

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, thanks! I missed that, sorry. I wonder why we are not using _user.email for that other spot as we are using that for Slack. Seems a little cleaner.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@justmiles friendly ping - was there are particular reason for overwriting _user.id with email? otherwise, i think populating _user.email would be more appropriate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wass3r , I absolutely agree that _user.email looks much more logical and cleaner, I left it as is, adding a new field only in order not to break backward compatibility in existing user scripts. However, I am happy to rename the fields if you see no issues with backward compatibility.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@vsychov i appreciate that you made the change in a thoughtful manner! i'll give it a day or so for chance of response here. otherwise, happy to merge this and might raise PR myself for making that breaking change and call it out in release notes.

Copy link
Member

Choose a reason for hiding this comment

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

i'll go ahead and merge. thanks again.

message.Vars["_user.displayname"] = event.User.DisplayName

// Try parsing as a domain message to get user email
Expand Down
19 changes: 11 additions & 8 deletions remote/gchat/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ Implementation for the Remote interface

// Client struct.
type Client struct {
Credentials string
ProjectID string
SubscriptionID string
Credentials string
ProjectID string
SubscriptionID string
ForceReplyToThread bool
}

// validate that Client adheres to remote interface.
Expand Down Expand Up @@ -95,15 +96,17 @@ func (c *Client) Send(message models.Message, _ *models.Bot) {
// Best effort. If the instance goes away, so be it.
msg := &chat.Message{
Text: message.Output,
Thread: &chat.Thread{
Name: message.ThreadID,
},
}

if message.ThreadID != "" {
msg.Thread = &chat.Thread{
Name: message.ThreadID,
}
request := msgService.Create(message.ChannelID, msg)
if c.ForceReplyToThread {
request = request.MessageReplyOption("REPLY_MESSAGE_FALLBACK_TO_NEW_THREAD")
}

_, err = msgService.Create(message.ChannelID, msg).Do()
_, err = request.Do()
if err != nil {
log.Error().Msgf("google_chat failed to create message: %s", err.Error())
}
Expand Down