#465 make .removequote work for users no longer in Manechat

Izzy-Moonbot/Modules/QuotesModule.cs

@@ -354,7 +354,7 @@ await context.Channel.SendMessageAsync(
             if (quoteUser == null)
                 throw new TargetException("The user this alias referenced to cannot be found.");
 
-            await _quoteService.RemoveQuote(quoteUser, number.Value - 1);
+            await _quoteService.RemoveQuote(quoteUser.Id, number.Value - 1);
 
             await context.Channel.SendMessageAsync(
                 $"Removed quote #{number.Value} from **{quoteUser.DisplayName}** ({quoteUser.Username}).", allowedMentions: AllowedMentions.None);
@@ -368,17 +368,10 @@ await context.Channel.SendMessageAsync(
             return;
         }
 
-        var member = context.Guild.GetUser(userId);
-        if (member == null)
-        {
-            await context.Channel.SendMessageAsync($"Sorry, I couldn't find that user");
-            return;
-        }
-
-        var newUserQuote = await _quoteService.RemoveQuote(member, number.Value - 1);
+        await _quoteService.RemoveQuote(userId, number.Value - 1);
 
         await context.Channel.SendMessageAsync(
-            $"Removed quote #{number.Value} from **{newUserQuote.Name}**.", allowedMentions: AllowedMentions.None);
+            $"Removed quote #{number.Value} from **<@{userId}>**.", allowedMentions: AllowedMentions.None);
         return;
     }
 

Izzy-Moonbot/Service/QuoteService.cs

@@ -163,34 +163,19 @@ public async Task<Quote> AddQuote(IIzzyUser user, string content)
         return new Quote(quoteId, quoteName, content);
     }
 
-    /// <summary>
-    /// Remove a quote from a user.
-    /// </summary>
-    /// <param name="user">The user to remove the quote from.</param>
-    /// <param name="id">The id of the quote to remove.</param>
-    /// <returns>The Quote that was removed.</returns>
-    /// <exception cref="NullReferenceException">If the user doesn't have any quotes.</exception>
-    /// <exception cref="IndexOutOfRangeException">If the quote id provided doesn't exist.</exception>
-    public async Task<Quote> RemoveQuote(IIzzyUser user, int id)
+    public async Task RemoveQuote(ulong userId, int id)
     {
-        if (!_quoteStorage.Quotes.TryGetValue(user.Id.ToString(), out var quotes))
+        if (!_quoteStorage.Quotes.TryGetValue(userId.ToString(), out var quotes))
             throw new NullReferenceException("That user does not have any quotes.");
-        
+
         if (quotes.Count <= id) throw new IndexOutOfRangeException("That quote ID does not exist.");
-
-        var quoteName = user.Username;
-        if (user is IGuildUser guildUser) quoteName = guildUser.DisplayName;
 
-        var quoteContent = quotes[id];
-
-        _quoteStorage.Quotes[user.Id.ToString()].RemoveAt(id);
+        _quoteStorage.Quotes[userId.ToString()].RemoveAt(id);
+
+        if (_quoteStorage.Quotes[userId.ToString()].Count == 0)
+            _quoteStorage.Quotes.Remove(userId.ToString());
 
-        if (_quoteStorage.Quotes[user.Id.ToString()].Count == 0)
-            _quoteStorage.Quotes.Remove(user.Id.ToString());
-
         await FileHelper.SaveQuoteStorageAsync(_quoteStorage);
-
-        return new Quote(id, quoteName, quoteContent);
     }
 }
 

Izzy-MoonbotTests/Tests/QuoteModuleTests.cs

@@ -306,11 +306,39 @@ public async Task AddAndRemoveQuotes_Tests()
         context = await client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, $".removequote <@{sunny.Id}> 1");
         await qm.TestableRemoveQuoteCommandAsync(context, $"<@{sunny.Id}> 1");
 
-        Assert.AreEqual($"Removed quote #1 from **{sunny.GlobalName}**.", generalChannel.Messages.Last().Content);
+        Assert.AreEqual($"Removed quote #1 from **<@{sunny.Id}>**.", generalChannel.Messages.Last().Content);
         TestUtils.AssertListsAreEqual(quotes.Quotes[sunny.Id.ToString()], new List<string> {
             "eat more vegetables"
         });
     }
 
+    // Regression test: This used to incorrectly produce "Sorry, I couldn't find that user",
+    // making a user's quotes effectively impossible to remove after they left the server.
+    [TestMethod()]
+    public async Task RemoveQuote_AncientUser_Test()
+    {
+        // setup
+
+        var (cfg, _, (_, sunny), _, (generalChannel, _, _), guild, client) = TestUtils.DefaultStubs();
+        DiscordHelper.DefaultGuildId = guild.Id;
+
+        // Celestia hasn't been seen since G4
+        var celestiaId = 7;
+        var quotes = new QuoteStorage();
+        quotes.Quotes.Add(celestiaId.ToString(), new List<string> { "my little ponies" });
+
+        var userinfo = new Dictionary<ulong, User>();
+        var qs = new QuoteService(quotes, userinfo);
+        var qm = new QuotesModule(cfg, qs, userinfo);
+
+        // setup end
+
+        var context = await client.AddMessageAsync(guild.Id, generalChannel.Id, sunny.Id, $".removequote {celestiaId} 1");
+        await qm.TestableRemoveQuoteCommandAsync(context, $"{celestiaId} 1");
+
+        Assert.AreEqual($"Removed quote #1 from **<@{celestiaId}>**.", generalChannel.Messages.Last().Content);
+        Assert.IsFalse(quotes.Quotes.ContainsKey(celestiaId.ToString()));
+    }
+
     // TODO: .quotealias command, and aliases in general
 }

Izzy-MoonbotTests/Tests/QuoteServiceTests.cs

@@ -41,7 +41,7 @@ public async Task BasicTests()
             "eat more vegetables"
         }, qs.GetQuotes(sunny.Id));
 
-        await qs.RemoveQuote(sunny, 0);
+        await qs.RemoveQuote(sunny.Id, 0);
 
         TestUtils.AssertListsAreEqual(new List<string> {
             "eat more vegetables"