Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport 1.18.x: UI Solve cntrl+f issue on KVv2 JSON details and edit views (#28808) #28893

Merged
merged 5 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
6 changes: 6 additions & 0 deletions changelog/28808.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
```release-note:improvement
ui: Replace KVv2 json secret details view with Hds::CodeBlock component allowing users to search the full secret height.
```
```release-note:bug
ui: Allow users to search the full json object within the json code-editor edit/create view.
```
3 changes: 2 additions & 1 deletion ui/app/templates/components/console/log-json.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
@showToolbar={{false}}
@value={{stringify this.content}}
@readOnly={{true}}
@viewportMargin="Infinity"
{{! ideally we calculate the "height" of the json data, but 100 should cover most cases }}
@viewportMargin="100"
@gutters={{false}}
@theme="hashi auto-height"
/>
Expand Down
3 changes: 2 additions & 1 deletion ui/app/templates/components/control-group-success.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
@showToolbar={{false}}
@value={{stringify this.unwrapData}}
@readOnly={{true}}
@viewportMargin="Infinity"
{{! ideally we calculate the "height" of the json data, but 100 should cover most cases }}
@viewportMargin="100"
@gutters={{false}}
@theme="hashi-read-only auto-height"
/>
Expand Down
2 changes: 1 addition & 1 deletion ui/lib/core/addon/components/json-editor.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
mode=@mode
readOnly=@readOnly
theme=@theme
viewportMarg=@viewportMargin
viewportMargin=@viewportMargin
onSetup=this.onSetup
onUpdate=this.onUpdate
onFocus=this.onFocus
Expand Down
2 changes: 1 addition & 1 deletion ui/lib/core/addon/components/json-editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { action } from '@ember/object';
* @param {Boolean} [readOnly] - Sets the view to readOnly, allowing for copying but no editing. It also hides the cursor. Defaults to false.
* @param {String} [theme] - Specify or customize the look via a named "theme" class in scss.
* @param {String} [value] - Value within the display. Generally, a json string.
* @param {String} [viewportMargin] - Size of viewport. Often set to "Infinity" to load/show all text regardless of length.
* @param {String} [viewportMargin] - Specifies the amount of lines rendered on the DOM (this is not the editor display height). The codemirror default is 10 which we set explicity in the code-mirror modifier per the recommendations from the codemirror docs.
* @param {string} [example] - Example to show when value is null -- when example is provided a restore action will render in the toolbar to clear the current value and show the example after input
* @param {string} [screenReaderLabel] - This label is read by the screen readers when CodeMirror text area is focused. This is helpful for accessibility.
* @param {string} [container] - **REQUIRED if rendering within a modal** Selector string or element object of containing element, set the focused element as the container value. This is for the Hds::Copy::Button and to set `autoRefresh=true` so content renders https://hds-website-hashicorp.vercel.app/components/copy/button?tab=code
Expand Down
2 changes: 1 addition & 1 deletion ui/lib/core/addon/modifiers/code-mirror.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export default class CodeMirrorModifier extends Modifier {
readOnly: namedArgs.readOnly || false,
theme: namedArgs.theme || 'hashi',
value: namedArgs.content || '',
viewportMargin: namedArgs.viewportMargin || '',
viewportMargin: namedArgs.viewportMargin || 10,
autoRefresh: namedArgs.autoRefresh,
});

Expand Down
27 changes: 21 additions & 6 deletions ui/lib/kv/addon/components/kv-data-fields.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,27 @@

