Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a Pin feature for a Discord bot, allowing users to pin messages that will be automatically reposted when new messages arrive in the channel. The implementation adds three commands: /pin (via modal), /unpin, and a message context menu option to pin selected messages.
Changes:
- Added pin command infrastructure with modal-based and context menu-based pinning
- Implemented automatic message resending when new messages arrive in channels with pinned content
- Added database operations for managing pin settings per channel
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| src/internal/repository/pin_setting.go | Adds DeleteByChannelID method to support unpinning by channel |
| src/internal/bot/handler/messageCreate.go | Implements automatic resending of pinned messages on new message events |
| src/internal/bot/handler/interaction.go | Adds modal submit handling for pin feature |
| src/internal/bot/command/registry.go | Registers pin, unpin, and pin select commands |
| src/internal/bot/command/general/pin.go | Implements /pin command with modal interface and permission checks |
| src/internal/bot/command/general/unpin.go | Implements /unpin command to remove pinned messages |
| src/internal/bot/command/general/pin_select.go | Implements message context menu for pinning selected messages |
| src/internal/bot/command/general/pin_modal.go | Handles modal submission for pin command |
| src/internal/bot/command/commands.go | Registers command contexts for pin-related commands |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // 自分のピン留めメッセージの場合は無視 | ||
| if r.Author != nil && r.Author.ID == s.State.User.ID { | ||
| if len(r.Embeds) == 0 { | ||
| return | ||
| } | ||
| if r.Embeds[0].Footer != nil && strings.Contains(r.Embeds[0].Footer.Text, "Pinned Message") { | ||
| return | ||
| } |
There was a problem hiding this comment.
The check for bot's own messages is incomplete. When r.Author is nil (which can happen in certain edge cases), the code will skip the embed check and proceed to resend the pinned message. This could cause the bot to enter an infinite loop where it keeps deleting and resending its own pinned messages. Move the bot check to be a top-level check before line 91, similar to the pattern at lines 26-29.
| // 自分のピン留めメッセージの場合は無視 | |
| if r.Author != nil && r.Author.ID == s.State.User.ID { | |
| if len(r.Embeds) == 0 { | |
| return | |
| } | |
| if r.Embeds[0].Footer != nil && strings.Contains(r.Embeds[0].Footer.Text, "Pinned Message") { | |
| return | |
| } | |
| // 自分のメッセージや Author が存在しないメッセージの場合は無視 | |
| if r.Author == nil || r.Author.ID == s.State.User.ID { | |
| return |
| repo := repository.NewPinSettingRepository(ctx.DB) | ||
| setting := &model.PinSetting{ | ||
| ID: i.ChannelID, | ||
| URL: sentMessage.ID, | ||
| Title: "Pinned Message", | ||
| Content: message, | ||
| GuildID: i.GuildID, | ||
| ChannelID: i.ChannelID, | ||
| } | ||
|
|
||
| if err := repo.Update(setting); err != nil { | ||
| if err := repo.Create(setting); err != nil { | ||
| replyPinError(s, i, config, "エラー", "ピン留めの保存に失敗しました。") | ||
| return true | ||
| } | ||
| } |
There was a problem hiding this comment.
The pin modal handler does not check if a pinned message already exists in the channel, unlike the PinSelect function which properly validates this at lines 61-64 in pin_select.go. This inconsistency means that using the modal via /pin command could overwrite existing pins without warning, while using the message context menu will reject the operation. Consider adding the same check here: query for existing settings and either reject with an error message or explicitly handle the overwrite case.
| sentMessage, err := s.ChannelMessageSendEmbed(i.ChannelID, embed) | ||
| if err != nil { | ||
| replyPinError(s, i, config, "エラー", "メッセージの送信に失敗しました。") | ||
| return | ||
| } | ||
|
|
||
| setting := &model.PinSetting{ | ||
| ID: i.ChannelID, | ||
| URL: sentMessage.ID, | ||
| Title: "Pinned Message", | ||
| Content: targetMsg.Content, | ||
| GuildID: i.GuildID, | ||
| ChannelID: i.ChannelID, | ||
| } | ||
|
|
||
| err = repo.Create(setting) | ||
| if err != nil { | ||
| replyPinError(s, i, config, "エラー", "ピン留めの保存に失敗しました。") | ||
| return | ||
| } |
There was a problem hiding this comment.
In the pin message context menu handler, after successfully pinning a message, there is no cleanup of the old pinned message if one exists. Unlike the modal handler and resendPinnedMessage function, this code doesn't check for or delete existing pinned messages before creating a new one. This is inconsistent with the error check at lines 61-64 which prevents creating a second pin. If that check were ever removed or if there's a race condition, this could result in multiple pinned messages accumulating in the channel. Consider deleting the old pinned message if settings already exist before the check at line 61.
|
|
||
| func replyPinSuccess(s *discordgo.Session, i *discordgo.InteractionCreate, config *internal.Config, title string) { | ||
| _ = s.InteractionRespond(i.Interaction, &discordgo.InteractionResponse{ | ||
| Type: discordgo.InteractionResponseChannelMessageWithSource, | ||
| Data: &discordgo.InteractionResponseData{ | ||
| Embeds: []*discordgo.MessageEmbed{ | ||
| { | ||
| Title: title, | ||
| Color: config.Colors.Success, | ||
| Timestamp: time.Now().Format(time.RFC3339), | ||
| }, | ||
| }, | ||
| Flags: discordgo.MessageFlagsEphemeral, | ||
| }, | ||
| }) | ||
| } |
There was a problem hiding this comment.
The replyPinSuccess function is defined but never used anywhere in the codebase. Consider removing this unused function to reduce code clutter and maintenance burden.
| func replyPinSuccess(s *discordgo.Session, i *discordgo.InteractionCreate, config *internal.Config, title string) { | |
| _ = s.InteractionRespond(i.Interaction, &discordgo.InteractionResponse{ | |
| Type: discordgo.InteractionResponseChannelMessageWithSource, | |
| Data: &discordgo.InteractionResponseData{ | |
| Embeds: []*discordgo.MessageEmbed{ | |
| { | |
| Title: title, | |
| Color: config.Colors.Success, | |
| Timestamp: time.Now().Format(time.RFC3339), | |
| }, | |
| }, | |
| Flags: discordgo.MessageFlagsEphemeral, | |
| }, | |
| }) | |
| } |
| CustomID: "message", | ||
| Label: "投稿内容", | ||
| Style: discordgo.TextInputParagraph, | ||
| Placeholder: "投稿内容を入力してください。すでにPinされたメッセージがある場合は上書きされます。", |
There was a problem hiding this comment.
The modal placeholder text states "すでにPinされたメッセージがある場合は上書きされます" (existing pinned messages will be overwritten), but this contradicts the actual behavior in pin_select.go (lines 61-64) which prevents pinning when a message already exists. This discrepancy in the user-facing message creates confusion about the expected behavior. Ensure the placeholder text accurately reflects the actual implementation, or align the implementation with this stated behavior by allowing overwrites.
| err = repo.DeleteByChannelID(i.ChannelID) | ||
| if err != nil { | ||
| replyPinError(s, i, config, "エラーが発生しました", "ピン留めの解除中にエラーが発生しました。") | ||
| return | ||
| } |
There was a problem hiding this comment.
The unpin command deletes the pin setting from the database but does not delete the actual pinned message from the Discord channel. Users will see the pinned message remain in the channel even after unpinning. Before deleting from the database (line 49), iterate through the settings and delete each message using s.ChannelMessageDelete(i.ChannelID, setting.URL) to clean up the Discord channel as well.
| func (r *PinSettingRepository) DeleteByChannelID(channelID string) error { | ||
| return r.db.Delete(&model.PinSetting{}, "channel_id = ?", channelID).Error | ||
| } |
There was a problem hiding this comment.
The newly added DeleteByChannelID method lacks test coverage. The repository test file (pin_setting_test.go) contains comprehensive tests for other methods but does not test this new functionality. Add test cases to verify that DeleteByChannelID correctly deletes all pin settings for a given channel ID, preserves settings for other channels, and handles non-existent channel IDs appropriately.
yuito-it
left a comment
There was a problem hiding this comment.
外部キー制約でエラーですね
2026/01/29 19:48:47 /Users/yutaka/Documents/Dev/UniProject/UniBot/src/internal/repository/pin_setting.go:24 ERROR: insert or update on table "pin_settings" violates foreign key constraint "fk_pin_settings_guild" (SQLSTATE 23503)
[21.482ms] [rows:0] INSERT INTO "pin_settings" ("id","url","title","content","guild_id","channel_id","created_at","updated_at") VALUES ('1383480147080118345','1466384575231164531','Pinned Message','test','1383480145486286879','1383480147080118345',1769683727144787000,1769683727144787000)
yuito-it
left a comment
There was a problem hiding this comment.
LGTM!と言いたいところですが...
pin_modalとpin_selectで既存のpin留めがある場合の挙動が異なります。
pin_modalでも警告を出すように統一する方が望ましいと思います。
|
@Aqua-218 ... |
Overview
Pin機能の実装
Issue
#213