rusackas opened a new pull request, #41399:
URL: https://github.com/apache/superset/pull/41399

   ### SUMMARY
   
   Fixes the dashboard "Embed dashboard" modal so that previously saved allowed 
domains are displayed in the input when the modal is reopened.
   
   This **adopts 
[anandraiin3/superset#15](https://github.com/anandraiin3/superset/pull/15)** 
(originally authored via Devin AI). Full credit to **@anandraiin3** for the 
original fix — this PR brings it onto current `master` with the regression test 
intact.
   
   **Root cause**
   
   The Ant Design v5 overhaul (#31590) restructured this form to use the 
`@superset-ui/core/components` `Form` / `FormItem` wrappers and moved 
`name="allowed-domains"` from the `<Input>` element onto the `<FormItem>` 
wrapper:
   
   ```tsx
   <Form layout="vertical">
     <FormItem
       name="allowed-domains"   // turns FormItem into a controlled antd field
       label={...}
     >
       <Input
         id="allowed-domains"
         value={allowedDomains}                  // ignored by antd
         onChange={e => setAllowedDomains(...)}  // wrapped/overridden by antd
       />
     </FormItem>
   </Form>
   ```
   
   When `FormItem` has a `name`, Ant Design takes over the field via its 
internal `Form` store: it clones the child and injects its own `value` / 
`onChange`, ignoring the explicit `value={allowedDomains}` passed to the 
`Input`. As a result, programmatic state updates from React (i.e. 
`setAllowedDomains(result.allowed_domains.join(', '))` after the `GET 
/embedded` call resolves) never propagate to the rendered input.
   
   This is why:
   
   - On first open with no embedded config, the user can type and save normally.
   - After navigating away and back, the modal remounts, the GET returns the 
saved domains, React state is updated, but the form's internal state stays 
empty — so the input renders empty even though `embedded.allowed_domains` is 
populated. This also matches the issue symptom where the input detects a 
duplicate domain and keeps Save disabled (the `isDirty` check uses React state, 
which *is* populated correctly).
   
   **Fix**
   
   Replace `name="allowed-domains"` on `<FormItem>` with 
`htmlFor="allowed-domains"`. The `FormItem` is now used purely as a 
label/layout wrapper (not a controlled antd field), so the `<Input>`'s explicit 
`value` / `onChange` are honored as a normal controlled component. The 
`htmlFor` keeps the label associated with the input for accessibility (matching 
the existing `id="allowed-domains"` on the `<Input>`).
   
   This is a minimal, focused change that restores the pre-AntD-v5 behavior 
without changing how data is loaded, saved, or validated.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   N/A — pure UI state-binding fix; no visual styling changes. Verified via the 
new unit test described below.
   
   ### TESTING INSTRUCTIONS
   
   Automated:
   
   ```bash
   cd superset-frontend
   npm run test -- src/dashboard/components/EmbeddedModal/EmbeddedModal.test.tsx
   ```
   
   A new test, `populates the allowed-domains input from the API response`, 
asserts that when the GET `/api/v1/dashboard/<id>/embedded` endpoint returns `{ 
allowed_domains: ['example.com'] }`, the input element's `value` is 
`"example.com"`. This test fails on `master` and passes with this fix.
   
   Manual (mirrors the issue reproduction):
   
   1. Enable the `EMBEDDED_SUPERSET` feature flag.
   2. Open a dashboard, click ⋮ → Embed dashboard.
   3. Enter `https://mytestdomain.com` and click Save changes.
   4. Navigate to Datasets, then back to Dashboards, and reopen the same 
dashboard.
   5. Click ⋮ → Embed dashboard.
   6. **Expected:** the "Allowed Domains (comma separated)" input shows 
`https://mytestdomain.com` (previously it was empty).
   
   ### ADDITIONAL INFORMATION
   
   Adopted from anandraiin3/superset#15 (originally authored via Devin AI). 
Credit to @anandraiin3.
   
   - [x] Has associated issue: Closes #35328
   - [ ] 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