You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "richardfogaca (via GitHub)" <gi...@apache.org> on 2026/02/03 01:49:00 UTC
[PR] fix(dataset-editor): include calculated columns in currency code dropdown [superset]
richardfogaca opened a new pull request, #37621:
URL: https://github.com/apache/superset/pull/37621
### SUMMARY
The **Currency code column** dropdown in the Dataset Editor excludes calculated columns, even though they are valid choices for currency code mapping (e.g. a `CASE` statement that maps country values to ISO currency codes like `USD`, `EUR`).
**Root cause:** The dropdown is built from `allColumns` (physical + calculated), but the filter at [`DatasourceEditor.jsx:1096`](https://github.com/apache/superset/blob/master/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.jsx#L1096) requires `type_generic === GenericDataType.String`. The backend's `fetch_metadata()` only resolves `type_generic` for physical columns — calculated columns are added back to the dataset with `type_generic` unset (`null`/`undefined`), so they silently fail the strict equality check.
**Fix:** Loosen the filter to also include columns with a truthy `expression` property (the same signal the component already uses to distinguish calculated from physical columns at construction time). This is safe because the downstream currency detection logic already handles invalid currency codes gracefully by falling back to neutral formatting.
```diff
const stringColumns = allColumns
- .filter(col => col.type_generic === GenericDataType.String)
+ .filter(
+ col => col.type_generic === GenericDataType.String || col.expression,
+ )
```
Fixes #37620
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
**Before** — calculated column `num_california` is missing from the dropdown:
| Before | After |
|--------|-------|
| Only physical string columns shown (`gender`, `name`, `state`) | Calculated columns also shown (`num_california`) alongside physical string columns |
**After** — calculated column `num_california` now appears in the dropdown:
### TESTING INSTRUCTIONS
1. Open a dataset in the Dataset Editor (e.g. `birth_names`)
2. Go to the **Calculated columns** tab and verify a calculated column exists (or add one, e.g. `CASE WHEN state = 'CA' THEN 'USD' ELSE 'EUR' END`)
3. In the **Default Column Settings** section at the top, open the **Currency code column** dropdown
4. Verify the calculated column appears in the dropdown alongside the physical string columns
5. Verify numeric physical columns (e.g. `num`, `num_boys`) do **not** appear
6. Select the calculated column and click Save — verify it persists
**Unit tests:**
```bash
npm run test -- --testPathPatterns='DatasourceEditorCurrency' --no-coverage
```
### ADDITIONAL INFORMATION
- [x] Has associated issue: #37620
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset-editor): include calculated columns in currency code dropdown [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #37621:
URL: https://github.com/apache/superset/pull/37621#issuecomment-3838381277
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 4e3da4e2dd8e4f2cc4582e5e7f47c93a1699e2a6 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6981540c7fe953000792b4ee |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-37621--superset-docs-preview.netlify.app](https://deploy-preview-37621--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-37621--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset-editor): include calculated columns in currency code dropdown [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #37621:
URL: https://github.com/apache/superset/pull/37621#discussion_r2759393457
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorCurrency.test.tsx:
##########
@@ -196,10 +219,22 @@ test('currency code column dropdown shows only string columns', async () => {
expect(currencyCodeOption).toBeDefined();
});
- // Verify NUMERIC column is NOT available
+ // Verify CALCULATED column is available despite null type_generic
const options = document.querySelectorAll('.ant-select-item-option');
+ const derivedCurrencyOption = Array.from(options).find(o =>
+ o.textContent?.includes('derived_currency'),
+ );
+ expect(derivedCurrencyOption).toBeDefined();
+
+ // Verify NUMERIC physical column is NOT available
const amountOption = Array.from(options).find(o =>
o.textContent?.includes('amount'),
);
Review Comment:
**Suggestion:** The check for the numeric physical column uses `includes('amount')`, which can also match the `total_amount` option text, so the test may incorrectly detect the calculated `total_amount` column as the physical `amount` column and either fail or pass for the wrong reason. To avoid this ambiguity, the match should explicitly exclude `total_amount` so that only the intended `amount` option is considered. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Test for currency column becomes flaky or misleading.
- ⚠️ CI could show intermittent failures for dropdown tests.
- ⚠️ Developer time wasted diagnosing false negatives.
```
</details>
```suggestion
const amountOption = Array.from(options).find(o => {
const text = o.textContent ?? '';
return text.includes('amount') && !text.includes('total_amount');
});
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the test suite including the file at
superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorCurrency.test.tsx.
2. The test navigates to the Columns tab and opens the "Currency code column" dropdown
(see test block starting around line 141 and the dropdown interaction at lines ~206-211).
3. After opening the dropdown, the test collects options via
document.querySelectorAll('.ant-select-item-option') and stores them in `options` (lines
~214-221 and 222-227).
4. The existing code at lines 230-233 uses Array.from(options).find(o =>
o.textContent?.includes('amount')) which matches any option whose visible text contains
the substring "amount". Because the dropdown contains both 'amount' (physical column) and
'total_amount' (calculated column), the find may return the 'total_amount' option instead
of the intended 'amount' option.
5. If the test runner or DOM ordering changes such that 'total_amount' appears before
'amount' in the option list, the current assertion (expect(amountOption).toBeUndefined())
may pass or fail for the wrong reason, making the test flaky or misleading.
6. The improved code (proposed) disambiguates the match by ensuring the matched option
includes "amount" but not "total_amount", eliminating the substring collision and making
the expectation target the physical 'amount' option explicitly.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorCurrency.test.tsx
**Line:** 230:232
**Comment:**
*Logic Error: The check for the numeric physical column uses `includes('amount')`, which can also match the `total_amount` option text, so the test may incorrectly detect the calculated `total_amount` column as the physical `amount` column and either fail or pass for the wrong reason. To avoid this ambiguity, the match should explicitly exclude `total_amount` so that only the intended `amount` option is considered.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset-editor): include calculated columns in currency code dropdown [superset]
Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca merged PR #37621:
URL: https://github.com/apache/superset/pull/37621
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset-editor): include calculated columns in currency code dropdown [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37621:
URL: https://github.com/apache/superset/pull/37621#issuecomment-3842045183
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset-editor): include calculated columns in currency code dropdown [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #37621:
URL: https://github.com/apache/superset/pull/37621#issuecomment-3838367254
## **Sequence Diagram**
Shows the fix where the Dataset Editor includes calculated columns (identified by a truthy expression) when building the currency code dropdown, despite those columns lacking backend-resolved type metadata.
```mermaid
sequenceDiagram
participant DatasetEditor
participant Backend
participant Dropdown
participant User
DatasetEditor->>Backend: fetch_metadata()
Backend-->>DatasetEditor: columns (physical cols have type_generic; calculated cols lack type_generic but include expression)
DatasetEditor->>Dropdown: Build options filtered by (type_generic == String OR expression)
Dropdown-->>User: Render currency code list (includes calculated columns)
```
---
*Generated by [CodeAnt AI](https://codeant.ai)*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset-editor): include calculated columns in currency code dropdown [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37621:
URL: https://github.com/apache/superset/pull/37621#issuecomment-3838364213
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(dataset-editor): include calculated columns in currency code dropdown [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37621:
URL: https://github.com/apache/superset/pull/37621#issuecomment-3844479799
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org