fix: (pbac) edit sheet readonly to all to readonly (#22672)

* fix readonly to all to readonly

* fix sheet not showing correct all,readonly,none state until refresh

* update test to include the correct roleId

---------

Co-authored-by: Alex van Andel <me@alexvanandel.com>
This commit is contained in:
sean-brydon
2025-07-22 14:46:16 +00:00
committed by GitHub
co-authored by Alex van Andel
parent dbaa0727ae
commit fa47456d5f
4 changed files with 110 additions and 2 deletions
@@ -0,0 +1,48 @@
import { describe, it, expect } from "vitest";
import { usePermissions } from "../usePermissions";
describe("usePermissions", () => {
const { getResourcePermissionLevel } = usePermissions();
describe("getResourcePermissionLevel", () => {
it("should return 'all' for any resource when *.* permission is present", () => {
const permissions = ["*.*", "eventType.create", "eventType.read"];
expect(getResourcePermissionLevel("eventType", permissions)).toBe("all");
expect(getResourcePermissionLevel("booking", permissions)).toBe("all");
expect(getResourcePermissionLevel("team", permissions)).toBe("all");
});
it("should return 'all' for resource with all individual permissions", () => {
const permissions = ["eventType.create", "eventType.read", "eventType.update", "eventType.delete"];
expect(getResourcePermissionLevel("eventType", permissions)).toBe("all");
});
it("should return 'read' for resource with only read permission", () => {
const permissions = ["eventType.read"];
expect(getResourcePermissionLevel("eventType", permissions)).toBe("read");
});
it("should return 'none' for resource with no permissions", () => {
const permissions = ["booking.create"];
expect(getResourcePermissionLevel("eventType", permissions)).toBe("none");
});
it("should handle * resource correctly", () => {
const permissionsWithAll = ["*.*"];
const permissionsWithoutAll = ["eventType.read"];
expect(getResourcePermissionLevel("*", permissionsWithAll)).toBe("all");
expect(getResourcePermissionLevel("*", permissionsWithoutAll)).toBe("none");
});
it("should prioritize *.* over individual permissions", () => {
const permissions = ["*.*", "eventType.read"]; // Has global all but only read for eventType individually
expect(getResourcePermissionLevel("eventType", permissions)).toBe("all");
});
});
});
@@ -46,6 +46,11 @@ export function usePermissions(): UsePermissionsReturn {
const resourceConfig = PERMISSION_REGISTRY[resource as keyof typeof PERMISSION_REGISTRY];
if (!resourceConfig) return "none";
// Check if global all permissions (*.*) is present
if (permissions.includes("*.*")) {
return "all";
}
// Filter out internal keys like _resource when checking permissions
const allResourcePerms = Object.keys(resourceConfig)
.filter((action) => !action.startsWith("_"))
@@ -108,7 +108,7 @@ export class RoleRepository {
await trx.rolePermission.deleteMany({
where: {
roleId,
AND: permissionChanges.toRemove.map((p) => ({
OR: permissionChanges.toRemove.map((p) => ({
resource: p.resource,
action: p.action,
})),
@@ -67,7 +67,15 @@ describe("RoleService", () => {
teamId: roleData.teamId,
color: "#000000",
type: RoleType.CUSTOM,
permissions: [{ id: "perm-1", resource: "eventType", action: "create" }],
permissions: [
{
id: "perm-1",
resource: "eventType",
action: "create",
roleId: "new-role",
createdAt: new Date(),
},
],
createdAt: new Date(),
updatedAt: new Date(),
};
@@ -215,6 +223,53 @@ describe("RoleService", () => {
expect(result).toBeDefined();
});
it("should properly remove multiple permissions when changing from all to none", async () => {
const roleId = "role-id";
const permissions = [] as PermissionString[]; // Setting to none
const existingPermissions = [
{ id: "1", roleId, resource: "eventType", action: "create" },
{ id: "2", roleId, resource: "eventType", action: "read" },
{ id: "3", roleId, resource: "eventType", action: "update" },
{ id: "4", roleId, resource: "eventType", action: "delete" },
];
const permissionChanges = {
toAdd: [],
toRemove: [
{ id: "1", roleId, resource: "eventType", action: "create" },
{ id: "2", roleId, resource: "eventType", action: "read" },
{ id: "3", roleId, resource: "eventType", action: "update" },
{ id: "4", roleId, resource: "eventType", action: "delete" },
],
};
const role: Role = {
id: roleId,
name: "Test Role",
teamId: 1,
type: RoleType.CUSTOM,
color: "#000000",
permissions: [],
createdAt: new Date(),
updatedAt: new Date(),
};
mockRepository.findById.mockResolvedValueOnce(role);
mockRepository.getPermissions.mockResolvedValueOnce(existingPermissions);
mockPermissionDiffService.calculateDiff.mockReturnValueOnce(permissionChanges);
mockRepository.update.mockResolvedValueOnce(role);
const result = await service.update({ roleId, permissions });
expect(mockPermissionDiffService.calculateDiff).toHaveBeenCalledWith(permissions, existingPermissions);
expect(mockRepository.update).toHaveBeenCalledWith(roleId, permissionChanges, {
color: undefined,
name: undefined,
});
expect(result).toBeDefined();
expect(result.permissions).toHaveLength(0);
});
it("should throw error if role does not exist", async () => {
const roleId = "non-existent-role";
const permissions = ["eventType.create"] as PermissionString[];