Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changeset/6605-permission-bulk-merge-not-replace.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
---
'@object-ui/app-shell': patch
---

Permission matrix bulk buttons (R / CRUD / All) now merge into the object's
permission row instead of replacing it, so spec-declared keys the matrix does
not author — `allowExport` and the ADR-0057 access-depth axis `readScope` /
`writeScope` — survive a bulk click the same way they already survived the
per-checkbox path. Previously one click on any bulk button silently dropped
them from the saved row, and the **All** button could widen effective read
access by deleting a `readScope: 'own'` narrowing with no diff and no error.
**None** deliberately keeps clearing the whole row, narrowings included:
merging there would leave `allowExport: true` alive after a click on the
button labelled None (objectui#6605).
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* objectui#6605 — the bulk buttons (R / CRUD / All / None) and the keys the
* matrix does not author.
*
* `bulkSetObject` used to REPLACE the object's permission row. Three
* spec-declared keys are modelled by neither the local `ObjectPerm` interface
* nor the `OBJECT_ACTIONS` column list — `allowExport`, and the ADR-0057
* access-depth axis `readScope` / `writeScope` — so one bulk click dropped
* whatever those held. The sharpest shape is the button labelled **All**: an
* admin clicking what reads as "grant everything" could WIDEN effective read
* access by deleting a `readScope: 'own'` narrowing, with no diff shown and no
* error.
*
* Every pin here asserts the SAVED payload, never editor state. That is the
* card's own argument for why the defect persisted: both save doors carry the
* row as-is — the environment door writes the whole record, and at package
* scope `mergePermissionSlice` takes in-scope rows entirely from `edited`
* (ADR-0086 P0), so `base` cannot restore what a bulk click dropped. An
* editor-state assertion would prove nothing about either door.
*
* ## `none` is pinned to keep REPLACING — that is the fix's fence, not a gap
*
* The dispatch on #6605 deliberately rejects the card's "`none` needs the same
* treatment" suggestion. The defect is a GRANT that silently drops a
* narrowing; `none` grants nothing, so nothing survives for a scope to
* narrow. Merging `none` would instead leave `allowExport: true` (and the
* scopes) alive after a click on the button labelled "None" — a permissive
* outcome that does not exist today, on a surface whose whole problem is
* silent permissiveness. What an admin's "None" means is a behaviour
* decision, made on #6605, not a mechanical merge. The `none` pin below makes
* that fence mechanical: a refactor that quietly adopts the card's suggestion
* goes red here and needs a maintainer decision, not a cleanup commit.
*/

import { describe, it, expect, vi, afterEach } from 'vitest';
import { render, screen, cleanup, fireEvent, waitFor, within } from '@testing-library/react';
import { MemoryRouter } from 'react-router-dom';

/** Every object-permission key the matrix authors, in column order. */
const MATRIX_KEYS = [
'allowCreate',
'allowRead',
'allowEdit',
'allowDelete',
'allowTransfer',
'viewAllRecords',
'modifyAllRecords',
];

interface FakeServer {
/** The stored record — what `layered()` answers (also the merge `base`). */
set: Record<string, any>;
/** What `list('object')` lists — the matrix rows (and, under a packageId, the slice scope). */
objectNames: string[];
saved: Record<string, any> | null;
savedOpts: Record<string, any> | undefined;
}

function makeClient(server: FakeServer) {
return {
layered: async () => ({ effective: server.set, code: null, overlay: null, overlayScope: null }),
getDraft: async () => null,
list: async (type: string) =>
type === 'object' ? server.objectNames.map((name) => ({ item: { name } })) : [],
get: async (type: string) => (type === 'object' ? { fields: [] } : null),
save: async (
_t: string,
_n: string,
payload: Record<string, any>,
opts?: Record<string, any>,
) => {
server.saved = payload;
server.savedOpts = opts;
return payload;
},
} as any;
}

let clientImpl: any;

vi.mock('./useMetadata', () => ({
useMetadataClient: () => clientImpl,
useMetadataTypes: () => ({
loading: false,
error: null,
entries: [{ type: 'permission', label: 'Permission', allowOrgOverride: true }],
}),
}));
vi.mock('./AssignedUsersSection', () => ({ AssignedUsersSection: () => null }));
vi.mock('@object-ui/fields', () => ({
CapabilityMultiSelectField: () => <div data-testid="cap-picker" />,
parseCapabilityNames: (v: unknown) => (typeof v === 'string' ? JSON.parse(v) : []),
}));

import { PermissionMatrixEditPage } from './PermissionMatrixEditor';

afterEach(cleanup);

async function renderMatrix(
objects: Record<string, unknown>,
opts: { objectNames?: string[]; packageId?: string } = {},
): Promise<FakeServer> {
const server: FakeServer = {
set: { name: 'sales_perms', label: 'Sales', objects, fields: {} },
objectNames: opts.objectNames ?? Object.keys(objects),
saved: null,
savedOpts: undefined,
};
clientImpl = makeClient(server);
render(
<MemoryRouter>
<PermissionMatrixEditPage type="permission" name="sales_perms" packageId={opts.packageId} />
</MemoryRouter>,
);
await screen.findByText('Sales');
return server;
}

/** The bulk button (`R` / `CRUD` / `All` / `None`) inside one object's row. */
function bulkButton(objectName: string, label: string) {
const row = screen
.getAllByRole('row')
.find((r) => within(r).queryByText(objectName) != null);
expect(row, `row for ${objectName}`).toBeTruthy();
return within(row!).getByRole('button', { name: new RegExp(`^${label}$`) });
}

/** Click Save and return the payload the client was handed. */
async function save(server: FakeServer) {
fireEvent.click(screen.getByRole('button', { name: /^Save$/ }));
await waitFor(() => expect(server.saved).not.toBeNull());
return server.saved!;
}

describe('PermissionMatrixEditor · bulk buttons vs unmodelled keys (objectui#6605)', () => {
it('"All" — the widening shape — grants every column AND the saved row keeps readScope / writeScope / allowExport', async () => {
const server = await renderMatrix({
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
});

fireEvent.click(bulkButton('a_account', 'All'));
const payload = await save(server);

const row = payload.objects.a_account;
for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true);
// The narrowings survive the click that used to delete them. `readScope`
// is the load-bearing one: dropping `own` silently widened read access.
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('own');
expect(row.allowExport).toBe(true);
expect(Object.keys(row).sort()).toEqual(
[...MATRIX_KEYS, 'allowExport', 'readScope', 'writeScope'].sort(),
);
});

it('"CRUD" merges the unmodelled keys through but still RESETS the matrix columns outside its grant', async () => {
const server = await renderMatrix({
a_account: {
allowTransfer: true,
viewAllRecords: true,
modifyAllRecords: true,
allowExport: true,
readScope: 'own',
writeScope: 'unit',
},
});

fireEvent.click(bulkButton('a_account', 'CRUD'));
const payload = await save(server);

const row = payload.objects.a_account;
// Falsification direction: merge must not decay into "add" — a bulk CRUD
// after a wider grant still means exactly CRUD for the keys the matrix owns.
expect(Object.keys(row).sort()).toEqual(
['allowCreate', 'allowRead', 'allowEdit', 'allowDelete', 'allowExport', 'readScope', 'writeScope'].sort(),
);
expect('allowTransfer' in row).toBe(false);
expect('viewAllRecords' in row).toBe(false);
expect('modifyAllRecords' in row).toBe(false);
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('unit');
expect(row.allowExport).toBe(true);
});

it('"R" saves a read-only row that still carries the unmodelled keys', async () => {
const server = await renderMatrix({
a_account: {
allowCreate: true,
allowEdit: true,
allowExport: true,
readScope: 'own',
writeScope: 'unit',
},
});

fireEvent.click(bulkButton('a_account', 'R'));
const payload = await save(server);

const row = payload.objects.a_account;
expect(Object.keys(row).sort()).toEqual(
['allowRead', 'allowExport', 'readScope', 'writeScope'].sort(),
);
expect(row.allowRead).toBe(true);
expect(row.readScope).toBe('own');
});

it('"None" still clears the WHOLE row — narrowings included (deliberate: the #6605 dispatch fence)', async () => {
// Read the header before "fixing" this pin: merging `none` would leave
// `allowExport: true` alive after a click on the button labelled "None".
const server = await renderMatrix({
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
a_contact: { allowRead: true, readScope: 'own' },
});

fireEvent.click(bulkButton('a_account', 'None'));
const payload = await save(server);

expect(payload.objects.a_account).toEqual({});
// Positive control in the same query shape: the untouched sibling row in
// the SAME saved payload still carries its narrowing, so the emptiness
// above is a measurement of `none`, not of a save path that drops keys.
expect(payload.objects.a_contact).toEqual({ allowRead: true, readScope: 'own' });
});

it('package door: the merged slice keeps the unmodelled keys after "All", and other packages\' rows survive byte-for-byte', async () => {
// In-scope: a_account (this package's row, carrying the narrowings).
// Out-of-scope: b_order — another package's contribution, not listed by
// this package, which `mergePermissionSlice` must copy verbatim from base.
const server = await renderMatrix(
{
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
b_order: { allowRead: true, viewAllRecords: true, readScope: 'unit' },
},
{ objectNames: ['a_account'], packageId: 'app.a' },
);

fireEvent.click(bulkButton('a_account', 'All'));
const payload = await save(server);

const row = payload.objects.a_account;
for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true);
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('own');
expect(row.allowExport).toBe(true);
// The other package's row is untouched — and the save went through the
// package door (a draft write), not a live record write.
expect(payload.objects.b_order).toEqual({ allowRead: true, viewAllRecords: true, readScope: 'unit' });
expect(server.savedOpts).toMatchObject({ mode: 'draft', packageId: 'app.a' });
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -638,14 +638,39 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved,

function bulkSetObject(objectName: string, action: 'all' | 'none' | 'crud' | 'read') {
setDraft((prev) => {
const next: ObjectPerm =
action === 'none'
? {}
: action === 'all'
? Object.fromEntries(OBJECT_ACTIONS.map((a) => [a.key, true])) as ObjectPerm
// `none` REPLACES the row with `{}` — deliberately, unlike the three
// granting arms below (#6605). The defect those arms had was a GRANT
// that silently dropped a narrowing: "All" deleting a `readScope: 'own'`
// widens effective read access with no diff and no error. `none` grants
// nothing, so nothing survives for a scope to narrow; merging here would
// instead leave `allowExport: true` (and the scopes) alive after a click
// on the button labelled "None" — a permissive outcome that does not
// exist today. What an admin's "None" means is a behaviour decision, not
// a mechanical merge; pinned by
// `PermissionMatrixEditor.bulkMergeKeys.test.tsx`.
if (action === 'none') {
return { ...prev, objects: { ...prev.objects, [objectName]: {} } };
}
// The granting arms MERGE (#6605): start from the current row, reset the
// keys this matrix authors (`OBJECT_ACTIONS`), then set the granted
// ones. Keys the matrix does not model — `allowExport`, `readScope`,
// `writeScope`, anything an older or newer editor wrote — ride through
// exactly as they do on the per-checkbox path (`updateObjectPerm`'s
// spread). Replacing the row wholesale is what silently deleted them,
// and both save doors persist the row as-is: the environment door writes
// the whole record, and at package scope `mergePermissionSlice` takes
// in-scope rows entirely from `edited` (ADR-0086 P0), so `base` cannot
// restore what a bulk click dropped.
const cur = prev.objects[objectName] ?? {};
const next: ObjectPerm = { ...cur };
for (const a of OBJECT_ACTIONS) delete next[a.key];
const grants: Array<keyof ObjectPerm> =
action === 'all'
? OBJECT_ACTIONS.map((a) => a.key)
: action === 'crud'
? { allowCreate: true, allowRead: true, allowEdit: true, allowDelete: true }
: { allowRead: true };
? ['allowCreate', 'allowRead', 'allowEdit', 'allowDelete']
: ['allowRead'];
for (const key of grants) next[key] = true;
return {
...prev,
objects: { ...prev.objects, [objectName]: next },
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changeset/6605-permission-bulk-merge-not-replace.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
---
'@object-ui/app-shell': patch
---

Permission matrix bulk buttons (R / CRUD / All) now merge into the object's
permission row instead of replacing it, so spec-declared keys the matrix does
not author — `allowExport` and the ADR-0057 access-depth axis `readScope` /
`writeScope` — survive a bulk click the same way they already survived the
per-checkbox path. Previously one click on any bulk button silently dropped
them from the saved row, and the **All** button could widen effective read
access by deleting a `readScope: 'own'` narrowing with no diff and no error.
**None** deliberately keeps clearing the whole row, narrowings included:
merging there would leave `allowExport: true` alive after a click on the
button labelled None (objectui#6605).
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* objectui#6605 — the bulk buttons (R / CRUD / All / None) and the keys the
* matrix does not author.
*
* `bulkSetObject` used to REPLACE the object's permission row. Three
* spec-declared keys are modelled by neither the local `ObjectPerm` interface
* nor the `OBJECT_ACTIONS` column list — `allowExport`, and the ADR-0057
* access-depth axis `readScope` / `writeScope` — so one bulk click dropped
* whatever those held. The sharpest shape is the button labelled **All**: an
* admin clicking what reads as "grant everything" could WIDEN effective read
* access by deleting a `readScope: 'own'` narrowing, with no diff shown and no
* error.
*
* Every pin here asserts the SAVED payload, never editor state. That is the
* card's own argument for why the defect persisted: both save doors carry the
* row as-is — the environment door writes the whole record, and at package
* scope `mergePermissionSlice` takes in-scope rows entirely from `edited`
* (ADR-0086 P0), so `base` cannot restore what a bulk click dropped. An
* editor-state assertion would prove nothing about either door.
*
* ## `none` is pinned to keep REPLACING — that is the fix's fence, not a gap
*
* The dispatch on #6605 deliberately rejects the card's "`none` needs the same
* treatment" suggestion. The defect is a GRANT that silently drops a
* narrowing; `none` grants nothing, so nothing survives for a scope to
* narrow. Merging `none` would instead leave `allowExport: true` (and the
* scopes) alive after a click on the button labelled "None" — a permissive
* outcome that does not exist today, on a surface whose whole problem is
* silent permissiveness. What an admin's "None" means is a behaviour
* decision, made on #6605, not a mechanical merge. The `none` pin below makes
* that fence mechanical: a refactor that quietly adopts the card's suggestion
* goes red here and needs a maintainer decision, not a cleanup commit.
*/

import { describe, it, expect, vi, afterEach } from 'vitest';
import { render, screen, cleanup, fireEvent, waitFor, within } from '@testing-library/react';
import { MemoryRouter } from 'react-router-dom';

/** Every object-permission key the matrix authors, in column order. */
const MATRIX_KEYS = [
'allowCreate',
'allowRead',
'allowEdit',
'allowDelete',
'allowTransfer',
'viewAllRecords',
'modifyAllRecords',
];

interface FakeServer {
/** The stored record — what `layered()` answers (also the merge `base`). */
set: Record<string, any>;
/** What `list('object')` lists — the matrix rows (and, under a packageId, the slice scope). */
objectNames: string[];
saved: Record<string, any> | null;
savedOpts: Record<string, any> | undefined;
}

function makeClient(server: FakeServer) {
return {
layered: async () => ({ effective: server.set, code: null, overlay: null, overlayScope: null }),
getDraft: async () => null,
list: async (type: string) =>
type === 'object' ? server.objectNames.map((name) => ({ item: { name } })) : [],
get: async (type: string) => (type === 'object' ? { fields: [] } : null),
save: async (
_t: string,
_n: string,
payload: Record<string, any>,
opts?: Record<string, any>,
) => {
server.saved = payload;
server.savedOpts = opts;
return payload;
},
} as any;
}

let clientImpl: any;

vi.mock('./useMetadata', () => ({
useMetadataClient: () => clientImpl,
useMetadataTypes: () => ({
loading: false,
error: null,
entries: [{ type: 'permission', label: 'Permission', allowOrgOverride: true }],
}),
}));
vi.mock('./AssignedUsersSection', () => ({ AssignedUsersSection: () => null }));
vi.mock('@object-ui/fields', () => ({
CapabilityMultiSelectField: () => <div data-testid="cap-picker" />,
parseCapabilityNames: (v: unknown) => (typeof v === 'string' ? JSON.parse(v) : []),
}));

import { PermissionMatrixEditPage } from './PermissionMatrixEditor';

afterEach(cleanup);

async function renderMatrix(
objects: Record<string, unknown>,
opts: { objectNames?: string[]; packageId?: string } = {},
): Promise<FakeServer> {
const server: FakeServer = {
set: { name: 'sales_perms', label: 'Sales', objects, fields: {} },
objectNames: opts.objectNames ?? Object.keys(objects),
saved: null,
savedOpts: undefined,
};
clientImpl = makeClient(server);
render(
<MemoryRouter>
<PermissionMatrixEditPage type="permission" name="sales_perms" packageId={opts.packageId} />
</MemoryRouter>,
);
await screen.findByText('Sales');
return server;
}

/** The bulk button (`R` / `CRUD` / `All` / `None`) inside one object's row. */
function bulkButton(objectName: string, label: string) {
const row = screen
.getAllByRole('row')
.find((r) => within(r).queryByText(objectName) != null);
expect(row, `row for ${objectName}`).toBeTruthy();
return within(row!).getByRole('button', { name: new RegExp(`^${label}$`) });
}

/** Click Save and return the payload the client was handed. */
async function save(server: FakeServer) {
fireEvent.click(screen.getByRole('button', { name: /^Save$/ }));
await waitFor(() => expect(server.saved).not.toBeNull());
return server.saved!;
}

describe('PermissionMatrixEditor · bulk buttons vs unmodelled keys (objectui#6605)', () => {
it('"All" — the widening shape — grants every column AND the saved row keeps readScope / writeScope / allowExport', async () => {
const server = await renderMatrix({
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
});

fireEvent.click(bulkButton('a_account', 'All'));
const payload = await save(server);

const row = payload.objects.a_account;
for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true);
// The narrowings survive the click that used to delete them. `readScope`
// is the load-bearing one: dropping `own` silently widened read access.
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('own');
expect(row.allowExport).toBe(true);
expect(Object.keys(row).sort()).toEqual(
[...MATRIX_KEYS, 'allowExport', 'readScope', 'writeScope'].sort(),
);
});

it('"CRUD" merges the unmodelled keys through but still RESETS the matrix columns outside its grant', async () => {
const server = await renderMatrix({
a_account: {
allowTransfer: true,
viewAllRecords: true,
modifyAllRecords: true,
allowExport: true,
readScope: 'own',
writeScope: 'unit',
},
});

fireEvent.click(bulkButton('a_account', 'CRUD'));
const payload = await save(server);

const row = payload.objects.a_account;
// Falsification direction: merge must not decay into "add" — a bulk CRUD
// after a wider grant still means exactly CRUD for the keys the matrix owns.
expect(Object.keys(row).sort()).toEqual(
['allowCreate', 'allowRead', 'allowEdit', 'allowDelete', 'allowExport', 'readScope', 'writeScope'].sort(),
);
expect('allowTransfer' in row).toBe(false);
expect('viewAllRecords' in row).toBe(false);
expect('modifyAllRecords' in row).toBe(false);
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('unit');
expect(row.allowExport).toBe(true);
});

it('"R" saves a read-only row that still carries the unmodelled keys', async () => {
const server = await renderMatrix({
a_account: {
allowCreate: true,
allowEdit: true,
allowExport: true,
readScope: 'own',
writeScope: 'unit',
},
});

fireEvent.click(bulkButton('a_account', 'R'));
const payload = await save(server);

const row = payload.objects.a_account;
expect(Object.keys(row).sort()).toEqual(
['allowRead', 'allowExport', 'readScope', 'writeScope'].sort(),
);
expect(row.allowRead).toBe(true);
expect(row.readScope).toBe('own');
});

it('"None" still clears the WHOLE row — narrowings included (deliberate: the #6605 dispatch fence)', async () => {
// Read the header before "fixing" this pin: merging `none` would leave
// `allowExport: true` alive after a click on the button labelled "None".
const server = await renderMatrix({
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
a_contact: { allowRead: true, readScope: 'own' },
});

fireEvent.click(bulkButton('a_account', 'None'));
const payload = await save(server);

expect(payload.objects.a_account).toEqual({});
// Positive control in the same query shape: the untouched sibling row in
// the SAME saved payload still carries its narrowing, so the emptiness
// above is a measurement of `none`, not of a save path that drops keys.
expect(payload.objects.a_contact).toEqual({ allowRead: true, readScope: 'own' });
});

it('package door: the merged slice keeps the unmodelled keys after "All", and other packages\' rows survive byte-for-byte', async () => {
// In-scope: a_account (this package's row, carrying the narrowings).
// Out-of-scope: b_order — another package's contribution, not listed by
// this package, which `mergePermissionSlice` must copy verbatim from base.
const server = await renderMatrix(
{
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
b_order: { allowRead: true, viewAllRecords: true, readScope: 'unit' },
},
{ objectNames: ['a_account'], packageId: 'app.a' },
);

fireEvent.click(bulkButton('a_account', 'All'));
const payload = await save(server);

const row = payload.objects.a_account;
for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true);
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('own');
expect(row.allowExport).toBe(true);
// The other package's row is untouched — and the save went through the
// package door (a draft write), not a live record write.
expect(payload.objects.b_order).toEqual({ allowRead: true, viewAllRecords: true, readScope: 'unit' });
expect(server.savedOpts).toMatchObject({ mode: 'draft', packageId: 'app.a' });
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -638,14 +638,39 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved,

function bulkSetObject(objectName: string, action: 'all' | 'none' | 'crud' | 'read') {
setDraft((prev) => {
const next: ObjectPerm =
action === 'none'
? {}
: action === 'all'
? Object.fromEntries(OBJECT_ACTIONS.map((a) => [a.key, true])) as ObjectPerm
// `none` REPLACES the row with `{}` — deliberately, unlike the three
// granting arms below (#6605). The defect those arms had was a GRANT
// that silently dropped a narrowing: "All" deleting a `readScope: 'own'`
// widens effective read access with no diff and no error. `none` grants
// nothing, so nothing survives for a scope to narrow; merging here would
// instead leave `allowExport: true` (and the scopes) alive after a click
// on the button labelled "None" — a permissive outcome that does not
// exist today. What an admin's "None" means is a behaviour decision, not
// a mechanical merge; pinned by
// `PermissionMatrixEditor.bulkMergeKeys.test.tsx`.
if (action === 'none') {
return { ...prev, objects: { ...prev.objects, [objectName]: {} } };
}
// The granting arms MERGE (#6605): start from the current row, reset the
// keys this matrix authors (`OBJECT_ACTIONS`), then set the granted
// ones. Keys the matrix does not model — `allowExport`, `readScope`,
// `writeScope`, anything an older or newer editor wrote — ride through
// exactly as they do on the per-checkbox path (`updateObjectPerm`'s
// spread). Replacing the row wholesale is what silently deleted them,
// and both save doors persist the row as-is: the environment door writes
// the whole record, and at package scope `mergePermissionSlice` takes
// in-scope rows entirely from `edited` (ADR-0086 P0), so `base` cannot
// restore what a bulk click dropped.
const cur = prev.objects[objectName] ?? {};
const next: ObjectPerm = { ...cur };
for (const a of OBJECT_ACTIONS) delete next[a.key];
const grants: Array<keyof ObjectPerm> =
action === 'all'
? OBJECT_ACTIONS.map((a) => a.key)
: action === 'crud'
? { allowCreate: true, allowRead: true, allowEdit: true, allowDelete: true }
: { allowRead: true };
? ['allowCreate', 'allowRead', 'allowEdit', 'allowDelete']
: ['allowRead'];
for (const key of grants) next[key] = true;
return {
...prev,
objects: { ...prev.objects, [objectName]: next },
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changeset/6605-permission-bulk-merge-not-replace.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
---
'@object-ui/app-shell': patch
---

Permission matrix bulk buttons (R / CRUD / All) now merge into the object's
permission row instead of replacing it, so spec-declared keys the matrix does
not author — `allowExport` and the ADR-0057 access-depth axis `readScope` /
`writeScope` — survive a bulk click the same way they already survived the
per-checkbox path. Previously one click on any bulk button silently dropped
them from the saved row, and the **All** button could widen effective read
access by deleting a `readScope: 'own'` narrowing with no diff and no error.
**None** deliberately keeps clearing the whole row, narrowings included:
merging there would leave `allowExport: true` alive after a click on the
button labelled None (objectui#6605).
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* objectui#6605 — the bulk buttons (R / CRUD / All / None) and the keys the
* matrix does not author.
*
* `bulkSetObject` used to REPLACE the object's permission row. Three
* spec-declared keys are modelled by neither the local `ObjectPerm` interface
* nor the `OBJECT_ACTIONS` column list — `allowExport`, and the ADR-0057
* access-depth axis `readScope` / `writeScope` — so one bulk click dropped
* whatever those held. The sharpest shape is the button labelled **All**: an
* admin clicking what reads as "grant everything" could WIDEN effective read
* access by deleting a `readScope: 'own'` narrowing, with no diff shown and no
* error.
*
* Every pin here asserts the SAVED payload, never editor state. That is the
* card's own argument for why the defect persisted: both save doors carry the
* row as-is — the environment door writes the whole record, and at package
* scope `mergePermissionSlice` takes in-scope rows entirely from `edited`
* (ADR-0086 P0), so `base` cannot restore what a bulk click dropped. An
* editor-state assertion would prove nothing about either door.
*
* ## `none` is pinned to keep REPLACING — that is the fix's fence, not a gap
*
* The dispatch on #6605 deliberately rejects the card's "`none` needs the same
* treatment" suggestion. The defect is a GRANT that silently drops a
* narrowing; `none` grants nothing, so nothing survives for a scope to
* narrow. Merging `none` would instead leave `allowExport: true` (and the
* scopes) alive after a click on the button labelled "None" — a permissive
* outcome that does not exist today, on a surface whose whole problem is
* silent permissiveness. What an admin's "None" means is a behaviour
* decision, made on #6605, not a mechanical merge. The `none` pin below makes
* that fence mechanical: a refactor that quietly adopts the card's suggestion
* goes red here and needs a maintainer decision, not a cleanup commit.
*/

import { describe, it, expect, vi, afterEach } from 'vitest';
import { render, screen, cleanup, fireEvent, waitFor, within } from '@testing-library/react';
import { MemoryRouter } from 'react-router-dom';

/** Every object-permission key the matrix authors, in column order. */
const MATRIX_KEYS = [
'allowCreate',
'allowRead',
'allowEdit',
'allowDelete',
'allowTransfer',
'viewAllRecords',
'modifyAllRecords',
];

interface FakeServer {
/** The stored record — what `layered()` answers (also the merge `base`). */
set: Record<string, any>;
/** What `list('object')` lists — the matrix rows (and, under a packageId, the slice scope). */
objectNames: string[];
saved: Record<string, any> | null;
savedOpts: Record<string, any> | undefined;
}

function makeClient(server: FakeServer) {
return {
layered: async () => ({ effective: server.set, code: null, overlay: null, overlayScope: null }),
getDraft: async () => null,
list: async (type: string) =>
type === 'object' ? server.objectNames.map((name) => ({ item: { name } })) : [],
get: async (type: string) => (type === 'object' ? { fields: [] } : null),
save: async (
_t: string,
_n: string,
payload: Record<string, any>,
opts?: Record<string, any>,
) => {
server.saved = payload;
server.savedOpts = opts;
return payload;
},
} as any;
}

let clientImpl: any;

vi.mock('./useMetadata', () => ({
useMetadataClient: () => clientImpl,
useMetadataTypes: () => ({
loading: false,
error: null,
entries: [{ type: 'permission', label: 'Permission', allowOrgOverride: true }],
}),
}));
vi.mock('./AssignedUsersSection', () => ({ AssignedUsersSection: () => null }));
vi.mock('@object-ui/fields', () => ({
CapabilityMultiSelectField: () => <div data-testid="cap-picker" />,
parseCapabilityNames: (v: unknown) => (typeof v === 'string' ? JSON.parse(v) : []),
}));

import { PermissionMatrixEditPage } from './PermissionMatrixEditor';

afterEach(cleanup);

async function renderMatrix(
objects: Record<string, unknown>,
opts: { objectNames?: string[]; packageId?: string } = {},
): Promise<FakeServer> {
const server: FakeServer = {
set: { name: 'sales_perms', label: 'Sales', objects, fields: {} },
objectNames: opts.objectNames ?? Object.keys(objects),
saved: null,
savedOpts: undefined,
};
clientImpl = makeClient(server);
render(
<MemoryRouter>
<PermissionMatrixEditPage type="permission" name="sales_perms" packageId={opts.packageId} />
</MemoryRouter>,
);
await screen.findByText('Sales');
return server;
}

/** The bulk button (`R` / `CRUD` / `All` / `None`) inside one object's row. */
function bulkButton(objectName: string, label: string) {
const row = screen
.getAllByRole('row')
.find((r) => within(r).queryByText(objectName) != null);
expect(row, `row for ${objectName}`).toBeTruthy();
return within(row!).getByRole('button', { name: new RegExp(`^${label}$`) });
}

/** Click Save and return the payload the client was handed. */
async function save(server: FakeServer) {
fireEvent.click(screen.getByRole('button', { name: /^Save$/ }));
await waitFor(() => expect(server.saved).not.toBeNull());
return server.saved!;
}

describe('PermissionMatrixEditor · bulk buttons vs unmodelled keys (objectui#6605)', () => {
it('"All" — the widening shape — grants every column AND the saved row keeps readScope / writeScope / allowExport', async () => {
const server = await renderMatrix({
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
});

fireEvent.click(bulkButton('a_account', 'All'));
const payload = await save(server);

const row = payload.objects.a_account;
for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true);
// The narrowings survive the click that used to delete them. `readScope`
// is the load-bearing one: dropping `own` silently widened read access.
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('own');
expect(row.allowExport).toBe(true);
expect(Object.keys(row).sort()).toEqual(
[...MATRIX_KEYS, 'allowExport', 'readScope', 'writeScope'].sort(),
);
});

it('"CRUD" merges the unmodelled keys through but still RESETS the matrix columns outside its grant', async () => {
const server = await renderMatrix({
a_account: {
allowTransfer: true,
viewAllRecords: true,
modifyAllRecords: true,
allowExport: true,
readScope: 'own',
writeScope: 'unit',
},
});

fireEvent.click(bulkButton('a_account', 'CRUD'));
const payload = await save(server);

const row = payload.objects.a_account;
// Falsification direction: merge must not decay into "add" — a bulk CRUD
// after a wider grant still means exactly CRUD for the keys the matrix owns.
expect(Object.keys(row).sort()).toEqual(
['allowCreate', 'allowRead', 'allowEdit', 'allowDelete', 'allowExport', 'readScope', 'writeScope'].sort(),
);
expect('allowTransfer' in row).toBe(false);
expect('viewAllRecords' in row).toBe(false);
expect('modifyAllRecords' in row).toBe(false);
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('unit');
expect(row.allowExport).toBe(true);
});

it('"R" saves a read-only row that still carries the unmodelled keys', async () => {
const server = await renderMatrix({
a_account: {
allowCreate: true,
allowEdit: true,
allowExport: true,
readScope: 'own',
writeScope: 'unit',
},
});

fireEvent.click(bulkButton('a_account', 'R'));
const payload = await save(server);

const row = payload.objects.a_account;
expect(Object.keys(row).sort()).toEqual(
['allowRead', 'allowExport', 'readScope', 'writeScope'].sort(),
);
expect(row.allowRead).toBe(true);
expect(row.readScope).toBe('own');
});

it('"None" still clears the WHOLE row — narrowings included (deliberate: the #6605 dispatch fence)', async () => {
// Read the header before "fixing" this pin: merging `none` would leave
// `allowExport: true` alive after a click on the button labelled "None".
const server = await renderMatrix({
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
a_contact: { allowRead: true, readScope: 'own' },
});

fireEvent.click(bulkButton('a_account', 'None'));
const payload = await save(server);

expect(payload.objects.a_account).toEqual({});
// Positive control in the same query shape: the untouched sibling row in
// the SAME saved payload still carries its narrowing, so the emptiness
// above is a measurement of `none`, not of a save path that drops keys.
expect(payload.objects.a_contact).toEqual({ allowRead: true, readScope: 'own' });
});

it('package door: the merged slice keeps the unmodelled keys after "All", and other packages\' rows survive byte-for-byte', async () => {
// In-scope: a_account (this package's row, carrying the narrowings).
// Out-of-scope: b_order — another package's contribution, not listed by
// this package, which `mergePermissionSlice` must copy verbatim from base.
const server = await renderMatrix(
{
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
b_order: { allowRead: true, viewAllRecords: true, readScope: 'unit' },
},
{ objectNames: ['a_account'], packageId: 'app.a' },
);

fireEvent.click(bulkButton('a_account', 'All'));
const payload = await save(server);

const row = payload.objects.a_account;
for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true);
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('own');
expect(row.allowExport).toBe(true);
// The other package's row is untouched — and the save went through the
// package door (a draft write), not a live record write.
expect(payload.objects.b_order).toEqual({ allowRead: true, viewAllRecords: true, readScope: 'unit' });
expect(server.savedOpts).toMatchObject({ mode: 'draft', packageId: 'app.a' });
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -638,14 +638,39 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved,

function bulkSetObject(objectName: string, action: 'all' | 'none' | 'crud' | 'read') {
setDraft((prev) => {
const next: ObjectPerm =
action === 'none'
? {}
: action === 'all'
? Object.fromEntries(OBJECT_ACTIONS.map((a) => [a.key, true])) as ObjectPerm
// `none` REPLACES the row with `{}` — deliberately, unlike the three
// granting arms below (#6605). The defect those arms had was a GRANT
// that silently dropped a narrowing: "All" deleting a `readScope: 'own'`
// widens effective read access with no diff and no error. `none` grants
// nothing, so nothing survives for a scope to narrow; merging here would
// instead leave `allowExport: true` (and the scopes) alive after a click
// on the button labelled "None" — a permissive outcome that does not
// exist today. What an admin's "None" means is a behaviour decision, not
// a mechanical merge; pinned by
// `PermissionMatrixEditor.bulkMergeKeys.test.tsx`.
if (action === 'none') {
return { ...prev, objects: { ...prev.objects, [objectName]: {} } };
}
// The granting arms MERGE (#6605): start from the current row, reset the
// keys this matrix authors (`OBJECT_ACTIONS`), then set the granted
// ones. Keys the matrix does not model — `allowExport`, `readScope`,
// `writeScope`, anything an older or newer editor wrote — ride through
// exactly as they do on the per-checkbox path (`updateObjectPerm`'s
// spread). Replacing the row wholesale is what silently deleted them,
// and both save doors persist the row as-is: the environment door writes
// the whole record, and at package scope `mergePermissionSlice` takes
// in-scope rows entirely from `edited` (ADR-0086 P0), so `base` cannot
// restore what a bulk click dropped.
const cur = prev.objects[objectName] ?? {};
const next: ObjectPerm = { ...cur };
for (const a of OBJECT_ACTIONS) delete next[a.key];
const grants: Array<keyof ObjectPerm> =
action === 'all'
? OBJECT_ACTIONS.map((a) => a.key)
: action === 'crud'
? { allowCreate: true, allowRead: true, allowEdit: true, allowDelete: true }
: { allowRead: true };
? ['allowCreate', 'allowRead', 'allowEdit', 'allowDelete']
: ['allowRead'];
for (const key of grants) next[key] = true;
return {
...prev,
objects: { ...prev.objects, [objectName]: next },
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changeset/6605-permission-bulk-merge-not-replace.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
---
'@object-ui/app-shell': patch
---

Permission matrix bulk buttons (R / CRUD / All) now merge into the object's
permission row instead of replacing it, so spec-declared keys the matrix does
not author — `allowExport` and the ADR-0057 access-depth axis `readScope` /
`writeScope` — survive a bulk click the same way they already survived the
per-checkbox path. Previously one click on any bulk button silently dropped
them from the saved row, and the **All** button could widen effective read
access by deleting a `readScope: 'own'` narrowing with no diff and no error.
**None** deliberately keeps clearing the whole row, narrowings included:
merging there would leave `allowExport: true` alive after a click on the
button labelled None (objectui#6605).
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* objectui#6605 — the bulk buttons (R / CRUD / All / None) and the keys the
* matrix does not author.
*
* `bulkSetObject` used to REPLACE the object's permission row. Three
* spec-declared keys are modelled by neither the local `ObjectPerm` interface
* nor the `OBJECT_ACTIONS` column list — `allowExport`, and the ADR-0057
* access-depth axis `readScope` / `writeScope` — so one bulk click dropped
* whatever those held. The sharpest shape is the button labelled **All**: an
* admin clicking what reads as "grant everything" could WIDEN effective read
* access by deleting a `readScope: 'own'` narrowing, with no diff shown and no
* error.
*
* Every pin here asserts the SAVED payload, never editor state. That is the
* card's own argument for why the defect persisted: both save doors carry the
* row as-is — the environment door writes the whole record, and at package
* scope `mergePermissionSlice` takes in-scope rows entirely from `edited`
* (ADR-0086 P0), so `base` cannot restore what a bulk click dropped. An
* editor-state assertion would prove nothing about either door.
*
* ## `none` is pinned to keep REPLACING — that is the fix's fence, not a gap
*
* The dispatch on #6605 deliberately rejects the card's "`none` needs the same
* treatment" suggestion. The defect is a GRANT that silently drops a
* narrowing; `none` grants nothing, so nothing survives for a scope to
* narrow. Merging `none` would instead leave `allowExport: true` (and the
* scopes) alive after a click on the button labelled "None" — a permissive
* outcome that does not exist today, on a surface whose whole problem is
* silent permissiveness. What an admin's "None" means is a behaviour
* decision, made on #6605, not a mechanical merge. The `none` pin below makes
* that fence mechanical: a refactor that quietly adopts the card's suggestion
* goes red here and needs a maintainer decision, not a cleanup commit.
*/

import { describe, it, expect, vi, afterEach } from 'vitest';
import { render, screen, cleanup, fireEvent, waitFor, within } from '@testing-library/react';
import { MemoryRouter } from 'react-router-dom';

/** Every object-permission key the matrix authors, in column order. */
const MATRIX_KEYS = [
'allowCreate',
'allowRead',
'allowEdit',
'allowDelete',
'allowTransfer',
'viewAllRecords',
'modifyAllRecords',
];

interface FakeServer {
/** The stored record — what `layered()` answers (also the merge `base`). */
set: Record<string, any>;
/** What `list('object')` lists — the matrix rows (and, under a packageId, the slice scope). */
objectNames: string[];
saved: Record<string, any> | null;
savedOpts: Record<string, any> | undefined;
}

function makeClient(server: FakeServer) {
return {
layered: async () => ({ effective: server.set, code: null, overlay: null, overlayScope: null }),
getDraft: async () => null,
list: async (type: string) =>
type === 'object' ? server.objectNames.map((name) => ({ item: { name } })) : [],
get: async (type: string) => (type === 'object' ? { fields: [] } : null),
save: async (
_t: string,
_n: string,
payload: Record<string, any>,
opts?: Record<string, any>,
) => {
server.saved = payload;
server.savedOpts = opts;
return payload;
},
} as any;
}

let clientImpl: any;

vi.mock('./useMetadata', () => ({
useMetadataClient: () => clientImpl,
useMetadataTypes: () => ({
loading: false,
error: null,
entries: [{ type: 'permission', label: 'Permission', allowOrgOverride: true }],
}),
}));
vi.mock('./AssignedUsersSection', () => ({ AssignedUsersSection: () => null }));
vi.mock('@object-ui/fields', () => ({
CapabilityMultiSelectField: () => <div data-testid="cap-picker" />,
parseCapabilityNames: (v: unknown) => (typeof v === 'string' ? JSON.parse(v) : []),
}));

import { PermissionMatrixEditPage } from './PermissionMatrixEditor';

afterEach(cleanup);

async function renderMatrix(
objects: Record<string, unknown>,
opts: { objectNames?: string[]; packageId?: string } = {},
): Promise<FakeServer> {
const server: FakeServer = {
set: { name: 'sales_perms', label: 'Sales', objects, fields: {} },
objectNames: opts.objectNames ?? Object.keys(objects),
saved: null,
savedOpts: undefined,
};
clientImpl = makeClient(server);
render(
<MemoryRouter>
<PermissionMatrixEditPage type="permission" name="sales_perms" packageId={opts.packageId} />
</MemoryRouter>,
);
await screen.findByText('Sales');
return server;
}

/** The bulk button (`R` / `CRUD` / `All` / `None`) inside one object's row. */
function bulkButton(objectName: string, label: string) {
const row = screen
.getAllByRole('row')
.find((r) => within(r).queryByText(objectName) != null);
expect(row, `row for ${objectName}`).toBeTruthy();
return within(row!).getByRole('button', { name: new RegExp(`^${label}$`) });
}

/** Click Save and return the payload the client was handed. */
async function save(server: FakeServer) {
fireEvent.click(screen.getByRole('button', { name: /^Save$/ }));
await waitFor(() => expect(server.saved).not.toBeNull());
return server.saved!;
}

describe('PermissionMatrixEditor · bulk buttons vs unmodelled keys (objectui#6605)', () => {
it('"All" — the widening shape — grants every column AND the saved row keeps readScope / writeScope / allowExport', async () => {
const server = await renderMatrix({
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
});

fireEvent.click(bulkButton('a_account', 'All'));
const payload = await save(server);

const row = payload.objects.a_account;
for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true);
// The narrowings survive the click that used to delete them. `readScope`
// is the load-bearing one: dropping `own` silently widened read access.
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('own');
expect(row.allowExport).toBe(true);
expect(Object.keys(row).sort()).toEqual(
[...MATRIX_KEYS, 'allowExport', 'readScope', 'writeScope'].sort(),
);
});

it('"CRUD" merges the unmodelled keys through but still RESETS the matrix columns outside its grant', async () => {
const server = await renderMatrix({
a_account: {
allowTransfer: true,
viewAllRecords: true,
modifyAllRecords: true,
allowExport: true,
readScope: 'own',
writeScope: 'unit',
},
});

fireEvent.click(bulkButton('a_account', 'CRUD'));
const payload = await save(server);

const row = payload.objects.a_account;
// Falsification direction: merge must not decay into "add" — a bulk CRUD
// after a wider grant still means exactly CRUD for the keys the matrix owns.
expect(Object.keys(row).sort()).toEqual(
['allowCreate', 'allowRead', 'allowEdit', 'allowDelete', 'allowExport', 'readScope', 'writeScope'].sort(),
);
expect('allowTransfer' in row).toBe(false);
expect('viewAllRecords' in row).toBe(false);
expect('modifyAllRecords' in row).toBe(false);
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('unit');
expect(row.allowExport).toBe(true);
});

it('"R" saves a read-only row that still carries the unmodelled keys', async () => {
const server = await renderMatrix({
a_account: {
allowCreate: true,
allowEdit: true,
allowExport: true,
readScope: 'own',
writeScope: 'unit',
},
});

fireEvent.click(bulkButton('a_account', 'R'));
const payload = await save(server);

const row = payload.objects.a_account;
expect(Object.keys(row).sort()).toEqual(
['allowRead', 'allowExport', 'readScope', 'writeScope'].sort(),
);
expect(row.allowRead).toBe(true);
expect(row.readScope).toBe('own');
});

it('"None" still clears the WHOLE row — narrowings included (deliberate: the #6605 dispatch fence)', async () => {
// Read the header before "fixing" this pin: merging `none` would leave
// `allowExport: true` alive after a click on the button labelled "None".
const server = await renderMatrix({
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
a_contact: { allowRead: true, readScope: 'own' },
});

fireEvent.click(bulkButton('a_account', 'None'));
const payload = await save(server);

expect(payload.objects.a_account).toEqual({});
// Positive control in the same query shape: the untouched sibling row in
// the SAME saved payload still carries its narrowing, so the emptiness
// above is a measurement of `none`, not of a save path that drops keys.
expect(payload.objects.a_contact).toEqual({ allowRead: true, readScope: 'own' });
});

it('package door: the merged slice keeps the unmodelled keys after "All", and other packages\' rows survive byte-for-byte', async () => {
// In-scope: a_account (this package's row, carrying the narrowings).
// Out-of-scope: b_order — another package's contribution, not listed by
// this package, which `mergePermissionSlice` must copy verbatim from base.
const server = await renderMatrix(
{
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
b_order: { allowRead: true, viewAllRecords: true, readScope: 'unit' },
},
{ objectNames: ['a_account'], packageId: 'app.a' },
);

fireEvent.click(bulkButton('a_account', 'All'));
const payload = await save(server);

const row = payload.objects.a_account;
for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true);
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('own');
expect(row.allowExport).toBe(true);
// The other package's row is untouched — and the save went through the
// package door (a draft write), not a live record write.
expect(payload.objects.b_order).toEqual({ allowRead: true, viewAllRecords: true, readScope: 'unit' });
expect(server.savedOpts).toMatchObject({ mode: 'draft', packageId: 'app.a' });
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -638,14 +638,39 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved,

function bulkSetObject(objectName: string, action: 'all' | 'none' | 'crud' | 'read') {
setDraft((prev) => {
const next: ObjectPerm =
action === 'none'
? {}
: action === 'all'
? Object.fromEntries(OBJECT_ACTIONS.map((a) => [a.key, true])) as ObjectPerm
// `none` REPLACES the row with `{}` — deliberately, unlike the three
// granting arms below (#6605). The defect those arms had was a GRANT
// that silently dropped a narrowing: "All" deleting a `readScope: 'own'`
// widens effective read access with no diff and no error. `none` grants
// nothing, so nothing survives for a scope to narrow; merging here would
// instead leave `allowExport: true` (and the scopes) alive after a click
// on the button labelled "None" — a permissive outcome that does not
// exist today. What an admin's "None" means is a behaviour decision, not
// a mechanical merge; pinned by
// `PermissionMatrixEditor.bulkMergeKeys.test.tsx`.
if (action === 'none') {
return { ...prev, objects: { ...prev.objects, [objectName]: {} } };
}
// The granting arms MERGE (#6605): start from the current row, reset the
// keys this matrix authors (`OBJECT_ACTIONS`), then set the granted
// ones. Keys the matrix does not model — `allowExport`, `readScope`,
// `writeScope`, anything an older or newer editor wrote — ride through
// exactly as they do on the per-checkbox path (`updateObjectPerm`'s
// spread). Replacing the row wholesale is what silently deleted them,
// and both save doors persist the row as-is: the environment door writes
// the whole record, and at package scope `mergePermissionSlice` takes
// in-scope rows entirely from `edited` (ADR-0086 P0), so `base` cannot
// restore what a bulk click dropped.
const cur = prev.objects[objectName] ?? {};
const next: ObjectPerm = { ...cur };
for (const a of OBJECT_ACTIONS) delete next[a.key];
const grants: Array<keyof ObjectPerm> =
action === 'all'
? OBJECT_ACTIONS.map((a) => a.key)
: action === 'crud'
? { allowCreate: true, allowRead: true, allowEdit: true, allowDelete: true }
: { allowRead: true };
? ['allowCreate', 'allowRead', 'allowEdit', 'allowDelete']
: ['allowRead'];
for (const key of grants) next[key] = true;
return {
...prev,
objects: { ...prev.objects, [objectName]: next },
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changeset/6605-permission-bulk-merge-not-replace.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
---
'@object-ui/app-shell': patch
---

Permission matrix bulk buttons (R / CRUD / All) now merge into the object's
permission row instead of replacing it, so spec-declared keys the matrix does
not author — `allowExport` and the ADR-0057 access-depth axis `readScope` /
`writeScope` — survive a bulk click the same way they already survived the
per-checkbox path. Previously one click on any bulk button silently dropped
them from the saved row, and the **All** button could widen effective read
access by deleting a `readScope: 'own'` narrowing with no diff and no error.
**None** deliberately keeps clearing the whole row, narrowings included:
merging there would leave `allowExport: true` alive after a click on the
button labelled None (objectui#6605).
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* objectui#6605 — the bulk buttons (R / CRUD / All / None) and the keys the
* matrix does not author.
*
* `bulkSetObject` used to REPLACE the object's permission row. Three
* spec-declared keys are modelled by neither the local `ObjectPerm` interface
* nor the `OBJECT_ACTIONS` column list — `allowExport`, and the ADR-0057
* access-depth axis `readScope` / `writeScope` — so one bulk click dropped
* whatever those held. The sharpest shape is the button labelled **All**: an
* admin clicking what reads as "grant everything" could WIDEN effective read
* access by deleting a `readScope: 'own'` narrowing, with no diff shown and no
* error.
*
* Every pin here asserts the SAVED payload, never editor state. That is the
* card's own argument for why the defect persisted: both save doors carry the
* row as-is — the environment door writes the whole record, and at package
* scope `mergePermissionSlice` takes in-scope rows entirely from `edited`
* (ADR-0086 P0), so `base` cannot restore what a bulk click dropped. An
* editor-state assertion would prove nothing about either door.
*
* ## `none` is pinned to keep REPLACING — that is the fix's fence, not a gap
*
* The dispatch on #6605 deliberately rejects the card's "`none` needs the same
* treatment" suggestion. The defect is a GRANT that silently drops a
* narrowing; `none` grants nothing, so nothing survives for a scope to
* narrow. Merging `none` would instead leave `allowExport: true` (and the
* scopes) alive after a click on the button labelled "None" — a permissive
* outcome that does not exist today, on a surface whose whole problem is
* silent permissiveness. What an admin's "None" means is a behaviour
* decision, made on #6605, not a mechanical merge. The `none` pin below makes
* that fence mechanical: a refactor that quietly adopts the card's suggestion
* goes red here and needs a maintainer decision, not a cleanup commit.
*/

import { describe, it, expect, vi, afterEach } from 'vitest';
import { render, screen, cleanup, fireEvent, waitFor, within } from '@testing-library/react';
import { MemoryRouter } from 'react-router-dom';

/** Every object-permission key the matrix authors, in column order. */
const MATRIX_KEYS = [
'allowCreate',
'allowRead',
'allowEdit',
'allowDelete',
'allowTransfer',
'viewAllRecords',
'modifyAllRecords',
];

interface FakeServer {
/** The stored record — what `layered()` answers (also the merge `base`). */
set: Record<string, any>;
/** What `list('object')` lists — the matrix rows (and, under a packageId, the slice scope). */
objectNames: string[];
saved: Record<string, any> | null;
savedOpts: Record<string, any> | undefined;
}

function makeClient(server: FakeServer) {
return {
layered: async () => ({ effective: server.set, code: null, overlay: null, overlayScope: null }),
getDraft: async () => null,
list: async (type: string) =>
type === 'object' ? server.objectNames.map((name) => ({ item: { name } })) : [],
get: async (type: string) => (type === 'object' ? { fields: [] } : null),
save: async (
_t: string,
_n: string,
payload: Record<string, any>,
opts?: Record<string, any>,
) => {
server.saved = payload;
server.savedOpts = opts;
return payload;
},
} as any;
}

let clientImpl: any;

vi.mock('./useMetadata', () => ({
useMetadataClient: () => clientImpl,
useMetadataTypes: () => ({
loading: false,
error: null,
entries: [{ type: 'permission', label: 'Permission', allowOrgOverride: true }],
}),
}));
vi.mock('./AssignedUsersSection', () => ({ AssignedUsersSection: () => null }));
vi.mock('@object-ui/fields', () => ({
CapabilityMultiSelectField: () => <div data-testid="cap-picker" />,
parseCapabilityNames: (v: unknown) => (typeof v === 'string' ? JSON.parse(v) : []),
}));

import { PermissionMatrixEditPage } from './PermissionMatrixEditor';

afterEach(cleanup);

async function renderMatrix(
objects: Record<string, unknown>,
opts: { objectNames?: string[]; packageId?: string } = {},
): Promise<FakeServer> {
const server: FakeServer = {
set: { name: 'sales_perms', label: 'Sales', objects, fields: {} },
objectNames: opts.objectNames ?? Object.keys(objects),
saved: null,
savedOpts: undefined,
};
clientImpl = makeClient(server);
render(
<MemoryRouter>
<PermissionMatrixEditPage type="permission" name="sales_perms" packageId={opts.packageId} />
</MemoryRouter>,
);
await screen.findByText('Sales');
return server;
}

/** The bulk button (`R` / `CRUD` / `All` / `None`) inside one object's row. */
function bulkButton(objectName: string, label: string) {
const row = screen
.getAllByRole('row')
.find((r) => within(r).queryByText(objectName) != null);
expect(row, `row for ${objectName}`).toBeTruthy();
return within(row!).getByRole('button', { name: new RegExp(`^${label}$`) });
}

/** Click Save and return the payload the client was handed. */
async function save(server: FakeServer) {
fireEvent.click(screen.getByRole('button', { name: /^Save$/ }));
await waitFor(() => expect(server.saved).not.toBeNull());
return server.saved!;
}

describe('PermissionMatrixEditor · bulk buttons vs unmodelled keys (objectui#6605)', () => {
it('"All" — the widening shape — grants every column AND the saved row keeps readScope / writeScope / allowExport', async () => {
const server = await renderMatrix({
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
});

fireEvent.click(bulkButton('a_account', 'All'));
const payload = await save(server);

const row = payload.objects.a_account;
for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true);
// The narrowings survive the click that used to delete them. `readScope`
// is the load-bearing one: dropping `own` silently widened read access.
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('own');
expect(row.allowExport).toBe(true);
expect(Object.keys(row).sort()).toEqual(
[...MATRIX_KEYS, 'allowExport', 'readScope', 'writeScope'].sort(),
);
});

it('"CRUD" merges the unmodelled keys through but still RESETS the matrix columns outside its grant', async () => {
const server = await renderMatrix({
a_account: {
allowTransfer: true,
viewAllRecords: true,
modifyAllRecords: true,
allowExport: true,
readScope: 'own',
writeScope: 'unit',
},
});

fireEvent.click(bulkButton('a_account', 'CRUD'));
const payload = await save(server);

const row = payload.objects.a_account;
// Falsification direction: merge must not decay into "add" — a bulk CRUD
// after a wider grant still means exactly CRUD for the keys the matrix owns.
expect(Object.keys(row).sort()).toEqual(
['allowCreate', 'allowRead', 'allowEdit', 'allowDelete', 'allowExport', 'readScope', 'writeScope'].sort(),
);
expect('allowTransfer' in row).toBe(false);
expect('viewAllRecords' in row).toBe(false);
expect('modifyAllRecords' in row).toBe(false);
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('unit');
expect(row.allowExport).toBe(true);
});

it('"R" saves a read-only row that still carries the unmodelled keys', async () => {
const server = await renderMatrix({
a_account: {
allowCreate: true,
allowEdit: true,
allowExport: true,
readScope: 'own',
writeScope: 'unit',
},
});

fireEvent.click(bulkButton('a_account', 'R'));
const payload = await save(server);

const row = payload.objects.a_account;
expect(Object.keys(row).sort()).toEqual(
['allowRead', 'allowExport', 'readScope', 'writeScope'].sort(),
);
expect(row.allowRead).toBe(true);
expect(row.readScope).toBe('own');
});

it('"None" still clears the WHOLE row — narrowings included (deliberate: the #6605 dispatch fence)', async () => {
// Read the header before "fixing" this pin: merging `none` would leave
// `allowExport: true` alive after a click on the button labelled "None".
const server = await renderMatrix({
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
a_contact: { allowRead: true, readScope: 'own' },
});

fireEvent.click(bulkButton('a_account', 'None'));
const payload = await save(server);

expect(payload.objects.a_account).toEqual({});
// Positive control in the same query shape: the untouched sibling row in
// the SAME saved payload still carries its narrowing, so the emptiness
// above is a measurement of `none`, not of a save path that drops keys.
expect(payload.objects.a_contact).toEqual({ allowRead: true, readScope: 'own' });
});

it('package door: the merged slice keeps the unmodelled keys after "All", and other packages\' rows survive byte-for-byte', async () => {
// In-scope: a_account (this package's row, carrying the narrowings).
// Out-of-scope: b_order — another package's contribution, not listed by
// this package, which `mergePermissionSlice` must copy verbatim from base.
const server = await renderMatrix(
{
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
b_order: { allowRead: true, viewAllRecords: true, readScope: 'unit' },
},
{ objectNames: ['a_account'], packageId: 'app.a' },
);

fireEvent.click(bulkButton('a_account', 'All'));
const payload = await save(server);

const row = payload.objects.a_account;
for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true);
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('own');
expect(row.allowExport).toBe(true);
// The other package's row is untouched — and the save went through the
// package door (a draft write), not a live record write.
expect(payload.objects.b_order).toEqual({ allowRead: true, viewAllRecords: true, readScope: 'unit' });
expect(server.savedOpts).toMatchObject({ mode: 'draft', packageId: 'app.a' });
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -638,14 +638,39 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved,

function bulkSetObject(objectName: string, action: 'all' | 'none' | 'crud' | 'read') {
setDraft((prev) => {
const next: ObjectPerm =
action === 'none'
? {}
: action === 'all'
? Object.fromEntries(OBJECT_ACTIONS.map((a) => [a.key, true])) as ObjectPerm
// `none` REPLACES the row with `{}` — deliberately, unlike the three
// granting arms below (#6605). The defect those arms had was a GRANT
// that silently dropped a narrowing: "All" deleting a `readScope: 'own'`
// widens effective read access with no diff and no error. `none` grants
// nothing, so nothing survives for a scope to narrow; merging here would
// instead leave `allowExport: true` (and the scopes) alive after a click
// on the button labelled "None" — a permissive outcome that does not
// exist today. What an admin's "None" means is a behaviour decision, not
// a mechanical merge; pinned by
// `PermissionMatrixEditor.bulkMergeKeys.test.tsx`.
if (action === 'none') {
return { ...prev, objects: { ...prev.objects, [objectName]: {} } };
}
// The granting arms MERGE (#6605): start from the current row, reset the
// keys this matrix authors (`OBJECT_ACTIONS`), then set the granted
// ones. Keys the matrix does not model — `allowExport`, `readScope`,
// `writeScope`, anything an older or newer editor wrote — ride through
// exactly as they do on the per-checkbox path (`updateObjectPerm`'s
// spread). Replacing the row wholesale is what silently deleted them,
// and both save doors persist the row as-is: the environment door writes
// the whole record, and at package scope `mergePermissionSlice` takes
// in-scope rows entirely from `edited` (ADR-0086 P0), so `base` cannot
// restore what a bulk click dropped.
const cur = prev.objects[objectName] ?? {};
const next: ObjectPerm = { ...cur };
for (const a of OBJECT_ACTIONS) delete next[a.key];
const grants: Array<keyof ObjectPerm> =
action === 'all'
? OBJECT_ACTIONS.map((a) => a.key)
: action === 'crud'
? { allowCreate: true, allowRead: true, allowEdit: true, allowDelete: true }
: { allowRead: true };
? ['allowCreate', 'allowRead', 'allowEdit', 'allowDelete']
: ['allowRead'];
for (const key of grants) next[key] = true;
return {
...prev,
objects: { ...prev.objects, [objectName]: next },
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changeset/6605-permission-bulk-merge-not-replace.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
---
'@object-ui/app-shell': patch
---

Permission matrix bulk buttons (R / CRUD / All) now merge into the object's
permission row instead of replacing it, so spec-declared keys the matrix does
not author — `allowExport` and the ADR-0057 access-depth axis `readScope` /
`writeScope` — survive a bulk click the same way they already survived the
per-checkbox path. Previously one click on any bulk button silently dropped
them from the saved row, and the **All** button could widen effective read
access by deleting a `readScope: 'own'` narrowing with no diff and no error.
**None** deliberately keeps clearing the whole row, narrowings included:
merging there would leave `allowExport: true` alive after a click on the
button labelled None (objectui#6605).
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* objectui#6605 — the bulk buttons (R / CRUD / All / None) and the keys the
* matrix does not author.
*
* `bulkSetObject` used to REPLACE the object's permission row. Three
* spec-declared keys are modelled by neither the local `ObjectPerm` interface
* nor the `OBJECT_ACTIONS` column list — `allowExport`, and the ADR-0057
* access-depth axis `readScope` / `writeScope` — so one bulk click dropped
* whatever those held. The sharpest shape is the button labelled **All**: an
* admin clicking what reads as "grant everything" could WIDEN effective read
* access by deleting a `readScope: 'own'` narrowing, with no diff shown and no
* error.
*
* Every pin here asserts the SAVED payload, never editor state. That is the
* card's own argument for why the defect persisted: both save doors carry the
* row as-is — the environment door writes the whole record, and at package
* scope `mergePermissionSlice` takes in-scope rows entirely from `edited`
* (ADR-0086 P0), so `base` cannot restore what a bulk click dropped. An
* editor-state assertion would prove nothing about either door.
*
* ## `none` is pinned to keep REPLACING — that is the fix's fence, not a gap
*
* The dispatch on #6605 deliberately rejects the card's "`none` needs the same
* treatment" suggestion. The defect is a GRANT that silently drops a
* narrowing; `none` grants nothing, so nothing survives for a scope to
* narrow. Merging `none` would instead leave `allowExport: true` (and the
* scopes) alive after a click on the button labelled "None" — a permissive
* outcome that does not exist today, on a surface whose whole problem is
* silent permissiveness. What an admin's "None" means is a behaviour
* decision, made on #6605, not a mechanical merge. The `none` pin below makes
* that fence mechanical: a refactor that quietly adopts the card's suggestion
* goes red here and needs a maintainer decision, not a cleanup commit.
*/

import { describe, it, expect, vi, afterEach } from 'vitest';
import { render, screen, cleanup, fireEvent, waitFor, within } from '@testing-library/react';
import { MemoryRouter } from 'react-router-dom';

/** Every object-permission key the matrix authors, in column order. */
const MATRIX_KEYS = [
'allowCreate',
'allowRead',
'allowEdit',
'allowDelete',
'allowTransfer',
'viewAllRecords',
'modifyAllRecords',
];

interface FakeServer {
/** The stored record — what `layered()` answers (also the merge `base`). */
set: Record<string, any>;
/** What `list('object')` lists — the matrix rows (and, under a packageId, the slice scope). */
objectNames: string[];
saved: Record<string, any> | null;
savedOpts: Record<string, any> | undefined;
}

function makeClient(server: FakeServer) {
return {
layered: async () => ({ effective: server.set, code: null, overlay: null, overlayScope: null }),
getDraft: async () => null,
list: async (type: string) =>
type === 'object' ? server.objectNames.map((name) => ({ item: { name } })) : [],
get: async (type: string) => (type === 'object' ? { fields: [] } : null),
save: async (
_t: string,
_n: string,
payload: Record<string, any>,
opts?: Record<string, any>,
) => {
server.saved = payload;
server.savedOpts = opts;
return payload;
},
} as any;
}

let clientImpl: any;

vi.mock('./useMetadata', () => ({
useMetadataClient: () => clientImpl,
useMetadataTypes: () => ({
loading: false,
error: null,
entries: [{ type: 'permission', label: 'Permission', allowOrgOverride: true }],
}),
}));
vi.mock('./AssignedUsersSection', () => ({ AssignedUsersSection: () => null }));
vi.mock('@object-ui/fields', () => ({
CapabilityMultiSelectField: () => <div data-testid="cap-picker" />,
parseCapabilityNames: (v: unknown) => (typeof v === 'string' ? JSON.parse(v) : []),
}));

import { PermissionMatrixEditPage } from './PermissionMatrixEditor';

afterEach(cleanup);

async function renderMatrix(
objects: Record<string, unknown>,
opts: { objectNames?: string[]; packageId?: string } = {},
): Promise<FakeServer> {
const server: FakeServer = {
set: { name: 'sales_perms', label: 'Sales', objects, fields: {} },
objectNames: opts.objectNames ?? Object.keys(objects),
saved: null,
savedOpts: undefined,
};
clientImpl = makeClient(server);
render(
<MemoryRouter>
<PermissionMatrixEditPage type="permission" name="sales_perms" packageId={opts.packageId} />
</MemoryRouter>,
);
await screen.findByText('Sales');
return server;
}

/** The bulk button (`R` / `CRUD` / `All` / `None`) inside one object's row. */
function bulkButton(objectName: string, label: string) {
const row = screen
.getAllByRole('row')
.find((r) => within(r).queryByText(objectName) != null);
expect(row, `row for ${objectName}`).toBeTruthy();
return within(row!).getByRole('button', { name: new RegExp(`^${label}$`) });
}

/** Click Save and return the payload the client was handed. */
async function save(server: FakeServer) {
fireEvent.click(screen.getByRole('button', { name: /^Save$/ }));
await waitFor(() => expect(server.saved).not.toBeNull());
return server.saved!;
}

describe('PermissionMatrixEditor · bulk buttons vs unmodelled keys (objectui#6605)', () => {
it('"All" — the widening shape — grants every column AND the saved row keeps readScope / writeScope / allowExport', async () => {
const server = await renderMatrix({
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
});

fireEvent.click(bulkButton('a_account', 'All'));
const payload = await save(server);

const row = payload.objects.a_account;
for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true);
// The narrowings survive the click that used to delete them. `readScope`
// is the load-bearing one: dropping `own` silently widened read access.
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('own');
expect(row.allowExport).toBe(true);
expect(Object.keys(row).sort()).toEqual(
[...MATRIX_KEYS, 'allowExport', 'readScope', 'writeScope'].sort(),
);
});

it('"CRUD" merges the unmodelled keys through but still RESETS the matrix columns outside its grant', async () => {
const server = await renderMatrix({
a_account: {
allowTransfer: true,
viewAllRecords: true,
modifyAllRecords: true,
allowExport: true,
readScope: 'own',
writeScope: 'unit',
},
});

fireEvent.click(bulkButton('a_account', 'CRUD'));
const payload = await save(server);

const row = payload.objects.a_account;
// Falsification direction: merge must not decay into "add" — a bulk CRUD
// after a wider grant still means exactly CRUD for the keys the matrix owns.
expect(Object.keys(row).sort()).toEqual(
['allowCreate', 'allowRead', 'allowEdit', 'allowDelete', 'allowExport', 'readScope', 'writeScope'].sort(),
);
expect('allowTransfer' in row).toBe(false);
expect('viewAllRecords' in row).toBe(false);
expect('modifyAllRecords' in row).toBe(false);
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('unit');
expect(row.allowExport).toBe(true);
});

it('"R" saves a read-only row that still carries the unmodelled keys', async () => {
const server = await renderMatrix({
a_account: {
allowCreate: true,
allowEdit: true,
allowExport: true,
readScope: 'own',
writeScope: 'unit',
},
});

fireEvent.click(bulkButton('a_account', 'R'));
const payload = await save(server);

const row = payload.objects.a_account;
expect(Object.keys(row).sort()).toEqual(
['allowRead', 'allowExport', 'readScope', 'writeScope'].sort(),
);
expect(row.allowRead).toBe(true);
expect(row.readScope).toBe('own');
});

it('"None" still clears the WHOLE row — narrowings included (deliberate: the #6605 dispatch fence)', async () => {
// Read the header before "fixing" this pin: merging `none` would leave
// `allowExport: true` alive after a click on the button labelled "None".
const server = await renderMatrix({
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
a_contact: { allowRead: true, readScope: 'own' },
});

fireEvent.click(bulkButton('a_account', 'None'));
const payload = await save(server);

expect(payload.objects.a_account).toEqual({});
// Positive control in the same query shape: the untouched sibling row in
// the SAME saved payload still carries its narrowing, so the emptiness
// above is a measurement of `none`, not of a save path that drops keys.
expect(payload.objects.a_contact).toEqual({ allowRead: true, readScope: 'own' });
});

it('package door: the merged slice keeps the unmodelled keys after "All", and other packages\' rows survive byte-for-byte', async () => {
// In-scope: a_account (this package's row, carrying the narrowings).
// Out-of-scope: b_order — another package's contribution, not listed by
// this package, which `mergePermissionSlice` must copy verbatim from base.
const server = await renderMatrix(
{
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
b_order: { allowRead: true, viewAllRecords: true, readScope: 'unit' },
},
{ objectNames: ['a_account'], packageId: 'app.a' },
);

fireEvent.click(bulkButton('a_account', 'All'));
const payload = await save(server);

const row = payload.objects.a_account;
for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true);
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('own');
expect(row.allowExport).toBe(true);
// The other package's row is untouched — and the save went through the
// package door (a draft write), not a live record write.
expect(payload.objects.b_order).toEqual({ allowRead: true, viewAllRecords: true, readScope: 'unit' });
expect(server.savedOpts).toMatchObject({ mode: 'draft', packageId: 'app.a' });
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -638,14 +638,39 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved,

function bulkSetObject(objectName: string, action: 'all' | 'none' | 'crud' | 'read') {
setDraft((prev) => {
const next: ObjectPerm =
action === 'none'
? {}
: action === 'all'
? Object.fromEntries(OBJECT_ACTIONS.map((a) => [a.key, true])) as ObjectPerm
// `none` REPLACES the row with `{}` — deliberately, unlike the three
// granting arms below (#6605). The defect those arms had was a GRANT
// that silently dropped a narrowing: "All" deleting a `readScope: 'own'`
// widens effective read access with no diff and no error. `none` grants
// nothing, so nothing survives for a scope to narrow; merging here would
// instead leave `allowExport: true` (and the scopes) alive after a click
// on the button labelled "None" — a permissive outcome that does not
// exist today. What an admin's "None" means is a behaviour decision, not
// a mechanical merge; pinned by
// `PermissionMatrixEditor.bulkMergeKeys.test.tsx`.
if (action === 'none') {
return { ...prev, objects: { ...prev.objects, [objectName]: {} } };
}
// The granting arms MERGE (#6605): start from the current row, reset the
// keys this matrix authors (`OBJECT_ACTIONS`), then set the granted
// ones. Keys the matrix does not model — `allowExport`, `readScope`,
// `writeScope`, anything an older or newer editor wrote — ride through
// exactly as they do on the per-checkbox path (`updateObjectPerm`'s
// spread). Replacing the row wholesale is what silently deleted them,
// and both save doors persist the row as-is: the environment door writes
// the whole record, and at package scope `mergePermissionSlice` takes
// in-scope rows entirely from `edited` (ADR-0086 P0), so `base` cannot
// restore what a bulk click dropped.
const cur = prev.objects[objectName] ?? {};
const next: ObjectPerm = { ...cur };
for (const a of OBJECT_ACTIONS) delete next[a.key];
const grants: Array<keyof ObjectPerm> =
action === 'all'
? OBJECT_ACTIONS.map((a) => a.key)
: action === 'crud'
? { allowCreate: true, allowRead: true, allowEdit: true, allowDelete: true }
: { allowRead: true };
? ['allowCreate', 'allowRead', 'allowEdit', 'allowDelete']
: ['allowRead'];
for (const key of grants) next[key] = true;
return {
...prev,
objects: { ...prev.objects, [objectName]: next },
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changeset/6605-permission-bulk-merge-not-replace.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
---
'@object-ui/app-shell': patch
---

Permission matrix bulk buttons (R / CRUD / All) now merge into the object's
permission row instead of replacing it, so spec-declared keys the matrix does
not author — `allowExport` and the ADR-0057 access-depth axis `readScope` /
`writeScope` — survive a bulk click the same way they already survived the
per-checkbox path. Previously one click on any bulk button silently dropped
them from the saved row, and the **All** button could widen effective read
access by deleting a `readScope: 'own'` narrowing with no diff and no error.
**None** deliberately keeps clearing the whole row, narrowings included:
merging there would leave `allowExport: true` alive after a click on the
button labelled None (objectui#6605).
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* objectui#6605 — the bulk buttons (R / CRUD / All / None) and the keys the
* matrix does not author.
*
* `bulkSetObject` used to REPLACE the object's permission row. Three
* spec-declared keys are modelled by neither the local `ObjectPerm` interface
* nor the `OBJECT_ACTIONS` column list — `allowExport`, and the ADR-0057
* access-depth axis `readScope` / `writeScope` — so one bulk click dropped
* whatever those held. The sharpest shape is the button labelled **All**: an
* admin clicking what reads as "grant everything" could WIDEN effective read
* access by deleting a `readScope: 'own'` narrowing, with no diff shown and no
* error.
*
* Every pin here asserts the SAVED payload, never editor state. That is the
* card's own argument for why the defect persisted: both save doors carry the
* row as-is — the environment door writes the whole record, and at package
* scope `mergePermissionSlice` takes in-scope rows entirely from `edited`
* (ADR-0086 P0), so `base` cannot restore what a bulk click dropped. An
* editor-state assertion would prove nothing about either door.
*
* ## `none` is pinned to keep REPLACING — that is the fix's fence, not a gap
*
* The dispatch on #6605 deliberately rejects the card's "`none` needs the same
* treatment" suggestion. The defect is a GRANT that silently drops a
* narrowing; `none` grants nothing, so nothing survives for a scope to
* narrow. Merging `none` would instead leave `allowExport: true` (and the
* scopes) alive after a click on the button labelled "None" — a permissive
* outcome that does not exist today, on a surface whose whole problem is
* silent permissiveness. What an admin's "None" means is a behaviour
* decision, made on #6605, not a mechanical merge. The `none` pin below makes
* that fence mechanical: a refactor that quietly adopts the card's suggestion
* goes red here and needs a maintainer decision, not a cleanup commit.
*/

import { describe, it, expect, vi, afterEach } from 'vitest';
import { render, screen, cleanup, fireEvent, waitFor, within } from '@testing-library/react';
import { MemoryRouter } from 'react-router-dom';

/** Every object-permission key the matrix authors, in column order. */
const MATRIX_KEYS = [
'allowCreate',
'allowRead',
'allowEdit',
'allowDelete',
'allowTransfer',
'viewAllRecords',
'modifyAllRecords',
];

interface FakeServer {
/** The stored record — what `layered()` answers (also the merge `base`). */
set: Record<string, any>;
/** What `list('object')` lists — the matrix rows (and, under a packageId, the slice scope). */
objectNames: string[];
saved: Record<string, any> | null;
savedOpts: Record<string, any> | undefined;
}

function makeClient(server: FakeServer) {
return {
layered: async () => ({ effective: server.set, code: null, overlay: null, overlayScope: null }),
getDraft: async () => null,
list: async (type: string) =>
type === 'object' ? server.objectNames.map((name) => ({ item: { name } })) : [],
get: async (type: string) => (type === 'object' ? { fields: [] } : null),
save: async (
_t: string,
_n: string,
payload: Record<string, any>,
opts?: Record<string, any>,
) => {
server.saved = payload;
server.savedOpts = opts;
return payload;
},
} as any;
}

let clientImpl: any;

vi.mock('./useMetadata', () => ({
useMetadataClient: () => clientImpl,
useMetadataTypes: () => ({
loading: false,
error: null,
entries: [{ type: 'permission', label: 'Permission', allowOrgOverride: true }],
}),
}));
vi.mock('./AssignedUsersSection', () => ({ AssignedUsersSection: () => null }));
vi.mock('@object-ui/fields', () => ({
CapabilityMultiSelectField: () => <div data-testid="cap-picker" />,
parseCapabilityNames: (v: unknown) => (typeof v === 'string' ? JSON.parse(v) : []),
}));

import { PermissionMatrixEditPage } from './PermissionMatrixEditor';

afterEach(cleanup);

async function renderMatrix(
objects: Record<string, unknown>,
opts: { objectNames?: string[]; packageId?: string } = {},
): Promise<FakeServer> {
const server: FakeServer = {
set: { name: 'sales_perms', label: 'Sales', objects, fields: {} },
objectNames: opts.objectNames ?? Object.keys(objects),
saved: null,
savedOpts: undefined,
};
clientImpl = makeClient(server);
render(
<MemoryRouter>
<PermissionMatrixEditPage type="permission" name="sales_perms" packageId={opts.packageId} />
</MemoryRouter>,
);
await screen.findByText('Sales');
return server;
}

/** The bulk button (`R` / `CRUD` / `All` / `None`) inside one object's row. */
function bulkButton(objectName: string, label: string) {
const row = screen
.getAllByRole('row')
.find((r) => within(r).queryByText(objectName) != null);
expect(row, `row for ${objectName}`).toBeTruthy();
return within(row!).getByRole('button', { name: new RegExp(`^${label}$`) });
}

/** Click Save and return the payload the client was handed. */
async function save(server: FakeServer) {
fireEvent.click(screen.getByRole('button', { name: /^Save$/ }));
await waitFor(() => expect(server.saved).not.toBeNull());
return server.saved!;
}

describe('PermissionMatrixEditor · bulk buttons vs unmodelled keys (objectui#6605)', () => {
it('"All" — the widening shape — grants every column AND the saved row keeps readScope / writeScope / allowExport', async () => {
const server = await renderMatrix({
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
});

fireEvent.click(bulkButton('a_account', 'All'));
const payload = await save(server);

const row = payload.objects.a_account;
for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true);
// The narrowings survive the click that used to delete them. `readScope`
// is the load-bearing one: dropping `own` silently widened read access.
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('own');
expect(row.allowExport).toBe(true);
expect(Object.keys(row).sort()).toEqual(
[...MATRIX_KEYS, 'allowExport', 'readScope', 'writeScope'].sort(),
);
});

it('"CRUD" merges the unmodelled keys through but still RESETS the matrix columns outside its grant', async () => {
const server = await renderMatrix({
a_account: {
allowTransfer: true,
viewAllRecords: true,
modifyAllRecords: true,
allowExport: true,
readScope: 'own',
writeScope: 'unit',
},
});

fireEvent.click(bulkButton('a_account', 'CRUD'));
const payload = await save(server);

const row = payload.objects.a_account;
// Falsification direction: merge must not decay into "add" — a bulk CRUD
// after a wider grant still means exactly CRUD for the keys the matrix owns.
expect(Object.keys(row).sort()).toEqual(
['allowCreate', 'allowRead', 'allowEdit', 'allowDelete', 'allowExport', 'readScope', 'writeScope'].sort(),
);
expect('allowTransfer' in row).toBe(false);
expect('viewAllRecords' in row).toBe(false);
expect('modifyAllRecords' in row).toBe(false);
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('unit');
expect(row.allowExport).toBe(true);
});

it('"R" saves a read-only row that still carries the unmodelled keys', async () => {
const server = await renderMatrix({
a_account: {
allowCreate: true,
allowEdit: true,
allowExport: true,
readScope: 'own',
writeScope: 'unit',
},
});

fireEvent.click(bulkButton('a_account', 'R'));
const payload = await save(server);

const row = payload.objects.a_account;
expect(Object.keys(row).sort()).toEqual(
['allowRead', 'allowExport', 'readScope', 'writeScope'].sort(),
);
expect(row.allowRead).toBe(true);
expect(row.readScope).toBe('own');
});

it('"None" still clears the WHOLE row — narrowings included (deliberate: the #6605 dispatch fence)', async () => {
// Read the header before "fixing" this pin: merging `none` would leave
// `allowExport: true` alive after a click on the button labelled "None".
const server = await renderMatrix({
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
a_contact: { allowRead: true, readScope: 'own' },
});

fireEvent.click(bulkButton('a_account', 'None'));
const payload = await save(server);

expect(payload.objects.a_account).toEqual({});
// Positive control in the same query shape: the untouched sibling row in
// the SAME saved payload still carries its narrowing, so the emptiness
// above is a measurement of `none`, not of a save path that drops keys.
expect(payload.objects.a_contact).toEqual({ allowRead: true, readScope: 'own' });
});

it('package door: the merged slice keeps the unmodelled keys after "All", and other packages\' rows survive byte-for-byte', async () => {
// In-scope: a_account (this package's row, carrying the narrowings).
// Out-of-scope: b_order — another package's contribution, not listed by
// this package, which `mergePermissionSlice` must copy verbatim from base.
const server = await renderMatrix(
{
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
b_order: { allowRead: true, viewAllRecords: true, readScope: 'unit' },
},
{ objectNames: ['a_account'], packageId: 'app.a' },
);

fireEvent.click(bulkButton('a_account', 'All'));
const payload = await save(server);

const row = payload.objects.a_account;
for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true);
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('own');
expect(row.allowExport).toBe(true);
// The other package's row is untouched — and the save went through the
// package door (a draft write), not a live record write.
expect(payload.objects.b_order).toEqual({ allowRead: true, viewAllRecords: true, readScope: 'unit' });
expect(server.savedOpts).toMatchObject({ mode: 'draft', packageId: 'app.a' });
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -638,14 +638,39 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved,

function bulkSetObject(objectName: string, action: 'all' | 'none' | 'crud' | 'read') {
setDraft((prev) => {
const next: ObjectPerm =
action === 'none'
? {}
: action === 'all'
? Object.fromEntries(OBJECT_ACTIONS.map((a) => [a.key, true])) as ObjectPerm
// `none` REPLACES the row with `{}` — deliberately, unlike the three
// granting arms below (#6605). The defect those arms had was a GRANT
// that silently dropped a narrowing: "All" deleting a `readScope: 'own'`
// widens effective read access with no diff and no error. `none` grants
// nothing, so nothing survives for a scope to narrow; merging here would
// instead leave `allowExport: true` (and the scopes) alive after a click
// on the button labelled "None" — a permissive outcome that does not
// exist today. What an admin's "None" means is a behaviour decision, not
// a mechanical merge; pinned by
// `PermissionMatrixEditor.bulkMergeKeys.test.tsx`.
if (action === 'none') {
return { ...prev, objects: { ...prev.objects, [objectName]: {} } };
}
// The granting arms MERGE (#6605): start from the current row, reset the
// keys this matrix authors (`OBJECT_ACTIONS`), then set the granted
// ones. Keys the matrix does not model — `allowExport`, `readScope`,
// `writeScope`, anything an older or newer editor wrote — ride through
// exactly as they do on the per-checkbox path (`updateObjectPerm`'s
// spread). Replacing the row wholesale is what silently deleted them,
// and both save doors persist the row as-is: the environment door writes
// the whole record, and at package scope `mergePermissionSlice` takes
// in-scope rows entirely from `edited` (ADR-0086 P0), so `base` cannot
// restore what a bulk click dropped.
const cur = prev.objects[objectName] ?? {};
const next: ObjectPerm = { ...cur };
for (const a of OBJECT_ACTIONS) delete next[a.key];
const grants: Array<keyof ObjectPerm> =
action === 'all'
? OBJECT_ACTIONS.map((a) => a.key)
: action === 'crud'
? { allowCreate: true, allowRead: true, allowEdit: true, allowDelete: true }
: { allowRead: true };
? ['allowCreate', 'allowRead', 'allowEdit', 'allowDelete']
: ['allowRead'];
for (const key of grants) next[key] = true;
return {
...prev,
objects: { ...prev.objects, [objectName]: next },
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changeset/6605-permission-bulk-merge-not-replace.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
---
'@object-ui/app-shell': patch
---

Permission matrix bulk buttons (R / CRUD / All) now merge into the object's
permission row instead of replacing it, so spec-declared keys the matrix does
not author — `allowExport` and the ADR-0057 access-depth axis `readScope` /
`writeScope` — survive a bulk click the same way they already survived the
per-checkbox path. Previously one click on any bulk button silently dropped
them from the saved row, and the **All** button could widen effective read
access by deleting a `readScope: 'own'` narrowing with no diff and no error.
**None** deliberately keeps clearing the whole row, narrowings included:
merging there would leave `allowExport: true` alive after a click on the
button labelled None (objectui#6605).
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* objectui#6605 — the bulk buttons (R / CRUD / All / None) and the keys the
* matrix does not author.
*
* `bulkSetObject` used to REPLACE the object's permission row. Three
* spec-declared keys are modelled by neither the local `ObjectPerm` interface
* nor the `OBJECT_ACTIONS` column list — `allowExport`, and the ADR-0057
* access-depth axis `readScope` / `writeScope` — so one bulk click dropped
* whatever those held. The sharpest shape is the button labelled **All**: an
* admin clicking what reads as "grant everything" could WIDEN effective read
* access by deleting a `readScope: 'own'` narrowing, with no diff shown and no
* error.
*
* Every pin here asserts the SAVED payload, never editor state. That is the
* card's own argument for why the defect persisted: both save doors carry the
* row as-is — the environment door writes the whole record, and at package
* scope `mergePermissionSlice` takes in-scope rows entirely from `edited`
* (ADR-0086 P0), so `base` cannot restore what a bulk click dropped. An
* editor-state assertion would prove nothing about either door.
*
* ## `none` is pinned to keep REPLACING — that is the fix's fence, not a gap
*
* The dispatch on #6605 deliberately rejects the card's "`none` needs the same
* treatment" suggestion. The defect is a GRANT that silently drops a
* narrowing; `none` grants nothing, so nothing survives for a scope to
* narrow. Merging `none` would instead leave `allowExport: true` (and the
* scopes) alive after a click on the button labelled "None" — a permissive
* outcome that does not exist today, on a surface whose whole problem is
* silent permissiveness. What an admin's "None" means is a behaviour
* decision, made on #6605, not a mechanical merge. The `none` pin below makes
* that fence mechanical: a refactor that quietly adopts the card's suggestion
* goes red here and needs a maintainer decision, not a cleanup commit.
*/

import { describe, it, expect, vi, afterEach } from 'vitest';
import { render, screen, cleanup, fireEvent, waitFor, within } from '@testing-library/react';
import { MemoryRouter } from 'react-router-dom';

/** Every object-permission key the matrix authors, in column order. */
const MATRIX_KEYS = [
'allowCreate',
'allowRead',
'allowEdit',
'allowDelete',
'allowTransfer',
'viewAllRecords',
'modifyAllRecords',
];

interface FakeServer {
/** The stored record — what `layered()` answers (also the merge `base`). */
set: Record<string, any>;
/** What `list('object')` lists — the matrix rows (and, under a packageId, the slice scope). */
objectNames: string[];
saved: Record<string, any> | null;
savedOpts: Record<string, any> | undefined;
}

function makeClient(server: FakeServer) {
return {
layered: async () => ({ effective: server.set, code: null, overlay: null, overlayScope: null }),
getDraft: async () => null,
list: async (type: string) =>
type === 'object' ? server.objectNames.map((name) => ({ item: { name } })) : [],
get: async (type: string) => (type === 'object' ? { fields: [] } : null),
save: async (
_t: string,
_n: string,
payload: Record<string, any>,
opts?: Record<string, any>,
) => {
server.saved = payload;
server.savedOpts = opts;
return payload;
},
} as any;
}

let clientImpl: any;

vi.mock('./useMetadata', () => ({
useMetadataClient: () => clientImpl,
useMetadataTypes: () => ({
loading: false,
error: null,
entries: [{ type: 'permission', label: 'Permission', allowOrgOverride: true }],
}),
}));
vi.mock('./AssignedUsersSection', () => ({ AssignedUsersSection: () => null }));
vi.mock('@object-ui/fields', () => ({
CapabilityMultiSelectField: () => <div data-testid="cap-picker" />,
parseCapabilityNames: (v: unknown) => (typeof v === 'string' ? JSON.parse(v) : []),
}));

import { PermissionMatrixEditPage } from './PermissionMatrixEditor';

afterEach(cleanup);

async function renderMatrix(
objects: Record<string, unknown>,
opts: { objectNames?: string[]; packageId?: string } = {},
): Promise<FakeServer> {
const server: FakeServer = {
set: { name: 'sales_perms', label: 'Sales', objects, fields: {} },
objectNames: opts.objectNames ?? Object.keys(objects),
saved: null,
savedOpts: undefined,
};
clientImpl = makeClient(server);
render(
<MemoryRouter>
<PermissionMatrixEditPage type="permission" name="sales_perms" packageId={opts.packageId} />
</MemoryRouter>,
);
await screen.findByText('Sales');
return server;
}

/** The bulk button (`R` / `CRUD` / `All` / `None`) inside one object's row. */
function bulkButton(objectName: string, label: string) {
const row = screen
.getAllByRole('row')
.find((r) => within(r).queryByText(objectName) != null);
expect(row, `row for ${objectName}`).toBeTruthy();
return within(row!).getByRole('button', { name: new RegExp(`^${label}$`) });
}

/** Click Save and return the payload the client was handed. */
async function save(server: FakeServer) {
fireEvent.click(screen.getByRole('button', { name: /^Save$/ }));
await waitFor(() => expect(server.saved).not.toBeNull());
return server.saved!;
}

describe('PermissionMatrixEditor · bulk buttons vs unmodelled keys (objectui#6605)', () => {
it('"All" — the widening shape — grants every column AND the saved row keeps readScope / writeScope / allowExport', async () => {
const server = await renderMatrix({
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
});

fireEvent.click(bulkButton('a_account', 'All'));
const payload = await save(server);

const row = payload.objects.a_account;
for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true);
// The narrowings survive the click that used to delete them. `readScope`
// is the load-bearing one: dropping `own` silently widened read access.
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('own');
expect(row.allowExport).toBe(true);
expect(Object.keys(row).sort()).toEqual(
[...MATRIX_KEYS, 'allowExport', 'readScope', 'writeScope'].sort(),
);
});

it('"CRUD" merges the unmodelled keys through but still RESETS the matrix columns outside its grant', async () => {
const server = await renderMatrix({
a_account: {
allowTransfer: true,
viewAllRecords: true,
modifyAllRecords: true,
allowExport: true,
readScope: 'own',
writeScope: 'unit',
},
});

fireEvent.click(bulkButton('a_account', 'CRUD'));
const payload = await save(server);

const row = payload.objects.a_account;
// Falsification direction: merge must not decay into "add" — a bulk CRUD
// after a wider grant still means exactly CRUD for the keys the matrix owns.
expect(Object.keys(row).sort()).toEqual(
['allowCreate', 'allowRead', 'allowEdit', 'allowDelete', 'allowExport', 'readScope', 'writeScope'].sort(),
);
expect('allowTransfer' in row).toBe(false);
expect('viewAllRecords' in row).toBe(false);
expect('modifyAllRecords' in row).toBe(false);
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('unit');
expect(row.allowExport).toBe(true);
});

it('"R" saves a read-only row that still carries the unmodelled keys', async () => {
const server = await renderMatrix({
a_account: {
allowCreate: true,
allowEdit: true,
allowExport: true,
readScope: 'own',
writeScope: 'unit',
},
});

fireEvent.click(bulkButton('a_account', 'R'));
const payload = await save(server);

const row = payload.objects.a_account;
expect(Object.keys(row).sort()).toEqual(
['allowRead', 'allowExport', 'readScope', 'writeScope'].sort(),
);
expect(row.allowRead).toBe(true);
expect(row.readScope).toBe('own');
});

it('"None" still clears the WHOLE row — narrowings included (deliberate: the #6605 dispatch fence)', async () => {
// Read the header before "fixing" this pin: merging `none` would leave
// `allowExport: true` alive after a click on the button labelled "None".
const server = await renderMatrix({
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
a_contact: { allowRead: true, readScope: 'own' },
});

fireEvent.click(bulkButton('a_account', 'None'));
const payload = await save(server);

expect(payload.objects.a_account).toEqual({});
// Positive control in the same query shape: the untouched sibling row in
// the SAME saved payload still carries its narrowing, so the emptiness
// above is a measurement of `none`, not of a save path that drops keys.
expect(payload.objects.a_contact).toEqual({ allowRead: true, readScope: 'own' });
});

it('package door: the merged slice keeps the unmodelled keys after "All", and other packages\' rows survive byte-for-byte', async () => {
// In-scope: a_account (this package's row, carrying the narrowings).
// Out-of-scope: b_order — another package's contribution, not listed by
// this package, which `mergePermissionSlice` must copy verbatim from base.
const server = await renderMatrix(
{
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
b_order: { allowRead: true, viewAllRecords: true, readScope: 'unit' },
},
{ objectNames: ['a_account'], packageId: 'app.a' },
);

fireEvent.click(bulkButton('a_account', 'All'));
const payload = await save(server);

const row = payload.objects.a_account;
for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true);
expect(row.readScope).toBe('own');
expect(row.writeScope).toBe('own');
expect(row.allowExport).toBe(true);
// The other package's row is untouched — and the save went through the
// package door (a draft write), not a live record write.
expect(payload.objects.b_order).toEqual({ allowRead: true, viewAllRecords: true, readScope: 'unit' });
expect(server.savedOpts).toMatchObject({ mode: 'draft', packageId: 'app.a' });
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -638,14 +638,39 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved,

function bulkSetObject(objectName: string, action: 'all' | 'none' | 'crud' | 'read') {
setDraft((prev) => {
const next: ObjectPerm =
action === 'none'
? {}
: action === 'all'
? Object.fromEntries(OBJECT_ACTIONS.map((a) => [a.key, true])) as ObjectPerm
// `none` REPLACES the row with `{}` — deliberately, unlike the three
// granting arms below (#6605). The defect those arms had was a GRANT
// that silently dropped a narrowing: "All" deleting a `readScope: 'own'`
// widens effective read access with no diff and no error. `none` grants
// nothing, so nothing survives for a scope to narrow; merging here would
// instead leave `allowExport: true` (and the scopes) alive after a click
// on the button labelled "None" — a permissive outcome that does not
// exist today. What an admin's "None" means is a behaviour decision, not
// a mechanical merge; pinned by
// `PermissionMatrixEditor.bulkMergeKeys.test.tsx`.
if (action === 'none') {
return { ...prev, objects: { ...prev.objects, [objectName]: {} } };
}
// The granting arms MERGE (#6605): start from the current row, reset the
// keys this matrix authors (`OBJECT_ACTIONS`), then set the granted
// ones. Keys the matrix does not model — `allowExport`, `readScope`,
// `writeScope`, anything an older or newer editor wrote — ride through
// exactly as they do on the per-checkbox path (`updateObjectPerm`'s
// spread). Replacing the row wholesale is what silently deleted them,
// and both save doors persist the row as-is: the environment door writes
// the whole record, and at package scope `mergePermissionSlice` takes
// in-scope rows entirely from `edited` (ADR-0086 P0), so `base` cannot
// restore what a bulk click dropped.
const cur = prev.objects[objectName] ?? {};
const next: ObjectPerm = { ...cur };
for (const a of OBJECT_ACTIONS) delete next[a.key];
const grants: Array<keyof ObjectPerm> =
action === 'all'
? OBJECT_ACTIONS.map((a) => a.key)
: action === 'crud'
? { allowCreate: true, allowRead: true, allowEdit: true, allowDelete: true }
: { allowRead: true };
? ['allowCreate', 'allowRead', 'allowEdit', 'allowDelete']
: ['allowRead'];
for (const key of grants) next[key] = true;
return {
...prev,
objects: { ...prev.objects, [objectName]: next },
Expand Down
Loading