codeant-ai-for-open-source[bot] commented on code in PR #43518:
URL: https://github.com/apache/superset/pull/43518#discussion_r3854656853
##########
superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx:
##########
@@ -495,76 +561,89 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> =
props => {
onChange: onDatePickerChange,
});
- useEffect(() => {
- const refreshComparatorSuggestions = () => {
- const { datasource } = props;
- const col = props.adhocFilter.subject;
- const having = props.adhocFilter.clause === Clauses.Having;
-
- if (col && datasource && datasource.filter_select && !having) {
- const controller = new AbortController();
- const { signal } = controller;
- if (loadingComparatorSuggestions) {
- controller.abort();
- }
- // Element-level array operators (Contains any / Contains all) search
- // inside the array, so suggest individual elements; whole-array
- // operators (=, In, โฆ) keep the default distinct-array suggestions.
- const { operatorId } = props.adhocFilter;
- const arrayElements =
- operatorId === Operators.ContainsAny ||
- operatorId === Operators.ContainsAll;
- setLoadingComparatorSuggestions(true);
- SupersetClient.get({
- signal,
- endpoint:
`/api/v1/datasource/${datasource.type}/${datasource.id}/column/${col}/values/${
- arrayElements ? '?array_elements=true' : ''
- }`,
- })
- .then(({ json }) => {
- setSuggestions(
- json.result.map((suggestion: unknown) => {
- // Complex column values arrive as JS arrays or objects: whole
- // arrays for MULTI_VALUE columns (e.g. [5, 6, 7]) and
Map/Tuple
- // objects for nested-container columns (e.g. {"a":
["x","y"]}).
- // A raw array/object is neither a valid single-select value
- // (antd collapses an array to its first element) nor
renderable
- // as a React child (an object throws). Render it as its
literal
- // string, which is also exactly what the backend's
- // parse_array_literal expects for the whole-array operators.
- if (suggestion !== null && typeof suggestion === 'object') {
- const literal = JSON.stringify(suggestion);
- return { value: literal, label: literal };
- }
- return {
- value: suggestion as null | number | boolean | string,
- label: optionLabel(
- suggestion as null | number | boolean | string,
- ),
- };
- }),
- );
- setLoadingComparatorSuggestions(false);
- })
- .catch(() => {
- setSuggestions([]);
- setLoadingComparatorSuggestions(false);
- });
+ // Element-level array operators (Contains any / Contains all) search inside
+ // the array, so suggest individual elements; whole-array operators (=, In,
โฆ)
+ // keep the default distinct-array suggestions.
+ const arrayElements =
+ props.adhocFilter.operatorId === Operators.ContainsAny ||
+ props.adhocFilter.operatorId === Operators.ContainsAll;
+
+ // AsyncSelect throws away every loaded option when the identity of its
+ // `options` callback changes, so this depends on plain values rather than on
+ // `props.datasource`, whose identity the parent does not guarantee.
+ const datasourceType = props.datasource?.type;
+ const datasourceId = props.datasource?.id;
+
+ const loadComparatorOptions = useCallback(
+ async (search: string): Promise<SelectOptionsTypePage> => {
+ const col = subjectString;
+ if (!col || !canSuggestComparatorValues) {
+ return { data: [], totalCount: 0 };
}
- };
- if (!datePicker) {
- refreshComparatorSuggestions();
- }
- // loadingComparatorSuggestions intentionally omitted - set inside effect,
would cause infinite loop
- // eslint-disable-next-line react-hooks/exhaustive-deps
- }, [
- props.adhocFilter.subject,
- props.adhocFilter.clause,
- props.adhocFilter.operatorId,
- props.datasource,
- datePicker,
- ]);
+ const params = new URLSearchParams();
+ if (arrayElements) {
+ params.set('array_elements', 'true');
+ }
+ if (search) {
+ params.set('q', search);
+ }
+ const query = params.toString();
+
+ try {
+ const { json } = await SupersetClient.get({
+ endpoint:
+ `/api/v1/datasource/${datasourceType}/${datasourceId}` +
+ `/column/${encodeURIComponent(col)}/values/${query ? `?${query}` :
''}`,
+ });
+ const data = json.result.map((suggestion: unknown) => {
+ // Complex column values arrive as JS arrays or objects: whole arrays
+ // for MULTI_VALUE columns (e.g. [5, 6, 7]) and Map/Tuple objects for
+ // nested-container columns (e.g. {"a": ["x","y"]}). A raw
+ // array/object is neither a valid single-select value (antd
collapses
+ // an array to its first element) nor renderable as a React child (an
+ // object throws). Render it as its literal string, which is also
+ // exactly what the backend's parse_array_literal expects for the
+ // whole-array operators.
+ if (suggestion !== null && typeof suggestion === 'object') {
+ const literal = JSON.stringify(suggestion);
+ return { value: literal, label: literal };
+ }
+ return {
+ value: suggestion as null | number | boolean | string,
+ label: optionLabel(suggestion as null | number | boolean | string),
+ };
+ });
+
+ setLoadedOptionCount(data.length);
+ setOptionsTruncated(isDefined(json.limit) && data.length >=
json.limit);
Review Comment:
**Suggestion:** An older request can resolve after a newer search and
overwrite `loadedOptionCount` and `optionsTruncated`, even though `AsyncSelect`
discards the stale option data. This can display the wrong truncation message
for the current search. Track the requested search term and only apply these
state updates when the response still matches it. [race condition]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ Explore comparator helper text can show stale counts.
- โ ๏ธ Truncation messaging may not match displayed search results.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7697716c732642dd978934b1b50f8072&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=7697716c732642dd978934b1b50f8072&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx
**Line:** 618:619
**Comment:**
*Race Condition: An older request can resolve after a newer search and
overwrite `loadedOptionCount` and `optionsTruncated`, even though `AsyncSelect`
discards the stale option data. This can display the wrong truncation message
for the current search. Track the requested search term and only apply these
state updates when the response still matches it.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43518&comment_hash=5e1cb45faa0eebf8a79782f2732fc8408d16f627ae5df5edc6d4ae883712cdd8&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43518&comment_hash=5e1cb45faa0eebf8a79782f2732fc8408d16f627ae5df5edc6d4ae883712cdd8&reaction=dislike'>๐</a>
##########
superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx:
##########
@@ -495,76 +561,89 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> =
props => {
onChange: onDatePickerChange,
});
- useEffect(() => {
- const refreshComparatorSuggestions = () => {
- const { datasource } = props;
- const col = props.adhocFilter.subject;
- const having = props.adhocFilter.clause === Clauses.Having;
-
- if (col && datasource && datasource.filter_select && !having) {
- const controller = new AbortController();
- const { signal } = controller;
- if (loadingComparatorSuggestions) {
- controller.abort();
- }
- // Element-level array operators (Contains any / Contains all) search
- // inside the array, so suggest individual elements; whole-array
- // operators (=, In, โฆ) keep the default distinct-array suggestions.
- const { operatorId } = props.adhocFilter;
- const arrayElements =
- operatorId === Operators.ContainsAny ||
- operatorId === Operators.ContainsAll;
- setLoadingComparatorSuggestions(true);
- SupersetClient.get({
- signal,
- endpoint:
`/api/v1/datasource/${datasource.type}/${datasource.id}/column/${col}/values/${
- arrayElements ? '?array_elements=true' : ''
- }`,
- })
- .then(({ json }) => {
- setSuggestions(
- json.result.map((suggestion: unknown) => {
- // Complex column values arrive as JS arrays or objects: whole
- // arrays for MULTI_VALUE columns (e.g. [5, 6, 7]) and
Map/Tuple
- // objects for nested-container columns (e.g. {"a":
["x","y"]}).
- // A raw array/object is neither a valid single-select value
- // (antd collapses an array to its first element) nor
renderable
- // as a React child (an object throws). Render it as its
literal
- // string, which is also exactly what the backend's
- // parse_array_literal expects for the whole-array operators.
- if (suggestion !== null && typeof suggestion === 'object') {
- const literal = JSON.stringify(suggestion);
- return { value: literal, label: literal };
- }
- return {
- value: suggestion as null | number | boolean | string,
- label: optionLabel(
- suggestion as null | number | boolean | string,
- ),
- };
- }),
- );
- setLoadingComparatorSuggestions(false);
- })
- .catch(() => {
- setSuggestions([]);
- setLoadingComparatorSuggestions(false);
- });
+ // Element-level array operators (Contains any / Contains all) search inside
+ // the array, so suggest individual elements; whole-array operators (=, In,
โฆ)
+ // keep the default distinct-array suggestions.
+ const arrayElements =
+ props.adhocFilter.operatorId === Operators.ContainsAny ||
+ props.adhocFilter.operatorId === Operators.ContainsAll;
+
+ // AsyncSelect throws away every loaded option when the identity of its
+ // `options` callback changes, so this depends on plain values rather than on
+ // `props.datasource`, whose identity the parent does not guarantee.
+ const datasourceType = props.datasource?.type;
+ const datasourceId = props.datasource?.id;
+
+ const loadComparatorOptions = useCallback(
+ async (search: string): Promise<SelectOptionsTypePage> => {
+ const col = subjectString;
+ if (!col || !canSuggestComparatorValues) {
+ return { data: [], totalCount: 0 };
}
- };
- if (!datePicker) {
- refreshComparatorSuggestions();
- }
- // loadingComparatorSuggestions intentionally omitted - set inside effect,
would cause infinite loop
- // eslint-disable-next-line react-hooks/exhaustive-deps
- }, [
- props.adhocFilter.subject,
- props.adhocFilter.clause,
- props.adhocFilter.operatorId,
- props.datasource,
- datePicker,
- ]);
+ const params = new URLSearchParams();
+ if (arrayElements) {
+ params.set('array_elements', 'true');
+ }
+ if (search) {
+ params.set('q', search);
+ }
+ const query = params.toString();
+
+ try {
+ const { json } = await SupersetClient.get({
+ endpoint:
+ `/api/v1/datasource/${datasourceType}/${datasourceId}` +
+ `/column/${encodeURIComponent(col)}/values/${query ? `?${query}` :
''}`,
+ });
+ const data = json.result.map((suggestion: unknown) => {
+ // Complex column values arrive as JS arrays or objects: whole arrays
+ // for MULTI_VALUE columns (e.g. [5, 6, 7]) and Map/Tuple objects for
+ // nested-container columns (e.g. {"a": ["x","y"]}). A raw
+ // array/object is neither a valid single-select value (antd
collapses
+ // an array to its first element) nor renderable as a React child (an
+ // object throws). Render it as its literal string, which is also
+ // exactly what the backend's parse_array_literal expects for the
+ // whole-array operators.
+ if (suggestion !== null && typeof suggestion === 'object') {
+ const literal = JSON.stringify(suggestion);
+ return { value: literal, label: literal };
+ }
+ return {
+ value: suggestion as null | number | boolean | string,
+ label: optionLabel(suggestion as null | number | boolean | string),
+ };
+ });
+
+ setLoadedOptionCount(data.length);
+ setOptionsTruncated(isDefined(json.limit) && data.length >=
json.limit);
+
+ // The count has to exceed what was returned. AsyncSelect treats
+ // `loaded >= totalCount` as "that is every value", sets
allValuesLoaded
+ // and from then on serves searches by filtering the loaded page
+ // client-side -- which is the behaviour this whole change exists to
+ // replace. Pagination is held off by COMPARATOR_PAGE_SIZE instead.
+ return { data, totalCount: data.length + 1 };
+ } catch {
+ setLoadedOptionCount(0);
+ setOptionsTruncated(false);
+ return { data: [], totalCount: 0 };
+ }
+ },
+ [
+ subjectString,
+ canSuggestComparatorValues,
+ datasourceType,
+ datasourceId,
+ arrayElements,
+ ],
+ );
+
+ // Options are cached per search term inside AsyncSelect; a different column
+ // or a switch to element-level suggestions invalidates all of them.
+ useEffect(() => {
+ comparatorSelectRef.current?.clearCache();
+ }, [subjectString, arrayElements]);
Review Comment:
**Suggestion:** Changing the subject or array-element mode clears
AsyncSelect's option cache but leaves `loadedOptionCount` and
`optionsTruncated` unchanged until the next request completes. During that
interval the new column displays the previous column's placeholder count and
capped-list helper text. Reset both pieces of state when invalidating the
cache. [stale reference]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ Explore filter shows the previous column's option count.
- โ ๏ธ Capped-list guidance can describe an unrelated column.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f650717ab82740c799bbedb159ac1d97&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f650717ab82740c799bbedb159ac1d97&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx
**Line:** 644:646
**Comment:**
*Stale Reference: Changing the subject or array-element mode clears
AsyncSelect's option cache but leaves `loadedOptionCount` and
`optionsTruncated` unchanged until the next request completes. During that
interval the new column displays the previous column's placeholder count and
capped-list helper text. Reset both pieces of state when invalidating the cache.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43518&comment_hash=8c093acfcb72f7e5590be7b0b039ec065ea4eac1038a18df104924b6f8bb4375&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43518&comment_hash=8c093acfcb72f7e5590be7b0b039ec065ea4eac1038a18df104924b6f8bb4375&reaction=dislike'>๐</a>
--
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]
