Skip to content

Commit 880a39b

Browse files
committed
fix(playground): address review blockers on the secret changes
The docs pages also mount ItemVariableResolverProvider, so hardcoding withExternalSecrets there resolved external secrets in the docs too. It now follows the writable flag, and the unit test asserts through that provider rather than one the docs never use. A literal {{$secrets.foo}} provider reference lost its read-only note when the scope was dropped from getReadOnlyNote. The note now keys off whether the variable is editable, which separates a provider reference from a declared external secret without a second scope label. Rebuilding external secret rows in the environments editor dropped the session value written by the hover card. The merge moved to a tested helper in utils/environments. Replaced a vacuous playground assertion: the card portals to document.body so it is never inside playground-view, and no snippet is rendered there at all.
1 parent 949451a commit 880a39b

8 files changed

Lines changed: 102 additions & 16 deletions

File tree

packages/bruno-api-docs/e2e/tests/playground/playground-variables.spec.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,14 @@ test.describe('Playground variables: highlight, hover card and inline edit', ()
133133
await expect(playground.variable.value).toHaveText('*'.repeat('vault-value'.length));
134134
});
135135

136-
test('a typed secret never reaches the generated code snippet', async ({ playground }) => {
136+
test('a typed secret stays masked in the card until revealed', async ({ playground }) => {
137137
await playground.variable.hoverInputToken('unsetSecret');
138138
await playground.variable.editTo('typed-secret');
139139

140-
await expect(playground.view).not.toContainText('typed-secret');
140+
await expect(playground.variable.card).not.toContainText('typed-secret');
141+
142+
await playground.variable.revealToggle.click();
143+
144+
await expect(playground.variable.card).toContainText('typed-secret');
141145
});
142146
});

packages/bruno-api-docs/e2e/tests/variableInfoPopup/variableInfoPopup.spec.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,16 @@ test.describe('Variable hover card', () => {
7575
await expect(variableCard.copyButton).toHaveCount(0);
7676
});
7777

78+
// External secrets are fillable in the playground only; the docs must not resolve them.
79+
test('leaves an external secret undefined', async ({ requestPage }) => {
80+
const { variableCard } = requestPage;
81+
await variableCard.hoverToken('vaultKey');
82+
83+
await expect(variableCard.card).toBeVisible();
84+
await expect(variableCard.scopeBadge).toHaveText('Undefined');
85+
await expect(variableCard.note).toHaveText('Variable is not defined');
86+
});
87+
7888
test('shows an (empty) placeholder with no copy for a defined variable that has no value', async ({ requestPage }) => {
7989
const { variableCard } = requestPage;
8090
await variableCard.hoverToken('emptyValue');

packages/bruno-api-docs/src/components/Playground/Content/Views/EnvironmentsView/EnvironmentsView.tsx

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { variableTypeColumn } from '../Common/VariableTypeControl/variableTypeCo
1313
import { GlobeIcon } from '../../../../../assets/icons';
1414
import { useAppDispatch } from '../../../../../store/hooks';
1515
import { cx } from '../../../../../utils/cx';
16-
import { envVariableToRow, envRowToVariable } from '../../../../../utils/environments';
16+
import { envVariableToRow, envRowToVariable, mergeExternalSecretRows } from '../../../../../utils/environments';
1717
import { isSecretVariable } from '../../../../../utils/variableResolution';
1818
import { updateCollectionEnvironments } from '@slices/playground';
1919

@@ -100,11 +100,7 @@ const EnvironmentsView: React.FC<EnvironmentsViewProps> = ({ collection, compact
100100
applyToSelectedEnv({
101101
externalSecrets: {
102102
...(selectedEnvironment?.externalSecrets ?? {}),
103-
variables: rows.map((row) => ({
104-
name: row.name,
105-
[secretPointerField]: row.value,
106-
disabled: !row.enabled
107-
}))
103+
variables: mergeExternalSecretRows(selectedEnvironment?.externalSecrets?.variables, rows, secretPointerField)
108104
}
109105
});
110106

packages/bruno-api-docs/src/components/VariableInfoCard/VariableInfoCard.spec.tsx

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,21 @@ describe('VariableInfoCard', () => {
8989
expect(root.querySelector(selector('copy'))).toBeNull();
9090
});
9191

92-
// External secrets are a playground affordance; the docs leave them unresolved.
93-
it('does not resolve an external secret', () => {
92+
// PageRouter wraps the docs pages in ItemVariableResolverProvider without
93+
// `writable`, so that mount must leave external secrets unresolved too.
94+
it('does not resolve an external secret on either docs provider', () => {
9495
expect(part(useRenderToDom(cardTree('vaultKey')), 'scope').text).toBe('Undefined');
96+
97+
const store = createOpenCollectionStore();
98+
store.dispatch(setActiveEnv('Dev'));
99+
const docsItemTree = (
100+
<Provider store={store}>
101+
<ItemVariableResolverProvider collection={collection} ancestry={[]} item={null}>
102+
<VariableInfoCard name="vaultKey" />
103+
</ItemVariableResolverProvider>
104+
</Provider>
105+
);
106+
expect(part(useRenderToDom(docsItemTree), 'scope').text).toBe('Undefined');
95107
});
96108

97109
it('pretty-prints an object-typed value', () => {

packages/bruno-api-docs/src/components/VariableInfoCard/VariableInfoCard.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,14 @@ interface VariableInfoCardProps {
1717
testId?: string;
1818
}
1919

20-
const getReadOnlyNote = (scope: VariableScope, activeEnvName: string | null): string | null => {
20+
/**
21+
* `$secrets` covers two things: a literal `{{$secrets.x}}` provider reference,
22+
* which nothing here can resolve, and an environment's declared external secret,
23+
* which the playground can fill in. Only the latter is editable.
24+
*/
25+
const getReadOnlyNote = (scope: VariableScope, activeEnvName: string | null, canEdit: boolean): string | null => {
2126
if (scope === 'process.env' || scope === 'oauth2') return 'read-only';
27+
if (scope === '$secrets' && !canEdit) return 'read-only';
2228
if (scope === 'undefined') return activeEnvName ? 'Variable is not defined' : 'No active environment';
2329
return null;
2430
};
@@ -130,7 +136,7 @@ export const VariableInfoCard: React.FC<VariableInfoCardProps> = ({
130136
);
131137
}
132138

133-
const readOnlyNote = getReadOnlyNote(info.scope, activeEnvName);
139+
const readOnlyNote = getReadOnlyNote(info.scope, activeEnvName, canEdit);
134140
const emptyLabel = !secretFillable && info.value === '' ? '(empty)' : null;
135141
const placeholder = info.secret && !editable ? '(Secret)' : canEdit ? null : emptyLabel;
136142

packages/bruno-api-docs/src/hooks/useVariableResolver.tsx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ const externalSecretVariables = (environment: Environment | undefined): SecretVa
120120
.filter((entry) => entry.name && entry.disabled !== true)
121121
.map((entry) => ({ name: entry.name, secret: true, value: entry.value ?? '' }) as unknown as SecretVariable);
122122

123-
/** `withExternalSecrets` is playground-only; the docs leave them unresolved. */
123+
/** `withExternalSecrets` tracks the writable (playground) mount; docs leave them unresolved. */
124124
const collectionAndEnvSources = (
125125
collection: OpenCollection | null,
126126
activeEnvName: string | null,
@@ -211,14 +211,16 @@ export const ItemVariableResolverProvider: React.FC<{
211211
const activeEnvName = useAppSelector(selectActiveEnvName);
212212
const showVars = useAppSelector(selectShowVars);
213213

214+
// This provider also backs the rendered docs pages, which pass no `writable`.
215+
// Only the writable (playground) mount folds in external secrets.
214216
const model = useMemo(() => {
215-
const sources: VariableSource[] = collectionAndEnvSources(collection, activeEnvName, true);
217+
const sources: VariableSource[] = collectionAndEnvSources(collection, activeEnvName, writable);
216218
for (const folder of ancestry) {
217219
sources.push({ scope: 'folder', variables: folderVariables(folder) });
218220
}
219221
if (item) sources.push(itemSource(item));
220222
return buildScopedVariableModel(sources);
221-
}, [collection, activeEnvName, ancestry, item]);
223+
}, [collection, activeEnvName, ancestry, item, writable]);
222224

223225
const resolver = useMemo(() => makeResolver(model, showVars, activeEnvName), [model, showVars, activeEnvName]);
224226

packages/bruno-api-docs/src/utils/environments.spec.ts

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, it, expect } from 'vitest';
2-
import { getEnvironmentVariables, envVariableToRow, envRowToVariable } from './environments';
2+
import { getEnvironmentVariables, envVariableToRow, envRowToVariable, mergeExternalSecretRows } from './environments';
33

44
describe('getEnvironmentVariables', () => {
55
it('splits regular and secret variables', () => {
@@ -274,3 +274,40 @@ describe('envVariableToRow / envRowToVariable round-trip', () => {
274274
expect(out.value).toBe('new-token');
275275
});
276276
});
277+
278+
describe('mergeExternalSecretRows', () => {
279+
const existing = [
280+
{ name: 'vaultKey', secretName: 'prod/api-key', value: 'typed-this-session' },
281+
{ name: 'dbPassword', secretName: 'prod/db' }
282+
];
283+
284+
it('keeps a session value when another field on the row is edited', () => {
285+
const rows = [
286+
{ name: 'vaultKey', value: 'prod/api-key-renamed', enabled: true },
287+
{ name: 'dbPassword', value: 'prod/db', enabled: true }
288+
];
289+
290+
const out = mergeExternalSecretRows(existing, rows, 'secretName') as Record<string, string | boolean>[];
291+
292+
expect(out[0].value).toBe('typed-this-session');
293+
expect(out[0].secretName).toBe('prod/api-key-renamed');
294+
expect(out[1].value).toBeUndefined();
295+
});
296+
297+
it('carries the session value through a disable toggle', () => {
298+
const rows = [{ name: 'vaultKey', value: 'prod/api-key', enabled: false }];
299+
300+
const out = mergeExternalSecretRows(existing, rows, 'secretName') as Record<string, string | boolean>[];
301+
302+
expect(out[0].value).toBe('typed-this-session');
303+
expect(out[0].disabled).toBe(true);
304+
});
305+
306+
it('adds a brand new row with no carried fields', () => {
307+
const rows = [{ name: 'fresh', value: 'prod/fresh', enabled: true }];
308+
309+
const out = mergeExternalSecretRows(existing, rows, 'secretName') as Record<string, string | boolean>[];
310+
311+
expect(out).toEqual([{ name: 'fresh', secretName: 'prod/fresh', disabled: false }]);
312+
});
313+
});

packages/bruno-api-docs/src/utils/environments.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,25 @@ interface ExternalSecretsConfig {
7676
variables?: { name?: string; secretName?: string; enabled?: boolean; type?: VariableValueType }[];
7777
}
7878

79+
/**
80+
* Rebuild an environment's external secrets from edited rows, carrying over any
81+
* field the row model does not round-trip. The hover card writes a session
82+
* `value` onto these entries, and rebuilding a row from scratch would drop it.
83+
*/
84+
export const mergeExternalSecretRows = (
85+
existing: { name?: string }[] | undefined,
86+
rows: { name: string; value: string; enabled: boolean }[],
87+
pointerField: string
88+
): Record<string, unknown>[] => {
89+
const byName = new Map((existing ?? []).map((variable) => [variable.name, variable]));
90+
return rows.map((row) => ({
91+
...(byName.get(row.name) ?? {}),
92+
name: row.name,
93+
[pointerField]: row.value,
94+
disabled: !row.enabled
95+
}));
96+
};
97+
7998
export interface EnvironmentVariableRow {
8099
name: string;
81100
value: string;

0 commit comments

Comments
 (0)