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

UI fix KVv2 in json editor when value is null #27094

Merged
merged 10 commits into from
May 17, 2024

Conversation

Monkeychip
Copy link
Contributor

@Monkeychip Monkeychip commented May 16, 2024

  • ent test pass
  • test coverage

Regression PR #24530
Fixes Issue #26709

Note: confirmed that you can in fact pass null as a value using curl, so this was a situation we needed to allow in the UI.

Before:

before.mov

After:

fixed.mov

@Monkeychip Monkeychip added this to the 1.17.0-rc milestone May 16, 2024
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label May 16, 2024
Copy link

github-actions bot commented May 16, 2024

CI Results:
All Go tests succeeded! ✅

@Monkeychip Monkeychip marked this pull request as ready for review May 16, 2024 22:19
@Monkeychip Monkeychip requested a review from a team as a code owner May 16, 2024 22:19
Copy link

github-actions bot commented May 16, 2024

Build Results:
All builds succeeded! ✅

Copy link
Collaborator

@hashishaw hashishaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great test coverage! just one suggestion

// unfortunately in javascript typeof null returns object
// this is due to a "historical js bug that will never be fixed"
// we handle this situation here
if (obj[key] === null) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we instead move this to its own else if block before the 'object' check? that would flatten the logic

@@ -30,7 +30,12 @@ export function obfuscateData(obj) {
for (const key of Object.keys(obj)) {
if (Array.isArray(obj[key])) {
newObj[key] = obj[key].map(() => '********');
} else if (typeof obj[key] === 'object') {
} else if (typeof obj[key] === 'object' && obj[key] === null) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just check if it === null

Suggested change
} else if (typeof obj[key] === 'object' && obj[key] === null) {
} else if (obj[key] === null) {

and then below just check if it's type object

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I was fearful that it would hit the second else if again, but testing while it does it doesn't seem to break anything.

@Monkeychip Monkeychip enabled auto-merge (squash) May 17, 2024 16:37
@Monkeychip Monkeychip merged commit 0554cda into main May 17, 2024
31 checks passed
@Monkeychip Monkeychip deleted the ui/VAULT-27209/fix-json-kv-when-value-null branch May 17, 2024 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants