Various fixes and improvements

PluralKit.Bot/CommandSystem/ContextChecksExt.cs

@@ -28,7 +28,7 @@ namespace PluralKit.Bot
         public static Context CheckOwnGroup(this Context ctx, PKGroup group)
         {
             if (group.System != ctx.System?.Id)
-                throw Errors.NotOwnMemberError;
+                throw Errors.NotOwnGroupError;
             return ctx;
         }
         

PluralKit.Bot/CommandSystem/ContextEntityArgumentsExt.cs

@@ -103,7 +103,7 @@ namespace PluralKit.Bot
             var input = ctx.PeekArgument();
 
             await using var conn = await ctx.Database.Obtain();
-            if (await conn.QueryGroupByName(input) is {} byName)
+            if (ctx.System != null && await conn.QueryGroupByName(ctx.System.Id, input) is {} byName)
                 return byName;
             if (await conn.QueryGroupByHid(input) is {} byHid)
                 return byHid;

PluralKit.Bot/Commands/CommandTree.cs

@@ -90,6 +90,18 @@ namespace PluralKit.Bot
             MemberRandom
         };
 
+        public static Command[] GroupCommands =
+        {
+            GroupInfo, GroupList, GroupNew, GroupAdd, GroupRemove, GroupMemberList, GroupRename, GroupDesc,
+            GroupIcon, GroupPrivacy, GroupDelete
+        };
+
+        public static Command[] GroupCommandsTargeted =
+        {
+            GroupInfo, GroupAdd, GroupRemove, GroupMemberList, GroupRename, GroupDesc, GroupIcon, GroupPrivacy,
+            GroupDelete
+        };
+
         public static Command[] SwitchCommands = {Switch, SwitchOut, SwitchMove, SwitchDelete};
 
         public static Command[] LogCommands = {LogChannel, LogEnable, LogDisable};
@@ -227,6 +239,8 @@ namespace PluralKit.Bot
                 await ctx.Execute<SystemEdit>(SystemPing, m => m.SystemPing(ctx));
             else if (ctx.Match("commands", "help"))
                 await PrintCommandList(ctx, "systems", SystemCommands);
+            else if (ctx.Match("groups", "gs"))
+                await ctx.Execute<Groups>(GroupList, g => g.ListSystemGroups(ctx, null));
             else if (!ctx.HasNext()) // Bare command
                 await ctx.Execute<System>(SystemInfo, m => m.Query(ctx, ctx.System));
             else
@@ -262,6 +276,8 @@ namespace PluralKit.Bot
                 await ctx.Execute<SystemFront>(SystemFrontPercent, m => m.SystemFrontPercent(ctx, target));
             else if (ctx.Match("info", "view", "show"))
                 await ctx.Execute<System>(SystemInfo, m => m.Query(ctx, target));
+            else if (ctx.Match("groups", "gs"))
+                await ctx.Execute<Groups>(GroupList, g => g.ListSystemGroups(ctx, target));
             else if (!ctx.HasNext())
                 await ctx.Execute<System>(SystemInfo, m => m.Query(ctx, target));
             else
@@ -332,6 +348,8 @@ namespace PluralKit.Bot
                 await ctx.Execute<Groups>(GroupNew, g => g.CreateGroup(ctx));
             else if (ctx.Match("list", "l"))
                 await ctx.Execute<Groups>(GroupList, g => g.ListSystemGroups(ctx, null));
+            else if (ctx.Match("commands", "help"))
+                await PrintCommandList(ctx, "groups", GroupCommands);
             else if (await ctx.MatchGroup() is {} target)
             {
                 // Commands with group argument
@@ -358,10 +376,10 @@ namespace PluralKit.Bot
                 else if (!ctx.HasNext())
                     await ctx.Execute<Groups>(GroupInfo, g => g.ShowGroupCard(ctx, target));
                 else
-                    await PrintCommandNotFoundError(ctx, GroupInfo, GroupRename, GroupDesc);
+                    await PrintCommandNotFoundError(ctx, GroupCommandsTargeted);
             }
             else if (!ctx.HasNext())
-                await PrintCommandNotFoundError(ctx, GroupInfo, GroupList, GroupNew, GroupRename, GroupDesc);
+                await PrintCommandNotFoundError(ctx, GroupCommands);
             else
                 await ctx.Reply($"{Emojis.Error} {ctx.CreateGroupNotFoundError(ctx.PopArgument())}");
         }

