Added appropriate error messages for every club related command. Removed obsolete error messages.

src/NadekoBot/Modules/Xp/Club/Club.cs

@@ -17,11 +17,14 @@ public partial class Xp
         [Cmd]
         public async Task ClubTransfer([Leftover] IUser newOwner)
         {
-            var club = _service.TransferClub(ctx.User, newOwner);
+            var result = _service.TransferClub(ctx.User, newOwner);
 
-            if (club is null)
+            if (!result.TryPickT0(out var club, out var error))
             {
-                await ReplyErrorLocalizedAsync(strs.club_transfer_failed);
+                if(error == ClubTransferError.NotOwner)
+                    await ReplyErrorLocalizedAsync(strs.club_owner_only);
+                else 
+                    await ReplyErrorLocalizedAsync(strs.club_target_not_member);
             }
             else
             {
@@ -37,14 +40,18 @@ public partial class Xp
         [Cmd]
         public async Task ClubAdmin([Leftover] IUser toAdmin)
         {
-            var admin = await _service.ToggleAdminAsync(ctx.User, toAdmin);
+            var result = await _service.ToggleAdminAsync(ctx.User, toAdmin);
 
-            if (admin is null)
-                await ReplyErrorLocalizedAsync(strs.club_admin_error);
-            else if (admin is true)
+            if (result == ToggleAdminResult.AddedAdmin)
                 await ReplyConfirmLocalizedAsync(strs.club_admin_add(Format.Bold(toAdmin.ToString())));
-            else
+            else if (result == ToggleAdminResult.RemovedAdmin)
                 await ReplyConfirmLocalizedAsync(strs.club_admin_remove(Format.Bold(toAdmin.ToString())));
+            else if (result == ToggleAdminResult.NotOwner)
+                await ReplyErrorLocalizedAsync(strs.club_owner_only);
+            else if (result == ToggleAdminResult.CantTargetThyself)
+                await ReplyErrorLocalizedAsync(strs.club_admin_invalid_target);
+            else if (result == ToggleAdminResult.TargetNotMember)
+                await ReplyErrorLocalizedAsync(strs.club_target_not_member);
         }
 
         [Cmd]
@@ -56,17 +63,22 @@ public partial class Xp
                 return;
             }
 
-            var succ = await _service.CreateClubAsync(ctx.User, clubName);
+            var result = await _service.CreateClubAsync(ctx.User, clubName);
 
-            if (succ is null)
+            if (result == ClubCreateResult.NameTaken)
             {
                 await ReplyErrorLocalizedAsync(strs.club_create_error_name);
                 return;
             }
 
-            if (succ is false)
+            if (result == ClubCreateResult.InsufficientLevel)
             {
-                await ReplyErrorLocalizedAsync(strs.club_create_error);
+                await ReplyErrorLocalizedAsync(strs.club_create_insuff_lvl);
+            }
+
+            if (result == ClubCreateResult.AlreadyInAClub)
+            {
+                await ReplyErrorLocalizedAsync(strs.club_already_in);
                 return;
             }
 
@@ -76,14 +88,21 @@ public partial class Xp
         [Cmd]
         public async Task ClubIcon([Leftover] string url = null)
         {
-            if ((!Uri.IsWellFormedUriString(url, UriKind.Absolute) && url is not null)
-                || !await _service.SetClubIconAsync(ctx.User.Id, url))
+            if ((!Uri.IsWellFormedUriString(url, UriKind.Absolute) && url is not null))
             {
-                await ReplyErrorLocalizedAsync(strs.club_icon_error);
+                await ReplyErrorLocalizedAsync(strs.club_icon_url_format);
                 return;
             }
 
-            await ReplyConfirmLocalizedAsync(strs.club_icon_set);
+            var result = await _service.SetClubIconAsync(ctx.User.Id, url);
+            if(result == SetClubIconResult.Success)
+                await ReplyConfirmLocalizedAsync(strs.club_icon_set);
+            else if (result == SetClubIconResult.NotOwner)
+                await ReplyErrorLocalizedAsync(strs.club_owner_only);
+            else if (result == SetClubIconResult.TooLarge)
+                await ReplyErrorLocalizedAsync(strs.club_icon_too_large);
+            else if (result == SetClubIconResult.InvalidFileType)
+                await ReplyErrorLocalizedAsync(strs.club_icon_invalid_filetype);
         }
 
         private async Task InternalClubInfoAsync(ClubInfo club)
@@ -254,14 +273,17 @@ public partial class Xp
         [Priority(0)]
         public async Task ClubAccept([Leftover] string userName)
         {
-            if (_service.AcceptApplication(ctx.User.Id, userName, out var discordUser))
+            var result = _service.AcceptApplication(ctx.User.Id, userName, out var discordUser);
+            if (result == ClubAcceptResult.Accepted)
                 await ReplyConfirmLocalizedAsync(strs.club_accepted(Format.Bold(discordUser.ToString())));
-            else
-                await ReplyErrorLocalizedAsync(strs.club_accept_error);
+            else if(result == ClubAcceptResult.NoSuchApplicant)
+                await ReplyErrorLocalizedAsync(strs.club_accept_invalid_applicant);
+            else if(result == ClubAcceptResult.NotOwnerOrAdmin)
+                await ReplyErrorLocalizedAsync(strs.club_admin_perms);
         }
 
         [Cmd]
-        public async Task Clubleave()
+        public async Task ClubLeave()
         {
             var res = _service.LeaveClub(ctx.User);
 
@@ -282,13 +304,20 @@ public partial class Xp
         [Priority(0)]
         public Task ClubKick([Leftover] string userName)
         {
-            if (_service.Kick(ctx.User.Id, userName, out var club))
+            var result = _service.Kick(ctx.User.Id, userName, out var club);
+            if(result == ClubKickResult.Success)
             {
                 return ReplyConfirmLocalizedAsync(strs.club_user_kick(Format.Bold(userName),
                     Format.Bold(club.ToString())));
             }
 
-            return ReplyErrorLocalizedAsync(strs.club_user_kick_fail);
+            if (result == ClubKickResult.Hierarchy)
+                return ReplyErrorLocalizedAsync(strs.club_kick_hierarchy);
+
+            if (result == ClubKickResult.NotOwnerOrAdmin)
+                return ReplyErrorLocalizedAsync(strs.club_admin_perms);
+            
+            return ReplyErrorLocalizedAsync(strs.club_target_not_member);
         }
 
         [Cmd]

src/NadekoBot/Modules/Xp/Club/ClubService.cs

@@ -4,6 +4,7 @@ using LinqToDB.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore;
 using NadekoBot.Db;
 using NadekoBot.Db.Models;
+using OneOf;
 
 namespace NadekoBot.Modules.Xp.Services;
 
@@ -18,15 +19,8 @@ public class ClubService : INService, IClubService
         _httpFactory = httpFactory;
     }
 
-    public enum CLubCreateResult
-    {
-        Success,
-        AlreadyInAClub,
-        NameTaken,
-        InsufficientLevel,
-    }
     
-    public async Task<bool?> CreateClubAsync(IUser user, string clubName)
+    public async Task<ClubCreateResult> CreateClubAsync(IUser user, string clubName)
     {
         //must be lvl 5 and must not be in a club already
 
@@ -34,11 +28,14 @@ public class ClubService : INService, IClubService
         var du = uow.GetOrCreateUser(user);
         var xp = new LevelStats(du.TotalXp);
         
-        if (xp.Level < 5 || du.ClubId is not null)
-            return false;
+        if (xp.Level < 5) 
+            return ClubCreateResult.InsufficientLevel;
+        
+        if (du.ClubId is not null)
+            return ClubCreateResult.AlreadyInAClub;
 
         if (await uow.Clubs.AnyAsyncEF(x => x.Name == clubName))
-            return null;
+            return ClubCreateResult.NameTaken;
         
         du.IsClubAdmin = true;
         du.Club = new()
@@ -52,23 +49,20 @@ public class ClubService : INService, IClubService
         await uow.GetTable<ClubApplicants>()
                  .DeleteAsync(x => x.UserId == du.Id);
 
-        return true;
-    }
-
-    public enum ClubTransferError
-    {
-        NotOwner,
-        TargetNotMember
+        return ClubCreateResult.Success;
     }
     
-    public ClubInfo TransferClub(IUser from, IUser newOwner)
+    public OneOf<ClubInfo, ClubTransferError> TransferClub(IUser from, IUser newOwner)
     {
         using var uow = _db.GetDbContext();
         var club = uow.Clubs.GetByOwner(@from.Id);
         var newOwnerUser = uow.GetOrCreateUser(newOwner);
 
-        if (club is null || club.Owner.UserId != from.Id || !club.Members.Contains(newOwnerUser))
-            return null;
+        if (club is null || club.Owner.UserId != from.Id)
+            return ClubTransferError.NotOwner;
+        
+        if (!club.Members.Contains(newOwnerUser))
+            return ClubTransferError.TargetNotMember;
 
         club.Owner.IsClubAdmin = true; // old owner will stay as admin
         newOwnerUser.IsClubAdmin = true;
@@ -76,30 +70,25 @@ public class ClubService : INService, IClubService
         uow.SaveChanges();
         return club;
     }
-
-    public enum ToggleAdminResult
-    {
-        AddedAdmin,
-        RemovedAdmin,
-        TargetNotMember,
-        CanTargetThyself,
-    }
     
-    public async Task<bool?> ToggleAdminAsync(IUser owner, IUser toAdmin)
+    public async Task<ToggleAdminResult> ToggleAdminAsync(IUser owner, IUser toAdmin)
     {
+        if (owner.Id == toAdmin.Id)
+            return ToggleAdminResult.CantTargetThyself;
+        
         await using var uow = _db.GetDbContext();
         var club = uow.Clubs.GetByOwner(owner.Id);
         var adminUser = uow.GetOrCreateUser(toAdmin);
 
-        if (club is null || club.Owner.UserId != owner.Id || !club.Members.Contains(adminUser))
-            return null;
-
-        if (club.OwnerId == adminUser.Id)
-            return true;
-
+        if (club is null)
+            return ToggleAdminResult.NotOwner;
+        
+        if(!club.Members.Contains(adminUser))
+            return ToggleAdminResult.TargetNotMember;
+        
         var newState = adminUser.IsClubAdmin = !adminUser.IsClubAdmin;
         await uow.SaveChangesAsync();
-        return newState;
+        return newState ? ToggleAdminResult.AddedAdmin : ToggleAdminResult.RemovedAdmin;
     }
 
     public ClubInfo GetClubByMember(IUser user)
@@ -108,35 +97,31 @@ public class ClubService : INService, IClubService
         var member = uow.Clubs.GetByMember(user.Id);
         return member;
     }