<hr class="is-marginless has-background-gray-200" />
{{#if @showJson}}
<JsonEditor
@title="{{if (eq @type 'create') 'Secret' 'Version'}} data"
@value={{this.stringifiedSecretData}}
@valueUpdated={{this.handleJson}}
@readOnly={{eq @type "details"}}
/>
{{#if (eq @type "details")}}
<Hds::CodeBlock
data-test-code-block="secret-data"
@value={{this.stringifiedSecretData}}
@language="json"
@hasCopyButton={{true}}
@maxHeight="300px"
as |CB|
>
<CB.Title @tag="h3">
Version data
</CB.Title>
</Hds::CodeBlock>
{{else}}
<JsonEditor
@title="{{if (eq @type 'create') 'Secret' 'Version'}} data"
@value={{this.stringifiedSecretData}}
@valueUpdated={{this.handleJson}}
@viewportMargin={{this.viewportMargin}}
/>
{{/if}}
{{#if (or @modelValidations.secretData.errors this.lintingErrors)}}
<AlertInline
@color={{if this.lintingErrors "warning" "critical"}}
Expand Down
9 changes: 9 additions & 0 deletions ui/lib/kv/addon/components/kv-data-fields.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,15 @@ export default class KvDataFields extends Component {
return this.args.secret?.secretData ? stringify([this.args.secret.secretData], {}) : this.startingValue;
}

get viewportMargin() {
if (!this.args?.secret?.secretData) return 10;
const jsonHeight = Object.keys(this.args.secret.secretData).length;
// return the higher of: 10 or the approimated number of lines in the json. jsonHeight only includes the first level of keys, so for objects
// with lots of nested values, it will undercount.
const max = Math.max(jsonHeight, 10);
return Math.min(max, 1000); // cap at 1000 lines to avoid performance implications
}

@action
handleJson(value, codemirror) {
codemirror.performLint();
Expand Down
2 changes: 1 addition & 1 deletion ui/lib/kv/addon/components/kv-patch/subkeys-reveal.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@
<Hds::Text::Body @tag="p" @weight="semibold">Reveal subkeys in JSON</Hds::Text::Body>
</Toggle>
{{#if this.showSubkeys}}
<Hds::CodeBlock @value={{stringify @subkeys}} @hasLineNumbers={{false}} data-test-subkeys />
<Hds::CodeBlock @value={{stringify @subkeys}} @hasLineNumbers={{false}} data-test-code-block="subkeys" />
{{/if}}
</div>
4 changes: 2 additions & 2 deletions ui/lib/kv/addon/components/kv-paths-card.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
for other CLI commands.
</p>
<Hds::CodeBlock
data-test-commands="cli"
data-test-code-block="cli"
@language="bash"
@hasLineNumbers={{false}}
@hasCopyButton={{true}}
Expand All @@ -71,7 +71,7 @@
</DocLink>
</p>
<Hds::CodeBlock
data-test-commands="api"
data-test-code-block="api"
@language="bash"
@hasLineNumbers={{false}}
@hasCopyButton={{true}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
<:content>
<Hds::CodeBlock
class="has-top-margin-s"
data-test-accounts-code-block
data-test-code-block="accounts"
@language="bash"
@hasLineNumbers={{false}}
@hasCopyButton={{true}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,22 +299,34 @@ module('Acceptance | kv-v2 workflow | edge cases', function (hooks) {
codemirror().getValue(),
`{
\"\": \"\"
}`
}`,
'JSON editor displays correct empty object'
);
codemirror().setValue('{ "foo3": { "name": "bar3" } }');
await click(FORM.saveBtn);

// Details view
await click(PAGE.secretTab('Secret'));
assert.dom(FORM.toggleJson).isNotDisabled();
assert.dom(FORM.toggleJson).isChecked();
assert.false(codemirror().getValue().includes('*'), 'Values are not obscured on details view');
assert.dom(FORM.toggleJson).isNotDisabled('JSON toggle is not disabled');
assert.dom(FORM.toggleJson).isChecked("JSON toggle is checked 'on'");

assert
.dom(GENERAL.codeBlock('secret-data'))
.hasText('Version data { "foo3": { "name": "bar3" } }', 'Values are displayed in the details view');

// New version view
await click(PAGE.detail.createNewVersion);
assert.dom(FORM.toggleJson).isNotDisabled();
assert.dom(FORM.toggleJson).isChecked();
assert.false(codemirror().getValue().includes('*'), 'Values are not obscured on edit view');
assert.deepEqual(
codemirror().getValue(),
`{
"foo3": {
"name": "bar3"
}
}`,
'Values are displayed in the new version view'
);
});

test('on enter the JSON editor cursor goes to the next line', async function (assert) {
Expand Down Expand Up @@ -359,12 +371,16 @@ module('Acceptance | kv-v2 workflow | edge cases', function (hooks) {
await click(PAGE.secretTab('Secret'));
await click(PAGE.detail.versionDropdown);
await click(`${PAGE.detail.version(1)} a`);
assert.strictEqual(codemirror().getValue(), expectedDataV1, 'Version one data is displayed');
assert
.dom(GENERAL.codeBlock('secret-data'))
.hasText(`Version data ${expectedDataV1}`, 'Version one data is displayed');

// Navigate back the second version and make sure the secret data is correct
await click(PAGE.detail.versionDropdown);
await click(`${PAGE.detail.version(2)} a`);
assert.strictEqual(codemirror().getValue(), expectedDataV2, 'Version two data is displayed');
assert
.dom(GENERAL.codeBlock('secret-data'))
.hasText(`Version data ${expectedDataV2}`, 'Version two data is displayed');
});

test('does not register as advanced when value includes {', async function (assert) {
Expand Down
2 changes: 2 additions & 0 deletions ui/tests/helpers/general-selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ export const GENERAL = {
navLink: (label: string) => `[data-test-sidebar-nav-link="${label}"]`,
cancelButton: '[data-test-cancel]',
saveButton: '[data-test-save]',
backButton: '[data-test-back-button]',
codeBlock: (label: string) => `[data-test-code-block="${label}"]`,
codemirror: `[data-test-component="code-mirror-modifier"]`,
codemirrorTextarea: `[data-test-component="code-mirror-modifier"] textarea`,
};
4 changes: 2 additions & 2 deletions ui/tests/helpers/kv/kv-selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ export const PAGE = {
},
paths: {
copyButton: (label) => `${PAGE.infoRowValue(label)} button`,
codeSnippet: (section) => `[data-test-commands="${section}"] code`,
snippetCopy: (section) => `[data-test-commands="${section}"] button`,
codeSnippet: (section) => `[data-test-code-block="${section}"] code`,
snippetCopy: (section) => `[data-test-code-block="${section}"] button`,
},
};

Expand Down
46 changes: 46 additions & 0 deletions ui/tests/helpers/secret-engine/secret-engine-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,3 +206,49 @@ export const fillInAwsConfig = async (situation = 'withAccess') => {
await fillIn(GENERAL.ttl.input('Identity token TTL'), '7200');
}
};

// Example usage
// createLongJson (2, 3) will create a json object with 2 original keys, each with 3 nested keys
// {
// "key-0": {
// "nested-key-0": {
// "nested-key-1": {
// "nested-key-2": "nested-value"
// }
// }
// },
// "key-1": {
// "nested-key-0": {
// "nested-key-1": {
// "nested-key-2": "nested-value"
// }
// }
// }
// }

export function createLongJson(lines = 10, nestLevel = 3) {
const keys = Array.from({ length: nestLevel }, (_, i) => `nested-key-${i}`);
const jsonObject = {};

for (let i = 0; i < lines; i++) {
nestLevel > 0
? (jsonObject[`key-${i}`] = createNestedObject({}, keys, 'nested-value'))
: (jsonObject[`key-${i}`] = 'non-nested-value');
}
return jsonObject;
}

function createNestedObject(obj = {}, keys, value) {
let current = obj;

for (let i = 0; i < keys.length - 1; i++) {
const key = keys[i];
if (!current[key]) {
current[key] = {};
}
current = current[key];
}

current[keys[keys.length - 1]] = value;
return obj;
}
31 changes: 31 additions & 0 deletions ui/tests/integration/components/json-editor-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import hbs from 'htmlbars-inline-precompile';
import jsonEditor from '../../pages/components/json-editor';
import sinon from 'sinon';
import { setRunOptions } from 'ember-a11y-testing/test-support';
import { createLongJson } from 'vault/tests/helpers/secret-engine/secret-engine-helpers';

const component = create(jsonEditor);

Expand All @@ -29,13 +30,16 @@ module('Integration | Component | json-editor', function (hooks) {
this.set('onFocusOut', sinon.spy());
this.set('json_blob', JSON_BLOB);
this.set('bad_json_blob', BAD_JSON_BLOB);
this.set('long_json', JSON.stringify(createLongJson(), null, `\t`));
this.set('hashi-read-only-theme', 'hashi-read-only auto-height');
setRunOptions({
rules: {
// CodeMirror has a secret textarea without a label that causes this problem
label: { enabled: false },
// TODO: investigate and fix Codemirror styling
'color-contrast': { enabled: false },
// failing on .CodeMirror-scroll
'scrollable-region-focusable': { enabled: false },
},
});
});
Expand Down Expand Up @@ -129,4 +133,31 @@ module('Integration | Component | json-editor', function (hooks) {
'even after hitting enter the value is still set correctly'
);
});

test('no viewportMargin renders only default 10 lines of data on the DOM', async function (assert) {
await render(hbs`
<JsonEditor
@value={{this.long_json}}
@mode="ruby"
@valueUpdated={{fn (mut this.value)}}
/>
`);
assert
.dom('.CodeMirror-code')
.doesNotIncludeText('key-9', 'Without viewportMargin, user cannot search for key-9');
});

test('when viewportMargin is set user is able to search a long secret', async function (assert) {
await render(hbs`
<JsonEditor
@value={{this.long_json}}
@mode="ruby"
@valueUpdated={{fn (mut this.value)}}
@viewportMargin="100"
/>
`);
assert
.dom('.CodeMirror-code')
.containsText('key-9', 'With viewportMargin set, user can search for key-9');
});
});
32 changes: 29 additions & 3 deletions ui/tests/integration/components/kv/kv-data-fields-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import { hbs } from 'ember-cli-htmlbars';
import { fillIn, render, click } from '@ember/test-helpers';
import codemirror from 'vault/tests/helpers/codemirror';
import { PAGE, FORM } from 'vault/tests/helpers/kv/kv-selectors';
import { GENERAL } from 'vault/tests/helpers/general-selectors';
import { setRunOptions } from 'ember-a11y-testing/test-support';
import { createLongJson } from 'vault/tests/helpers/secret-engine/secret-engine-helpers';

module('Integration | Component | kv-v2 | KvDataFields', function (hooks) {
setupRenderingTest(hooks);
Expand All @@ -22,6 +25,12 @@ module('Integration | Component | kv-v2 | KvDataFields', function (hooks) {
this.backend = 'my-kv-engine';
this.path = 'my-secret';
this.secret = this.store.createRecord('kv/data', { backend: this.backend });
setRunOptions({
rules: {
// failing on .CodeMirror-scroll
'scrollable-region-focusable': { enabled: false },
},
});
});

test('it updates the secret model', async function (assert) {
Expand Down Expand Up @@ -88,7 +97,7 @@ module('Integration | Component | kv-v2 | KvDataFields', function (hooks) {
assert.dom(PAGE.infoRowValue('foo')).hasText('bar', 'secret value shows after toggle');
});

test('it shows readonly json editor when viewing secret details of complex secret', async function (assert) {
test('it shows hds codeblock when viewing secret details of complex secret', async function (assert) {
this.secret.secretData = {
foo: {
bar: 'baz',
Expand All @@ -100,7 +109,24 @@ module('Integration | Component | kv-v2 | KvDataFields', function (hooks) {
owner: this.engine,
});
assert.dom(PAGE.infoRowValue('foo')).doesNotExist('does not render rows of secret data');
assert.dom('[data-test-component="code-mirror-modifier"]').hasClass('readonly-codemirror');
assert.dom('[data-test-component="code-mirror-modifier"]').includesText(`{ "foo": { "bar": "baz" }}`);
assert.dom(GENERAL.codeBlock('secret-data')).exists('hds codeBlock exists');
assert
.dom(GENERAL.codeBlock('secret-data'))
.hasText(`Version data { "foo": { "bar": "baz" } } `, 'Json data is displayed');
});

test('it defaults to a viewportMargin 10 when there is no secret data', async function (assert) {
await render(hbs`<KvDataFields @showJson={{true}} @secret={{this.secret}} />`, { owner: this.engine });
assert.strictEqual(codemirror().options.viewportMargin, 10, 'viewportMargin defaults to 10');
});

test('it calculates viewportMargin based on secret size', async function (assert) {
this.secret.secretData = createLongJson(100);
await render(hbs`<KvDataFields @showJson={{true}} @secret={{this.secret}} />`, { owner: this.engine });
assert.strictEqual(
codemirror().options.viewportMargin,
100,
'viewportMargin is set to 100 matching the height of the json'
);
});
});
Loading
Loading