codeant-ai-for-open-source[bot] commented on PR #36734: URL: https://github.com/apache/superset/pull/36734#issuecomment-3669773610
## 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/36734/files#diff-db83e65fa23a09ee5180a86cdc9c9161958a6ed7b5e7fbb588483c877d4f5541R468-R576'><strong>Missing request validation</strong></a><br>The new endpoints (create_join, update_join, update_table, update_column, generate_dashboard) accept and index into `request.json` without validating required fields or types. This can cause KeyError/TypeError or lead to ambiguous 500 errors instead of returning clear 400 validation errors.<br> - [ ] <a href='https://github.com/apache/superset/pull/36734/files#diff-db83e65fa23a09ee5180a86cdc9c9161958a6ed7b5e7fbb588483c877d4f5541R550-R558'><strong>Enum conversion errors</strong></a><br>The code calls `JoinType(data["join_type"])` and `Cardinality(data["cardinality"])` directly. Passing an invalid enum value will raise a ValueError leading to a 500 response. These should be validated and surfaced as 4xx errors with clear messages.<br> - [ ] <a href='https://github.com/apache/superset/pull/36734/files#diff-db83e65fa23a09ee5180a86cdc9c9161958a6ed7b5e7fbb588483c877d4f5541R294-R303'><strong>Inconsistent JSON handling</strong></a><br>The DB columns `source_columns` and `target_columns` are stored as JSON text. The create_join response returns the Python lists from the request, update_join response json.loads the DB field, but get_report returns the raw DB field directly. This inconsistency can lead to different types (string vs list) returned by different endpoints.<br> - [ ] <a href='https://github.com/apache/superset/pull/36734/files#diff-b759c453ed690e39b91e251719e30761a0bef19ec062fcbf61d5e72589e78b6dR19-R22'><strong>Possible Circular Import</strong></a><br>This file re-exports defaults and types from './JoinEditorModal'. If './JoinEditorModal' imports from this barrel (or another barrel that re-exports this file), a circular dependency can occur at runtime and cause values to be undefined. Verify that the re-export source files do not import from this barrel or break the module graph.<br> - [ ] <a href='https://github.com/apache/superset/pull/36734/files#diff-5584ae704abe49fa4a5ecbd0b52eaf8266acaef5c3f6b9ee3e84e88e7cb0aa2cR223-R256'><strong>Identifier ambiguity</strong></a><br>The Selects for source/target tables use table.name as the Select value (string). If two tables share the same name, or names change, mapping to table objects (ids and columns) will be ambiguous and error-prone. Prefer using the table id as the Select value and keep the name as label, or store both id and name to guarantee stable lookups.<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]