-
-    public enum SetClubIconResult
-    {
-        Success,
-        InvalidFiletype,
-        TooLarge,
-        NotOwner,
-    }
     
-    public async Task<bool> SetClubIconAsync(ulong ownerUserId, string url)
+    public async Task<SetClubIconResult> SetClubIconAsync(ulong ownerUserId, string url)
     {
         if (url is not null)
         {
             using var http = _httpFactory.CreateClient();
             using var temp = await http.GetAsync(url, HttpCompletionOption.ResponseHeadersRead);
-            if (!temp.IsImage() || temp.GetContentLength() > 5.Megabytes().Bytes)
-                return false;
+            
+            if (!temp.IsImage()) 
+                return SetClubIconResult.InvalidFileType;
+            
+            if (temp.GetContentLength() > 5.Megabytes().Bytes)
+                return SetClubIconResult.TooLarge;
         }
 
         await using var uow = _db.GetDbContext();
         var club = uow.Clubs.GetByOwner(ownerUserId);
 
         if (club is null)
-            return false;
+            return SetClubIconResult.NotOwner;
 
         club.ImageUrl = url;
         await uow.SaveChangesAsync();
 
-        return true;
+        return SetClubIconResult.Success;
     }
 
     public bool GetClubByName(string clubName, out ClubInfo club)
