Skip to content

Commit 7f0fb57

Browse files
committed
Merge branch 'main' into fix/incident-comment-feed-33112
Resolves one conflict in playwright/eslint-rules/tests/corpus.test.mjs. Both sides lowered the recorded no-positional-locator total from 1298: this branch to 1291 (the TaskComments.spec.ts burn-down of 7) and main to 1243 (55 from other burn-downs). Neither side alone is right, so take the combined total of 1236, which is what the merged eslint-suppressions.json actually sums to.
2 parents 4fb9389 + 42d1535 commit 7f0fb57

1,690 files changed

Lines changed: 131460 additions & 32106 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
---
2+
description: How UI code reads permissions — the useEntityPermissions hook, derived flags, and the ban on raw permission-key reads
3+
paths: "openmetadata-ui/src/main/resources/ui/**/*.{ts,tsx}"
4+
---
5+
6+
# Frontend permissions
7+
8+
Applies to UI `*.{ts,tsx}`. Concept, full flag table and rationale:
9+
`openmetadata-ui/src/main/resources/ui/docs/permissions.md`.
10+
11+
**One rule: never read a permission key directly. Ask for the intent.**
12+
13+
```ts
14+
// ✗ banned — `openmetadata-permissions/no-raw-permission-access` fails the build
15+
if (permissions.EditAll || permissions.EditDescription) { … }
16+
17+
// ✓ derived flag
18+
const { canEditDescription } = useEntityPermissions(ResourceEntity.TABLE, fqn);
19+
```
20+
21+
The rule guards `EditAll`, `ViewAll` and `ViewBasic` at **error**. Other keys are unguarded but
22+
the same rule applies — use the flag.
23+
24+
## Fetching
25+
26+
```ts
27+
const {
28+
canEditTags, hasViewAccess, /* …flags */
29+
permissions, // raw OperationPermission, for props that still require it
30+
isLoading, error, refresh,
31+
} = useEntityPermissions(resource, identifier, options);
32+
```
33+
34+
- `identifier` — an **fqn string**, or `{ id }` to look up by id.
35+
- `options``{ deleted?: boolean; enabled?: boolean }`; `enabled: false` skips the fetch when
36+
the fqn/id isn't known yet.
37+
38+
Results are cached in React Query under `permissionQueryKeys`, so components asking for the same
39+
entity share one request. `PermissionProvider` reads the same cache — mixing old and new code
40+
does not double-fetch.
41+
42+
**Always pass `deleted` when the entity can be soft-deleted.** Every `canEdit*` is gated on it;
43+
view and delete flags deliberately are not, since delete surfaces restore/purge.
44+
45+
> **Restore affordances need an ungated derivation.** Gating a manage button on a
46+
> `deleted`-aware flag removes the only path to restore a soft-deleted entity — derive a second,
47+
> ungated set for those controls (`DataAssetsHeader`'s `ungatedFlags` is the precedent).
48+
49+
## No named flag for the operation?
50+
51+
```ts
52+
getDerivedPermissionFlags(permissions, deleted).can(Operation.EditStatus);
53+
```
54+
55+
Same prioritization and `deleted` gating as the named flags.
56+
57+
## Already holding permissions
58+
59+
For a prop-supplied `OperationPermission` or a per-row bulk lookup, call the pure function
60+
instead of fetching:
61+
62+
```ts
63+
const flags = getDerivedPermissionFlags(permission ?? DEFAULT_ENTITY_PERMISSION, deleted);
64+
```
65+
66+
For lists, `useBulkEntityPermissions(resource, fqns)` returns `flagsByFqn` and shares the
67+
single-entity cache.
68+
69+
## Prioritization — explicit deny beats a broader grant
70+
71+
`canEditTags` is **not** `EditTags || EditAll`. When the payload carries the field key that key
72+
wins, including when it is `false`; `EditAll` is only the fallback when the key is absent. Do not
73+
"restore" the old OR — an explicit `EditTags: false` alongside `EditAll: true` must deny.
74+
75+
## Testing
76+
77+
Mock the **hook**, run the **real** derivation over a minimal permission object:
78+
79+
```ts
80+
mockUseEntityPermissions.mockReturnValue({
81+
permissions, isLoading: false, error: null, refresh: jest.fn(),
82+
...getDerivedPermissionFlags(permissions, false),
83+
});
84+
```
85+
86+
Components reaching these hooks need a `QueryClientProvider` — use `renderWithQueryClient`
87+
(`src/test/unit/test-utils.tsx`), or compose a wrapper when the test also needs a router.
88+
Without it React Query throws *"No QueryClient set"* and the whole suite fails to run.
89+
90+
## Changing behaviour
91+
92+
Behavioural decisions live in `src/utils/permissionPolicy.ts` — change them there, once, rather
93+
than at call sites.

.claude/rules/frontend-react.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ paths: "openmetadata-ui/src/main/resources/ui/**/*.{ts,tsx}"
66
# Frontend React/TypeScript conventions
77

88
Applies to UI `*.{ts,tsx}`. Styling/tokens are in `frontend-styling.md`; component-library choice in
9-
`component-library.md`; strings/i18n in `i18n.md`; Playwright in `frontend-playwright.md`. For the
9+
`component-library.md`; strings/i18n in `i18n.md`; Playwright in `frontend-playwright.md`;
10+
reading permissions in `frontend-permissions.md`. For the
1011
**formatting procedure** invoke the `ui-checkstyle` skill — do not hand-edit formatting.
1112
**Folder structure and file naming for new code:
1213
`openmetadata-ui/src/main/resources/ui/DEVELOPER_HANDBOOK.md`** — read it before creating any new

.github/CODEOWNERS

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
/openmetadata-service/ @open-metadata/backend
1414

1515
# Review from Ingestion owners for changes around Ingestion code
16-
/ingestion @open-metadata/ingestion @akashverma0786
16+
/ingestion @open-metadata/ingestion
1717

1818
# Review from Devops owners for changes around workflows
1919
/.github @akash-jain-10 @harshach @tutte @open-metadata/mergers

.github/playwright/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ SSO stays in its dedicated workflow, while knowledge graph and ontology share on
1717
ceil(total weighted worker time / (3 workers * 21 minutes * 0.85))
1818
```
1919

20-
The common matrix is bounded to 5–24 runners and uses a 21-minute allocation budget. Dedicated lanes use a 20-minute allocation budget. The common lane previously sat a minute below the dedicated lanes, but the chromium suite outgrew what 24 runners could hold at 19 minutes, so full-mode planning aborted outright; 21 minutes restores headroom while staying inside the 25-minute `timeout` wrapper around `npx playwright test` and the 35-minute `playwright-ci` job clock. Note that the allocation budget bounds a whole *shard*, whereas the strict 20-minute ceiling below bounds a single *atomic unit*, so the two are independent. Planner weights use the hook-inclusive observed duration, including retries. Only an exact stable test ID explicitly reported as skipped may retain a zero weight; every other zero-duration observation and every unseen test uses the conservative fallback, and zero weights never transfer through the file/title identity fallback. The versioned bootstrap baseline uses stable expected and skipped observations from the coverage-complete but failed full run `29984209316`, while unexpected and flaky tests retain their prior duration weights from run `29980474263`. It is bootstrap data, not a fabricated successful history; normal planning still uses p75 from the latest three successful full runs when those artifacts exist. This keeps expensive internally parallel suites together instead of multiplying their shared setup across runners. Serial/global behavior stays in one-worker lanes. Large suites listed in `AUDITED_PARALLEL_SUITES` are split at test granularity only after confirming that they are not serial and do not depend on earlier tests. The planner fails when any remaining atomic unit or bounded lane exceeds the 20-minute ceiling.
20+
The common matrix is bounded to 5–24 runners and uses a 21-minute allocation budget. Dedicated lanes use a 20-minute allocation budget. The common lane previously sat a minute below the dedicated lanes, but the chromium suite outgrew what 24 runners could hold at 19 minutes, so full-mode planning aborted outright; 21 minutes restores headroom while staying inside the `timeout` wrapper around `npx playwright test` (in `playwright-e2e-reusable.yml`) and the `playwright-ci` job clock. Note that the allocation budget bounds a whole *shard*, whereas the strict 20-minute ceiling below bounds a single *atomic unit*, so the two are independent. Planner weights use the hook-inclusive observed duration, including retries. Only an exact stable test ID explicitly reported as skipped may retain a zero weight; every other zero-duration observation and every unseen test uses the conservative fallback, and zero weights never transfer through the file/title identity fallback. The versioned bootstrap baseline uses stable expected and skipped observations from the coverage-complete but failed full run `29984209316`, while unexpected and flaky tests retain their prior duration weights from run `29980474263`. It is bootstrap data, not a fabricated successful history; normal planning still uses p75 from the latest three successful full runs when those artifacts exist. This keeps expensive internally parallel suites together instead of multiplying their shared setup across runners. Serial/global behavior stays in one-worker lanes. Large suites listed in `AUDITED_PARALLEL_SUITES` are split at test granularity only after confirming that they are not serial and do not depend on earlier tests. The planner fails when any remaining atomic unit or bounded lane exceeds the 20-minute ceiling.
2121

2222
The `Basic` and `chromium` projects share that common 24-runner cap and are balanced together; they are not separate pools of standard hosted runners. Isolated ingestion, search, reindex, permission, and global-state lanes are additional because they cannot safely share mutable server state with the common matrix.
2323

0 commit comments

Comments
 (0)