Skip to content

Conversation

@eschutho
Copy link
Member

SUMMARY

When editing in the Metadata Parameters in the database editor, if possible, we parse the string to a javascript object, but were not converting it back to a string in order for the ace editor to display the text, resulting in an error.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

After:
Screenshot 2025-05-14 at 12 10 44 PM

TESTING INSTRUCTIONS

edit the metadata parameters and save, reload the database

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@dosubot dosubot bot added the change:frontend Requires changing the frontend label May 15, 2025
Copy link

@korbit-ai korbit-ai bot left a comment

Choose a reason for hiding this comment

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

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.
Category Issue Status
Performance Inefficient Empty Object Check ▹ view 🧠 Incorrect
Readability Nested ternary making value assignment hard to read ▹ view 🧠 Not in scope
Files scanned
File Path Reviewed
superset-frontend/src/features/databases/DatabaseModal/ExtraOptions.tsx

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

width="100%"
height="160px"
value={
!Object.keys(extraJson?.metadata_params || {}).length

This comment was marked as resolved.

Comment on lines 534 to 540
value={
!Object.keys(extraJson?.metadata_params || {}).length
? ''
: extraJson?.metadata_params
: typeof extraJson?.metadata_params === 'string'
? extraJson?.metadata_params
: JSON.stringify(extraJson?.metadata_params)
}

This comment was marked as resolved.

@eschutho eschutho force-pushed the elizabeth/metadata-bug branch from 6546b2e to 89db23f Compare May 15, 2025 01:03
@eschutho eschutho force-pushed the elizabeth/metadata-bug branch from 89db23f to 9b0e180 Compare May 15, 2025 18:20
Copy link
Member

@betodealmeida betodealmeida left a comment

Choose a reason for hiding this comment

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

In times like this I wish we used something like thrift or protobuf to define types, and then autogenerate the server/client code...

@github-actions
Copy link
Contributor

@eschutho Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments

@github-actions
Copy link
Contributor

@eschutho Ephemeral environment spinning up at http://54.191.220.223:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.

@eschutho eschutho merged commit b050897 into master May 16, 2025
58 checks passed
@eschutho eschutho deleted the elizabeth/metadata-bug branch May 16, 2025 18:27
@michael-s-molina michael-s-molina added the v5.0 Label added by the release manager to track PRs to be included in the 5.0 branch label May 23, 2025
michael-s-molina pushed a commit that referenced this pull request May 23, 2025
LevisNgigi pushed a commit to LevisNgigi/superset that referenced this pull request Jun 18, 2025
alexandrusoare pushed a commit to alexandrusoare/superset that referenced this pull request Jun 19, 2025
@mistercrunch mistercrunch added 🍒 5.0.0 Cherry-picked to 5.0.0 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels labels Jul 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels change:frontend Requires changing the frontend size/L v5.0 Label added by the release manager to track PRs to be included in the 5.0 branch 🍒 5.0.0 Cherry-picked to 5.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants