From f912805eccbee374a2fab08f560deaf8766ef221 Mon Sep 17 00:00:00 2001 From: spiral Date: Wed, 25 Aug 2021 19:51:33 -0400 Subject: [PATCH] feat: make group member add/remove response code less confusing; add tests --- PluralKit.Bot/Commands/Groups.cs | 2 +- PluralKit.Bot/Commands/MemberGroup.cs | 2 +- .../Services/GroupAddRemoveResponseService.cs | 109 +++++++++++ PluralKit.Bot/Utils/MiscUtils.cs | 31 --- .../GroupAddRemoveResponseTests.cs | 185 ++++++++++++++++++ 5 files changed, 296 insertions(+), 33 deletions(-) create mode 100644 PluralKit.Bot/Services/GroupAddRemoveResponseService.cs create mode 100644 PluralKit.Tests/GroupAddRemoveResponseTests.cs diff --git a/PluralKit.Bot/Commands/Groups.cs b/PluralKit.Bot/Commands/Groups.cs index f3404004..a9011559 100644 --- a/PluralKit.Bot/Commands/Groups.cs +++ b/PluralKit.Bot/Commands/Groups.cs @@ -457,7 +457,7 @@ namespace PluralKit.Bot } else return; // otherwise toAction "may be undefined" - await ctx.Reply(MiscUtils.GroupAddRemoveResponse(members, toAction, op)); + await ctx.Reply(GroupAddRemoveResponseService.GenerateResponse(op, members.Count, 1, members.Count - existingMembersInGroup.Count, existingMembersInGroup.Count)); } public async Task ListGroupMembers(Context ctx, PKGroup target) diff --git a/PluralKit.Bot/Commands/MemberGroup.cs b/PluralKit.Bot/Commands/MemberGroup.cs index 7d19e5e7..1c67a30f 100644 --- a/PluralKit.Bot/Commands/MemberGroup.cs +++ b/PluralKit.Bot/Commands/MemberGroup.cs @@ -55,7 +55,7 @@ namespace PluralKit.Bot } else return; // otherwise toAction "may be unassigned" - await ctx.Reply(MiscUtils.GroupAddRemoveResponse(groups, toAction, op)); + await ctx.Reply(GroupAddRemoveResponseService.GenerateResponse(op, 1, groups.Count, groups.Count - existingGroups.Count, existingGroups.Count)); } public async Task List(Context ctx, PKMember target) diff --git a/PluralKit.Bot/Services/GroupAddRemoveResponseService.cs b/PluralKit.Bot/Services/GroupAddRemoveResponseService.cs new file mode 100644 index 00000000..ceb20aba --- /dev/null +++ b/PluralKit.Bot/Services/GroupAddRemoveResponseService.cs @@ -0,0 +1,109 @@ +using System; + +using PluralKit.Core; + +namespace PluralKit.Bot +{ + public static class GroupAddRemoveResponseService + { + public static string GenerateResponse(Groups.AddRemoveOperation action, int memberCount, int groupCount, int actionedOn, int notActionedOn) + => new Response(action, memberCount, groupCount, actionedOn, notActionedOn).ToString(); + + private class Response + { + private readonly Groups.AddRemoveOperation _op; + + private readonly string _actionStr; + private readonly string _containStr; + private readonly string _emojiStr; + + private readonly bool _memberPlural; + private readonly bool _groupPlural; + + private readonly int _actionedOn; + private readonly int _notActionedOn; + + public Response(Groups.AddRemoveOperation action, int memberCount, int groupCount, int actionedOn, + int notActionedOn) + { + _op = action; + + _actionStr = action == Groups.AddRemoveOperation.Add ? "added to" : "removed from"; + _containStr = action == Groups.AddRemoveOperation.Add ? "in" : "not in"; + _emojiStr = actionedOn > 0 ? Emojis.Success : Emojis.Error; + + _memberPlural = memberCount > 1; + _groupPlural = groupCount > 1; + + // sanity checking: we can't add multiple groups to multiple members (at least for now) + if (_memberPlural && _groupPlural) + throw new ArgumentOutOfRangeException(); + + // sanity checking: we can't act/not act on a different number of entities than we have + if (_memberPlural && (actionedOn + notActionedOn) != memberCount) + throw new ArgumentOutOfRangeException(); + if (_groupPlural && (actionedOn + notActionedOn) != groupCount) + throw new ArgumentOutOfRangeException(); + + _actionedOn = actionedOn; + _notActionedOn = notActionedOn; + } + + // name generators + private string MemberString(bool capitalize = false) + => capitalize + ? (_memberPlural ? "Members" : "Member") + : (_memberPlural ? "members" : "member"); + + private string MemberString(int count, bool capitalize = false) + => capitalize + ? (count == 1 ? "Member" : "Members") + : (count == 1 ? "member" : "members"); + + private string GroupString() => _groupPlural ? "groups" : "group"; + + private string GroupString(int count) + => count == 1 ? "group" : "groups"; + + // string generators + + private string ResponseString() + { + if (_actionedOn > 0 && _notActionedOn > 0 && _memberPlural) + return $"{_actionedOn} {MemberString(_actionedOn)} {_actionStr} {GroupString()}"; + if (_actionedOn > 0 && _notActionedOn > 0 && _groupPlural) + return $"{MemberString(capitalize: true)} {_actionStr} {_actionedOn} {GroupString(_actionedOn)}"; + if (_notActionedOn == 0) + return $"{MemberString(capitalize: true)} {_actionStr} {GroupString()}"; + if (_actionedOn == 0) + return $"{MemberString(capitalize: true)} not {_actionStr} {GroupString()}"; + + throw new ArgumentOutOfRangeException(); + } + + private string InfoMessage() + { + if (_notActionedOn == 0) return $""; + + var msg = ""; + if (_actionedOn > 0 && _memberPlural) + msg += $"{_notActionedOn} {MemberString(_notActionedOn)}"; + else + msg += $"{MemberString()}"; + + msg += $" already {_containStr}"; + + if (_actionedOn > 0 && _groupPlural) + msg += $" {_notActionedOn} {GroupString(_notActionedOn)}"; + else + msg += $" {GroupString()}"; + + return $" ({msg})"; + } + + public string ToString() => $"{_emojiStr} {ResponseString()}{InfoMessage()}."; + + // | + } + } +} \ No newline at end of file diff --git a/PluralKit.Bot/Utils/MiscUtils.cs b/PluralKit.Bot/Utils/MiscUtils.cs index 0a5929f1..377e22bf 100644 --- a/PluralKit.Bot/Utils/MiscUtils.cs +++ b/PluralKit.Bot/Utils/MiscUtils.cs @@ -20,37 +20,6 @@ namespace PluralKit.Bot public static string ProxyTagsString(this PKMember member, string separator = ", ") => string.Join(separator, member.ProxyTags.Select(t => t.ProxyString.AsCode())); - - private static String entityTerm(int count, bool isTarget) - { - var ret = ""; - ret += isTarget ? "Member" : "Group"; - if (( - (typeof(T) == typeof(GroupId) && !isTarget) || - (typeof(T) == typeof(MemberId) && isTarget) - ) && count > 1) - ret += "s"; - return ret; - } - - public static String GroupAddRemoveResponse(List entityList, List actionedOn, Groups.AddRemoveOperation op) - { - var opStr = op == Groups.AddRemoveOperation.Add ? "added to" : "removed from"; - var inStr = op == Groups.AddRemoveOperation.Add ? "in" : "not in"; - var notActionedOn = entityList.Count - actionedOn.Count; - - var groupNotActionedPosStr = typeof(T) == typeof(GroupId) ? notActionedOn.ToString() + " " : ""; - var memberNotActionedPosStr = typeof(T) == typeof(MemberId) ? notActionedOn.ToString() + " " : ""; - - if (actionedOn.Count == 0) - return $"{Emojis.Error} {entityTerm(notActionedOn, true)} not {opStr} {entityTerm(entityList.Count, false).ToLower()} ({entityTerm(notActionedOn, true).ToLower()} already {inStr} {entityTerm(entityList.Count, false).ToLower()})."; - else - if (notActionedOn == 0) - return $"{Emojis.Success} {entityTerm(actionedOn.Count, true)} {opStr} {entityTerm(actionedOn.Count, false).ToLower()}."; - else - return $"{Emojis.Success} {actionedOn.Count} {entityTerm(actionedOn.Count, true).ToLower()} {opStr} {entityTerm(actionedOn.Count, false).ToLower()} ({memberNotActionedPosStr}{entityTerm(actionedOn.Count, true).ToLower()} already {inStr} {groupNotActionedPosStr}{entityTerm(notActionedOn, false).ToLower()})."; - } - public static bool IsOurProblem(this Exception e) { // This function filters out sporadic errors out of our control from being reported to Sentry diff --git a/PluralKit.Tests/GroupAddRemoveResponseTests.cs b/PluralKit.Tests/GroupAddRemoveResponseTests.cs new file mode 100644 index 00000000..e4dcc892 --- /dev/null +++ b/PluralKit.Tests/GroupAddRemoveResponseTests.cs @@ -0,0 +1,185 @@ +using System; + +using PluralKit.Bot; +using PluralKit.Core; + +using Xunit; + +namespace PluralKit.Tests +{ + public class GroupAddRemoveResponseTests + { + private static Func + func = GroupAddRemoveResponseService.GenerateResponse; + private static Groups.AddRemoveOperation addOp = Groups.AddRemoveOperation.Add; + private static Groups.AddRemoveOperation removeOp = Groups.AddRemoveOperation.Remove; + private static string success = Emojis.Success; + private static string failure = Emojis.Error; + + public class AddOp + { + public class MemberGroup + { + [Fact] + public void Success() + => Assert.Equal( + $"{success} Member added to group.", + func(addOp, 1, 1, 1, 0) + ); + + [Fact] + public void Failure() + => Assert.Equal( + $"{failure} Member not added to group (member already in group).", + func(addOp, 1, 1, 0, 1) + ); + } + + public class MemberGroups + { + [Fact] + public void Success() + => Assert.Equal( + $"{success} Member added to groups.", + func(addOp, 1, 2, 2, 0) + ); + + [Fact] + public void PartialSuccess1() + => Assert.Equal( + $"{success} Member added to 2 groups (member already in 1 group).", + func(addOp, 1, 3, 2, 1) + ); + + [Fact] + public void PartialSuccess2() + => Assert.Equal( + $"{success} Member added to 1 group (member already in 2 groups).", + func(addOp, 1, 3, 1, 2) + ); + + [Fact] + public void Failure() + => Assert.Equal( + $"{failure} Member not added to groups (member already in groups).", + func(addOp, 1, 2, 0, 2) + ); + } + + public class MembersGroup + { + [Fact] + public void Success() + => Assert.Equal( + $"{success} Members added to group.", + func(addOp, 2, 1, 2, 0) + ); + + [Fact] + public void PartialSuccess1() + => Assert.Equal( + $"{success} 2 members added to group (1 member already in group).", + func(addOp, 3, 1, 2, 1) + ); + + [Fact] + public void PartialSuccess2() + => Assert.Equal( + $"{success} 1 member added to group (2 members already in group).", + func(addOp, 3, 1, 1, 2) + ); + + [Fact] + public void Failure() + => Assert.Equal( + $"{failure} Members not added to group (members already in group).", + func(addOp, 2, 1, 0, 2) + ); + } + + } + + public class RemoveOp + { + public class MemberGroup + { + [Fact] + public void Success() + => Assert.Equal( + $"{success} Member removed from group.", + func(removeOp, 1, 1, 1, 0) + ); + + [Fact] + public void Failure() + => Assert.Equal( + $"{failure} Member not removed from group (member already not in group).", + func(removeOp, 1, 1, 0, 1) + ); + } + + public class MemberGroups + { + [Fact] + public void Success() + => Assert.Equal( + $"{success} Member removed from groups.", + func(removeOp, 1, 3, 3, 0) + ); + + [Fact] + public void PartialSuccess1() + => Assert.Equal( + $"{success} Member removed from 1 group (member already not in 2 groups).", + func(removeOp, 1, 3, 1, 2) + ); + + [Fact] + public void PartialSuccess2() + => Assert.Equal( + $"{success} Member removed from 2 groups (member already not in 1 group).", + func(removeOp, 1, 3, 2, 1) + ); + + [Fact] + public void Failure() + => Assert.Equal( + $"{failure} Member not removed from groups (member already not in groups).", + func(removeOp, 1, 3, 0, 3) + ); + } + + public class MembersGroup + { + [Fact] + public void Success() + => Assert.Equal( + $"{success} Members removed from group.", + func(removeOp, 2, 1, 2, 0) + ); + + + public void PartialSuccess1() + => Assert.Equal( + $"{success} 1 member removed from group (2 members already not in group).", + func(removeOp, 3, 1, 1, 2) + ); + + [Fact] + public void PartialSuccess2() + => Assert.Equal( + $"{success} 2 members removed from group (1 member already not in group).", + func(removeOp, 3, 1, 2, 1) + ); + + [Fact] + public void Failure() + => Assert.Equal( + $"{failure} Members not removed from group (members already not in group).", + func(removeOp, 2, 1, 0, 2) + ); + } + + } + } +} \ No newline at end of file