-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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: Updated characters types, Discord & Telegram enhancements #957
feat: Updated characters types, Discord & Telegram enhancements #957
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM outside of minor comments
@@ -69,6 +71,102 @@ export class MessageManager { | |||
return; | |||
} | |||
|
|||
const isDirectlyMentioned = this._isMessageForMe(message); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add a check if discord settings is even configured before doing discord settings logic
} | ||
|
||
private _isTeamCoordinationRequest(content: string): boolean { | ||
const coordinationKeywords = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's move this to a constant's file
@@ -32,6 +32,7 @@ import { sendMessageInChunks, canSendMessage } from "./utils.ts"; | |||
|
|||
export type InterestChannels = { | |||
[key: string]: { | |||
currentHandler: any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this me typed
Updated, fixed items, and added security feature for Telegram to limit bot add to groups, which if not wanted could get excessive usage of bot, leading to increased costs. |
@azep-ninja this is great stuff, but would it be possible / not too painful to break up some of the more opinionated changes from the more general stuff? Like the telegram group and team stuff should probably be separate PRs for evaluation. |
@lalalune yeah I can do that! This PR is now updated for the more generic items and separate PRs have been created below for team specific settings per client. |
All issues resolved here as well @odilitime . |
Relates to:
Issue 399
Risks
Low: Additional settings to character.json types are low level changes, and should not break any core functionality. Additionally discord messages.ts and telegram file additions are specific to the new settings, so risk is low.
Background
What does this PR do?
Enhances the Discord and Telegram bot's message handling to support:
What kind of change is this?
Features (non-breaking change which adds functionality)
Improvements (misc. changes to existing features)
Documentation changes needed?
My changes require a change to the project documentation.
Add documentation for new shouldRespondOnlyToMentions config option
Add documentation for new shouldOnlyJoinInAllowedGroups config option
Testing
Where should a reviewer start?
Detailed testing steps
ninja_dev - Discord Username