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](https://github.com/user-attachments/assets/bug-before-fix)
   
   | 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:
   
   ![after](https://github.com/user-attachments/assets/bug-after-fix)
   
   ### 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to