chore(settings): address review comments from PR 21072 (#21121)

## Summary

Round through bosiraphael's 31 review threads on the merged PR #21072
(discovery hero + ephemeral playground token). The user asked to apply
each suggestion only where it adds value, so this PR is split into three
buckets.

### Comments (~17 threads)

- Tightened security-rationale / CSS-gotcha / API-doc comments to one or
two factual lines
- Kept (shortened) the comments above `RequireAccessTokenGuard` call
sites — without them a future reader could remove the guard and silently
reopen the escalation hole
- Kept (shortened) the in-memory-only rationale on
`playgroundApiKeyState` for the same reason
- Kept `flex: 1 + min-height: 0` CSS gotcha on `SubMenuTopBarContainer`
— non-obvious and easy to break

### Structure / extraction

- Move `WEBHOOK_TABLE_ROW_GRID_TEMPLATE_COLUMNS` to its own constants
file (one-export-per-file)
- Split `SettingsAgentToolsTab` and `SettingsAgentToolsTable` across
queries/, hooks/, types/, utils/:
  - `graphql/queries/findManyApplicationsForToolTable.ts`
  - `graphql/queries/findManyMarketplaceAppsForToolTable.ts`
  - `hooks/useSettingsAgentToolsTable.ts` (data loading + index merging)
  - `types/SettingsAgentToolItem|Application|MarketplaceApp`
  - `utils/getToolApplicationId|getToolLink`
- Extract `SettingsAiModelsTab` optimistic mutations into
`hooks/useSettingsAiModelsActions` (handleModelFieldChange,
handleUseRecommendedToggle, handleModelToggle,
handleToggleAllVisibleModels)
- Extract `SettingsAI.handleCreateTool` into `hooks/useCreateTool`
- Drop unnecessary `useMemo` wrappers on `heroTabs` arrays
(SettingsObjects, SettingsLayout)
- Simplify `MenuItemToggle` handler in SettingsAgentSkillsTab:
`onToggleChange={setShowDeactivated}` (no longer wrapping with arrow +
read of stale `!showDeactivated`)

### Hero assets

- Replace placeholder `customize-illustration` with per-page exports
- Rename `layout/customize-illustration-{light,dark}.png` →
`layout/cover-{light,dark}.png`
- Add `cover-{light,dark}.png` for **applications** and **members**
(they were both pointing at the layout placeholder as a TODO)
- Overwrite `data-model/cover-*.png`, `playground/cover-*.png`,
`ai/ai-tools-cover-*.png` with the new exports

## Test plan

- [ ] `npx nx typecheck twenty-front` 
- [ ] `npx nx typecheck twenty-server` 
- [ ] `npx nx lint twenty-front`  (oxlint + oxfmt, 0 warnings/errors)
- [ ] `/settings/layout`, `/settings/data-model`,
`/settings/applications`, `/settings/ai`, `/settings/api-webhooks`,
`/settings/members` each render the new hero illustration (light + dark)
- [ ] AI tab: tool list still loads, search + Custom/Managed/Standard
filters still work, "New Tool" still navigates to detail
- [ ] AI tab: Models tab — smart/fast model select, "Use best models
only" toggle, per-model checkboxes, toggle-all all still
optimistic+revert on error
- [ ] Skills tab: "Deactivated" toggle still flips show/hide
- [ ] Webhooks table still uses the 1fr 28px grid
This commit is contained in:
Félix Malfait
2026-06-02 07:23:14 +02:00
committed by GitHub
parent d6b3527552
commit 75df1f3997
62 changed files with 523 additions and 528 deletions
@@ -944,11 +944,7 @@ describe('JwtAuthStrategy', () => {
});
describe('PLAYGROUND token validation', () => {
// A PLAYGROUND token is access-shaped but must never impersonate. This is the
// same payload that, as an ACCESS token with isImpersonating set but without
// impersonation ids, throws 'Invalid or missing user workspace ID in
// impersonation token' above; as a PLAYGROUND token it must skip the
// impersonation path entirely and resolve to the first-person context.
// PLAYGROUND tokens are access-shaped but must never impersonate.
it('ignores isImpersonating and resolves first-person', async () => {
const validUserId = 'valid-user-id';
const validUserWorkspaceId = randomUUID();
@@ -7,9 +7,8 @@ export type AppOAuthStateJwtPayload = CommonPropertiesJwtPayload & {
connectionProviderId: string;
userId: string;
userWorkspaceId: string;
// 'user' = the resulting credential is private to userWorkspaceId.
// 'workspace' = visible to anyone in the workspace.
// Named `visibility` to disambiguate from OAuth `scopes` on the row.
// 'user' scopes the credential to userWorkspaceId; 'workspace' makes it
// visible to anyone in the workspace.
visibility: 'user' | 'workspace';
// If set, the callback updates this existing connectedAccount row instead
// of creating a new one (used by the UI's "Reconnect" action).