betodealmeida commented on a change in pull request #19314:
URL: https://github.com/apache/superset/pull/19314#discussion_r838845914
##########
File path:
superset-frontend/src/views/CRUD/data/database/DatabaseModal/ModalHeader.tsx
##########
@@ -142,19 +154,23 @@ const ModalHeader = ({
</StyledFormHeader>
);
+ const importDbHeader = (
+ <StyledStickyHeader>
+ <StyledFormHeader>
+ <p className="helper-top"> STEP 2 OF 2 </p>
+ <h4>Enter the required {dbModel.name} credentials</h4>
+ <p className="helper-bottom">{fileCheck ? file[0].name : ''}</p>
+ </StyledFormHeader>
+ </StyledStickyHeader>
+ );
+
+ if (fileCheck) return importDbHeader;
if (isLoading) return <></>;
Review comment:
`<></>;` looks like a fish
##########
File path:
superset-frontend/src/views/CRUD/data/database/DatabaseModal/ModalHeader.tsx
##########
@@ -142,19 +154,23 @@ const ModalHeader = ({
</StyledFormHeader>
);
+ const importDbHeader = (
+ <StyledStickyHeader>
+ <StyledFormHeader>
+ <p className="helper-top"> STEP 2 OF 2 </p>
+ <h4>Enter the required {dbModel.name} credentials</h4>
+ <p className="helper-bottom">{fileCheck ? file[0].name : ''}</p>
+ </StyledFormHeader>
+ </StyledStickyHeader>
+ );
+
+ if (fileCheck) return importDbHeader;
if (isLoading) return <></>;
- if (isEditMode) {
- return isEditHeader;
- }
- if (useSqlAlchemyForm) {
- return useSqlAlchemyFormHeader;
- }
- if (hasConnectedDb && !editNewDb) {
- return hasConnectedDbHeader;
- }
- if (db || editNewDb) {
- return hasDbHeader;
- }
+ if (isEditMode) return isEditHeader;
+ if (useSqlAlchemyForm) return useSqlAlchemyFormHeader;
+ if (hasConnectedDb && !editNewDb) return hasConnectedDbHeader;
+ if (db || editNewDb) return hasDbHeader;
Review comment:
We could also use a switch here:
```js
switch (true) {
fileCheck: return importDbHeader;
isLoading: return <></>;
isEditMode: return isEditHeader;
useSqlAlchemyForm: return useSqlAlchemyFormHeader;
hasConnectedDb && !editNewDb: return hasConnectedDbHeader;
db || editNewDb: return hasDbHeader;
default: noDbHeader;
}
```
##########
File path:
superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
##########
@@ -444,6 +469,11 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps>
= ({
const [editNewDb, setEditNewDb] = useState<boolean>(false);
const [isLoading, setLoading] = useState<boolean>(false);
const [testInProgress, setTestInProgress] = useState<boolean>(false);
+ const [passwords, setPasswords] = useState<Record<string, string>>({});
+ const [confirmedOverwrite, setConfirmedOverwrite] = useState<boolean>(false);
+ const [file, setFile] = useState<UploadFile[]>([]);
Review comment:
Can we rename this back to `fileList` and `setFileList`, so we know that
it should indeed be a list of files (even if it will have only one)? It also
matches the name of the prop in `<Upload/>`.
##########
File path: superset-frontend/src/views/CRUD/data/database/DatabaseList.test.jsx
##########
@@ -41,17 +37,6 @@ import { act } from 'react-dom/test-utils';
const mockStore = configureStore([thunk]);
const store = mockStore({});
-const mockAppState = {
Review comment:
My suggestion is, if we don't need it let's remove it.
##########
File path:
superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx
##########
@@ -814,6 +858,25 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps>
= ({
</>
);
}
+
+ // Import doesn't use db, so footer will not render in the if statement
above
Review comment:
Can you clarify what you mean with "Import doesn't use db"? Do you mean
the `db` state?
--
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: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
