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
12 changes: 12 additions & 0 deletions .changeset/7611-grant-by-name.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
'@object-ui/app-shell': minor
---

Setup's "Assigned users" on a permission set writes a new grant by the set's name alone (objectui#7611, after objectstack#22863, ADR-0131 stage 6b-2-pre).

- **Add.** The section creates `sys_user_permission_set` with `{ user_id, permission_set }`, where `permission_set` is the set's machine name. It no longer reads the set's `sys_permission_set` row and no longer writes `permission_set_id`, which no writer fills now. A set with no catalog row, such as a platform or package set on a database the platform no longer seeds rows into, is granted like any other; before, the section refused it with "This set has no catalog row yet" and wrote nothing.
- **A name the server's catalog does not hold** is refused by the server with `400` (`reference_not_found` at `permission_set`), and the section shows the server's message.
- **Remove** is unchanged: it deletes the direct grant's own row.
- **Server floor.** A server from before objectstack#22863 still requires `permission_set_id` on a grant, so there the add is refused with `400` and the section shows that refusal.

Clause-②: no. No export, prop or locale key changes; the two removed strings were the section's own copy.
2 changes: 1 addition & 1 deletion .changeset/7611-setup-catalog-registry.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ The Setup catalog for positions and permission sets reads the registry. It is bu
- **The environment scope (`?scope=environment`).** `…/metadata/position` and `…/metadata/permission` with this parameter list every item the registry serves for the type, including the platform's own sets. Without it, the list shows one project package's items, as Studio does. Every link the pages emit keeps the scope: the item links, the create link and the editor's breadcrumb. `ENVIRONMENT_SCOPE_QUERY` is exported for hosts that route their own URLs onto it.
- **Gated for Setup.** A caller without `manage_metadata` gets no create affordance on the list and a read-only editor, and the page says why. Under `single`, the reason is that the platform administrator defines these items and the caller's organization assigns them. Under a wall, it is that the operator defines them for every organization. The generic editor and the permission-set editor now apply this caller gate wherever they render. The metadata door refuses that caller's save anyway; this states the refusal before the click.
- **The active switch.** The catalog list has an active column and an active/inactive filter. Both show the activation ledger's state (`sys_metadata_activation`), which is what the authorization resolver reads; an item with no ledger row is active. The switch writes through the ledger's door, `POST /api/v1/security/_activation/:type/:name` with `{ enabled }`, so switching a set or a position off takes it away from its holders, and switching it on gives it back. It reads and writes no catalog row, so the catalog row's own `active` column is neither shown nor changed. The door's refusals are shown in the server's own words. The audience anchors `everyone` and `guest`, which the door refuses to switch, show a disabled switch and the reason. The door is objectstack's ADR-0131 stage 2c.
- **Holders.** In the environment scope, the permission-set editor shows who holds the set under its definition, and the position editor shows who holds the position. A set's holders are read by name: grants come from the `permission_set` column, and the positions that distribute the set come from the registry's position definitions (`permissionSets`). They are no longer read from `sys_position_permission_set` or `sys_position` rows, so on a framework from before objectstack's ADR-0131 stage 1, where positions declare no `permissionSets`, a set lists no holders through a position. A new grant still carries `permission_set_id`, because the server does not accept a grant by name alone yet. A position is assigned by its name.
- **Holders.** In the environment scope, the permission-set editor shows who holds the set under its definition, and the position editor shows who holds the position. A set's holders are read by name: grants come from the `permission_set` column, and the positions that distribute the set come from the registry's position definitions (`permissionSets`). They are no longer read from `sys_position_permission_set` or `sys_position` rows, so on a framework from before objectstack's ADR-0131 stage 1, where positions declare no `permissionSets`, a set lists no holders through a position. A position is assigned by its name.
- **The sharing-rule recipient picker lists the `position` registry** through the console's metadata store. It no longer reads `sys_position` rows. Without a metadata store mounted, the `position` kind falls back to the plain text input.
- **The console's `system/roles`, `system/positions` and `system/permissions`** now go to the catalog list, not to the object pages.
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,7 @@ const asItems: Envelope = (rows) => ({ items: rows, total: rows.length });
* answers no positions).
*/
const DIRECTORY: Record<string, Record<string, unknown>[]> = {
sys_permission_set: [{ id: 'ps_1', name: 'showcase_contributor' }],
sys_user_permission_set: [
{ id: 'grant_1', permission_set_id: 'ps_1', permission_set: 'showcase_contributor', user_id: 'u_direct' },
],
sys_user_permission_set: [{ id: 'grant_1', permission_set: 'showcase_contributor', user_id: 'u_direct' }],
sys_user_position: [],
sys_user: [{ id: 'u_direct', name: 'Direct Dana', email: 'dana@example.com' }],
};
Expand Down Expand Up @@ -112,7 +109,7 @@ describe('AssignedUsersSection — the find() envelope it accepts (objectui#5945

it('does NOT read `records` — not a QueryResult member', async () => {
await renderThrough(asRecords);
// The permission set itself never resolved, so the section reports the
// The grant read's envelope was refused, so the section reports the
// empty state. Before the fix this rendered `Direct Dana`.
expect(screen.queryByText('Direct Dana')).toBeNull();
await waitFor(() =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,33 @@
// `permission_set` name column, and the positions that distribute the set are
// the registry's position definitions whose `permissionSets` names it
// (ADR-0131 D4) — never `sys_position_permission_set` / `sys_position` rows.
// A new grant is written by name alone, and the fixture is a database with no
// `sys_permission_set` row at all (objectstack ADR-0131 D10, stage 6b-2-pre).

import * as React from 'react';
import { describe, expect, it, vi } from 'vitest';
import { render, screen, waitFor } from '@testing-library/react';
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import { normaliseClientError } from '@object-ui/data-objectstack';
import { AssignedUsersSection } from './AssignedUsersSection';

vi.mock('@object-ui/react', async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
useAdapter: () => mockAdapter,
useMetadata: () => mockMetadataStore,
}));
vi.mock('@object-ui/fields', async (importOriginal) => ({
...(await importOriginal<typeof import('@object-ui/fields')>()),
RecordPickerDialog: () => null,
}));
// The picker stands in as one button, rendered while it is open, that selects
// `pickedUsers` — the same `onSelectRecords` hand-off the real dialog makes.
vi.mock('@object-ui/fields', async (importOriginal) => {
const { createElement } = await import('react');
return {
...(await importOriginal<typeof import('@object-ui/fields')>()),
RecordPickerDialog: (props: { open: boolean; onSelectRecords?: (records: unknown[]) => void }) =>
props.open
? createElement('button', { type: 'button', onClick: () => props.onSelectRecords?.(pickedUsers) }, 'pick')
: null,
};
});
const pickedUsers = [{ id: 'u_new', name: 'New Nora' }];

// The registry's position definitions — the binding is part of the definition.
const registryPositions = [
Expand All @@ -35,10 +47,7 @@ const registryPositions = [
const mockMetadataStore = { ensureType: vi.fn(async () => registryPositions) };

const data: Record<string, any[]> = {
sys_permission_set: [{ id: 'ps_1', name: 'showcase_contributor' }],
sys_user_permission_set: [
{ id: 'grant_1', permission_set_id: 'ps_1', permission_set: 'showcase_contributor', user_id: 'u_direct' },
],
sys_user_permission_set: [{ id: 'grant_1', permission_set: 'showcase_contributor', user_id: 'u_direct' }],
sys_user_position: [
{ id: 'up_1', user_id: 'u_held', position: 'contributor' },
// The direct grantee ALSO holds the position — must merge into one row.
Expand Down Expand Up @@ -117,3 +126,58 @@ describe('AssignedUsersSection — by name, against the registry (objectui#7611)
await waitFor(() => expect(screen.getByText(/No users assigned yet/)).toBeTruthy());
});
});

/** Open the picker and select `pickedUsers` (the stand-in above). */
async function addThroughPicker() {
await waitFor(() => expect(screen.getByText('Held Henry')).toBeTruthy());
fireEvent.click(screen.getByText('Add user'));
fireEvent.click(await screen.findByText('pick'));
}

describe('AssignedUsersSection — a grant by name, on a database with no catalog row (objectui#7611)', () => {
it('ADD writes { user_id, permission_set } alone and reads no sys_permission_set row', async () => {
mockAdapter.find.mockClear();
mockAdapter.create.mockClear();
render(<AssignedUsersSection permissionSetName="showcase_contributor" />);
await addThroughPicker();

await waitFor(() => expect(mockAdapter.create).toHaveBeenCalledTimes(1));
// Exactly these two keys: no legacy `permission_set_id`, whatever its value.
expect(mockAdapter.create).toHaveBeenCalledWith('sys_user_permission_set', {
user_id: 'u_new',
permission_set: 'showcase_contributor',
});
const reads = mockAdapter.find.mock.calls.map(([object]) => object);
expect(reads).not.toContain('sys_permission_set');
expect(screen.queryByRole('alert')).toBeNull();
});

it("shows the server's refusal of a name its catalog does not hold, in the server's words", async () => {
// The error as the adapter throws it: the client's error for a
// `400 VALIDATION_FAILED` with one `reference_not_found` entry at
// `permission_set`, through the adapter's own `normaliseClientError`.
const said = "Permission Set: no permission set is named 'showcase_contributor'.";
const clientError = Object.assign(new Error(said), {
code: 'VALIDATION_FAILED',
httpStatus: 400,
details: { fields: [{ field: 'permission_set', code: 'reference_not_found', message: said }] },
});
mockAdapter.create.mockClear();
mockAdapter.create.mockRejectedValueOnce(normaliseClientError(clientError));
render(<AssignedUsersSection permissionSetName="showcase_contributor" />);
await addThroughPicker();

const alert = await screen.findByRole('alert');
expect(alert.textContent).toBe(said);
});

it('REMOVE deletes the direct grant by its row id', async () => {
mockAdapter.delete.mockClear();
render(<AssignedUsersSection permissionSetName="showcase_contributor" />);
await waitFor(() => expect(screen.getByText('Held Henry')).toBeTruthy());
fireEvent.click(screen.getByLabelText('Remove'));

await waitFor(() => expect(mockAdapter.delete).toHaveBeenCalledTimes(1));
expect(mockAdapter.delete).toHaveBeenCalledWith('sys_user_permission_set', 'grant_1');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,28 @@
* rules on the junction insert (e.g. the AI-seat cap) are caught and shown as a
* friendly, localized inline message — not a raw developer error.
*
* ## By name, against the registry (objectui#7611, ADR-0131 D3/D4)
* ## By name, against the registry (objectui#7611, ADR-0131 D3/D4/D10)
*
* The set is addressed by its machine NAME throughout:
* The set is addressed by its machine NAME throughout, and no
* `sys_permission_set` row is read or written:
*
* - direct grants are the `sys_user_permission_set` rows whose `permission_set`
* column names the set — the by-name column the platform's grant readers
* key on (a grant with no name grants nothing through them);
* - ADD writes a grant by name alone, `{ user_id, permission_set }`. The set is
* the security catalog's definition of that name, so a set with no catalog
* row is granted like any other. The grant's legacy `permission_set_id`
* column is never written: no writer fills it and the server derives
* nothing from it (objectstack ADR-0131 D10, stage 6b-2-pre). A name the
* catalog does not hold is refused by the server (`400 VALIDATION_FAILED`,
* `reference_not_found` at `permission_set`), and the refusal is shown in
* the server's own words;
* - REMOVE deletes the direct grant's own row, by its id;
* - the positions that distribute the set are the registry's `position`
* definitions whose `permissionSets` list names it (ADR-0131 D4 — the
* binding is part of the position's definition), read through the console's
* metadata store, never `sys_position_permission_set` / `sys_position` rows.
*
* ⚠️ One row read remains, and only on ADD: the grant door still requires
* `permission_set_id` and refuses a grant that carries the name alone
* (measured on objectstack `main`: 400 `VALIDATION_FAILED` on
* `permission_set`). {@link resolveGrantRowId} is that read, pending the
* server half that accepts a grant by name.
*
* Permission-set-agnostic: every role gets the same UI, and the AI seat
* (`ai_seat`) is just one of them. The generic add-by-picker engine (spec
* RecordRelatedListProps.add) powers the capability; this is the polished
Expand Down Expand Up @@ -80,7 +84,6 @@ function useCopy() {
direct: '直授',
viaPosition: (p: string) => '经岗位 ' + p,
everyoneNote: '已绑定到 everyone 锚点 — 所有登录成员都持有此权限集。',
noGrantRow: '此权限集还没有目录行,服务端暂时无法按名称分配。',
positionHeldHint: '经岗位持有 — 在岗位的指派中移除。',
}
: {
Expand All @@ -97,7 +100,6 @@ function useCopy() {
direct: 'direct',
viaPosition: (p: string) => 'via position ' + p,
everyoneNote: 'Bound to the everyone anchor — every signed-in member holds this set.',
noGrantRow: 'This set has no catalog row yet, and the server does not accept a grant by name alone yet.',
positionHeldHint: 'Held via a position — remove it on the position’s assignments.',
},
[zh],
Expand Down Expand Up @@ -145,18 +147,6 @@ export function positionsDistributing(positions: readonly unknown[], setName: st
return out;
}

/**
* The `sys_permission_set` row id a NEW grant must carry — the one row read
* left here, pending the server half: the grant door requires
* `permission_set_id` and stamps the name from it (see the module doc).
*/
async function resolveGrantRowId(adapter: any, setName: string): Promise<string | null> {
const rows = asArray(
await adapter.find('sys_permission_set', { $filter: { name: setName }, $select: ['id'], $top: 1 }),
);
return rows[0]?.id != null ? String(rows[0].id) : null;
}

export function AssignedUsersSection({ permissionSetName }: AssignedUsersSectionProps) {
const adapter = useAdapter() as any;
const c = useCopy();
Expand Down Expand Up @@ -256,17 +246,14 @@ export function AssignedUsersSection({ permissionSetName }: AssignedUsersSection
setBusy(true);
setError(null);
try {
const setId = await resolveGrantRowId(adapter, permissionSetName);
if (!setId) throw new Error(c.noGrantRow);
for (const u of records || []) {
const uid = u?.id != null ? String(u.id) : null;
if (!uid || assignedIds.has(uid)) continue;
// The id is what the grant door requires today; the name rides
// beside it and the door refuses a mismatch (module doc).
// By name alone: no catalog row is read and no legacy
// `permission_set_id` is written (module doc).
await adapter.create('sys_user_permission_set', {
permission_set_id: setId,
permission_set: permissionSetName,
user_id: uid,
permission_set: permissionSetName,
});
}
await load();
Expand Down
Loading