codeant-ai-for-open-source[bot] commented on PR #36831: URL: https://github.com/apache/superset/pull/36831#issuecomment-3690288334
## Nitpicks 🔍 <table> <tr><td>🔒 <strong>No security issues identified</strong></td></tr> <tr><td>⚡ <strong>Recommended areas for review</strong><br><br> - [ ] <a href='https://github.com/apache/superset/pull/36831/files#diff-5564b0f52907a96466688c65507b0b4da44dbb6e70d6b8f78a55aefbad846cf6R233-R256'><strong>Error handling / defensive response parsing</strong></a><br>loadDatasources directly maps response.json.result without validating response shape or handling request failures. Network or server errors or unexpected response shapes can cause runtime errors. Also, returning an empty, predictable structure on failure will make callers more robust.<br> - [ ] <a href='https://github.com/apache/superset/pull/36831/files#diff-5564b0f52907a96466688c65507b0b4da44dbb6e70d6b8f78a55aefbad846cf6R191-R199'><strong>Missing result guard</strong></a><br>componentDidMount assumes the dataset lookup returns at least one result and immediately reads r.data[0] and sets state. If the API returns an empty list (no match) this will set datasource to undefined and still show a success toast. The flow should verify a result exists and handle the "not found" / error cases (avoid showing a success toast when nothing was loaded).<br> - [ ] <a href='https://github.com/apache/superset/pull/36831/files#diff-9b8597679cdcd683673e676bdb27bb5de40ebead48cde3d9ed270c9059094c63R219-R245'><strong>Flaky override of window.location</strong></a><br>Tests override `window.location` using Object.defineProperty but restore it later. If an assertion or render throws, the restore code may not run, leaving global state mutated for subsequent tests and causing flakiness. Wrap overrides in try/finally or use utilities that restore globals automatically.<br> - [ ] <a href='https://github.com/apache/superset/pull/36831/files#diff-9b8597679cdcd683673e676bdb27bb5de40ebead48cde3d9ed270c9059094c63R266-R287'><strong>Special-character handling verification</strong></a><br>The test for special characters asserts only presence of `opr:eq` in any call. It should also assert that the encoded dataset name is included (or parse query params) to ensure correct behavior for URL-encoded characters.<br> - [ ] <a href='https://github.com/apache/superset/pull/36831/files#diff-9b8597679cdcd683673e676bdb27bb5de40ebead48cde3d9ed270c9059094c63R197-R216'><strong>Weak fetch assertion</strong></a><br>Several tests validate the fetch request by checking substrings in the raw URL (e.g., `opr:ct` / `opr:eq`). This can lead to false positives/negatives if multiple requests are made or encoding differs. Prefer parsing the request URL or using a regex that verifies both operator and encoded dataset value to make the assertion robust.<br> </td></tr> </table> -- 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]
