Mostly finished, needs to be double-checked/documented

PluralKit.Bot/Commands/SystemList.cs

@@ -5,17 +5,25 @@ using System.Threading.Tasks;
 
 using Dapper;
 
+using NodaTime;
+
 using PluralKit.Core;
 
+using Serilog;
+
 namespace PluralKit.Bot
 {
     public class SystemList
     {
+        private readonly IClock _clock;
         private readonly DbConnectionFactory _db;
+        private readonly ILogger _logger;
         
-        public SystemList(DbConnectionFactory db)
+        public SystemList(DbConnectionFactory db, ILogger logger, IClock clock)
         {
             _db = db;
+            _logger = logger;
+            _clock = clock;
         }
 
         public async Task MemberList(Context ctx, PKSystem target)
@@ -34,8 +42,8 @@ namespace PluralKit.Bot
                 GetEmbedTitle(target, opts),
                 (eb, ms) =>
                 {
-                    eb.WithFooter($"{members.Count} total.");
+                    eb.WithFooter($"{opts.CreateFilterString()}. {members.Count} results.");
-                    renderer.RenderPage(eb, ms);
+                    renderer.RenderPage(eb, ctx.System, ms);
                     return Task.CompletedTask;
                 });
         }
@@ -43,8 +51,15 @@ namespace PluralKit.Bot
         private async Task<IReadOnlyList<PKListMember>> GetMemberList(PKSystem target, SortFilterOptions opts)
         {
             using var conn = await _db.Obtain();
+            var query = opts.BuildQuery();
             var args = new {System = target.Id, opts.Filter};
-            return (await conn.QueryAsync<PKListMember>(opts.BuildQuery(), args)).ToList();
+
+            var timeBefore = _clock.GetCurrentInstant();
+            var results = (await conn.QueryAsync<PKListMember>(query, args)).ToList();
+            var timeAfter = _clock.GetCurrentInstant();
+            _logger.Debug("Executing sort/filter query `{Query}` with arguments {Args} returning {ResultCount} results in {QueryTime}", query, args, results.Count, timeAfter - timeBefore);
+
+            return results;
         }
 
         private string GetEmbedTitle(PKSystem target, SortFilterOptions opts)
@@ -54,7 +69,7 @@ namespace PluralKit.Bot
             if (target.Name != null) title.Append($"{target.Name.SanitizeMentions()} (`{target.Hid}`)");
             else title.Append($"`{target.Hid}`");
  
-            if (opts.Filter != null) title.Append($"matching **{opts.Filter.SanitizeMentions()}**");
+            if (opts.Filter != null) title.Append($" matching **{opts.Filter.SanitizeMentions()}**");
             
             return title.ToString();
         }

PluralKit.Bot/Lists/IListRenderer.cs

@@ -2,11 +2,13 @@
 
 using DSharpPlus.Entities;
 
+using PluralKit.Core;
+
 namespace PluralKit.Bot
 {
     public interface IListRenderer
     {
         int MembersPerPage { get; }
-        void RenderPage(DiscordEmbedBuilder eb, IEnumerable<PKListMember> members);
+        void RenderPage(DiscordEmbedBuilder eb, PKSystem system, IEnumerable<PKListMember> members);
     }
 }

PluralKit.Bot/Lists/LongRenderer.cs

@@ -5,6 +5,8 @@ using DSharpPlus.Entities;
 
 using Humanizer;
 
+using NodaTime;
+
 using PluralKit.Core;
 
 namespace PluralKit.Bot
@@ -19,7 +21,7 @@ namespace PluralKit.Bot
             _fields = fields;
         }
 
-        public void RenderPage(DiscordEmbedBuilder eb, IEnumerable<PKListMember> members)
+        public void RenderPage(DiscordEmbedBuilder eb, PKSystem system, IEnumerable<PKListMember> members)
         {
             foreach (var m in members)
             {
@@ -29,6 +31,8 @@ namespace PluralKit.Bot
                 if (_fields.ShowBirthday && m.Birthday != null) profile += $"\n**Birthdate**: {m.BirthdayString}";
                 if (_fields.ShowPronouns && m.ProxyTags.Count > 0) profile += $"\n**Proxy tags:** {m.ProxyTagsString()}";
                 if (_fields.ShowMessageCount && m.MessageCount > 0) profile += $"\n**Message count:** {m.MessageCount}";
+                if (_fields.ShowLastMessage && m.LastMessage != null) profile += $"\n**Last message:** {FormatTimestamp(system, DiscordUtils.SnowflakeToInstant(m.LastMessage.Value))}";
+                if (_fields.ShowLastSwitch && m.LastSwitchTime != null) profile += $"\n**Last switched in:** {FormatTimestamp(system, m.LastSwitchTime.Value)}";
                 if (_fields.ShowDescription && m.Description != null) profile += $"\n\n{m.Description}";
                 if (_fields.ShowPrivacy && m.MemberPrivacy == PrivacyLevel.Private)
                     profile += "\n*(this member is private)*";
@@ -37,25 +41,32 @@ namespace PluralKit.Bot
             }
         }
 
+        private static string FormatTimestamp(PKSystem system, Instant timestamp) => DateTimeFormats.ZonedDateTimeFormat.Format(timestamp.InZone(system.Zone ?? DateTimeZone.Utc));
+
         public class MemberFields
         {
             public bool ShowDisplayName = true;
-            public bool ShowCreated = true;
+            public bool ShowCreated = false;
-            public bool ShowMessageCount = true;
             public bool ShowPronouns = true;
             public bool ShowBirthday = true;
             public bool ShowProxyTags = true;
             public bool ShowDescription = true;
             public bool ShowPrivacy = true;
             
+            public bool ShowMessageCount = false;
+            public bool ShowLastSwitch = false;
+            public bool ShowLastMessage = false;
+
             public static MemberFields FromFlags(Context ctx)
             {
-                // TODO
+                var def = new MemberFields();
-                return new MemberFields
+                if (ctx.MatchFlag("with-last-switch", "with-last-fronted", "with-last-front", "wls", "wlf"))
-                {
+                    def.ShowLastSwitch = true;
-                    ShowMessageCount = false,
+                if (ctx.MatchFlag("with-message-count", "wmc"))
-                    ShowCreated = false
+                    def.ShowMessageCount = true;
-                };
+                if (ctx.MatchFlag("with-last-message", "with-last-proxy", "wlm", "wlp"))
+                    def.ShowLastMessage = true;
+                return def;
             }
         }
     }

PluralKit.Bot/Lists/PKListMember.cs

@@ -1,9 +1,13 @@
-using PluralKit.Core;
+using NodaTime;
+
+using PluralKit.Core;
 
 namespace PluralKit.Bot
 {
     public class PKListMember: PKMember
     {
         public int MessageCount { get; set; }
+        public ulong? LastMessage { get; set; }
+        public Instant? LastSwitchTime { get; set; }
     }
 }

PluralKit.Bot/Lists/ShortRenderer.cs

@@ -4,15 +4,17 @@ using System.Linq;
 
 using DSharpPlus.Entities;
 
+using PluralKit.Core;
+
 namespace PluralKit.Bot
 {
     public class ShortRenderer: IListRenderer
     {
         public int MembersPerPage => 25;
         
-        public void RenderPage(DiscordEmbedBuilder eb, IEnumerable<PKListMember> members)
+        public void RenderPage(DiscordEmbedBuilder eb, PKSystem system, IEnumerable<PKListMember> members)
         {
-            eb.Description = string.Join("\n", members.Select(m =>
+            string RenderLine(PKListMember m)
             {
                 if (m.HasProxyTags)
                 {
@@ -24,7 +26,9 @@ namespace PluralKit.Bot
                 }
 
                 return $"[`{m.Hid}`] **{m.Name.SanitizeMentions()}**";
-            }));
+            }
+
+            eb.Description = string.Join("\n", members.Select(RenderLine));
         }
     }
 }

PluralKit.Bot/Lists/SortFilterOptions.cs

@@ -1,4 +1,5 @@
-using System.Text;
+using System;
+using System.Text;
 
 using PluralKit.Core;
 
