Merge branch 'feature/sort-filter'

PluralKit.Bot/Commands/CommandTree.cs

@@ -100,7 +100,7 @@ namespace PluralKit.Bot
             if (ctx.Match("list", "l", "members"))
                 return ctx.Execute<SystemList>(SystemList, m => m.MemberList(ctx, ctx.System));
             if (ctx.Match("f", "find", "search", "query", "fd"))
-                return ctx.Execute<SystemList>(SystemFind, m => m.MemberFind(ctx, ctx.System));
+                return ctx.Execute<SystemList>(SystemFind, m => m.MemberList(ctx, ctx.System));
             if (ctx.Match("link"))
                 return ctx.Execute<SystemLink>(Link, m => m.LinkSystem(ctx));
             if (ctx.Match("unlink"))
@@ -188,7 +188,7 @@ namespace PluralKit.Bot
             else if (ctx.Match("list", "l", "members"))
                 await ctx.Execute<SystemList>(SystemList, m => m.MemberList(ctx, ctx.System));
             else if (ctx.Match("find", "search", "query", "fd", "s"))
-                await ctx.Execute<SystemList>(SystemFind, m => m.MemberFind(ctx, ctx.System));
+                await ctx.Execute<SystemList>(SystemFind, m => m.MemberList(ctx, ctx.System));
             else if (ctx.Match("f", "front", "fronter", "fronters"))
             {
                 if (ctx.Match("h", "history"))
@@ -225,7 +225,7 @@ namespace PluralKit.Bot
             else if (ctx.Match("list", "l", "members"))
                 await ctx.Execute<SystemList>(SystemList, m => m.MemberList(ctx, target));
             else if (ctx.Match("find", "search", "query", "fd", "s"))
-                await ctx.Execute<SystemList>(SystemFind, m => m.MemberFind(ctx, target));
+                await ctx.Execute<SystemList>(SystemFind, m => m.MemberList(ctx, target));
             else if (ctx.Match("f", "front", "fronter", "fronters"))
             {
                 if (ctx.Match("h", "history"))

PluralKit.Bot/Commands/SystemList.cs

@@ -1,136 +1,95 @@
-using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Text;
 using System.Threading.Tasks;
 
-using DSharpPlus.Entities;
+using Dapper;
 
-using Humanizer;
+using NodaTime;
 
 using PluralKit.Core;
 
+using Serilog;
+
 namespace PluralKit.Bot
 {
     public class SystemList
     {
-        private IDataStore _data;
+        private readonly IClock _clock;
-
+        private readonly DbConnectionFactory _db;
-        public SystemList(IDataStore data)
+        private readonly ILogger _logger;
+        
+        public SystemList(DbConnectionFactory db, ILogger logger, IClock clock)
         {
-            _data = data;
+            _db = db;
+            _logger = logger;
+            _clock = clock;
         }
 
-        private async Task RenderMemberList(Context ctx, PKSystem system, bool canShowPrivate, int membersPerPage, string embedTitle, Func<PKMember, bool> filter,
+        public async Task MemberList(Context ctx, PKSystem target)
-                                            Func<DiscordEmbedBuilder, IEnumerable<PKMember>, Task>
-                                                renderer)
         {
-            var authCtx = ctx.LookupContextFor(system);
+            if (target == null) throw Errors.NoSystemError;
-            var shouldShowPrivate = authCtx == LookupContext.ByOwner && canShowPrivate;
+            ctx.CheckSystemPrivacy(target, target.MemberListPrivacy);
-
-            var membersToShow = await _data.GetSystemMembers(system)
-                .Where(filter)
-                .OrderBy(m => m.Name, StringComparer.InvariantCultureIgnoreCase)
-                .ToListAsync();
-
-            var membersToShowWithPrivacy = membersToShow
-                .Where(m => m.MemberPrivacy == PrivacyLevel.Public || shouldShowPrivate)
-                .ToList();
-            
-            var anyMembersHidden = !shouldShowPrivate && membersToShowWithPrivacy.Count != membersToShow.Count;
 
+            // GetRendererFor must be called before GetOptions as it consumes a potential positional full argument that'd otherwise land in the filter
+            var renderer = GetRendererFor(ctx);
+            var opts = GetOptions(ctx, target);
+            var members = await GetMemberList(target, opts);
             await ctx.Paginate(
-                membersToShowWithPrivacy.ToAsyncEnumerable(),
+                members.ToAsyncEnumerable(),
-                membersToShowWithPrivacy.Count,
+                members.Count,
-                membersPerPage,
+                renderer.MembersPerPage,
-                embedTitle,
+                GetEmbedTitle(target, opts),
                 (eb, ms) =>
                 {
-                    var footer = $"{membersToShowWithPrivacy.Count} total.";
+                    eb.WithFooter($"{opts.CreateFilterString()}. {members.Count} results.");
-                    if (anyMembersHidden && authCtx == LookupContext.ByOwner)
+                    renderer.RenderPage(eb, ctx.System, ms);
-                        footer += " Private members have been hidden. Add \"all\" to the command to include them.";
+                    return Task.CompletedTask;
-                    eb.WithFooter(footer);
-                    
-                    return renderer(eb, ms);
                 });
         }
 
-        private Task ShortRenderer(DiscordEmbedBuilder eb, IEnumerable<PKMember> members)
+        private async Task<IReadOnlyList<PKListMember>> GetMemberList(PKSystem target, SortFilterOptions opts)
         {
-            eb.Description = string.Join("\n", members.Select((m) =>
+            using var conn = await _db.Obtain();
-            {
+            var query = opts.BuildQuery();
-                if (m.HasProxyTags)
+            var args = new {System = target.Id, opts.Filter};
-                {
-                    var proxyTagsString = m.ProxyTagsString().SanitizeMentions();
-                    if (proxyTagsString.Length > 100) // arbitrary threshold for now, tweak?
-                        proxyTagsString = "tags too long, see member card";
 
-                    return $"[`{m.Hid}`] **{m.Name.SanitizeMentions()}** *({proxyTagsString})*";
+            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 $"[`{m.Hid}`] **{m.Name.SanitizeMentions()}**";
+            return results;
-            }));
-            
-            return Task.CompletedTask;
         }
 
-        private Task LongRenderer(DiscordEmbedBuilder eb, IEnumerable<PKMember> members)
+        private string GetEmbedTitle(PKSystem target, SortFilterOptions opts)
         {
-            foreach (var m in members)
+            var title = new StringBuilder("Members of ");
-            {
+            
-                var profile = $"**ID**: {m.Hid}";
+            if (target.Name != null) title.Append($"{target.Name.SanitizeMentions()} (`{target.Hid}`)");
-                if (m.DisplayName != null) profile += $"\n**Display name**: {m.DisplayName}";
+            else title.Append($"`{target.Hid}`");
-                if (m.Pronouns != null) profile += $"\n**Pronouns**: {m.Pronouns}";
+ 
-                if (m.Birthday != null) profile += $"\n**Birthdate**: {m.BirthdayString}";
+            if (opts.Filter != null) title.Append($" matching **{opts.Filter.SanitizeMentions()}**");
-                if (m.ProxyTags.Count > 0) profile += $"\n**Proxy tags:** {m.ProxyTagsString()}";
+            
-                if (m.Description != null) profile += $"\n\n{m.Description}";
+            return title.ToString();
-                if (m.MemberPrivacy == PrivacyLevel.Private)
-                    profile += "\n*(this member is private)*";
-
-                eb.AddField(m.Name, profile.Truncate(1024));
-            }
-
-            return Task.CompletedTask;
         }
 
-        public async Task MemberList(Context ctx, PKSystem system)
+        private SortFilterOptions GetOptions(Context ctx, PKSystem target)
         {
-            if (system == null) throw Errors.NoSystemError;
+            var opts = SortFilterOptions.FromFlags(ctx);
-            ctx.CheckSystemPrivacy(system, system.MemberListPrivacy);
+            opts.Filter = ctx.RemainderOrNull();
-
+            // If we're *explicitly* trying to access non-public members of another system, error
-            var embedTitle = system.Name != null
+            if (opts.PrivacyFilter != PrivacyFilter.PublicOnly && ctx.LookupContextFor(target) != LookupContext.ByOwner)
-                ? $"Members of {system.Name.SanitizeMentions()} (`{system.Hid}`)"
+                throw new PKError("You cannot look up private members of another system.");
-                : $"Members of `{system.Hid}`";
+            return opts;
-
-            var shouldShowLongList = ctx.Match("f", "full", "big", "details", "long");
-            var canShowPrivate = ctx.Match("a", "all", "everyone", "private");
-            if (shouldShowLongList)
-                await RenderMemberList(ctx, system,  canShowPrivate, 5, embedTitle, _ => true, LongRenderer);
-            else 
-                await RenderMemberList(ctx, system,  canShowPrivate, 25, embedTitle, _ => true, ShortRenderer);
         }
 
-        public async Task MemberFind(Context ctx, PKSystem system)
+        private IListRenderer GetRendererFor(Context ctx)
         {
-            if (system == null) throw Errors.NoSystemError;
+            var longList = ctx.Match("f", "full", "big", "details", "long") || ctx.MatchFlag("f", "full");
-            ctx.CheckSystemPrivacy(system, system.MemberListPrivacy);
+            if (longList)
-
+                return new LongRenderer(LongRenderer.MemberFields.FromFlags(ctx));
-            var shouldShowLongList = ctx.Match("full", "big", "details", "long") || ctx.MatchFlag("f", "full");
+            return new ShortRenderer();
-            var canShowPrivate = ctx.Match("all", "everyone", "private") || ctx.MatchFlag("a", "all");
-
-            var searchTerm = ctx.RemainderOrNull() ?? throw new PKSyntaxError("You must specify a search term.");
-            
-            var embedTitle = system.Name != null
-                ? $"Members of {system.Name.SanitizeMentions()} (`{system.Hid}`) matching **{searchTerm.SanitizeMentions()}**"
-                : $"Members of `{system.Hid}` matching **{searchTerm.SanitizeMentions()}**";
-
-            bool Filter(PKMember member) =>
-                member.Name.Contains(searchTerm, StringComparison.InvariantCultureIgnoreCase) ||
-                (member.DisplayName?.Contains(searchTerm, StringComparison.InvariantCultureIgnoreCase) ?? false);
-            
-            if (shouldShowLongList)
-                await RenderMemberList(ctx, system,  canShowPrivate, 5, embedTitle, Filter, LongRenderer);
-            else 
-                await RenderMemberList(ctx, system,  canShowPrivate, 25, embedTitle, Filter, ShortRenderer);
         }
     }
 }

PluralKit.Bot/Lists/IListRenderer.cs

@@ -0,0 +1,14 @@
+using System.Collections.Generic;
+
+using DSharpPlus.Entities;
+
+using PluralKit.Core;
+
+namespace PluralKit.Bot
+{
+    public interface IListRenderer
+    {
+        int MembersPerPage { get; }
+        void RenderPage(DiscordEmbedBuilder eb, PKSystem system, IEnumerable<PKListMember> members);
+    }
+}

PluralKit.Bot/Lists/LongRenderer.cs

@@ -0,0 +1,73 @@
+using System;
+using System.Collections.Generic;
+
+using DSharpPlus.Entities;
+
+using Humanizer;
+
+using NodaTime;
+
+using PluralKit.Core;
+
+namespace PluralKit.Bot
+{
+    public class LongRenderer: IListRenderer
+    {
+        public int MembersPerPage => 5;
+
+        private readonly MemberFields _fields;
+        public LongRenderer(MemberFields fields)
+        {
+            _fields = fields;
+        }
+
+        public void RenderPage(DiscordEmbedBuilder eb, PKSystem system, IEnumerable<PKListMember> members)
+        {
+            foreach (var m in members)
+            {
+                var profile = $"**ID**: {m.Hid}";
+                if (_fields.ShowDisplayName && m.DisplayName != null) profile += $"\n**Display name**: {m.DisplayName}";
+                if (_fields.ShowPronouns && m.Pronouns != null) profile += $"\n**Pronouns**: {m.Pronouns}";
+                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)*";
+
+                eb.AddField(m.Name, profile.Truncate(1024));
+            }
+        }
+
+        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 = false;
+            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)
+            {
+                var def = new MemberFields();
+                if (ctx.MatchFlag("with-last-switch", "with-last-fronted", "with-last-front", "wls", "wlf"))
+                    def.ShowLastSwitch = true;
+                if (ctx.MatchFlag("with-message-count", "wmc"))
+                    def.ShowMessageCount = true;
+                if (ctx.MatchFlag("with-last-message", "with-last-proxy", "wlm", "wlp"))
+                    def.ShowLastMessage = true;
+                return def;
+            }
+        }
+    }
+}