@@ -175,25 +160,19 @@ public class ClubService : INService, IClubService
         return ClubApplyResult.Success;
     }
 
-    public enum ClubAcceptResult
-    {
-        Accepted,
-        NotOwnerOrAdmin,
-        NoSuchApplicant,
-    }
     
-    public bool AcceptApplication(ulong clubOwnerUserId, string userName, out DiscordUser discordUser)
+    public ClubAcceptResult AcceptApplication(ulong clubOwnerUserId, string userName, out DiscordUser discordUser)
     {
         discordUser = null;
         using var uow = _db.GetDbContext();
         var club = uow.Clubs.GetByOwnerOrAdmin(clubOwnerUserId);
         if (club is null)
-            return false;
+            return ClubAcceptResult.NotOwnerOrAdmin;
 
         var applicant =
             club.Applicants.FirstOrDefault(x => x.User.ToString().ToUpperInvariant() == userName.ToUpperInvariant());
         if (applicant is null)
-            return false;
+            return ClubAcceptResult.NoSuchApplicant;
 
         applicant.User.Club = club;
         applicant.User.IsClubAdmin = false;
@@ -205,7 +184,7 @@ public class ClubService : INService, IClubService
 
         discordUser = applicant.User;
         uow.SaveChanges();
-        return true;
+        return ClubAcceptResult.Accepted;
     }
 
     public ClubInfo GetClubWithBansAndApplications(ulong ownerUserId)
