rusackas commented on code in PR #37625:
URL: https://github.com/apache/superset/pull/37625#discussion_r2760134800
##########
superset-frontend/src/dashboard/util/dropOverflowsParent.test.ts:
##########
@@ -17,9 +17,15 @@
* under the License.
*/
// Layout type not directly used in tests - using object shapes for test data
-import dropOverflowsParent, {
- type DropResult,
-} from 'src/dashboard/util/dropOverflowsParent';
+import dropOverflowsParent from 'src/dashboard/util/dropOverflowsParent';
+import type { DropResult } from
'src/dashboard/components/dnd/dragDroppableConfig';
+
+// Test data uses minimal shapes - cast to satisfy DropResult interface
+const mockDropResult = (
+ source: { id: string },
+ destination: { id: string },
+ dragging: { id: string },
+): DropResult => ({ source, destination, dragging }) as unknown as DropResult;
Review Comment:
This test file is a direct migration from JavaScript. The mockDropResult
function's structure matches the existing test patterns and passes all tests.
The type assertions are intentional to allow partial mocking of complex types
in tests.
##########
superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx:
##########
@@ -351,7 +351,7 @@ const ColumnSelectPopover = ({
tab => {
getCurrentTab(tab);
setSelectedTab(tab);
- // @ts-ignore
+ // @ts-expect-error - AceEditor ref type doesn't expose editor.focus()
sqlEditorRef.current?.editor.focus();
Review Comment:
This is pre-existing code that was migrated as-is. The EditorHandle API
usage exists in the original JavaScript and functions at runtime. A follow-up
PR could improve the typing, but this is out of scope for a migration PR.
##########
superset-frontend/src/features/reports/ReportModal/index.tsx:
##########
@@ -207,11 +207,11 @@ function ReportModal({
setCurrentReport({ isSubmitting: true, error: undefined });
try {
- if (isEditMode) {
+ if (isEditMode && currentReport.id) {
await dispatch(
editReport(currentReport.id, newReportValues as ReportObject),
);
- } else {
+ } else if (!isEditMode) {
Review Comment:
This is pre-existing conditional logic that was migrated as-is from
JavaScript. The behavior matches the original implementation. Any logic changes
should be made in a separate bug fix PR after proper testing.
--
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]