@@ -7,45 +8,63 @@ namespace PluralKit.Bot
     public class SortFilterOptions
     {
         public SortProperty SortProperty = SortProperty.Name;
-        public SortDirection Direction = SortDirection.Ascending;
+        public bool Reverse = false;
         public PrivacyFilter PrivacyFilter = PrivacyFilter.PublicOnly;
         public string Filter = null;
         public bool SearchInDescription = false;
 
         public string CreateFilterString()
         {
-            // TODO
+            StringBuilder str = new StringBuilder();
-            return "uwu";
+            str.Append("Sorting by ");
+            str.Append(SortProperty switch
+            {
+                SortProperty.Name => "member name",
+                SortProperty.Hid => "member ID",
+                SortProperty.DisplayName => "display name",
+                SortProperty.CreationDate => "creation date",
+                SortProperty.LastMessage => "last message",
+                SortProperty.LastSwitch => "last switch",
+                SortProperty.MessageCount => "message count",
+                SortProperty.Birthdate => "birthday",
+                _ => new ArgumentOutOfRangeException($"Couldn't find readable string for sort property {SortProperty}")
+            });
+            
+            if (Filter != null)
+            {
+                str.Append($", searching for \"{Filter}\"");
+                if (SearchInDescription) str.Append(" (including description)");
+            }
+
+            str.Append(PrivacyFilter switch
+            {
+                PrivacyFilter.All => ", showing all members",
+                PrivacyFilter.PrivateOnly => ", showing only private members",
+                PrivacyFilter.PublicOnly => "", // (default, no extra line needed)
+                _ => new ArgumentOutOfRangeException($"Couldn't find readable string for privacy filter {PrivacyFilter}")
+            });
+            
+            return str.ToString();
         }
 
         public string BuildQuery()
         {
+            // For best performance, add index:
+            // - `on switch_members using btree (member asc nulls last) include (switch);`
+            // TODO: add a migration adding this, perhaps lumped with the rest of the DB changes (it's there in prod)
+            // TODO: also, this should be moved to a view, ideally
+            
             // Select clause
             StringBuilder query = new StringBuilder();
-            query.Append(SortProperty switch
+            query.Append("select members.*, message_info.*");
-            {
+            query.Append(", (select max(switches.timestamp) from switch_members inner join switches on switches.id = switch_members.switch where switch_members.member = members.id) as last_switch_time");
-                SortProperty.MessageCount =>
+            query.Append(" from members");
-                "select members.*, count(messages.*) as message_count, max(messages.mid) as last_message from members",
-                SortProperty.LastMessage =>
-                "select members.*, count(messages.*) as message_count, max(messages.mid) as last_message from members",
-                SortProperty.LastSwitch => "select members.*, max(switches.timestamp) as last_switch_time from members",
-                _ => "select members.* from members"
-            });
             
-            // Join clauses
+            // Join here to enforce index scan on messages table by member, collect both max and count in one swoop
-            query.Append(SortProperty switch
+            query.Append(" left join lateral (select count(messages.mid) as message_count, max(messages.mid) as last_message from messages where messages.member = members.id) as message_info on true");
-            {
-                SortProperty.MessageCount => " left join messages on messages.member = members.id",
-                SortProperty.LastMessage => " left join messages on messages.member = members.id",
-                SortProperty.LastSwitch =>
-                " left join switch_members on switch_members.member = members.id left join switches on switch_members.switch = switches.id",
-                _ => ""
-            });
 
-            // Where clauses
+            // Filtering
             query.Append(" where members.system = @System");
-
-            // Privacy filter
             query.Append(PrivacyFilter switch
             {
                 PrivacyFilter.PrivateOnly => $" and members.member_privacy = {(int) PrivacyLevel.Private}",
@@ -65,31 +84,33 @@ namespace PluralKit.Bot
                 query.Append(")");
             }
 
-            // Group clause
-            query.Append(SortProperty switch
-            {
-                SortProperty.MessageCount => " group by members.id",
-                SortProperty.LastMessage => " group by members.id",
-                SortProperty.LastSwitch => " group by members.id",
-                _ => ""
-            });
-
             // Order clause
             query.Append(SortProperty switch
             {
+                SortProperty.Name => " order by members.name",
+                SortProperty.DisplayName => " order by members.display_name, members.name",
                 SortProperty.Hid => " order by members.hid",
                 SortProperty.CreationDate => " order by members.created",
                 SortProperty.Birthdate =>
                 " order by extract(month from members.birthday), extract(day from members.birthday)",
-                SortProperty.MessageCount => " order by count(messages.mid)",
+                SortProperty.MessageCount => " order by message_count",
-                SortProperty.LastMessage => " order by max(messages.mid)",
+                SortProperty.LastMessage => " order by last_message",
-                SortProperty.LastSwitch => " order by max(switches.timestamp)",
+                SortProperty.LastSwitch => " order by last_switch_time",
-                _ => " order by members.name"
+                _ => throw new ArgumentOutOfRangeException($"Couldn't find order clause for sort property {SortProperty}")
             });
 
             // Order direction
-            if (Direction == SortDirection.Descending)
+            var direction = SortProperty switch
-                query.Append(" desc");
+            {
+                // Some of these make more "logical sense" as descending (ie. "last message" = descending order of message timestamp/ID)
+                SortProperty.MessageCount => SortDirection.Descending,
+                SortProperty.LastMessage => SortDirection.Descending,
+                SortProperty.LastSwitch => SortDirection.Descending,
+                _ => SortDirection.Ascending
+            };
+            if (Reverse) direction = direction == SortDirection.Ascending ? SortDirection.Descending : SortDirection.Ascending;
+            query.Append(direction == SortDirection.Ascending ? " asc" : " desc");
+            query.Append(" nulls last");
 
             return query.ToString();
         }
@@ -98,39 +119,38 @@ namespace PluralKit.Bot
         {
             var p = new SortFilterOptions();
             
-            // Direction
+            // Sort property (default is by name, but adding a flag anyway, 'cause why not)
-            if (ctx.MatchFlag("r", "rev", "reverse", "desc", "descending"))
+            if (ctx.MatchFlag("by-name", "bn")) p.SortProperty = SortProperty.Name;
-                p.Direction = SortDirection.Descending;
+            if (ctx.MatchFlag("by-display-name", "bdn")) p.SortProperty = SortProperty.DisplayName;
+            if (ctx.MatchFlag("by-id", "bid")) p.SortProperty = SortProperty.Hid;
+            if (ctx.MatchFlag("by-message-count", "bmc")) p.SortProperty = SortProperty.MessageCount;
+            if (ctx.MatchFlag("by-created", "bc")) p.SortProperty = SortProperty.CreationDate;
+            if (ctx.MatchFlag("by-last-fronted", "by-last-front", "by-last-switch", "blf", "bls")) p.SortProperty = SortProperty.LastSwitch;
+            if (ctx.MatchFlag("by-last-message", "blm", "blp")) p.SortProperty = SortProperty.LastMessage;
+            if (ctx.MatchFlag("by-birthday", "by-birthdate", "bbd")) p.SortProperty = SortProperty.Birthdate;
             
-            // Sort
+            // Sort reverse
-            if (ctx.MatchFlag("by-id", "bi"))
+            if (ctx.MatchFlag("r", "rev", "reverse"))
-                p.SortProperty = SortProperty.Hid;
+                p.Reverse = true;
-            else if (ctx.MatchFlag("by-msgcount", "by-mc", "by-msgs", "bm"))
-                p.SortProperty = SortProperty.MessageCount;
-            else if (ctx.MatchFlag("by-date", "by-time", "by-created", "bc"))
-                p.SortProperty = SortProperty.CreationDate;
-            else if (ctx.MatchFlag("by-switch"))
-                p.SortProperty = SortProperty.LastSwitch;
-            else if (ctx.MatchFlag("by-last-message"))
-                p.SortProperty = SortProperty.LastMessage;
 
-            // Description
+            // Include description in filter?
-            if (ctx.MatchFlag("desc"))
+            if (ctx.MatchFlag("search-description", "filter-description", "in-description", "description", "desc"))
                 p.SearchInDescription = true;
             
-            // Privacy
+            // Privacy filter (default is public only)
-            if (ctx.MatchFlag("a", "all"))
+            if (ctx.MatchFlag("a", "all")) p.PrivacyFilter = PrivacyFilter.All; 
-                p.PrivacyFilter = PrivacyFilter.All;
+            if (ctx.MatchFlag("private-only")) p.PrivacyFilter = PrivacyFilter.PrivateOnly;
-            else if (ctx.MatchFlag("po", "private", "private-only", "only-private", "priv"))
+            if (ctx.MatchFlag("public-only")) p.PrivacyFilter = PrivacyFilter.PublicOnly;
-                p.PrivacyFilter = PrivacyFilter.PrivateOnly;
             
             return p;
         }
+        
     }
 
     public enum SortProperty
     {
         Name,
+        DisplayName,
         Hid,
         MessageCount,
         CreationDate,

PluralKit.Bot/Modules.cs

@@ -6,6 +6,8 @@ using Autofac;
 using DSharpPlus;
 using DSharpPlus.EventArgs;
 
+using NodaTime;
+
 using PluralKit.Core;
 
 using Sentry;
@@ -86,6 +88,7 @@ namespace PluralKit.Bot
             {
                 Timeout = TimeSpan.FromSeconds(5)
             }).AsSelf().SingleInstance();
+            builder.RegisterInstance(SystemClock.Instance).As<IClock>();
         }
     }
 }