codeant-ai-for-open-source[bot] commented on code in PR #39984:
URL: https://github.com/apache/superset/pull/39984#discussion_r3212165944
##########
superset-frontend/src/pages/ChartList/ChartList.listview.test.tsx:
##########
@@ -267,7 +267,7 @@ test('sorts table when clicking column headers', async ()
=> {
.filter(
call =>
call.url.includes('order_column') &&
- call.url.includes('last_saved_at'),
+ call.url.includes('changed_on_delta_humanized'),
Review Comment:
**Suggestion:** This matcher now counts both the initial page-load request
and the click-triggered sort request, because the initial chart fetch already
uses `order_column=changed_on_delta_humanized`. Keeping `toHaveLength(1)` makes
the test flaky/failing. Narrow the assertion to the latest call (or also assert
sort direction transition) instead of counting all matching calls. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Jest test `sorts table when clicking column headers` fails.
- ⚠️ CI for `superset-frontend/src/pages/ChartList` blocked by failing test.
- ⚠️ Sorting behavior for Last modified column not accurately verified.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In `superset-frontend/src/pages/ChartList/index.tsx:253`, `initialSort`
is set to `[{
id: 'changed_on_delta_humanized', desc: true }]`, which is passed as
`initialSort` into
the `ListView` component at `index.tsx:181`.
2. `ListView` delegates data loading to `useListViewState`
(`superset-frontend/src/components/ListView/utils.ts:206`), whose
`useEffect` at
`utils.ts:51-87` calls `fetchData({ pageIndex, pageSize, sortBy, filters })`
on mount,
with `sortBy[0].id === 'changed_on_delta_humanized'`.
3. `useListViewResource` in
`superset-frontend/src/views/CRUD/hooks.ts:73-118` builds the
query with `order_column: sortBy[0].id` and issues a GET to
`/api/v1/chart/?q=...`, so the
initial page-load request URL contains both `order_column` and
`changed_on_delta_humanized` (also asserted in `ChartList.test.tsx:162-167`
via
`expect(dataCalls[0].url).toContain('order_column:changed_on_delta_humanized,order_direction:desc,page:0,page_size:25')`).
4. When running `test('sorts table when clicking column headers', ...)` in
`ChartList.listview.test.tsx:219-235`, `renderChartList(mockUser)` triggers
the initial
fetch above, then clicking the "Last modified" header at
`ChartList.listview.test.tsx:22-23` causes a second fetch with the same
`order_column=changed_on_delta_humanized`; the filter at lines 269-270
(`call.url.includes('order_column') &&
call.url.includes('changed_on_delta_humanized')`)
therefore matches both the initial and the sort-triggered requests, so
`lastModifiedSortCalls` has length 2 and
`expect(lastModifiedSortCalls).toHaveLength(1)`
fails whenever this test runs.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fsrc%2Fpages%2FChartList%2FChartList.listview.test.tsx%0A%2A%2ALine%3A%2A%2A%20269%3A270%0A%2A%2AComment%3A%2A%2A%0A%09%2ALogic%20Error%3A%20This%20matcher%20now%20counts%20both%20the%20initial%20page-load%20request%20and%20the%20click-triggered%20sort%20request%2C%20because%20the%20initial%20chart%20fetch%20already%20uses%20%60order_column%3Dchanged_on_delta_humanized%60.%20Keeping%20%60toHaveLength%281%29%60%20makes%20the%20test%20flaky%2Ffailing.%20Narrow%20the%20assertion%20to%20the%20latest%20call%20%28or%20also%20assert%20sort%20direction%20transition%29%20instead%20of%20counting%20all%20matching%20calls.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20co
ncise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fsrc%2Fpages%2FChartList%2FChartList.listview.test.tsx%0A%2A%2ALine%3A%2A%2A%20269%3A270%0A%2A%2AComment%3A%2A%2A%0A%09%2ALogic%20Error%3A%20This%20matcher%20now%20counts%20both%20the%20initial%20page-load%20request%20and%20the%20click-triggered%20sort%20request%2C%20because%20the%20initial%20chart%20fetch%20already%20uses%20%60order_column%3Dchanged_on_delta_humanized%60.%20Keeping%20%60toHaveLength%281%29%60%20makes%20the%20test%20flaky%2Ffailing.%20Narrow%20th
e%20assertion%20to%20the%20latest%20call%20%28or%20also%20assert%20sort%20direction%20transition%29%20instead%20of%20counting%20all%20matching%20calls.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/pages/ChartList/ChartList.listview.test.tsx
**Line:** 269:270
**Comment:**
*Logic Error: This matcher now counts both the initial page-load
request and the click-triggered sort request, because the initial chart fetch
already uses `order_column=changed_on_delta_humanized`. Keeping
`toHaveLength(1)` makes the test flaky/failing. Narrow the assertion to the
latest call (or also assert sort direction transition) instead of counting all
matching calls.
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%2F39984&comment_hash=019209caf0c0785f1fd899a3b74b13a54a75371b0cbfbe9f5feb8fa70fa34279&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39984&comment_hash=019209caf0c0785f1fd899a3b74b13a54a75371b0cbfbe9f5feb8fa70fa34279&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]