PluralKit.Bot/Commands/Groups.cs

@@ -9,6 +9,8 @@ using Dapper;
 
 using DSharpPlus.Entities;
 
+using Humanizer;
+
 using PluralKit.Core;
 
 namespace PluralKit.Bot
@@ -26,16 +28,26 @@ namespace PluralKit.Bot
         {
             ctx.CheckSystem();
             
+            // Check group name length
             var groupName = ctx.RemainderOrNull() ?? throw new PKSyntaxError("You must pass a group name.");
             if (groupName.Length > Limits.MaxGroupNameLength)
                 throw new PKError($"Group name too long ({groupName.Length}/{Limits.MaxGroupNameLength} characters).");
             
             await using var conn = await _db.Obtain();
             
-            var existingGroupCount = await conn.QuerySingleAsync<int>("select count(*) from groups where system = @System", ctx.System.Id);
+            // Check group cap
+            var existingGroupCount = await conn.QuerySingleAsync<int>("select count(*) from groups where system = @System", new { System = ctx.System.Id });
             if (existingGroupCount >= Limits.MaxGroupCount)
                 throw new PKError($"System has reached the maximum number of groups ({Limits.MaxGroupCount}). Please delete unused groups first in order to create new ones.");
 
+            // Warn if there's already a group by this name
+            var existingGroup = await conn.QueryGroupByName(ctx.System.Id, groupName);
+            if (existingGroup != null) {
+                var msg = $"{Emojis.Warn} You already have a group in your system with the name \"{existingGroup.Name}\" (with ID `{existingGroup.Hid}`). Do you want to create another group with the same name?";
+                if (!await ctx.PromptYesNo(msg))
+                    throw new PKError("Group creation cancelled.");
+            }
+            
             var newGroup = await conn.CreateGroup(ctx.System.Id, groupName);
             
             var eb = new DiscordEmbedBuilder()
@@ -51,11 +63,21 @@ namespace PluralKit.Bot
         {
             ctx.CheckOwnGroup(target);
             
+            // Check group name length
             var newName = ctx.RemainderOrNull() ?? throw new PKSyntaxError("You must pass a new group name.");
             if (newName.Length > Limits.MaxGroupNameLength)
                 throw new PKError($"New group name too long ({newName.Length}/{Limits.MaxMemberNameLength} characters).");
             
             await using var conn = await _db.Obtain();
+            
+            // Warn if there's already a group by this name
+            var existingGroup = await conn.QueryGroupByName(ctx.System.Id, newName);
+            if (existingGroup != null && existingGroup.Id != target.Id) {
+                var msg = $"{Emojis.Warn} You already have a group in your system with the name \"{existingGroup.Name}\" (with ID `{existingGroup.Hid}`). Do you want to rename this member to that name too?";
+                if (!await ctx.PromptYesNo(msg))
+                    throw new PKError("Group creation cancelled.");
+            }
+
             await conn.UpdateGroup(target.Id, new GroupPatch {Name = newName});
 
             await ctx.Reply($"{Emojis.Success} Group name changed from \"**{target.Name}**\" to \"**{newName}**\".");
@@ -173,14 +195,26 @@ namespace PluralKit.Bot
                 system = ctx.System;
             }
             
+            // should this be split off to a separate permission?
+            ctx.CheckSystemPrivacy(system, system.MemberListPrivacy);
+            
             // TODO: integrate with the normal "search" system
             await using var conn = await _db.Obtain();
 
             var pctx = LookupContext.ByNonOwner;
-            if (ctx.MatchFlag("a", "all") && system.Id == ctx.System.Id)
+            if (ctx.MatchFlag("a", "all"))
+            {
+                if (system.Id == ctx.System.Id)
                     pctx = LookupContext.ByOwner;
+                else
+                    throw new PKError("You do not have permission to access this information.");
+            }
+                
+
+            var groups = (await conn.QueryGroupsInSystem(system.Id))
+                .Where(g => g.Visibility.CanAccess(pctx))
+                .ToList();
             
-            var groups = (await conn.QueryGroupsInSystem(system.Id)).Where(g => g.Visibility.CanAccess(pctx)).ToList();
             if (groups.Count == 0)
             {
                 if (system.Id == ctx.System?.Id)
@@ -195,14 +229,8 @@ namespace PluralKit.Bot
             
             Task Renderer(DiscordEmbedBuilder eb, IEnumerable<PKGroup> page)
             {
-                var sb = new StringBuilder();
+                eb.WithSimpleLineContent(page.Select(g => $"[`{g.Hid}`] **{g.Name}**"));
-                foreach (var g in page)
+                eb.WithFooter($"{groups.Count} total.");
-                {
-                    sb.Append($"[`{g.Hid}`] **{g.Name}**\n");
-                }
-
-                eb.WithDescription(sb.ToString());
-                eb.WithFooter($"{groups.Count} total");
                 return Task.CompletedTask;
             }
         }
@@ -244,23 +272,46 @@ namespace PluralKit.Bot
             var members = await ParseMemberList(ctx);
             
             await using var conn = await _db.Obtain();
+            
+            var existingMembersInGroup = (await conn.QueryMemberList(target.System,
+                new DatabaseViewsExt.MemberListQueryOptions {GroupFilter = target.Id}))
+                .Select(m => m.Id)
+                .ToHashSet();
+            
             if (op == AddRemoveOperation.Add)
             {
-                await conn.AddMembersToGroup(target.Id, members.Select(m => m.Id));
+                var membersNotInGroup = members
-                await ctx.Reply($"{Emojis.Success} Members added to group.");
+                    .Where(m => !existingMembersInGroup.Contains(m.Id))
+                    .Select(m => m.Id)
+                    .ToList();
+                await conn.AddMembersToGroup(target.Id, membersNotInGroup);
+                
+                if (membersNotInGroup.Count == members.Count)
+                    await ctx.Reply($"{Emojis.Success} {"members".ToQuantity(membersNotInGroup.Count)} added to group.");
+                else 
+                    await ctx.Reply($"{Emojis.Success} {"members".ToQuantity(membersNotInGroup.Count)} added to group ({"members".ToQuantity(members.Count - membersNotInGroup.Count)} already in group).");
             }
             else if (op == AddRemoveOperation.Remove)
             {
-                await conn.RemoveMembersFromGroup(target.Id, members.Select(m => m.Id));
+                var membersInGroup = members
-                await ctx.Reply($"{Emojis.Success} Members removed from group.");
+                    .Where(m => existingMembersInGroup.Contains(m.Id))
+                    .Select(m => m.Id)
+                    .ToList();
+                await conn.RemoveMembersFromGroup(target.Id, membersInGroup);
+                
+                if (membersInGroup.Count == members.Count)
+                    await ctx.Reply($"{Emojis.Success} {"members".ToQuantity(membersInGroup.Count)} removed from group.");
+                else 
+                    await ctx.Reply($"{Emojis.Success} {"members".ToQuantity(membersInGroup.Count)} removed from group ({"members".ToQuantity(members.Count - membersInGroup.Count)} already not in group).");
             }
         }
 
         public async Task ListGroupMembers(Context ctx, PKGroup target)
         {
             await using var conn = await _db.Obtain();
+            
             var targetSystem = await GetGroupSystem(ctx, target, conn);
-            ctx.CheckSystemPrivacy(targetSystem, targetSystem.MemberListPrivacy);
+            ctx.CheckSystemPrivacy(targetSystem, target.Visibility);
             
             var opts = ctx.ParseMemberListOptions(ctx.LookupContextFor(target.System));
             opts.GroupFilter = target.Id;
@@ -313,7 +364,7 @@ namespace PluralKit.Bot
                     .AddField("Description", target.DescriptionPrivacy.Explanation())
                     .AddField("Icon", target.IconPrivacy.Explanation())
                     .AddField("Visibility", target.Visibility.Explanation())
-                    .WithDescription($"To edit privacy settings, use the command:\n`pk;group **{GroupReference(target)}** privacy <subject> <level>`\n\n- `subject` is one of `description`, `icon`, `visibility`, or `all`\n- `level` is either `public` or `private`.")
+                    .WithDescription($"To edit privacy settings, use the command:\n> pk;group **{GroupReference(target)}** privacy **<subject>** **<level>**\n\n- `subject` is one of `description`, `icon`, `visibility`, or `all`\n- `level` is either `public` or `private`.")
                     .Build()); 
                 return;
             }
