You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "minwookshin (via GitHub)" <gi...@apache.org> on 2026/10/03 03:11:46 UTC
[PR] fix(explore): preserve pending column configuration edits [superset]
minwookshin opened a new pull request, #44931:
URL: https://github.com/apache/superset/pull/44931
### SUMMARY
Keep pending column-format edits together before notifying the parent. Debounced updates now use the latest values and callback, and cancel when the form unmounts or the parent replaces its value.
Fixes #44922. This is scoped to lost updates; #44843 and #44668 cover separate test/reset work.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
No visual changes. Rapid edits to two fields previously allowed a stale update to overwrite the other field.
### TESTING INSTRUCTIONS
In a Table chart, quickly edit two column-format fields before the debounce finishes. Both changes should persist. Close the control with a pending edit and confirm it does not emit afterward.
All 12 ColumnConfigControl tests passed, including seven regressions covering mixed delays, unmount, callback replacement and external values. All applicable pre-commit checks passed, including targeted TypeScript, formatting and lint. Backend/browser E2E was not run.
### ADDITIONAL INFORMATION
- [x] Has associated issue: #44922
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] 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(explore): preserve pending column configuration edits [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #44931:
URL: https://github.com/apache/superset/pull/44931#issuecomment-5965134657
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/44931?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 82.29%. Comparing base ([`8d026b4`](https://app.codecov.io/gh/apache/superset/commit/8d026b4a5682fa3c5fc24f75f1ac4271c35f424d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`ce21510`](https://app.codecov.io/gh/apache/superset/commit/ce215107347331bfa4d73edd826f633e4941ae9c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #44931 +/- ##
==========================================
+ Coverage 82.26% 82.29% +0.02%
==========================================
Files 2995 2995
Lines 185034 185051 +17
Branches 42822 42825 +3
==========================================
+ Hits 152225 152287 +62
+ Misses 30047 30002 -45
Partials 2762 2762
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/44931/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/44931/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `77.94% <100.00%> (+0.05%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/44931?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</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(explore): preserve pending column configuration edits [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe merged PR #44931:
URL: https://github.com/apache/superset/pull/44931
--
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(explore): preserve pending column configuration edits [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44931:
URL: https://github.com/apache/superset/pull/44931#issuecomment-5964954081
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(explore): preserve pending column configuration edits [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #44931:
URL: https://github.com/apache/superset/pull/44931#discussion_r4171550349
##########
superset-frontend/src/explore/components/controls/ColumnConfigControl/ControlForm/index.tsx:
##########
@@ -70,13 +72,27 @@ export default function ControlForm({
children,
}: ControlFormProps) {
const theme = useTheme();
+ const latestValue = useRef<JsonObject>(value ?? {});
+ const latestOnChange = useRef(onChange);
const debouncedOnChange = useMemo(
- () =>
- ({
- 0: onChange,
- [Constants.FAST_DEBOUNCE]: debounce(onChange, Constants.FAST_DEBOUNCE),
- }) as Record<number, typeof onChange>,
- [onChange],
+ () => ({}) as Record<number, DebouncedFunc<() => void>>,
+ [],
+ );
+
+ useEffect(() => {
+ latestOnChange.current = onChange;
+ }, [onChange]);
+
+ useEffect(() => {
+ latestValue.current = value ?? {};
+ Object.values(debouncedOnChange).forEach(callback => callback.cancel());
+ }, [value, debouncedOnChange]);
+
+ useEffect(
+ () => () => {
+ Object.values(debouncedOnChange).forEach(callback => callback.cancel());
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Duplicated cancel-all logic</b></div>
<div id="fix">
The cancel-all logic `Object.values(debouncedOnChange).forEach(callback => callback.cancel())` now appears three times: in the `value` sync effect, the unmount cleanup, and the `debounceDelay === 0` branch. Extract one `cancelAllPending` helper so a future scheduling site cannot forget a cancel and silently leak a pending edit.
</div>
</div>
<small><i>Code Review Run #d94eef</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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