@@ -305,27 +284,20 @@ public class ClubService : INService, IClubService
         return ClubUnbanResult.Success;
     }
 
-    public enum ClubKickResult
-    {
-        Success,
-        NotOwnerOrAdmin,
-        TargetNotAMember,
-        Hierarchy
-    }
     
-    public bool Kick(ulong kickerId, string userName, out ClubInfo club)
+    public ClubKickResult Kick(ulong kickerId, string userName, out ClubInfo club)
     {
         using var uow = _db.GetDbContext();
         club = uow.Clubs.GetByOwnerOrAdmin(kickerId);
         if (club is null)
-            return false;
+            return ClubKickResult.NotOwnerOrAdmin;
 
         var usr = club.Members.FirstOrDefault(x => x.ToString().ToUpperInvariant() == userName.ToUpperInvariant());
         if (usr is null)
-            return false;
+            return ClubKickResult.TargetNotAMember;
 
         if (club.OwnerId == usr.Id || (usr.IsClubAdmin && club.Owner.UserId != kickerId))
-            return false;
+            return ClubKickResult.Hierarchy;
 
         club.Members.Remove(usr);
         var app = club.Applicants.FirstOrDefault(x => x.UserId == usr.Id);
@@ -333,7 +305,7 @@ public class ClubService : INService, IClubService
             club.Applicants.Remove(app);
         uow.SaveChanges();
 
-        return true;
+        return ClubKickResult.Success;
     }
 
     public List<ClubInfo> GetClubLeaderboardPage(int page)

src/NadekoBot/Modules/Xp/Club/IClubService.cs

@@ -1,24 +1,25 @@
 using NadekoBot.Db.Models;
