From 6f99338a623c4f90316b4ca7d2bbd22cbdfa9560 Mon Sep 17 00:00:00 2001 From: darbyjack Date: Thu, 22 Oct 2026 05:48:30 +0000 Subject: [PATCH 1/4] fix: make /guild code redeem join a guild the way /guild accept does Redeeming a code added the player to the guild's member list and stopped there. It skipped the join cooldown, fired no join event, granted no permissions and registered the member differently from an invite, so a player who joined by code was a member in name only. /guild code redeem now refuses while the join cooldown runs, and GuildHandler.handleInvite fires the cancellable GuildJoinEvent before anything is consumed, registers the member through Guild.addMember and applies the guild tier and role permissions. Guild.addMember and Guild.addMemberByCode share one registration, which identifies a member by uuid and stamps the join date. The use count, the redeemer list and the creator of a code are unchanged. --- docs/e2e-coverage.md | 6 +- .../me/glaremasters/guilds/guild/Guild.java | 34 ++- .../guilds/guild/GuildHandler.java | 21 +- .../guilds/commands/codes/CommandCode.kt | 12 + src/test/e2e/tests/codes.spec.ts | 62 +++- .../guilds/guild/GuildCodeTest.java | 81 ++++++ .../guilds/guild/GuildHandlerRedeemTest.java | 267 ++++++++++++++++++ .../guild/GuildMemberRegistrationTest.java | 135 +++++++++ 8 files changed, 607 insertions(+), 11 deletions(-) create mode 100644 src/test/java/me/glaremasters/guilds/guild/GuildCodeTest.java create mode 100644 src/test/java/me/glaremasters/guilds/guild/GuildHandlerRedeemTest.java create mode 100644 src/test/java/me/glaremasters/guilds/guild/GuildMemberRegistrationTest.java diff --git a/docs/e2e-coverage.md b/docs/e2e-coverage.md index 10d87eacd..659c37b4c 100644 --- a/docs/e2e-coverage.md +++ b/docs/e2e-coverage.md @@ -3,7 +3,7 @@ Which Guilds feature is tested where, and what is deliberately left out. See `e2e-testing.md` for how to run any of it. -141 tests across 12 spec files, plus 3 preflight tests that run before every suite and fail the run +143 tests across 12 spec files, plus 3 preflight tests that run before every suite and fail the run if Guilds or any of its hard dependencies did not load. ## Preflight — `plugins/guilds-preflight.spec.ts` @@ -128,13 +128,15 @@ meaningful. | Alliance chat between guilds | `chat.spec.ts`, ally chat reaches an allied guild and not the third party | | Cleanup on deletion | a deleted guild drops out of its allies | -## H. Invite codes — `tests/codes.spec.ts` (12, of which 5 are one `describe.serial` chain) +## H. Invite codes — `tests/codes.spec.ts` (14, of which 5 are one `describe.serial` chain) | Requirement | Test | | --- | --- | | Creating invite codes | a code can be created, listed and deleted | | Viewing and listing codes | a code can be created, listed and deleted / *and it is listed for the guild master* | | Redeeming a code | a code can be redeemed and puts the player in the guild | +| Redeeming a code grants what the guild grants | a player who joins by code gets the permissions their guild grants them | +| The join cooldown applies to a code | a code cannot be redeemed while the join cooldown is running | | Deleting codes | a code can be created, listed and deleted | | Invalid codes | an invalid code is refused | | Reusing codes where restricted | a code runs out of uses and then says so | diff --git a/src/main/java/me/glaremasters/guilds/guild/Guild.java b/src/main/java/me/glaremasters/guilds/guild/Guild.java index eeb950ab9..c9e4f9173 100644 --- a/src/main/java/me/glaremasters/guilds/guild/Guild.java +++ b/src/main/java/me/glaremasters/guilds/guild/Guild.java @@ -180,23 +180,43 @@ public ItemStack getSkull() { /** * Add player to a guild + * * @param player the player being added * @param guildHandler guild handler */ public void addMember(OfflinePlayer player, GuildHandler guildHandler) { - GuildMember member = new GuildMember(player.getUniqueId(), guildHandler.getLowestGuildRole()); - if (members.contains(member)) return; - removeInvitedMember(member.getUuid()); - members.add(member); - member.setJoinDate(System.currentTimeMillis()); + addMember(new GuildMember(player.getUniqueId(), guildHandler.getLowestGuildRole())); } /** * Invite a member by guild code - * @param guildMember + * + *