PluralKit.Bot/Lists/PKListMember.cs

@@ -0,0 +1,13 @@
+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

@@ -0,0 +1,34 @@
+using System;
+using System.Collections.Generic;
+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, PKSystem system, IEnumerable<PKListMember> members)
+        {
+            string RenderLine(PKListMember m)
+            {
+                if (m.HasProxyTags)
+                {
+                    var proxyTagsString = m.ProxyTagsString().SanitizeMentions();
+                    if (proxyTagsString.Length > 100) // arbitrary threshold for now, tweak?
+                        proxyTagsString = "tags too long, see member card";
+
+                    return $"[`{m.Hid}`] **{m.Name.SanitizeMentions()}** *({proxyTagsString})*";
+                }
+
+                return $"[`{m.Hid}`] **{m.Name.SanitizeMentions()}**";
+            }
+
+            eb.Description = string.Join("\n", members.Select(RenderLine));
+        }
+    }
+}

PluralKit.Bot/Lists/SortFilterOptions.cs

@@ -0,0 +1,174 @@
+using System;
+using System.Text;
+
+using PluralKit.Core;
+
+namespace PluralKit.Bot
+{
+    public class SortFilterOptions
+    {
+        public SortProperty SortProperty = SortProperty.Name;
+        public bool Reverse = false;
+        public PrivacyFilter PrivacyFilter = PrivacyFilter.PublicOnly;
+        public string Filter = null;
+        public bool SearchInDescription = false;
+
+        public string CreateFilterString()
+        {
+            StringBuilder str = new StringBuilder();
+            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("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");
+            query.Append(" from members");
+            
+            // Join here to enforce index scan on messages table by member, collect both max and count in one swoop
+            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");
+
+            // Filtering
+            query.Append(" where members.system = @System");
+            query.Append(PrivacyFilter switch
+            {
+                PrivacyFilter.PrivateOnly => $" and members.member_privacy = {(int) PrivacyLevel.Private}",
+                PrivacyFilter.PublicOnly => $" and members.member_privacy = {(int) PrivacyLevel.Public}",
+                _ => ""
+            });
+
+            // String filter
+            if (Filter != null)
+            {
+                // Use position rather than ilike to not bother with escaping and such
+                query.Append(" and (");
+                query.Append(
+                    "position(lower(@Filter) in lower(members.name)) > 0 or position(lower(@Filter) in lower(coalesce(members.display_name, ''))) > 0");
+                if (SearchInDescription)
+                    query.Append(" or position(lower(@Filter) in lower(coalesce(members.description, ''))) > 0");
+                query.Append(")");
+            }
+
+            // 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 message_count",
+                SortProperty.LastMessage => " order by last_message",
+                SortProperty.LastSwitch => " order by last_switch_time",
+                _ => throw new ArgumentOutOfRangeException($"Couldn't find order clause for sort property {SortProperty}")
+            });
+
+            // Order direction
+            var direction = SortProperty switch
+            {
+                // 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();
+        }
+        
+        public static SortFilterOptions FromFlags(Context ctx)
+        {
+            var p = new SortFilterOptions();
+            
+            // Sort property (default is by name, but adding a flag anyway, 'cause why not)
+            if (ctx.MatchFlag("by-name", "bn")) p.SortProperty = SortProperty.Name;
+            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 reverse
+            if (ctx.MatchFlag("r", "rev", "reverse"))
+                p.Reverse = true;
+
+            // Include description in filter?
+            if (ctx.MatchFlag("search-description", "filter-description", "in-description", "description", "desc"))
+                p.SearchInDescription = true;
+            
+            // Privacy filter (default is public only)
+            if (ctx.MatchFlag("a", "all")) p.PrivacyFilter = PrivacyFilter.All; 
+            if (ctx.MatchFlag("private-only")) p.PrivacyFilter = PrivacyFilter.PrivateOnly;
+            if (ctx.MatchFlag("public-only")) p.PrivacyFilter = PrivacyFilter.PublicOnly;
+            
+            return p;
+        }
+        
+    }
+
+    public enum SortProperty
+    {
+        Name,
+        DisplayName,
+        Hid,
+        MessageCount,
+        CreationDate,
+        LastSwitch,
+        LastMessage,
+        Birthdate
+    }
+
+    public enum SortDirection
+    {
+        Ascending,
+        Descending
+    }
+
+    public enum PrivacyFilter
+    {
+        All,
+        PublicOnly,
+        PrivateOnly
+    }
+}

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>();
         }
     }
 }