+using OneOf;
 
 namespace NadekoBot.Modules.Xp.Services;
 
 public interface IClubService
 {
-    Task<bool?> CreateClubAsync(IUser user, string clubName);
-    ClubInfo? TransferClub(IUser from, IUser newOwner);
-    Task<bool?> ToggleAdminAsync(IUser owner, IUser toAdmin);
+    Task<ClubCreateResult> CreateClubAsync(IUser user, string clubName);
+    OneOf<ClubInfo,ClubTransferError> TransferClub(IUser from, IUser newOwner);
+    Task<ToggleAdminResult> ToggleAdminAsync(IUser owner, IUser toAdmin);
     ClubInfo? GetClubByMember(IUser user);
-    Task<bool> SetClubIconAsync(ulong ownerUserId, string? url);
+    Task<SetClubIconResult> SetClubIconAsync(ulong ownerUserId, string? url);
     bool GetClubByName(string clubName, out ClubInfo club);
     ClubApplyResult ApplyToClub(IUser user, ClubInfo club);
-    bool AcceptApplication(ulong clubOwnerUserId, string userName, out DiscordUser discordUser);
+    ClubAcceptResult AcceptApplication(ulong clubOwnerUserId, string userName, out DiscordUser discordUser);
     ClubInfo? GetClubWithBansAndApplications(ulong ownerUserId);
     ClubLeaveResult LeaveClub(IUser user);
     bool SetDescription(ulong userId, string? desc);
     bool Disband(ulong userId, out ClubInfo club);
     ClubBanResult Ban(ulong bannerId, string userName, out ClubInfo club);
     ClubUnbanResult UnBan(ulong ownerUserId, string userName, out ClubInfo club);
-    bool Kick(ulong kickerId, string userName, out ClubInfo club);
+    ClubKickResult Kick(ulong kickerId, string userName, out ClubInfo club);
     List<ClubInfo> GetClubLeaderboardPage(int page);
 }
 

src/NadekoBot/Modules/Xp/Club/Results/ClubAcceptResult.cs

@@ -0,0 +1,8 @@
+namespace NadekoBot.Modules.Xp.Services;
+
+public enum ClubAcceptResult
+{
+    Accepted,
+    NotOwnerOrAdmin,
+    NoSuchApplicant,
+}

src/NadekoBot/Modules/Xp/Club/Results/ClubCreateResult.cs

@@ -0,0 +1,9 @@
+namespace NadekoBot.Modules.Xp.Services;
+
+public enum ClubCreateResult
+{
+    Success,
+    AlreadyInAClub,
+    NameTaken,
+    InsufficientLevel,
+}

src/NadekoBot/Modules/Xp/Club/Results/ClubKickResult.cs

@@ -0,0 +1,9 @@
+namespace NadekoBot.Modules.Xp.Services;
+
+public enum ClubKickResult
+{
+    Success,
+    NotOwnerOrAdmin,
+    TargetNotAMember,
+    Hierarchy
+}

src/NadekoBot/Modules/Xp/Club/Results/ClubTransferError.cs

@@ -0,0 +1,7 @@
+namespace NadekoBot.Modules.Xp.Services;
+
+public enum ClubTransferError
+{
+    NotOwner,
+    TargetNotMember
+}

src/NadekoBot/Modules/Xp/Club/Results/SetClubIconResult.cs

@@ -0,0 +1,9 @@
+namespace NadekoBot.Modules.Xp.Services;
+
+public enum SetClubIconResult
+{
+    Success,
+    InvalidFileType,
+    TooLarge,
+    NotOwner,
+}

src/NadekoBot/Modules/Xp/Club/Results/ToggleAdminResult.cs

@@ -0,0 +1,10 @@
+namespace NadekoBot.Modules.Xp.Services;
+
+public enum ToggleAdminResult
+{
+    AddedAdmin,
+    RemovedAdmin,
+    NotOwner,
+    TargetNotMember,
+    CantTargetThyself,
+}

src/NadekoBot/data/strings/responses/responses.en-US.json

@@ -150,7 +150,6 @@
   "rc": "Color of {0} role has been changed.",
   "rc_perms": "Error occurred due to invalid color or insufficient permissions.",
   "color": "Color",
-  "icon": "Icon",
   "hoisted": "Hoisted",
   "mentionable": "Mentionable",
   "remrole": "Successfully removed role {0} from user {1}",
@@ -271,7 +270,6 @@
   "usage": "Usage",
   "options": "Options",
   "requires": "Requires",
-  "tag": "Tag",
   "animal_race": "Animal race",
   "animal_race_failed": "Failed to start since there was not enough participants.",
   "animal_race_full": "Race is full! Starting immediately.",
@@ -443,7 +441,6 @@
   "removed": "removed permission #{0} - {1}",
   "rx_disable": "Disabled usage of {0} {1} for {2} role.",
   "rx_enable": "Enabled usage of {0} {1} for {2} role.",
-  "sec": "sec.",
   "sx_disable": "Disabled usage of {0} {1} on this server.",
   "sx_enable": "Enabled usage of {0} {1} on this server.",
   "unblacklisted": "Unblacklisted {0} with ID {1}",
@@ -841,20 +838,22 @@
   "club_join_banned": "You're banned from that club.",
   "club_already_in": "You are already a member of a club.",
   "club_create_error_name": "Failed creating the club. A club with that name already exists.",
-  "club_create_error": "Failed creating the club. Make sure you're above level 5 and not a member of a club already.",
   "club_name_too_long": "Club name is too long.",
   "club_created": "Club {0} successfully created!",
+  "club_create_insuff_lvl": "You don't meet the minimum level requirements in order to create a club.",
   "club_not_exists": "That club doesn't exist.",
   "club_applied": "You've applied for membership in {0} club.",
   "club_accepted": "Accepted user {0} to the club.",
-  "club_accept_error": "User not found",
+  "club_accept_invalid_applicant": "That user has not applied to your club.",
   "club_left": "You've left the club.",
   "club_not_in_a_club": "You are not in a club.",
   "club_owner_cant_leave": "Club owner can't leave the club - you must either transfer ownership or disband the club.",
   "club_user_kick": "User {0} kicked from {1} club.",
   "club_user_not_in_club": "{0} is not in a club.",
-  "club_user_kick_fail": "Error kicking. You're either not the club owner, or that user is not in your club.",
   "club_user_banned": "Banned user {0} from {1} club.",
+  "club_owner_only": "This action can only be performed by the club owner.",
+  "club_admin_invalid_target": "You can't target yourself or the club owner.",
+  "club_target_not_member": "Specified user is not a member of your club.",
   "club_admin_perms": "You must be a club admin or owner in order to perform this action.",
   "club_ban_fail_user_not_found": "That user is not in your club or applied to it.",
   "club_ban_fail_unbannable": "Only the club owner can ban club admins.",
@@ -864,11 +863,14 @@
   "club_desc_update_failed": "Failed changing club description.",
   "club_disbanded": "Club {0} has been disbanded",
   "club_disband_error": "Error. You are either not in a club, or you are not the owner of your club.",
-  "club_icon_error": "Not a valid image url or you're not the club owner.",
+  "club_icon_too_large": "Image is too large.",
+  "club_icon_invalid_filetype": "Specified image has an invalid filetype. Make sure you're specifying a direct image url.",
+  "club_icon_url_format": "You must specify an absolute image url/.",
   "club_icon_set": "New club icon set.",
   "club_bans_for": "Bans for {0} club",
   "club_apps_for": "Applicants for {0} club",
   "club_leaderboard": "Club leaderboard - page {0}",
+  "club_kick_hierarchy": "Only club owner can kick club admins. Owner can't be kicked.",
   "template_reloaded": "Xp template has been reloaded.",
   "expr_edited": "Expression Edited",
   "self_assign_are_exclusive": "You can only choose 1 role from each group.",
@@ -882,7 +884,6 @@
   "poll_closed": "Poll Closed!",
   "club_admin_add": "{0} is now a club admin.",
   "club_admin_remove": "{0} is no longer club admin.",
-  "club_admin_error": "Error. You are either not the owner of the club, or that user is not in your club.",
   "started": "Started. Reposting every {0}s.",
   "stopped": "Stopped reposting.",
   "feed_added": "Feed added.",
@@ -929,7 +930,6 @@
   "mass_ban_completed": "Banned {0} users.",
   "mass_kill_completed": "Mass Banning and Blacklisting of {0} users is complete.",
   "club_transfered": "Ownership of the club {0} has been transferred to {1}",
-  "club_transfer_failed": "Transfer failed. You must be the club owner. Target must be a member of your club.",
   "roll_duel_challenge": "challenged {1} for a roll duel for {2}",
   "roll_duel": "Roll Duel",
   "roll_duel_no_funds": "Either you or your opponent don't have enough funds.",