The same registration as {@link #addMember(OfflinePlayer, GuildHandler)}: a player is only + * in the guild once, a stale invite is dropped, and the member carries the date they joined. + * The role is whatever the caller decided, which for a code is the lowest role in the guild. + * + * @param guildMember the member to add */ public void addMemberByCode(GuildMember guildMember) { - members.add(guildMember); + addMember(guildMember); + } + + /** + * Registers a member with the guild. + * + *

A member is identified by their uuid, not by the object: {@link GuildMember} does not + * override {@code equals}, so comparing objects would never find the member that is already in + * the guild and every caller would be free to add a second entry for the same player. + * + * @param member the member to register + */ + private void addMember(GuildMember member) { + if (getMember(member.getUuid()) != null) return; + removeInvitedMember(member.getUuid()); + members.add(member); + if (member.getJoinDate() == 0L) { + member.setJoinDate(System.currentTimeMillis()); + } } /** diff --git a/src/main/java/me/glaremasters/guilds/guild/GuildHandler.java b/src/main/java/me/glaremasters/guilds/guild/GuildHandler.java index eaa16e32e..cdf38a89c 100644 --- a/src/main/java/me/glaremasters/guilds/guild/GuildHandler.java +++ b/src/main/java/me/glaremasters/guilds/guild/GuildHandler.java @@ -6,6 +6,7 @@ import co.aikar.commands.ACFUtil; import co.aikar.commands.PaperCommandManager; import me.glaremasters.guilds.Guilds; +import me.glaremasters.guilds.api.events.GuildJoinEvent; import me.glaremasters.guilds.configuration.sections.GuildSettings; import me.glaremasters.guilds.configuration.sections.GuildVaultSettings; import me.glaremasters.guilds.configuration.sections.PluginSettings; @@ -1107,6 +1108,11 @@ public void removeRolePermsFromAll(final Permission permission, final Guild guil /** * Handle inviting player when redeeming a code * + *

A code is a way into a guild, so this does what {@code /guild accept} does: it fires the + * cancellable {@link GuildJoinEvent} before anything is consumed, registers the member the + * same way an invite does, and hands the member the permissions their guild and role grant. + * A plugin that cancels the join event stops the redeem, and the code keeps its use. + * * @param manager command manager * @param player the player redeeming the code * @param guild the guild they are trying to join @@ -1116,11 +1122,24 @@ public void handleInvite(PaperCommandManager manager, Player player, Guild guild if (code.getUses() <= 0) ACFUtil.sneaky(new ExpectationNotMet(Messages.CODES__OUT)); + final GuildJoinEvent event = new GuildJoinEvent(player, guild); + Bukkit.getPluginManager().callEvent(event); + + if (event.isCancelled()) { + return; + } + code.addRedeemer(player); - guild.addMemberByCode(new GuildMember(player.getUniqueId(), getLowestGuildRole())); + guild.addMember(player, this); addToMemberCache(player.getUniqueId(), guild.getId()); + final Permission permission = getGuildsPlugin().getPermissions(); + if (permission != null) { + addGuildPerms(permission, player); + addRolePerm(permission, player); + } + if (ClaimUtils.isEnabled(settingsManager)) { WorldGuardWrapper wrapper = WorldGuardWrapper.getInstance(); ClaimUtils.getGuildClaim(wrapper, player, guild).ifPresent(region -> ClaimUtils.addMember(region, player)); diff --git a/src/main/kotlin/me/glaremasters/guilds/commands/codes/CommandCode.kt b/src/main/kotlin/me/glaremasters/guilds/commands/codes/CommandCode.kt index df3c4b570..368f1d9b7 100644 --- a/src/main/kotlin/me/glaremasters/guilds/commands/codes/CommandCode.kt +++ b/src/main/kotlin/me/glaremasters/guilds/commands/codes/CommandCode.kt @@ -15,6 +15,8 @@ import co.aikar.commands.annotation.Syntax import co.aikar.commands.annotation.Values import me.glaremasters.guilds.Guilds import me.glaremasters.guilds.configuration.sections.CodeSettings +import me.glaremasters.guilds.cooldowns.Cooldown +import me.glaremasters.guilds.cooldowns.CooldownHandler import me.glaremasters.guilds.exceptions.ExpectationNotMet import me.glaremasters.guilds.guild.Guild import me.glaremasters.guilds.guild.GuildHandler @@ -32,6 +34,8 @@ internal class CommandCode : BaseCommand() { lateinit var settingsManager: SettingsManager @Dependency lateinit var guildHandler: GuildHandler + @Dependency + lateinit var cooldownHandler: CooldownHandler @Subcommand("code create") @Description("{@@descriptions.code-create}") @@ -98,6 +102,14 @@ internal class CommandCode : BaseCommand() { fun redeem(@Conditions("NoGuild") player: Player, code: String) { val guild = guildHandler.getGuildByCode(code) ?: throw ExpectationNotMet(Messages.CODES__INVALID_CODE) + // A code is a way into a guild, so it is refused while the join cooldown runs, the same + // way `/guild accept` and `/guild create` refuse it. + val cooldown = Cooldown.Type.Join.name + + if (cooldownHandler.hasCooldown(cooldown, player.uniqueId)) { + throw ExpectationNotMet(Messages.ACCEPT__COOLDOWN, "{amount}", cooldownHandler.getRemaining(cooldown, player.uniqueId).toString()) + } + if (guildHandler.checkIfFull(guild)) { throw ExpectationNotMet(Messages.ACCEPT__GUILD_FULL) } diff --git a/src/test/e2e/tests/codes.spec.ts b/src/test/e2e/tests/codes.spec.ts index 2e51faf30..faeaf33d0 100644 --- a/src/test/e2e/tests/codes.spec.ts +++ b/src/test/e2e/tests/codes.spec.ts @@ -1,7 +1,8 @@ import { describe, expect, test } from '@plugwright/runner'; import { commandWithout, expectReceived } from '../plugins/support/assertions.js'; import { MSG } from '../plugins/support/expected.js'; -import { guildList } from '../plugins/support/state.js'; +import { ROLE } from '../plugins/support/fixtures.js'; +import { guildList, guildMembers } from '../plugins/support/state.js'; import { stripColors } from '../plugins/support/text.js'; /** @@ -63,6 +64,65 @@ test('a code can be redeemed and puts the player in the guild', async ({ player, const entry = (await guildList(joiner)).find(item => item.name === guild)!; expect(entry.memberCount).toBe(2); + + // The roster is the one an invited member appears on, with the same role. + const roster = await guildMembers(player); + expect(roster.map(member => member.name).sort()).toEqual([player.username, joiner.username].sort()); + expect(roster.find(member => member.name === joiner.username)!.role).toBe(ROLE.member); +}); + +test('a player who joins by code gets the permissions their guild grants them', async ({ + player, + createPlayer, + guilds, +}) => { + const guild = await guilds.createGuild(player); + const created = player.getMessageBufferIndex(); + await guilds.run(player, '/guild code create 2'); + await expectReceived(player, MSG.codeCreated, created); + const code = parseCode(player.messageBuffer.slice(created).join('\n'))!; + + const joiner = await createPlayer({ username: guilds.playerName() }); + const joining = joiner.getMessageBufferIndex(); + await guilds.run(joiner, `/guild code redeem ${code}`); + await expectReceived(joiner, MSG.codeJoined(guild), joining); + + // tiers.yml grants guilds.command.admin to every member, and the runner denies that node on + // LuckPerms' default group, so an admin command answering at all is the proof that the guild + // handed the node over. LuckPerms applies it to an online player on its own schedule, so the + // probe is re-issued until it works. + const asMember = joiner.getMessageBufferIndex(); + try { + await guilds.awaitEffect(joiner, `/guild admin motd ${guild}`, MSG.motdNotSet); + } catch { + const told = joiner.messageBuffer.slice(asMember).map(stripColors).join(' | '); + throw new Error(`the joiner never got the permissions their guild grants; they were told: ${told}`); + } +}); + +test('a code cannot be redeemed while the join cooldown is running', async ({ player, createPlayer, guilds }) => { + const guild = await guilds.createGuild(player); + const created = player.getMessageBufferIndex(); + await guilds.run(player, '/guild code create 2'); + await expectReceived(player, MSG.codeCreated, created); + const code = parseCode(player.messageBuffer.slice(created).join('\n'))!; + + const joiner = await createPlayer({ username: guilds.playerName() }); + const joining = joiner.getMessageBufferIndex(); + await guilds.run(joiner, `/guild code redeem ${code}`); + await expectReceived(joiner, MSG.codeJoined(guild), joining); + + // Leaving starts the join cooldown, and joining a guild is what that cooldown is about. A code + // is a way into a guild, so it is refused the same way an invite is. + await guilds.run(joiner, '/guild leave'); + await expect(joiner).toHaveReceivedMessage(MSG.leaveWarning); + await guilds.confirm(joiner); + await expect(joiner).toHaveReceivedMessage(MSG.leaveSuccess); + + const blocked = joiner.getMessageBufferIndex(); + await guilds.run(joiner, `/guild code redeem ${code}`); + await expectReceived(joiner, MSG.acceptCooldown, blocked); + await expect(joiner).not.toHaveReceivedMessage(MSG.codeJoined(guild), { since: blocked }); }); test('an invalid code is refused', async ({ player, createPlayer, guilds }) => { diff --git a/src/test/java/me/glaremasters/guilds/guild/GuildCodeTest.java b/src/test/java/me/glaremasters/guilds/guild/GuildCodeTest.java new file mode 100644 index 000000000..2ea257f69 --- /dev/null +++ b/src/test/java/me/glaremasters/guilds/guild/GuildCodeTest.java @@ -0,0 +1,81 @@ +package me.glaremasters.guilds.guild; + +import org.bukkit.entity.Player; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; + +/** + * Covers what one use of an invite code means. + * + *

A code is a counter, a list of the players who spent it, and the player who made it. Those + * three things are the whole of its stored shape, and the code redeem path is not allowed to + * change any of them: a redeem spends exactly one use, records exactly one redeemer, and the + * creator and the remaining count are what the next reader sees. + */ +class GuildCodeTest { + + private static final String ID = "ABC1234"; + + private GuildCode code; + private UUID creatorId; + + @BeforeEach + void setUp() { + creatorId = UUID.randomUUID(); + code = new GuildCode(ID, 3, creatorId, new ArrayList<>()); + } + + @Test + @DisplayName("a redeem spends one use and records one redeemer") + void aRedeemSpendsOneUse() { + final UUID redeemer = UUID.randomUUID(); + + code.addRedeemer(player(redeemer)); + + assertEquals(2, code.getUses()); + assertEquals(Arrays.asList(redeemer), code.getRedeemers()); + } + + @Test + @DisplayName("a redeem leaves the creator and the id alone") + void aRedeemKeepsTheCreator() { + code.addRedeemer(player(UUID.randomUUID())); + + assertEquals(ID, code.getId()); + assertSame(creatorId, code.getCreator()); + } + + @Test + @DisplayName("the same player can spend the code twice, and it costs two uses") + void theSamePlayerCanRedeemTwice() { + // Documented, not endorsed: a player who leaves the guild they joined with a code can + // come back and spend the code again, once the join cooldown has run out. The code + // charges them for it, and the redeemers list names them twice. + final UUID redeemer = UUID.randomUUID(); + final Player player = player(redeemer); + + code.addRedeemer(player); + code.addRedeemer(player); + + assertEquals(1, code.getUses()); + assertEquals(Arrays.asList(redeemer, redeemer), code.getRedeemers()); + } + + /** + * A player with the given uuid. + */ + private static Player player(final UUID uuid) { + final Player player = Mockito.mock(Player.class); + Mockito.when(player.getUniqueId()).thenReturn(uuid); + return player; + } +} diff --git a/src/test/java/me/glaremasters/guilds/guild/GuildHandlerRedeemTest.java b/src/test/java/me/glaremasters/guilds/guild/GuildHandlerRedeemTest.java new file mode 100644 index 000000000..4dedce0d3 --- /dev/null +++ b/src/test/java/me/glaremasters/guilds/guild/GuildHandlerRedeemTest.java @@ -0,0 +1,267 @@ +package me.glaremasters.guilds.guild; + +import ch.jalu.configme.SettingsManager; +import co.aikar.commands.PaperCommandManager; +import me.glaremasters.guilds.Guilds; +import me.glaremasters.guilds.api.events.GuildJoinEvent; +import me.glaremasters.guilds.configuration.sections.HooksSettings; +import me.glaremasters.guilds.configuration.sections.PluginSettings; +import net.milkbowl.vault.permission.Permission; +import org.bukkit.Bukkit; +import org.bukkit.OfflinePlayer; +import org.bukkit.entity.Player; +import org.bukkit.plugin.PluginManager; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.ArgumentCaptor; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.Collections; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Covers what {@code /guild code redeem} does to the player who redeems a code. + * + *

{@code /guild accept} and {@code /guild code redeem} are the two ways into a guild, and they + * were not doing the same work. Accept fired the cancellable {@link GuildJoinEvent}, applied the + * guild's tier permissions and the member's role permissions, and registered the member through + * {@link Guild#addMember}; redeeming a code did none of that. A player who joined by code was a + * member in name only: third-party plugins listening for a join never heard about it, a listener + * that cancelled the event could not stop it, and the player kept no permissions until their next + * login. + * + *

Only the bookkeeping is exercised here. The claim and the announcement need a claim plugin and + * a real command manager, and the permissions are asserted through the Vault {@link Permission} + * mock, which is the same call the plugin makes. + */ +class GuildHandlerRedeemTest { + + private static final String CODE = "ABC1234"; + private static final String TIER_PERM = "guilds.command.admin"; + private static final String ROLE_NODE = "guilds.roles.member"; + + @TempDir + File dataFolder; + + private MockedStatic bukkit; + private PluginManager pluginManager; + private Permission permission; + private GuildHandler handler; + private Guild guild; + private GuildCode code; + private Player player; + private UUID playerId; + private UUID creatorId; + private PaperCommandManager manager; + + @BeforeEach + void setUp() throws IOException { + writeRoles(); + writeTiers(); + + final SettingsManager settingsManager = Mockito.mock(SettingsManager.class); + Mockito.when(settingsManager.getProperty(PluginSettings.RUN_VAULT_ASYNC)).thenReturn(false); + Mockito.when(settingsManager.getProperty(HooksSettings.WORLDGUARD)).thenReturn(false); + + permission = Mockito.mock(Permission.class); + + final Guilds guildsPlugin = Mockito.mock(Guilds.class, Mockito.RETURNS_DEEP_STUBS); + Mockito.when(guildsPlugin.getDataFolder()).thenReturn(dataFolder); + Mockito.when(guildsPlugin.getPermissions()).thenReturn(permission); + Mockito.when(guildsPlugin.getDatabase().getGuildAdapter().getAllGuilds()) + .thenReturn(Collections.emptyList()); + + handler = new GuildHandler(guildsPlugin, settingsManager); + + playerId = UUID.randomUUID(); + creatorId = UUID.randomUUID(); + + player = Mockito.mock(Player.class); + Mockito.when(player.getUniqueId()).thenReturn(playerId); + Mockito.when(player.getName()).thenReturn("Joiner"); + + pluginManager = Mockito.mock(PluginManager.class); + + // The mocks have to exist before the stubbing starts: Mockito refuses to create a mock + // inside a `thenReturn` argument, because that is what an unfinished stubbing looks like. + final OfflinePlayer joiner = offlinePlayer(playerId, "Joiner"); + final OfflinePlayer creator = offlinePlayer(creatorId, "Creator"); + + bukkit = Mockito.mockStatic(Bukkit.class); + bukkit.when(Bukkit::getPluginManager).thenReturn(pluginManager); + // The generic answer first: a later stub replaces an earlier one, so the named players + // below have to be registered after this one to win. + bukkit.when(() -> Bukkit.getOfflinePlayer(Mockito.any(UUID.class))).thenAnswer(invocation -> { + final UUID uuid = invocation.getArgument(0); + return offlinePlayer(uuid, "Someone"); + }); + bukkit.when(() -> Bukkit.getOfflinePlayer(playerId)).thenReturn(joiner); + bukkit.when(() -> Bukkit.getOfflinePlayer(creatorId)).thenReturn(creator); + + manager = Mockito.mock(PaperCommandManager.class, Mockito.RETURNS_DEEP_STUBS); + + final GuildMember master = new GuildMember(UUID.randomUUID(), handler.getGuildRole(0)); + guild = new Guild(UUID.randomUUID()); + guild.setName("TestGuild"); + guild.setPrefix("Test"); + guild.setStatus(Guild.Status.Private); + guild.setTier(handler.getLowestGuildTier()); + guild.setMembers(new ArrayList<>(Collections.singletonList(master))); + guild.setInvitedMembers(new ArrayList<>()); + guild.setAllies(new ArrayList<>()); + guild.setPendingAllies(new ArrayList<>()); + guild.setVaults(new ArrayList<>()); + guild.setCodes(new ArrayList<>()); + guild.setGuildMaster(master); + + code = new GuildCode(CODE, 2, creatorId, new ArrayList<>()); + guild.getCodes().add(code); + + handler.addGuild(guild); + } + + @AfterEach + void tearDown() { + bukkit.close(); + } + + @Test + @DisplayName("redeeming a code fires the join event") + void redeemFiresTheJoinEvent() { + handler.handleInvite(manager, player, guild, code); + + final ArgumentCaptor event = ArgumentCaptor.forClass(GuildJoinEvent.class); + Mockito.verify(pluginManager).callEvent(event.capture()); + + assertEquals(playerId, event.getValue().getPlayer().getUniqueId()); + assertEquals(guild.getId(), event.getValue().getGuild().getId()); + } + + @Test + @DisplayName("a cancelled join event stops the redeem") + void cancelledJoinEventStopsTheRedeem() { + Mockito.doAnswer(invocation -> { + ((GuildJoinEvent) invocation.getArgument(0)).setCancelled(true); + return null; + }).when(pluginManager).callEvent(Mockito.any()); + + handler.handleInvite(manager, player, guild, code); + + // Nothing was consumed: the use is still there, the player is not a redeemer and the + // guild did not gain a member. + assertEquals(2, code.getUses()); + assertTrue(code.getRedeemers().isEmpty()); + assertNull(guild.getMember(playerId)); + assertNull(handler.getGuildByPlayerId(playerId)); + } + + @Test + @DisplayName("redeeming a code puts the player in the guild") + void redeemAddsTheMember() { + handler.handleInvite(manager, player, guild, code); + + final GuildMember member = guild.getMember(playerId); + assertNotNull(member, "the redeemer should be a member of the guild"); + assertEquals(handler.getLowestGuildRole().getLevel(), member.getRole().getLevel()); + assertEquals(guild.getId(), handler.getGuildByPlayerId(playerId).getId()); + + // The member is registered with a join date, like a member who accepted an invite. + assertTrue(member.getJoinDate() > 0L, "the member should have a join date"); + } + + @Test + @DisplayName("redeeming a code consumes a use and records the redeemer") + void redeemConsumesAUse() { + handler.handleInvite(manager, player, guild, code); + + assertEquals(1, code.getUses()); + assertEquals(Collections.singletonList(playerId), code.getRedeemers()); + } + + @Test + @DisplayName("redeeming a code grants the guild and role permissions") + void redeemGrantsPermissions() { + handler.handleInvite(manager, player, guild, code); + + Mockito.verify(permission).playerAdd(Mockito.isNull(), Mockito.eq(player), Mockito.eq(TIER_PERM)); + Mockito.verify(permission).playerAdd(Mockito.isNull(), Mockito.eq(player), Mockito.eq(ROLE_NODE)); + } + + @Test + @DisplayName("redeeming a code clears a stale invite") + void redeemClearsAStaleInvite() { + guild.inviteMember(playerId); + assertTrue(guild.checkIfInvited(player)); + + handler.handleInvite(manager, player, guild, code); + + assertFalse(guild.checkIfInvited(player), "the invite should have been cleared"); + } + + /** + * Writes a roles file with the same shape the plugin generates: the master first, the lowest + * role last, because {@link GuildHandler#getLowestGuildRole()} reads the last entry. + */ + private void writeRoles() throws IOException { + final String yaml = String.join("\n", + "roles:", + " \"0\":", + " name: GuildMaster", + " permission-node: guilds.roles.master", + " permissions:", + " invite: true", + " \"3\":", + " name: Member", + " permission-node: " + ROLE_NODE, + " permissions:", + " invite: false", + ""); + Files.write(new File(dataFolder, "roles.yml").toPath(), yaml.getBytes(StandardCharsets.UTF_8)); + } + + /** + * Writes a tiers file whose single tier grants one node, so the tier permission the redeem + * path is expected to hand over is observable. + */ + private void writeTiers() throws IOException { + final String yaml = String.join("\n", + "tiers:", + " list:", + " 1:", + " level: 1", + " name: Bronze", + " cost: 0.0", + " max-members: 2", + " vault-amount: 1", + " permissions:", + " - " + TIER_PERM, + ""); + Files.write(new File(dataFolder, "tiers.yml").toPath(), yaml.getBytes(StandardCharsets.UTF_8)); + } + + /** + * Builds an offline player whose name and online state the plugin can read. + */ + private static OfflinePlayer offlinePlayer(final UUID uuid, final String name) { + final OfflinePlayer offline = Mockito.mock(OfflinePlayer.class); + Mockito.when(offline.getUniqueId()).thenReturn(uuid); + Mockito.when(offline.getName()).thenReturn(name); + Mockito.when(offline.isOnline()).thenReturn(false); + return offline; + } +} diff --git a/src/test/java/me/glaremasters/guilds/guild/GuildMemberRegistrationTest.java b/src/test/java/me/glaremasters/guilds/guild/GuildMemberRegistrationTest.java new file mode 100644 index 000000000..5a9309422 --- /dev/null +++ b/src/test/java/me/glaremasters/guilds/guild/GuildMemberRegistrationTest.java @@ -0,0 +1,135 @@ +package me.glaremasters.guilds.guild; + +import org.bukkit.OfflinePlayer; +import org.bukkit.entity.Player; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import java.util.ArrayList; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Covers how a member is registered with a guild, for both ways in. + * + *

{@code /guild accept} and {@code /guild code redeem} are the two ways into a guild, and they + * used to register the member differently. Accept went through {@link Guild#addMember}, which + * cleared a stale invite and stamped the member with the date they joined. A code went through + * {@link Guild#addMemberByCode}, which did neither, so a member who joined by code showed a join + * date of zero — the epoch, in the members list — until their next login patched it up. + * + *

Both now go through the same registration. + */ +class GuildMemberRegistrationTest { + + private static final GuildRole MASTER = new GuildRole("GuildMaster", "guilds.roles.master", 0); + private static final GuildRole MEMBER = new GuildRole("Member", "guilds.roles.member", 3); + + private Guild guild; + private GuildMember master; + private UUID playerId; + + @BeforeEach + void setUp() { + master = new GuildMember(UUID.randomUUID(), MASTER); + playerId = UUID.randomUUID(); + + guild = new Guild(UUID.randomUUID()); + guild.setMembers(new ArrayList<>()); + guild.getMembers().add(master); + guild.setInvitedMembers(new ArrayList<>()); + guild.setGuildMaster(master); + } + + @Test + @DisplayName("a member added by code is stamped with the date they joined") + void codeMemberHasAJoinDate() { + guild.addMemberByCode(new GuildMember(playerId, MEMBER)); + + final GuildMember member = guild.getMember(playerId); + assertNotNull(member); + assertTrue(member.getJoinDate() > 0L, "the member should have a join date"); + } + + @Test + @DisplayName("a member added by code clears the invite they came with") + void codeMemberClearsTheInvite() { + guild.inviteMember(playerId); + + guild.addMemberByCode(new GuildMember(playerId, MEMBER)); + + assertFalse(guild.checkIfInvited(invitedPlayer()), "the invite should have been cleared"); + } + + @Test + @DisplayName("a member who is already in the guild is not added twice") + void aMemberIsOnlyInTheGuildOnce() { + guild.addMemberByCode(new GuildMember(playerId, MEMBER)); + guild.addMemberByCode(new GuildMember(playerId, MEMBER)); + + assertEquals(1, guild.getMembers().stream().filter(m -> m.getUuid().equals(playerId)).count()); + } + + @Test + @DisplayName("a member added by code keeps the role the caller chose") + void codeMemberKeepsItsRole() { + guild.addMemberByCode(new GuildMember(playerId, MEMBER)); + + assertEquals(MEMBER, guild.getMember(playerId).getRole()); + } + + @Test + @DisplayName("an invited member is registered the same way a code member is") + void invitedMemberIsRegisteredTheSameWay() { + final GuildHandler handler = Mockito.mock(GuildHandler.class); + Mockito.when(handler.getLowestGuildRole()).thenReturn(MEMBER); + + final OfflinePlayer player = Mockito.mock(OfflinePlayer.class); + Mockito.when(player.getUniqueId()).thenReturn(playerId); + guild.inviteMember(playerId); + + guild.addMember(player, handler); + + final GuildMember member = guild.getMember(playerId); + assertNotNull(member); + assertTrue(member.getJoinDate() > 0L, "the member should have a join date"); + assertFalse(guild.checkIfInvited(invitedPlayer()), "the invite should have been cleared"); + } + + @Test + @DisplayName("a player who is already in the guild is not added a second time") + void anInvitedPlayerIsNotAddedTwice() { + final GuildHandler handler = Mockito.mock(GuildHandler.class); + Mockito.when(handler.getLowestGuildRole()).thenReturn(MEMBER); + + final OfflinePlayer player = Mockito.mock(OfflinePlayer.class); + Mockito.when(player.getUniqueId()).thenReturn(playerId); + + guild.addMember(player, handler); + guild.addMember(player, handler); + + assertEquals(1, guild.getMembers().stream().filter(m -> m.getUuid().equals(playerId)).count()); + } + + @Test + @DisplayName("a member who is not in the guild is not found") + void anUnknownMemberIsNotFound() { + assertNull(guild.getMember(playerId)); + } + + /** + * The invited player, as the guild sees them. + */ + private Player invitedPlayer() { + final Player player = Mockito.mock(Player.class); + Mockito.when(player.getUniqueId()).thenReturn(playerId); + return player; + } +} From 22c5c627101780d1e703e09572043c323d3621c0 Mon Sep 17 00:00:00 2001 From: t3 Date: Thu, 22 Oct 2026 12:08:24 +0000 Subject: [PATCH 2/4] docs: put the invite-code coverage counts at the merged value --- docs/e2e-coverage.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/e2e-coverage.md b/docs/e2e-coverage.md index 659c37b4c..6fb0698b8 100644 --- a/docs/e2e-coverage.md +++ b/docs/e2e-coverage.md @@ -3,7 +3,7 @@ Which Guilds feature is tested where, and what is deliberately left out. See `e2e-testing.md` for how to run any of it. -143 tests across 12 spec files, plus 3 preflight tests that run before every suite and fail the run +147 tests across 12 spec files, plus 3 preflight tests that run before every suite and fail the run if Guilds or any of its hard dependencies did not load. ## Preflight — `plugins/guilds-preflight.spec.ts` @@ -128,17 +128,19 @@ meaningful. | Alliance chat between guilds | `chat.spec.ts`, ally chat reaches an allied guild and not the third party | | Cleanup on deletion | a deleted guild drops out of its allies | -## H. Invite codes — `tests/codes.spec.ts` (14, of which 5 are one `describe.serial` chain) +## H. Invite codes — `tests/codes.spec.ts` (18, of which 5 are one `describe.serial` chain) | Requirement | Test | | --- | --- | | Creating invite codes | a code can be created, listed and deleted | | Viewing and listing codes | a code can be created, listed and deleted / *and it is listed for the guild master* | | Redeeming a code | a code can be redeemed and puts the player in the guild | +| Redeeming a code more than once | a code with two uses can be redeemed twice | | Redeeming a code grants what the guild grants | a player who joins by code gets the permissions their guild grants them | | The join cooldown applies to a code | a code cannot be redeemed while the join cooldown is running | | Deleting codes | a code can be created, listed and deleted | | Invalid codes | an invalid code is refused | +| Use counts that could never be redeemed | a code with no uses is refused and nothing is stored / a code with a negative number of uses is refused and nothing is stored / a refused code does not use up the guild allowance of codes | | Reusing codes where restricted | a code runs out of uses and then says so | | Permission restrictions | code information is restricted by role / *a member can be added and cannot manage codes* | | Full lifecycle | *one code through its whole life* — a `describe.serial` chain: create → listed → redeem → deleted, asserting membership at each step rather than the command's reply | From 509e991ea1d2ae037d3df1b9156daac0ccddac9f Mon Sep 17 00:00:00 2001 From: t3 Date: Thu, 22 Oct 2026 12:34:17 +0000 Subject: [PATCH 3/4] docs: use the canonical coverage totals for the merged set of PRs --- docs/e2e-coverage.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/e2e-coverage.md b/docs/e2e-coverage.md index 6fb0698b8..aca5e66c2 100644 --- a/docs/e2e-coverage.md +++ b/docs/e2e-coverage.md @@ -3,7 +3,7 @@ Which Guilds feature is tested where, and what is deliberately left out. See `e2e-testing.md` for how to run any of it. -147 tests across 12 spec files, plus 3 preflight tests that run before every suite and fail the run +160 tests across 14 spec files, plus 3 preflight tests that run before every suite and fail the run if Guilds or any of its hard dependencies did not load. ## Preflight — `plugins/guilds-preflight.spec.ts` @@ -17,12 +17,13 @@ meaningful. | The economy and permission providers Vault needs are registered | Vault's `Economy` and `Permission` services resolve | | Guilds commands are reachable and its language is English | `/guild` answers, and does so in English | -## A. Guild lifecycle — `tests/lifecycle.spec.ts` (19) +## A. Guild lifecycle — `tests/lifecycle.spec.ts` (21) | Requirement | Test | | --- | --- | | Successful guild creation | creates a guild, charges for it, and shows it in the guild list | | Creation confirmation and cancellation | cancelling the prompt leaves no guild and no charge | +| The confirmation is checked again | refuses the confirmation when the player joined a guild in the meantime / refuses the confirmation when another guild took the name in the meantime | | Invalid guild names and prefixes | rejects a guild name that breaks the configured requirements / rejects a name too long to serve as its own prefix / rejects an explicit prefix that breaks the prefix requirements | | Duplicate guild names | rejects a guild name another guild already holds | | Insufficient funds | rejects creation when the player cannot afford it | @@ -32,7 +33,7 @@ meaningful. | Status and information commands | toggles the guild between public and private / changes the guild prefix / sets, shows and clears the guild MOTD / reports guild membership through the guild list and rejects guild-scoped commands elsewhere | | Name blacklist | rejects a guild name that is a substring of a blacklisted word / accepts a guild name that contains a blacklisted word | -## B. Membership management — `tests/membership.spec.ts` (19) +## B. Membership management — `tests/membership.spec.ts` (20) | Requirement | Test | | --- | --- | @@ -41,6 +42,7 @@ meaningful. | Declining an invitation | an invited player can decline and stays without a guild | | Preventing duplicate invitations | a player cannot be invited twice / a player who is already in a guild cannot be invited | | Preventing joining when already in another guild | a player cannot join a second guild while in one / a player cannot accept a guild they were never invited to / a player accepting a guild that does not exist is told the guild does not exist | +| Finding a guild by name | a guild is found when its name is typed in a different case | | Leaving a guild | a member can leave and the guild is told | | Leaving and being invited back | a member can leave, be invited back and join the guild again | | Master leaving deletes the guild | a member leaving warns that the guild master deletes the guild by leaving | @@ -51,7 +53,7 @@ meaningful. | Guild capacity | a full guild refuses another member | | Permissions after leaving or being removed | a player who leaves loses the permissions their guild granted them / a player who is kicked loses the permissions their guild granted them | -## C. Roles and permissions — `tests/roles.spec.ts` (8), `tests/admin.spec.ts` (7) +## C. Roles and permissions — `tests/roles.spec.ts` (8), `tests/admin.spec.ts` (8) | Requirement | Test | | --- | --- | @@ -64,6 +66,7 @@ meaningful. | Tier permissions after upgrade | upgrading charges the tier price and moves the guild onto the new tier / **an upgrade leaves the master with the permissions the new tier grants** | | A player in no guild | a player in no guild is told so rather than refused by role | | Admin commands that name a guild and a second argument | an admin command that names a guild and a second argument reaches its handler / an admin command that names a guild which does not exist says so / an admin command that names a guild and a second argument is refused for an unknown player | +| Admin commands that name a guild in a different case | an admin command finds its guild when the name is typed in a different case | | Admin commands that act on a player | an admin command can put a player into a guild / an admin command can hand a player upgrade tickets / an admin command can hand a guild to one of its members | | Admin commands that act on an arena | an admin command can teleport to an arena spawn point | | The player-facing bank commands | the player-facing bank commands work | From 7b05ad5067ab172a6dc23e215450493698f389b6 Mon Sep 17 00:00:00 2001 From: t3 Date: Thu, 22 Oct 2026 13:19:06 +0000 Subject: [PATCH 4/4] docs: set the canonical coverage totals to 162 and mark the sections other PRs own --- docs/e2e-coverage.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/e2e-coverage.md b/docs/e2e-coverage.md index aca5e66c2..3fd86147f 100644 --- a/docs/e2e-coverage.md +++ b/docs/e2e-coverage.md @@ -3,7 +3,7 @@ Which Guilds feature is tested where, and what is deliberately left out. See `e2e-testing.md` for how to run any of it. -160 tests across 14 spec files, plus 3 preflight tests that run before every suite and fail the run +162 tests across 14 spec files, plus 3 preflight tests that run before every suite and fail the run if Guilds or any of its hard dependencies did not load. ## Preflight — `plugins/guilds-preflight.spec.ts` @@ -33,7 +33,7 @@ meaningful. | Status and information commands | toggles the guild between public and private / changes the guild prefix / sets, shows and clears the guild MOTD / reports guild membership through the guild list and rejects guild-scoped commands elsewhere | | Name blacklist | rejects a guild name that is a substring of a blacklisted word / accepts a guild name that contains a blacklisted word | -## B. Membership management — `tests/membership.spec.ts` (20) +## B. Membership management — `tests/membership.spec.ts` (21) | Requirement | Test | | --- | --- | @@ -195,6 +195,10 @@ the server answering in another language for the rest of the run. ACF's `usePerI false)` is what makes that safe, and one player switching language leaves everyone else in English asserts it rather than assuming it. +## L. Startup and the guild data — `tests/startup-loading.spec.ts` (4) + +## Runtime smoke coverage — `tests/smoke.spec.ts` (7) + ## Fixture self-check — `tests/bank.spec.ts` `STARTING_BALANCE is what the fixtures actually fund` is not a Guilds test. It checks that the