@@ -385,7 +436,7 @@ namespace PluralKit.Bot
 
         private static string GroupReference(PKGroup group)
         {
-            if (Regex.IsMatch(group.Name, "[A-Za-z0-9\\-_]+"))
+            if (Regex.IsMatch(group.Name, "^[A-Za-z0-9\\-_]+$"))
                 return group.Name;
             return group.Hid;
         }

PluralKit.Bot/Errors.cs

@@ -106,7 +106,7 @@ namespace PluralKit.Bot {
         public static PKError ProxyNameTooShort(string name) => new PKError($"The webhook's name, `{name}`, is shorter than two characters, and thus cannot be proxied. Please change the member name or use a longer system tag.");
         public static PKError ProxyNameTooLong(string name) => new PKError($"The webhook's name, {name}, is too long ({name.Length} > {Limits.MaxProxyNameLength} characters), and thus cannot be proxied. Please change the member name, display name or server display name, or use a shorter system tag.");
 
-        public static PKError ProxyTagAlreadyExists(ProxyTag tagToAdd, PKMember member) => new PKError($"That member already has the proxy tag `` {tagToAdd.ProxyString.EscapeBacktickPair()}``. The member currently has these tags: {member.ProxyTagsString()}");
+        public static PKError ProxyTagAlreadyExists(ProxyTag tagToAdd, PKMember member) => new PKError($"That member already has the proxy tag `` {tagToAdd.ProxyString.EscapeBacktickPair()} ``. The member currently has these tags: {member.ProxyTagsString()}");
         public static PKError ProxyTagDoesNotExist(ProxyTag tagToRemove, PKMember member) => new PKError($"That member does not have the proxy tag ``{tagToRemove.ProxyString.EscapeBacktickPair()}``. The member currently has these tags: {member.ProxyTagsString()}");
         public static PKError LegacyAlreadyHasProxyTag(ProxyTag requested, PKMember member) => new PKError($"This member already has more than one proxy tag set: {member.ProxyTagsString()}\nConsider using the ``pk;member {member.Hid} proxy add {requested.ProxyString.EscapeBacktickPair()}`` command instead.");
         public static PKError EmptyProxyTags(PKMember member) => new PKError($"The example proxy `text` is equivalent to having no proxy tags at all, since there are no symbols or brackets on either end. If you'd like to clear your proxy tags, use `pk;member {member.Hid} proxy clear`.");

PluralKit.Bot/Services/EmbedService.cs

@@ -129,9 +129,6 @@ namespace PluralKit.Bot {
                     description += "*(this member has a server-specific avatar set)*\n";
             if (description != "") eb.WithDescription(description);
             
-            if (groups.Count > 0)
-                eb.AddField($"Groups ({groups.Count})", string.Join("\n", groups.Select(g => $"[`{g.Hid}`] **{g.Name}**")).Truncate(1000));
-
             if (avatar != null) eb.WithThumbnail(avatar);
 
             if (!member.DisplayName.EmptyOrNull() && member.NamePrivacy.CanAccess(ctx)) eb.AddField("Display Name", member.DisplayName.Truncate(1024), true);
@@ -146,6 +143,15 @@ namespace PluralKit.Bot {
             // if (!member.Color.EmptyOrNull() && member.ColorPrivacy.CanAccess(ctx)) eb.AddField("Color", $"#{member.Color}", true);
             if (!member.Color.EmptyOrNull()) eb.AddField("Color", $"#{member.Color}", true);
 
+            if (groups.Count > 0)
+            {
+                // More than 5 groups show in "compact" format without ID
+                var content = groups.Count > 5
+                    ? string.Join(", ", groups.Select(g => g.Name))
+                    : string.Join("\n", groups.Select(g => $"[`{g.Hid}`] **{g.Name}**"));
+                eb.AddField($"Groups ({groups.Count})", content.Truncate(1000));
+            }
+
             if (member.DescriptionFor(ctx) is {} desc) eb.AddField("Description", member.Description.NormalizeLineEndSpacing(), false);
 
             return eb.Build();

PluralKit.Bot/Utils/DiscordUtils.cs

@@ -212,7 +212,7 @@ namespace PluralKit.Bot
         public static string EscapeBacktickPair(this string input){
             Regex doubleBacktick = new Regex(@"``", RegexOptions.Multiline);
             //Run twice to catch any pairs that are created from the first pass, pairs shouldn't be created in the second as they are created from odd numbers of backticks, even numbers are all caught on the first pass
-            if(input != null) return doubleBacktick.Replace(doubleBacktick.Replace(input, @"`‌ `"),@"`‌`");
+            if(input != null) return doubleBacktick.Replace(doubleBacktick.Replace(input, @"`‌ `"),@"`‌ `");
             else return input;
         }
 

PluralKit.Core/Database/Migrations/9.sql

@@ -20,7 +20,8 @@ create table groups (
 
 create table group_members (
     group_id int not null references groups(id) on delete cascade,
-    member_id int not null references members(id) on delete cascade
+    member_id int not null references members(id) on delete cascade,
+    primary key (group_id, member_id)
 );
 
 update info set schema_version = 9;

PluralKit.Core/Models/ModelQueryExt.cs

@@ -29,8 +29,8 @@ namespace PluralKit.Core
         public static Task<PKMember?> QueryMemberByHid(this IPKConnection conn, string hid) =>
             conn.QueryFirstOrDefaultAsync<PKMember?>("select * from members where hid = @hid", new {hid = hid.ToLowerInvariant()});
         
-        public static Task<PKGroup?> QueryGroupByName(this IPKConnection conn, string name) =>
+        public static Task<PKGroup?> QueryGroupByName(this IPKConnection conn, SystemId system, string name) =>
-            conn.QueryFirstOrDefaultAsync<PKGroup?>("select * from groups where lower(name) = lower(@name)", new {name = name});
+            conn.QueryFirstOrDefaultAsync<PKGroup?>("select * from groups where system = @System and lower(Name) = lower(@Name)", new {System = system, Name = name});
         
         public static Task<PKGroup?> QueryGroupByHid(this IPKConnection conn, string hid) =>
             conn.QueryFirstOrDefaultAsync<PKGroup?>("select * from groups where hid = @hid", new {hid = hid.ToLowerInvariant()});

PluralKit.Core/Utils/Limits.cs

@@ -7,7 +7,7 @@ namespace PluralKit.Core {
         public static readonly int MaxSystemTagLength = MaxProxyNameLength - 1;
         public static readonly int MaxMemberCount = 1500;
         public static readonly int MaxMembersWarnThreshold = MaxMemberCount - 50;
-        public static readonly int MaxGroupCount = 50; // TODO: up to 100+?
+        public static readonly int MaxGroupCount = 100; // TODO: up to 200+?
         public static readonly int MaxDescriptionLength = 1000;
         public static readonly int MaxMemberNameLength = 100; // Fair bit larger than MaxProxyNameLength for bookkeeping
         public static readonly int MaxGroupNameLength = 100;

docs/content/user-guide.md

@@ -415,6 +415,62 @@ To look at the per-member breakdown of the front over a given time period, use t
 
 Note that in cases of switches with multiple members, each involved member will have the full length of the switch counted towards it. This means that the percentages may add up to over 100%.
 
+## Member groups
+PluralKit allows you to categorize system members in different **groups**.
+You can add members to a group, and each member can be in multiple groups.
+The groups a member is in will show on the group card.
+
+### Creating a new group
+To create a new group, use the `pk;group new` command:
+
+    pk;group new MyGroup
+    
+This will create a new group. Groups all have a 5-letter ID, similar to systems and members.
+
+### Adding and removing members to groups
+To add a member to a group, use the `pk;group <group> add` command, eg:
+
+    pk;group MyGroup add Craig
+    
+You can add multiple members to a group by separating them with spaces, eg:
+
+    pk;group MyGroup add Bob John Charlie
+
+Similarly, you can remove members from a group, eg:
+
+    pk;group MyGroup remove Bob Craig
+    
+### Listing members in a group
+To list all the members in a group, use the `pk;group <group> list` command.
+The syntax works the same as `pk;system list`, and also allows searching and sorting, eg:
+
+    pk;group MyGroup list
+    pk;group MyGroup list --by-message-count jo
+    
+### Listing all your groups
+In the same vein, you can list all the groups in your system with the `pk;group list` command:
+
+    pk;group list
+    
+### Group name, description, icon, delete
+(TODO: write this better)
+
+Groups can be renamed:
+
+    pk;group MyGroup rename SuperCoolGroup
+
+Groups can have icons that show in on the group card:
+    
+    pk;group MyGroup icon https://my.link.to/image.png
+    
+Groups can have descriptions:
+
+    pk;group MyGroup description This is my cool group description! :)
+    
+Groups can be deleted:
+
+    pk;group MyGroup delete
+
 ## Privacy
 There are various reasons you may not want information about your system or your members to be public. As such, there are a few controls to manage which information is publicly accessible or not.