Skip to content

Clearing a data source field doesn't actually clear it - #7785

Open
yoshiokatsuneo wants to merge 2 commits into
getredash:masterfrom
yoshiokatsuneo:fix/dynamic-form-empty-value-normalization
Open

Clearing a data source field doesn't actually clear it#7785
yoshiokatsuneo wants to merge 2 commits into
getredash:masterfrom
yoshiokatsuneo:fix/dynamic-form-empty-value-normalization

Conversation

@yoshiokatsuneo

@yoshiokatsuneo yoshiokatsuneo commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Why

If you edit an existing data source and clear a field that already had a value, it doesn't actually get cleared — it saves an empty string instead of removing the setting. If you leave a field empty on a new data source, it's correctly saved as "not set." Same user intent ("no value"), two different outcomes depending on whether the field had a value before, because the code used the field's initialValue to guess intent instead of just looking at the current value.

What changed

normalizeEmptyValuesToNull in DynamicForm.jsx no longer compares against initialValue. Any empty string is now normalized to null, so a cleared field always ends up unset, regardless of whether it had a saved value before.

Relation to #3886 ("Fix empty values sent in dynamic form")

This initialValue comparison was introduced deliberately in #3886, with the stated intent: "don't submit form values if they are empty (unless they are intentionally set to empty string)". The idea was to distinguish a field the user never touched from one the user actively cleared, treating the latter as an intentional empty-string value worth preserving.

In practice, clearing a field is how users expect to unset it, not to explicitly assign an empty string. Preserving "" instead of null means the field looks "set" server-side (the key is present) even though the user's intent was to remove it. This PR revisits that assumption and removes the distinction, treating both cases as "no value."

Relation to #7756 ("fix(configuration): treat empty secret as explicit clear")

#7756 addresses a related symptom: clearing a secret field (e.g. an AWS credential) persists it as "", which some downstream SDKs (e.g. boto3) treat as an explicit credential rather than "unset," breaking default credential-chain fallback. That fix is scoped to secret fields in ConfigurationContainer.update() on the backend.

This PR fixes the underlying cause on the frontend for all DynamicForm fields (not limited to secrets): once a cleared field is normalized to null before submission, the empty-string case #7756 handles no longer originates from the UI.

Validation

  • pnpm run lint

🤖 This PR was authored with the help of Claude Code (Anthropic), using Claude Sonnet 5 (claude-sonnet-5).

@yoshiokatsuneo yoshiokatsuneo changed the title Fix inconsistent empty-value handling in DynamicForm Clearing a data source field doesn't actually clear it Aug 11, 2026
@greptile-apps

greptile-apps Bot commented Aug 11, 2026

Copy link
Copy Markdown

Greptile Summary

This PR changes DynamicForm submission normalization so every empty string is submitted as null, allowing cleared data-source settings to be treated as unset.

  • Removes the initial-value-dependent normalization behavior.
  • Removes the now-unused Lodash find import.
  • Updates surrounding formatting without changing behavior.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
client/app/components/dynamic-form/DynamicForm.jsx Empty-string normalization is simplified to consistently produce null, with no eligible follow-up defect established.

Reviews (3): Last reviewed commit: "Apply prettier formatting" | Re-trigger Greptile

@yoshiokatsuneo
yoshiokatsuneo force-pushed the fix/dynamic-form-empty-value-normalization branch from 317606e to cea411e Compare August 11, 2026 17:15
Previously, clearing a field that had a previously saved value sent an
empty string to the server, while never filling in a field sent null.
Both cases represent the same user intent (no value), so normalize any
empty string to null regardless of the field's initial value.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@yoshiokatsuneo
yoshiokatsuneo force-pushed the fix/dynamic-form-empty-value-normalization branch from cea411e to cbc3757 Compare August 11, 2026 17:16

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 1 file

Re-trigger cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant