rusackas commented on PR #40528: URL: https://github.com/apache/superset/pull/40528#issuecomment-4598993642
Thanks for the review @aminghadersohi — all the follow-ups are in bed150cfa5: - **MEDIUM (no unit test for the fix):** Added. I extracted the parsing into a pure, fully-typed `parseSelectedOwners()` (`PropertiesModal/utils.ts`) and unit-tested the regression directly — remove an owner whose data lives only in the controlled `value` (partial option cache) and the remaining owner keeps its name/email from state — plus the new-owner and label-fallback paths. - **NIT (missing comment on the AsyncSelect cache workaround):** Added inline docs on the helper explaining why we prefer the owner object already in state over the partial option cache. - **NIT (redundant runtime type guard):** It turned out not to be redundant — the option `label` is a rendered `OwnerSelectLabel` React element, not a string, so I made the type honest (`ReactNode`, shared `OwnerOption` type across the handler and `AccessSection`) and removed the unsafe `as` casts. The `typeof label === 'string'` guard now matches the type. Existing `PropertiesModal`/`AccessSection` suites still pass and frontend type-check + pre-commit are clean. -- 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]
