Repository navigation
enh: Enhance message saving functionality and /channels command - #43
Conversation
- Refactor ChannelsCommand to improve text channel handling and pagination. - Introduce saveMessagesForGuild function for better message saving logic. - Add support for thread creation and deletion events. - Implement getAllTextChannels utility for fetching viewable text channels. - Update localization files for new responses.
There was a problem hiding this comment.
Copilot reviewed 9 out of 11 changed files in this pull request and generated no comments.
Files not reviewed (2)
- src/locales/en-US.json: Language not supported
- src/locales/fr.json: Language not supported
Comments suppressed due to low confidence (2)
src/commands/admin/savechannel.ts:37
- Consider awaiting the saveMessagesForGuild call to properly handle errors and provide timely user feedback. If fire-and-forget is intentional, please document the rationale.
saveMessagesForGuild(interaction.guild!, undefined, interaction, channel.id);
src/commands/admin/save.ts:191
- Ensure that using 'viewableChannels' (which may include non-text channels) to derive channel IDs for cleanup is the intended behavior, as it differs from the earlier use of the filtered text channels.
const channelsIds = viewableChannels.map((channel) => channel.id);
…r handling and readability
There was a problem hiding this comment.
Copilot reviewed 11 out of 13 changed files in this pull request and generated no comments.
Files not reviewed (2)
- src/locales/en-US.json: Language not supported
- src/locales/fr.json: Language not supported
Comments suppressed due to low confidence (2)
src/database/bee-database.ts:16
- Ensure that the immediate invocation of the transaction callback (using () after db.transaction) is fully supported by your database driver API and that error handling within the transaction is sufficient.
db.transaction(() => { ... })();
src/commands/admin/channels.ts:135
- [nitpick] Consider adding a comment to explain the regex replacement for thread formatting to improve code clarity and maintainability.
.replace(/├ ([^\n]*)$/, "└ $1")
…tegrity and fixing a bug
…e accurate message counts
There was a problem hiding this comment.
Copilot reviewed 16 out of 19 changed files in this pull request and generated no comments.
Files not reviewed (3)
- .dockerignore: Language not supported
- src/locales/en-US.json: Language not supported
- src/locales/fr.json: Language not supported
Comments suppressed due to low confidence (2)
src/database/bee-database.ts:16
- Verify that the immediate invocation pattern (i.e. appending '()' after db.transaction) is supported by your DB library. If not, refactor to explicitly commit or rollback the transaction with proper error handling.
db.transaction(() => {
src/events/threads/thread-create.ts:8
- [nitpick] Consider renaming the parameter 'newlyCreate' to 'isNewlyCreated' to enhance code readability and clarity.
async execute(thread: AnyThreadChannel, newlyCreate: boolean) {
There was a problem hiding this comment.
Pull Request Overview
This PR enhances Abeille’s message saving functionality and channel management by refactoring command logic, improving database transactions, and adding support for thread events. Key changes include:
- Refactored ChannelsCommand to support pagination and detailed channel/thread statistics.
- Updated message saving flows with atomic transactions and improved filtering.
- Added new event handlers for thread creation and deletion, along with documentation improvements.
Reviewed Changes
Copilot reviewed 19 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/loaders.ts | Added informative logging for locale loading |
| src/events/threads/thread-delete.ts | Introduced thread deletion handling with message cleanup |
| src/events/threads/thread-create.ts | Added thread creation event with asynchronous join logic |
| src/events/messages/message-create.ts | Updated message saving logic using improved filtering |
| src/events/client-ready.ts | Refactored guild message sync using Promise.all |
| src/database/bee-database.ts | Wrapped inserts and updates in atomic transactions with counter updates |
| src/commands/admin/savechannel.ts | Simplified channel save command by delegating to saveMessagesForGuild |
| src/commands/admin/save.ts | Extended save functionality to filter messages by channel ID |
| src/commands/admin/channels.ts | Implemented pagination and enhanced grouping of channel statistics |
| src/commands/activity/* | Replaced deferred reply variable with direct await for clarity |
| docs/* and README.md | Updated documentation to reflect new features and guidelines |
| CONTRIBUTING.md | Added contributing guidelines and style conventions |
Files not reviewed (4)
- .dockerignore: Language not supported
- .env.template: Language not supported
- src/locales/en-US.json: Language not supported
- src/locales/fr.json: Language not supported
Comments suppressed due to low confidence (1)
src/commands/admin/channels.ts:240
- [nitpick] While updating button states for pagination, ensure that edge cases (such as dynamic changes to the number of pages) are handled to avoid potential UI inconsistencies.
row.components[0]!.setDisabled(currentPage === 0).setLabel("◀");
| // Atomic update with check to prevent negative counter | ||
| const updateStatement = db.query(` | ||
| UPDATE messageday | ||
| SET count = MAX(0, count ${isDelete ? "-" : "+"} 1) |
There was a problem hiding this comment.
[nitpick] The inline arithmetic operation in the SQL query may be misinterpreted due to tight formatting; consider adding explicit spacing or parentheses to improve clarity and maintainability.
| SET count = MAX(0, count ${isDelete ? "-" : "+"} 1) | |
| SET count = MAX(0, (count ${isDelete ? " - " : " + "} 1)) |
| threadManager: ThreadManager, | ||
| textChannels: Collection<string, GuildBasedChannel & TextBasedChannel> | ||
| ): Promise<void> { | ||
| const activeChannelThreads = await threadManager.fetchActive(true); |
There was a problem hiding this comment.
I have a "Missing access" error when running fetchActive and fetchArchived for every text channels in my test server :(
I don't know why (yet), but it does stop the saving process. Could you add something to catch the error?
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the message saving functionality and improves channel management commands. Key changes include:
- Refactoring message saving logic (including handling threads) and optimizing database transactions.
- Adding events for thread creation and deletion.
- Improving the /channels command with pagination and enhanced channel statistics formatting.
- Updating documentation to reflect new features, architecture, and security best practices.
Reviewed Changes
Copilot reviewed 19 out of 23 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/loaders.ts | Added logging for locale loading progress. |
| src/events/threads/thread-delete.ts | New event to handle thread deletion and cleanup messages. |
| src/events/threads/thread-create.ts | New event to handle thread creation and joining new threads. |
| src/events/messages/message-create.ts | Updated message filtering logic with improved filtering function. |
| src/events/client-ready.ts | Refactored guild message syncing logic using Promise.all. |
| src/database/bee-database.ts | Refactored message saving transactions and daily counter updates. |
| src/commands/admin/savechannel.ts | Simplified channel saving by calling saveMessagesForGuild. |
| src/commands/admin/save.ts | Improved message fetching and thread handling in save flow. |
| src/commands/admin/channels.ts | Enhanced channel statistics command with embed pagination. |
| src/commands/activity/* | Removed redundant reply deferral in activity commands. |
| docs/*, README.md, CONTRIBUTING.md | Updated and added documentation regarding architecture and security. |
Files not reviewed (4)
- .dockerignore: Language not supported
- .env.template: Language not supported
- src/locales/en-US.json: Language not supported
- src/locales/fr.json: Language not supported
Comments suppressed due to low confidence (2)
src/events/threads/thread-create.ts:8
- [nitpick] Consider renaming the 'newlyCreate' parameter to 'newlyCreated' for improved clarity and consistency.
async execute(thread: AnyThreadChannel, newlyCreate: boolean) {
src/database/bee-database.ts:42
- The immediately invoked transaction function pattern may be unconventional. Verify that the db.transaction API supports this usage pattern without introducing side effects.
})();
This PR also:
Closes #29
Closes #40