From 00ee66523afd6019fa7d081f6667d42c19cde217 Mon Sep 17 00:00:00 2001 From: Michael McCarty Date: Fri, 23 Oct 2026 17:39:02 -0700 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20feat:=20avoid=20deleting=20empty/ol?= =?UTF-8?q?d=20list=20messages=20and=20add=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/boost/boost.go | 43 ++++ src/boost/boost_button_reactions.go | 56 ++++- src/boost/boost_reactions.go | 35 +++- src/boost/message_redraw.go | 26 ++- src/boost/stale_boost_list_test.go | 303 ++++++++++++++++++++++++++++ src/dc/dctest/events.go | 28 +++ src/dc/dctest/fake.go | 6 + src/dc/event.go | 12 ++ 8 files changed, 489 insertions(+), 20 deletions(-) create mode 100644 src/boost/stale_boost_list_test.go diff --git a/src/boost/boost.go b/src/boost/boost.go index 07746266..1f3cd325 100644 --- a/src/boost/boost.go +++ b/src/boost/boost.go @@ -483,6 +483,49 @@ func setChickenRunMessageID(contract *Contract, channelID, messageID string) { saveData(contract.ContractHash) } +// findLocation returns the LocationData for the given channelID, or nil if not found. +func (c *Contract) findLocation(channelID string) *LocationData { + if c == nil || channelID == "" { + return nil + } + for _, loc := range c.Location { + if loc.ChannelID == channelID { + return loc + } + } + return nil +} + +// isOldBoostListMessage checks if messageID belongs to contract in channelID as a former boost list message +// that is no longer the active boost list. +func isOldBoostListMessage(contract *Contract, channelID string, messageID string) bool { + if contract == nil || channelID == "" || messageID == "" { + return false + } + for _, loc := range contract.Location { + if loc.ChannelID == channelID { + if messageID == loc.ListMsgID || messageID == loc.ReactionID { + return false + } + if contract.CRMessageIDs != nil && contract.CRMessageIDs[channelID] == messageID { + return false + } + return slices.Contains(loc.MessageIDs, messageID) + } + } + return false +} + +// cleanupOldBoostListMessage deletes an old boost list message from Discord. +func cleanupOldBoostListMessage(client dc.Client, contract *Contract, channelID string, messageID string) { + if client == nil || contract == nil || channelID == "" || messageID == "" { + return + } + log.Printf("cleanupOldBoostListMessage: deleting stale boost list message %s in channel %s for contract %s", + messageID, channelID, contract.ContractHash) + _ = client.DeleteMessage(channelID, messageID) +} + func getTokenCountString(tokenStr string, tokensWanted int, tokensReceived int) (string, string) { countStr := "" signupCountStr := "" diff --git a/src/boost/boost_button_reactions.go b/src/boost/boost_button_reactions.go index 0673ab55..45c3c5de 100644 --- a/src/boost/boost_button_reactions.go +++ b/src/boost/boost_button_reactions.go @@ -91,26 +91,66 @@ func HandleContractReactions(client dc.Client, e *dc.ComponentEvent) { redraw := false + // Check if this interaction came from an old/stale boost list message + isStale := isOldBoostListMessage(contract, e.ChannelID(), e.MessageID()) + if !isStale { + if loc := contract.findLocation(e.ChannelID()); loc != nil { + if loc.ListMsgID != "" && e.MessageID() != loc.ListMsgID && e.MessageID() != loc.ReactionID { + isStale = true + } + } + } + + if isStale { + cleanupOldBoostListMessage(client, contract, e.ChannelID(), e.MessageID()) + redraw = true + } + switch cmd { case "boost": - if e.MessageID() == contract.Location[0].ListMsgID { - redraw = buttonReactionBoost(client, e.GuildID(), e.ChannelID(), contract, userID) + if isStale { + redraw = true + } else { + loc := contract.findLocation(e.ChannelID()) + if loc != nil && e.MessageID() == loc.ListMsgID { + redraw = buttonReactionBoost(client, e.GuildID(), e.ChannelID(), contract, userID) + } } case "bag": - _, redraw = buttonReactionBag(client, e.GuildID(), e.ChannelID(), contract, userID) + var r bool + _, r = buttonReactionBag(client, e.GuildID(), e.ChannelID(), contract, userID) + if r || isStale { + redraw = true + } case "token": - _, redraw = buttonReactionToken(client, e.GuildID(), e.ChannelID(), contract, userID, 1, "") + var r bool + _, r = buttonReactionToken(client, e.GuildID(), e.ChannelID(), contract, userID, 1, "") + if r || isStale { + redraw = true + } sendOrUpdateUserReactionSummary(e, contract, userID) case "2token": - _, redraw = buttonReactionToken(client, e.GuildID(), e.ChannelID(), contract, userID, 2, "") + var r bool + _, r = buttonReactionToken(client, e.GuildID(), e.ChannelID(), contract, userID, 2, "") + if r || isStale { + redraw = true + } sendOrUpdateUserReactionSummary(e, contract, userID) case "swap": - redraw = buttonReactionSwap(client, e.GuildID(), e.ChannelID(), contract, userID) + if !isStale { + redraw = buttonReactionSwap(client, e.GuildID(), e.ChannelID(), contract, userID) + } case "last": - _, redraw = buttonReactionLast(client, e.GuildID(), e.ChannelID(), contract, userID) + if !isStale { + _, redraw = buttonReactionLast(client, e.GuildID(), e.ChannelID(), contract, userID) + } case "cr": var str string - redraw, str = buttonReactionRunChickens(client, contract, userID) + var r bool + r, str = buttonReactionRunChickens(client, contract, userID) + if r || isStale { + redraw = true + } // Ack the message for every other command _ = e.Followup(dc.Message{ Content: str, diff --git a/src/boost/boost_reactions.go b/src/boost/boost_reactions.go index c2725845..c71fefc6 100644 --- a/src/boost/boost_reactions.go +++ b/src/boost/boost_reactions.go @@ -27,6 +27,12 @@ func ReactionAdd(client dc.Client, e *dc.ReactionEvent) string { return returnVal } + isStale := isOldBoostListMessage(contract, e.ChannelID(), e.MessageID()) + if isStale { + cleanupOldBoostListMessage(client, contract, e.ChannelID(), e.MessageID()) + redraw = true + } + defer saveData(contract.ContractHash) if emojiName == "โš™๏ธ" || emojiName == "โš™" { @@ -107,25 +113,30 @@ func ReactionAdd(client dc.Client, e *dc.ReactionEvent) string { switch e.EmojiName() { case boostIconName: - if e.MessageID() == contract.Location[0].ListMsgID { + loc := contract.findLocation(e.ChannelID()) + if loc != nil && e.MessageID() == loc.ListMsgID { result := buttonReactionBoost(client, e.GuildID(), e.ChannelID(), contract, userID) if result { return returnVal } } case "๐Ÿ”ƒ": - result := buttonReactionSwap(client, e.GuildID(), e.ChannelID(), contract, userID) - if result { - return returnVal + if !isStale { + result := buttonReactionSwap(client, e.GuildID(), e.ChannelID(), contract, userID) + if result { + return returnVal + } } case "โคต๏ธ": - willReturn := false - willReturn, redraw = buttonReactionLast(client, e.GuildID(), e.ChannelID(), contract, userID) - if willReturn { - return returnVal + if !isStale { + willReturn := false + willReturn, redraw = buttonReactionLast(client, e.GuildID(), e.ChannelID(), contract, userID) + if willReturn { + return returnVal + } } case "๐Ÿšฝ": - if contract.Boosters[userID].BoostState == BoostStateUnboosted { + if !isStale && contract.Boosters[userID].BoostState == BoostStateUnboosted { // Bounds check: ensure currentBoosterIdx is valid before using it if currentBoosterIdx < 0 { _, _ = client.SendMessage(e.ChannelID(), dc.Message{Content: "Unable to move booster right now because the current booster position could not be determined."}) @@ -214,7 +225,11 @@ func ReactionAdd(client dc.Client, e *dc.ReactionEvent) string { userID := e.UserID() if strings.ToLower(e.EmojiName()) == tokenReactionStr { - _, redraw = buttonReactionToken(client, e.GuildID(), e.ChannelID(), contract, userID, 1, "") + var r bool + _, r = buttonReactionToken(client, e.GuildID(), e.ChannelID(), contract, userID, 1, "") + if r || isStale { + redraw = true + } } } else { keepReaction = false diff --git a/src/boost/message_redraw.go b/src/boost/message_redraw.go index 198e59f2..7fc0fd2b 100644 --- a/src/boost/message_redraw.go +++ b/src/boost/message_redraw.go @@ -84,7 +84,18 @@ func RedrawBoostList(client dc.Client, guildID string, channelID string) error { buttonComponents := getContractReactionsComponents(contract) for _, loc := range contract.Location { if loc.GuildID == guildID && loc.ChannelID == channelID { - _ = client.DeleteMessage(loc.ChannelID, loc.ListMsgID) + oldMsgID := loc.ListMsgID + if oldMsgID != "" { + if err := client.DeleteMessage(loc.ChannelID, oldMsgID); err != nil { + log.Printf("RedrawBoostList: unable to delete old boost list message %s in channel %s for contract %s: %v", + oldMsgID, loc.ChannelID, contract.ContractHash, err) + time.AfterFunc(2*time.Second, func() { + if retryErr := client.DeleteMessage(loc.ChannelID, oldMsgID); retryErr != nil { + log.Printf("RedrawBoostList: retry delete old boost list message %s failed: %v", oldMsgID, retryErr) + } + }) + } + } var components []dc.LayoutComponent listComp := DrawBoostList(contract) components = append(components, listComp...) @@ -232,7 +243,18 @@ func sendNextNotification(client dc.Client, contract *Contract, pingUsers bool) if contract.State == ContractStateArchive { _ = client.UnpinMessage(loc.ChannelID, loc.ReactionID) } - _ = client.DeleteMessage(loc.ChannelID, loc.ListMsgID) + oldMsgID := loc.ListMsgID + if oldMsgID != "" { + if err := client.DeleteMessage(loc.ChannelID, oldMsgID); err != nil { + log.Printf("sendNextNotification: unable to delete old boost list message %s in channel %s for contract %s: %v", + oldMsgID, loc.ChannelID, contract.ContractHash, err) + time.AfterFunc(2*time.Second, func() { + if retryErr := client.DeleteMessage(loc.ChannelID, oldMsgID); retryErr != nil { + log.Printf("sendNextNotification: retry delete old boost list message %s failed: %v", oldMsgID, retryErr) + } + }) + } + } // Compose the message without a Ping listComp := DrawBoostList(contract) diff --git a/src/boost/stale_boost_list_test.go b/src/boost/stale_boost_list_test.go new file mode 100644 index 00000000..1c196f43 --- /dev/null +++ b/src/boost/stale_boost_list_test.go @@ -0,0 +1,303 @@ +package boost + +import ( + "testing" + "time" + + "github.com/mkmccarty/TokenTimeBoostBot/src/dc/dctest" +) + +func TestIsOldBoostListMessage(t *testing.T) { + contract := &Contract{ + ContractHash: "test-stale-hash", + Location: []*LocationData{ + { + ChannelID: "200000000000000001", + ListMsgID: "300000000000000100", + ReactionID: "300000000000000001", + MessageIDs: []string{"300000000000000001", "300000000000000010", "300000000000000020", "300000000000000100"}, + }, + }, + CRMessageIDs: map[string]string{ + "200000000000000001": "300000000000000050", + }, + } + + // Active list message is not old + if isOldBoostListMessage(contract, "200000000000000001", "300000000000000100") { + t.Errorf("expected isOldBoostListMessage to be false for active ListMsgID") + } + + // Signup message is not an old boost list + if isOldBoostListMessage(contract, "200000000000000001", "300000000000000001") { + t.Errorf("expected isOldBoostListMessage to be false for ReactionID") + } + + // CR message is not an old boost list + if isOldBoostListMessage(contract, "200000000000000001", "300000000000000050") { + t.Errorf("expected isOldBoostListMessage to be false for CR message") + } + + // Previous list messages should be identified as old + if !isOldBoostListMessage(contract, "200000000000000001", "300000000000000010") { + t.Errorf("expected isOldBoostListMessage to be true for previous list message") + } + if !isOldBoostListMessage(contract, "200000000000000001", "300000000000000020") { + t.Errorf("expected isOldBoostListMessage to be true for previous list message") + } + + // Unrelated message ID should return false + if isOldBoostListMessage(contract, "200000000000000001", "300000000000000999") { + t.Errorf("expected isOldBoostListMessage to be false for unknown message") + } + + // Unrelated channel should return false + if isOldBoostListMessage(contract, "200000000000000999", "300000000000000010") { + t.Errorf("expected isOldBoostListMessage to be false for unknown channel") + } +} + +func setupStaleTestContract(contractHash, channelID, currentListMsgID, oldListMsgID, signupMsgID string) *Contract { + const userAID = "400000000000000001" + const userBID = "400000000000000002" + + c := &Contract{ + ContractHash: contractHash, + ContractID: "test-contract", + CoopID: "test-coop", + StartTime: time.Now(), + EstimatedDuration: 2 * time.Hour, + Style: ContractFlagFastrun, + State: ContractStateFastrun, + CurrentBoosterUserID: userAID, + CreatorID: []string{"400000000000000000"}, + Order: []string{userAID, userBID}, + Boosters: make(map[string]*Booster), + Location: []*LocationData{ + { + GuildID: "100000000000000001", + ChannelID: channelID, + ListMsgID: currentListMsgID, + ReactionID: signupMsgID, + MessageIDs: []string{signupMsgID, oldListMsgID, currentListMsgID}, + }, + }, + } + + c.Boosters[userAID] = &Booster{ + UserID: userAID, + Name: "Farmer A", + Nick: "Farmer A", + BoostState: BoostStateTokenTime, + TokensWanted: 6, + TokensReceived: 0, + StartTime: time.Now(), + } + c.Boosters[userBID] = &Booster{ + UserID: userBID, + Name: "Farmer B", + Nick: "Farmer B", + BoostState: BoostStateUnboosted, + TokensWanted: 6, + TokensReceived: 0, + } + + ContractsMutex.Lock() + Contracts[contractHash] = c + ContractsMutex.Unlock() + + return c +} + +func TestStaleBoostList_TokenButtonClickCleansUpOldList(t *testing.T) { + contractHash := "test-race-token-btn" + chanID := "200000000000000001" + activeListID := "300000000000000100" + oldListID := "300000000000000090" + signupID := "300000000000000001" + userAID := "400000000000000001" + userBID := "400000000000000002" + + contract := setupStaleTestContract(contractHash, chanID, activeListID, oldListID, signupID) + defer func() { + ContractsMutex.Lock() + delete(Contracts, contractHash) + ContractsMutex.Unlock() + }() + + client := dctest.New() + + // User B clicks token button on the old message (oldListID) + event := dctest.ComponentButtonEventFull("100000000000000001", chanID, oldListID, userBID, "rc_#token#"+contractHash) + + HandleContractReactions(client, event) + + // 1. Verify old message was deleted + deleteCalls := client.CallsTo("DeleteMessage") + foundDelete := false + for _, call := range deleteCalls { + if len(call.Args) >= 2 && call.Args[0] == chanID && call.Args[1] == oldListID { + foundDelete = true + break + } + } + if !foundDelete { + t.Errorf("expected DeleteMessage to be called for stale message %s, calls: %+v", oldListID, deleteCalls) + } + + // 2. Verify token was credited to the active booster (userA) + contract.mutex.Lock() + tokensRecv := contract.Boosters[userAID].TokensReceived + contract.mutex.Unlock() + if tokensRecv != 1 { + t.Errorf("expected booster userA to receive 1 token despite clicking on old message, got %d", tokensRecv) + } + + // 3. Verify active boost list message was refreshed via EditMessage + editCalls := client.CallsTo("EditMessage") + foundEditActive := false + for _, call := range editCalls { + if len(call.Args) >= 2 && call.Args[0] == chanID && call.Args[1] == activeListID { + foundEditActive = true + break + } + } + if !foundEditActive { + t.Errorf("expected EditMessage on active list %s, got calls: %+v", activeListID, editCalls) + } +} + +func TestStaleBoostList_RocketButtonClickCleansUpWithoutAdvancing(t *testing.T) { + contractHash := "test-race-rocket-btn" + chanID := "200000000000000002" + activeListID := "300000000000000200" + oldListID := "300000000000000190" + signupID := "300000000000000002" + userAID := "400000000000000001" + userBID := "400000000000000002" + + contract := setupStaleTestContract(contractHash, chanID, activeListID, oldListID, signupID) + defer func() { + ContractsMutex.Lock() + delete(Contracts, contractHash) + ContractsMutex.Unlock() + }() + + client := dctest.New() + + // Active booster userA clicks rocket on stale message oldListID + event := dctest.ComponentButtonEventFull("100000000000000001", chanID, oldListID, userAID, "rc_#boost#"+contractHash) + + HandleContractReactions(client, event) + + // 1. Verify stale message was deleted + deleteCalls := client.CallsTo("DeleteMessage") + foundDelete := false + for _, call := range deleteCalls { + if len(call.Args) >= 2 && call.Args[0] == chanID && call.Args[1] == oldListID { + foundDelete = true + break + } + } + if !foundDelete { + t.Errorf("expected DeleteMessage to be called for stale message %s, calls: %+v", oldListID, deleteCalls) + } + + // 2. Verify contract did NOT advance again (userA is still active booster, not userB) + if contract.Boosters[userAID].BoostState != BoostStateTokenTime { + t.Errorf("expected userA to remain in BoostStateTokenTime, got %v", contract.Boosters[userAID].BoostState) + } + if contract.Boosters[userBID].BoostState != BoostStateUnboosted { + t.Errorf("expected userB to remain unboosted, got %v", contract.Boosters[userBID].BoostState) + } +} + +func TestStaleBoostList_TokenEmojiReactionCleansUpOldList(t *testing.T) { + contractHash := "test-race-token-emoji" + chanID := "200000000000000003" + activeListID := "300000000000000300" + oldListID := "300000000000000290" + signupID := "300000000000000003" + userAID := "400000000000000001" + userBID := "400000000000000002" + + contract := setupStaleTestContract(contractHash, chanID, activeListID, oldListID, signupID) + defer func() { + ContractsMutex.Lock() + delete(Contracts, contractHash) + ContractsMutex.Unlock() + }() + + client := dctest.New() + + // User B reacts with "token" emoji on the old message (oldListID) + rxEvent := dctest.ReactionEvent("100000000000000001", chanID, oldListID, userBID, "token") + + ReactionAdd(client, rxEvent) + + // 1. Verify old message was deleted + deleteCalls := client.CallsTo("DeleteMessage") + foundDelete := false + for _, call := range deleteCalls { + if len(call.Args) >= 2 && call.Args[0] == chanID && call.Args[1] == oldListID { + foundDelete = true + break + } + } + if !foundDelete { + t.Errorf("expected DeleteMessage to be called for stale message %s, calls: %+v", oldListID, deleteCalls) + } + + // 2. Verify token was credited to booster userA + contract.mutex.Lock() + tokensRecv := contract.Boosters[userAID].TokensReceived + contract.mutex.Unlock() + if tokensRecv != 1 { + t.Errorf("expected booster userA to receive 1 token from emoji reaction on old message, got %d", tokensRecv) + } +} + +func TestStaleBoostList_RocketEmojiReactionCleansUpWithoutAdvancing(t *testing.T) { + contractHash := "test-race-rocket-emoji" + chanID := "200000000000000004" + activeListID := "300000000000000400" + oldListID := "300000000000000390" + signupID := "300000000000000004" + userAID := "400000000000000001" + userBID := "400000000000000002" + + contract := setupStaleTestContract(contractHash, chanID, activeListID, oldListID, signupID) + defer func() { + ContractsMutex.Lock() + delete(Contracts, contractHash) + ContractsMutex.Unlock() + }() + + client := dctest.New() + + // User A reacts with rocket emoji on stale message oldListID + rxEvent := dctest.ReactionEvent("100000000000000001", chanID, oldListID, userAID, boostIconName) + + ReactionAdd(client, rxEvent) + + // 1. Verify old message was deleted + deleteCalls := client.CallsTo("DeleteMessage") + foundDelete := false + for _, call := range deleteCalls { + if len(call.Args) >= 2 && call.Args[0] == chanID && call.Args[1] == oldListID { + foundDelete = true + break + } + } + if !foundDelete { + t.Errorf("expected DeleteMessage to be called for stale message %s, calls: %+v", oldListID, deleteCalls) + } + + // 2. Verify userA is still the active booster and was not advanced again + if contract.Boosters[userAID].BoostState != BoostStateTokenTime { + t.Errorf("expected userA to remain in BoostStateTokenTime, got %v", contract.Boosters[userAID].BoostState) + } + if contract.Boosters[userBID].BoostState != BoostStateUnboosted { + t.Errorf("expected userB to remain unboosted, got %v", contract.Boosters[userBID].BoostState) + } +} diff --git a/src/dc/dctest/events.go b/src/dc/dctest/events.go index 4ac3f45c..69eeb084 100644 --- a/src/dc/dctest/events.go +++ b/src/dc/dctest/events.go @@ -117,6 +117,34 @@ func ComponentButtonEventWithUser(customID, userID string) *dc.ComponentEvent { return event } +// ComponentButtonEventFull builds a dc.ComponentEvent for a button click with specific guild, channel, message, user, and customID. +func ComponentButtonEventFull(guildID, channelID, messageID, userID, customID string) *dc.ComponentEvent { + payload := fmt.Sprintf(`{ + "id": "1", + "application_id": "2", + "type": 3, + "token": "test-token", + "version": 1, + "guild_id": %q, + "channel": {"id": %q, "type": 0}, + "user": {"id": %q, "username": "tester", "discriminator": "0"}, + "message": {"id": %q, "channel_id": %q, "content": "", "timestamp": "2026-01-01T00:00:00Z", + "author": {"id": "1", "username": "bot", "discriminator": "0"}}, + "data": {"custom_id": %q, "component_type": 2} + }`, guildID, channelID, userID, messageID, channelID, customID) + + event, err := dc.NewComponentEventFromPayload([]byte(payload)) + if err != nil { + panic(fmt.Sprintf("dctest: building a button event: %v", err)) + } + return event +} + +// ReactionEvent builds a dc.ReactionEvent with the given parameters. +func ReactionEvent(guildID, channelID, messageID, userID, emojiName string) *dc.ReactionEvent { + return dc.NewReactionEvent(guildID, channelID, messageID, userID, emojiName, "") +} + // AutocompleteEvent builds a dc.AutocompleteEvent for an autocomplete interaction. func AutocompleteEvent(commandName, optionName, optionValue string) *dc.AutocompleteEvent { return AutocompleteEventWithUser(commandName, optionName, optionValue, "4") diff --git a/src/dc/dctest/fake.go b/src/dc/dctest/fake.go index 8d691c1f..bcdb82b1 100644 --- a/src/dc/dctest/fake.go +++ b/src/dc/dctest/fake.go @@ -316,6 +316,12 @@ func (f *FakeClient) DeleteMessage(channelID, messageID string) error { return f.DeleteErr } +// RemoveMessageReaction records the reaction removal. +func (f *FakeClient) RemoveMessageReaction(channelID, messageID, emojiID, userID string) error { + f.record("RemoveMessageReaction", channelID, messageID, emojiID, userID) + return nil +} + // CreateGuildRole records the request and hands back a role with a fresh ID, // which is what a guild that accepted the create looks like. func (f *FakeClient) CreateGuildRole(guildID string, params dc.RoleParams) (*dc.Role, error) { diff --git a/src/dc/event.go b/src/dc/event.go index 796ba568..89f9af28 100644 --- a/src/dc/event.go +++ b/src/dc/event.go @@ -839,6 +839,18 @@ func reactionEventFrom(r *events.GenericReaction) *ReactionEvent { return event } +// NewReactionEvent creates a ReactionEvent with the specified fields (useful for testing). +func NewReactionEvent(guildID, channelID, messageID, userID, emojiName, emojiID string) *ReactionEvent { + return &ReactionEvent{ + guildID: guildID, + channelID: channelID, + messageID: messageID, + userID: userID, + emojiName: emojiName, + emojiID: emojiID, + } +} + // MessageID is the message the reaction is on. func (e *ReactionEvent) MessageID() string { return e.messageID }