diff --git a/panel/src/i18n/resources/en-US/servers.json b/panel/src/i18n/resources/en-US/servers.json index 2c483a6..83e6c36 100644 --- a/panel/src/i18n/resources/en-US/servers.json +++ b/panel/src/i18n/resources/en-US/servers.json @@ -175,6 +175,9 @@ "luckperms_recent_actions": "Recent Actions History", "luckperms_no_recent_actions": "No recent actions.", "luckperms_revert": "Revert", + "luckperms_revert_to_set": "Revert sets {{node}} back to {{value}}", + "luckperms_revert_to_unset": "Revert removes {{node}}", + "luckperms_revert_unknown": "Can't revert: value unknown", "luckperms_remove_group": "Remove group {{group}}", "luckperms_remove_perm": "Remove permission {{node}}", "luckperms_reverting": "Reverting...", diff --git a/panel/src/i18n/resources/zh-CN/servers.json b/panel/src/i18n/resources/zh-CN/servers.json index a7f25cc..8a92e97 100644 --- a/panel/src/i18n/resources/zh-CN/servers.json +++ b/panel/src/i18n/resources/zh-CN/servers.json @@ -175,6 +175,9 @@ "luckperms_recent_actions": "最近操作历史", "luckperms_no_recent_actions": "暂无最近操作历史。", "luckperms_revert": "撤销", + "luckperms_revert_to_set": "撤销会把 {{node}} 改回 {{value}}", + "luckperms_revert_to_unset": "撤销会移除 {{node}}", + "luckperms_revert_unknown": "无法撤销：原值未知", "luckperms_remove_group": "移除权限组 {{group}}", "luckperms_remove_perm": "移除权限 {{node}}", "luckperms_reverting": "正在撤销...", diff --git a/panel/src/pages/ServerLuckPerms.test.tsx b/panel/src/pages/ServerLuckPerms.test.tsx index 527cfba..a0e21d2 100644 --- a/panel/src/pages/ServerLuckPerms.test.tsx +++ b/panel/src/pages/ServerLuckPerms.test.tsx @@ -181,3 +181,108 @@ describe("ServerLuckPerms when LuckPerms does not answer", () => { expect(screen.queryByText(/Couldn't read this player's/)).toBeNull(); }); }); + +// A revert puts a permission back as it was. A removed deny comes back as a deny +// (it used to come back as a grant), a set that replaced a value restores that +// value, and a removal whose earlier value was never read offers no revert. +describe("ServerLuckPerms reverting a permission change", () => { + const read = { + player: "Alex", + groups: [], + permissions: [{ node: "essentials.fly", value: false, world: "world_nether" }], + output: "Alex's permissions: essentials.fly (false) world=world_nether", + }; + + beforeEach(() => { + calls.accessLuckPermsInfo.mockResolvedValue(read); + calls.accessPermission.mockResolvedValue({ output: "" }); + }); + + async function lookUpAlex() { + renderPage(); + await userEvent.type(await screen.findByPlaceholderText("Steve"), "Alex{Enter}"); + } + + async function typeNode(node: string, world = "") { + await userEvent.type(screen.getByLabelText("Permission Node"), node); + if (world) await userEvent.type(screen.getByLabelText("World Context (Optional)"), world); + } + + it("puts a removed deny back as a deny", async () => { + await lookUpAlex(); + await userEvent.click(await screen.findByRole("button", { name: "Remove permission essentials.fly" })); + expect(calls.accessPermission).toHaveBeenLastCalledWith("lobby", "unset", "Alex", "essentials.fly", undefined, "world_nether"); + expect(await screen.findByText("- essentials.fly (FALSE) [world_nether]")).toBeTruthy(); + await userEvent.click(screen.getByRole("button", { name: "Revert" })); + expect(calls.accessPermission).toHaveBeenLastCalledWith("lobby", "set", "Alex", "essentials.fly", false, "world_nether"); + }); + + it("puts back the value a typed removal took, matched without case", async () => { + await lookUpAlex(); + await screen.findByRole("button", { name: "Remove permission essentials.fly" }); + await typeNode("Essentials.Fly", "World_Nether"); + await userEvent.click(screen.getByRole("button", { name: /^remove permission$/i })); + await userEvent.click(await screen.findByRole("button", { name: "Revert" })); + expect(calls.accessPermission).toHaveBeenLastCalledWith("lobby", "set", "Alex", "Essentials.Fly", false, "World_Nether"); + }); + + it("restores the value a set replaced", async () => { + await lookUpAlex(); + await screen.findByRole("button", { name: "Remove permission essentials.fly" }); + await typeNode("essentials.fly", "world_nether"); + await userEvent.click(screen.getByRole("button", { name: /^add permission$/i })); + expect(calls.accessPermission).toHaveBeenLastCalledWith("lobby", "set", "Alex", "essentials.fly", true, "world_nether"); + await userEvent.click(await screen.findByRole("button", { name: "Revert" })); + expect(calls.accessPermission).toHaveBeenLastCalledWith("lobby", "set", "Alex", "essentials.fly", false, "world_nether"); + }); + + it("removes a set node that held nothing in that world", async () => { + await lookUpAlex(); + await screen.findByRole("button", { name: "Remove permission essentials.fly" }); + await typeNode("essentials.fly"); + await userEvent.click(screen.getByRole("button", { name: /^add permission$/i })); + await userEvent.click(await screen.findByRole("button", { name: "Revert" })); + expect(calls.accessPermission).toHaveBeenLastCalledWith("lobby", "unset", "Alex", "essentials.fly", undefined, undefined); + }); + + it("offers no revert for a removal whose value was never read", async () => { + calls.accessLuckPermsInfo.mockResolvedValue({ player: "Alex", groups: [], permissions: [], output: "" }); + await lookUpAlex(); + await screen.findByText(/Couldn't read this player's permission nodes/); + await typeNode("essentials.fly", "world_nether"); + await userEvent.click(screen.getByRole("button", { name: /^remove permission$/i })); + expect(await screen.findByText("Can't revert: value unknown")).toBeTruthy(); + expect(screen.getByText("- essentials.fly [world_nether]")).toBeTruthy(); + expect(screen.queryByRole("button", { name: "Revert" })).toBeNull(); + + await typeNode("essentials.home"); + await userEvent.click(screen.getByRole("button", { name: /^add permission$/i })); + await userEvent.click(await screen.findByRole("button", { name: "Revert" })); + expect(calls.accessPermission).toHaveBeenLastCalledWith("lobby", "unset", "Alex", "essentials.home", undefined, undefined); + }); + + // The read predates the set that just landed: it still shows the deny, but the + // node now holds a grant, so the removal after it must not offer to restore the + // deny. + async function setThenRemoveOnStaleRead() { + await lookUpAlex(); + await screen.findByRole("button", { name: "Remove permission essentials.fly" }); + await typeNode("essentials.fly", "world_nether"); + await userEvent.click(screen.getByRole("button", { name: /^add permission$/i })); + await typeNode("essentials.fly", "world_nether"); + await userEvent.click(screen.getByRole("button", { name: /^remove permission$/i })); + expect(calls.accessPermission).toHaveBeenLastCalledWith("lobby", "unset", "Alex", "essentials.fly", undefined, "world_nether"); + expect(await screen.findByText("Can't revert: value unknown")).toBeTruthy(); + expect(screen.getByText("- essentials.fly [world_nether]")).toBeTruthy(); + } + + it("trusts no read whose refresh failed", async () => { + calls.accessLuckPermsInfo.mockResolvedValueOnce(read).mockRejectedValue({ status: 502, code: "rcon_unavailable", message: "console down" }); + await setThenRemoveOnStaleRead(); + }); + + it("trusts no read still being refreshed", async () => { + calls.accessLuckPermsInfo.mockResolvedValueOnce(read).mockReturnValue(new Promise(() => {})); + await setThenRemoveOnStaleRead(); + }); +}); diff --git a/panel/src/pages/ServerLuckPerms.tsx b/panel/src/pages/ServerLuckPerms.tsx index 54f0c9a..5ffcba3 100644 --- a/panel/src/pages/ServerLuckPerms.tsx +++ b/panel/src/pages/ServerLuckPerms.tsx @@ -48,6 +48,10 @@ interface ActionHistoryItem { target: string; value?: boolean; world?: string; + // undo is the exact write that puts a permission back as it was before; absent + // when that state is unknown, so the entry offers no revert (a guess would turn + // a removed deny into a grant). Group entries invert their action instead. + undo?: { action: "set" | "unset"; value?: boolean }; status: "success" | "error"; output: string; reverted?: boolean; @@ -207,12 +211,26 @@ export function ServerLuckPerms() { return { node, world }; }; + // priorValue is the value the last read showed for a node in a world (LuckPerms + // matches both case-insensitively), or undefined when no row showed one: the + // node was unset, or nothing could be read. A read still in flight or one whose + // refresh failed is stale (it predates the last write), so it knows nothing. + const priorValue = (node: string, world: string): boolean | undefined => { + if (lpUnread || lpLoading) return undefined; + return lpInfo?.permissions?.find( + (p) => p.node.toLowerCase() === node.toLowerCase() && (p.world ?? "").toLowerCase() === world.toLowerCase(), + )?.value; + }; + const handleAddPermission = async (e: React.FormEvent) => { e.preventDefault(); if (!selectedPlayer || !permNodeInput.trim() || submitting) return; const typed = typedPermission(); if (!typed) return; const { node, world } = typed; + // Reverting a set restores the value the node had; with none read, it removes + // the node, which undoes the set whenever the node was not there before. + const prior = priorValue(node, world); setSubmitting(true); setFormFeedback(null); @@ -225,6 +243,7 @@ export function ServerLuckPerms() { target: node, value: permValueInput, world: world || undefined, + undo: prior === undefined ? { action: "unset" } : { action: "set", value: prior }, status: "success", output: res.output || t("luckperms_no_output"), }); @@ -239,7 +258,8 @@ export function ServerLuckPerms() { }; // Returns whether the server took the unset, so the typed form can clear itself. - const handleRemovePermission = async (node: string, world?: string): Promise => { + // value is what the node held, when known; only then can the removal be reverted. + const handleRemovePermission = async (node: string, world?: string, value?: boolean): Promise => { if (!selectedPlayer || submitting) return false; setSubmitting(true); setFormFeedback(null); @@ -250,7 +270,9 @@ export function ServerLuckPerms() { player: selectedPlayer, action: "unset", target: node, + value, world: world || undefined, + undo: value === undefined ? undefined : { action: "set", value }, status: "success", output: res.output || t("luckperms_no_output"), }); @@ -270,7 +292,7 @@ export function ServerLuckPerms() { if (!selectedPlayer || !permNodeInput.trim() || submitting) return; const typed = typedPermission(); if (!typed) return; - if (await handleRemovePermission(typed.node, typed.world)) { + if (await handleRemovePermission(typed.node, typed.world, priorValue(typed.node, typed.world))) { setPermNodeInput(""); setPermWorldInput(""); } @@ -286,13 +308,14 @@ export function ServerLuckPerms() { const inverseAction = item.action === "add" ? "remove" : "add"; await api.accessGroup(name, inverseAction, item.player, item.target); } else { - const inverseAction = item.action === "set" ? "unset" : "set"; + const undo = item.undo; + if (!undo) return; await api.accessPermission( name, - inverseAction, + undo.action, item.player, item.target, - item.action === "set" ? item.value : undefined, + undo.action === "set" ? undo.value : undefined, item.world || undefined ); } @@ -671,7 +694,7 @@ export function ServerLuckPerms() { variant="ghost" size="sm" disabled={submitting} - onClick={() => handleRemovePermission(p.node, p.world)} + onClick={() => handleRemovePermission(p.node, p.world, p.value)} aria-label={t("luckperms_remove_perm", { node: p.node })} title={t("luckperms_remove_perm", { node: p.node })} className="h-7 w-7 p-0 text-muted-foreground hover:text-destructive hover:bg-muted rounded transition-all focus:outline-none" @@ -834,9 +857,16 @@ export function ServerLuckPerms() {
{history.map((h) => { const isSuccess = h.status === "success"; + const shownValue = h.value === undefined ? "" : ` (${h.value ? "TRUE" : "FALSE"})`; const actionLabel = h.type === "group" ? `${h.action === "add" ? "+" : "-"} ${t("luckperms_group_name")}: ${h.target}` - : `${h.action === "set" ? `+ ${h.target} (${h.value ? "TRUE" : "FALSE"})` : `- ${h.target}`}${h.world ? ` [${h.world}]` : ""}`; + : `${h.action === "set" ? "+" : "-"} ${h.target}${shownValue}${h.world ? ` [${h.world}]` : ""}`; + const revertible = h.type === "group" || !!h.undo; + const revertTitle = h.undo + ? h.undo.action === "set" + ? t("luckperms_revert_to_set", { node: h.target, value: h.undo.value ? "TRUE" : "FALSE" }) + : t("luckperms_revert_to_unset", { node: h.target }) + : undefined; return (
{h.timestamp} - {isSuccess && !h.reverted && ( + {isSuccess && !h.reverted && revertible && ( )} + {isSuccess && !h.reverted && !revertible && ( + + {t("luckperms_revert_unknown")} + + )} {h.reverted && ( {t("luckperms_revert")}