You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/12/02 05:24:22 UTC

[GitHub] [incubator-superset] betodealmeida opened a new pull request #11884: feat: add modal to import databases

betodealmeida opened a new pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884


   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   
   This PR adds a new button to import databases from the UI if the `VERSIONED_EXPORT` feature flag is enabled. It allows users to import databases in the new format proposed in https://github.com/apache/incubator-superset/issues/11167.
   
   The biggest challenge in importing databases is that we export them without passwords, extra security information, nor certificates. When importing databases we can inspect the `sqlalchemy_uri` and see if it has a masked password, and if true we can prompt the user for the password.
   
   The second biggest challenge is that we don't know how many databases are present in the imported file, since users can export multiple databases at once in a single file. We also don't know how many of those require passwords, since not all databases use them.
   
   To work around this when the user tries to import a file the frontend will upload it to the backend. The backend performs validation on the file, and if there are databases with masked passwords it returns a validation error (422). The frontend will parse the error message, and if the only errors are from missing passwords it will prompt the user for the passwords and resubmit the file, this time with the associated passwords.
   
   This PR adds the backend validation, as well as the import modal. For the API calls I added a new method to `useSingleViewResource` called `importResource`, which will be reused for other imports (dataset, chart, dashboard and saved queries).
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   Will add.
   
   ### TEST PLAN
   <!--- What steps should be taken to verify the changes -->
   
   Manually tested importing a database, and confirmed it prompts for the password and the database is configured correctly.
   
   Also added tests.
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [X] Has associated issue: https://github.com/apache/incubator-superset/issues/11167
   - [ ] Changes UI
   - [ ] Requires DB Migration.
   - [ ] Confirm DB Migration upgrade and downgrade tested.
   - [X] Introduces new feature or API
   - [ ] Removes existing feature or API
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (4e7a189) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9c69679e14c2fa2b031c5c22b5cb95b640925176?el=desc) (9c69679) will **increase** coverage by `0.41%`.
   > The diff coverage is `58.71%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   + Coverage   67.48%   67.89%   +0.41%     
   ==========================================
     Files         916      919       +3     
     Lines       44487    45546    +1059     
     Branches     4227     4245      +18     
   ==========================================
   + Hits        30020    30923     +903     
   - Misses      14364    14520     +156     
     Partials      103      103              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.99% <31.57%> (-0.13%)` | :arrow_down: |
   | javascript | `62.82% <45.78%> (-0.10%)` | :arrow_down: |
   | python | `64.81% <100.00%> (+0.82%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `60.24% <13.04%> (-8.30%)` | :arrow_down: |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `83.83% <41.66%> (-5.82%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `61.70% <61.70%> (ø)` | |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `72.00% <100.00%> (ø)` | |
   | [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `89.06% <100.00%> (+1.31%)` | :arrow_up: |
   | [...uperset/databases/commands/importers/dispatcher.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy9kaXNwYXRjaGVyLnB5) | `80.64% <100.00%> (+1.33%)` | :arrow_up: |
   | [...perset/databases/commands/importers/v1/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy92MS9fX2luaXRfXy5weQ==) | `97.14% <100.00%> (+0.17%)` | :arrow_up: |
   | [superset/databases/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.97% <100.00%> (+0.02%)` | :arrow_up: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `89.78% <100.00%> (+0.04%)` | :arrow_up: |
   | ... and [31 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [9c69679...4e7a189](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (90569fd) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9c69679e14c2fa2b031c5c22b5cb95b640925176?el=desc) (9c69679) will **decrease** coverage by `1.15%`.
   > The diff coverage is `52.85%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   - Coverage   67.48%   66.32%   -1.16%     
   ==========================================
     Files         916      932      +16     
     Lines       44487    46214    +1727     
     Branches     4227     4749     +522     
   ==========================================
   + Hits        30020    30650     +630     
   - Misses      14364    15460    +1096     
   - Partials      103      104       +1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.85% <35.29%> (-0.26%)` | :arrow_down: |
   | javascript | `62.83% <47.31%> (-0.10%)` | :arrow_down: |
   | python | `63.84% <100.00%> (-0.14%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `55.40% <19.23%> (-13.14%)` | :arrow_down: |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `83.83% <41.66%> (-5.82%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `65.30% <65.30%> (ø)` | |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `72.00% <100.00%> (ø)` | |
   | [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `87.85% <100.00%> (+0.09%)` | :arrow_up: |
   | [...uperset/databases/commands/importers/dispatcher.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy9kaXNwYXRjaGVyLnB5) | `80.64% <100.00%> (+1.33%)` | :arrow_up: |
   | [...perset/databases/commands/importers/v1/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy92MS9fX2luaXRfXy5weQ==) | `97.14% <100.00%> (+0.17%)` | :arrow_up: |
   | [superset/databases/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.70% <100.00%> (-0.24%)` | :arrow_down: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `89.78% <100.00%> (+0.04%)` | :arrow_up: |
   | ... and [65 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [9c69679...90569fd](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (4e7a189) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9c69679e14c2fa2b031c5c22b5cb95b640925176?el=desc) (9c69679) will **decrease** coverage by `4.23%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   - Coverage   67.48%   63.24%   -4.24%     
   ==========================================
     Files         916      461     -455     
     Lines       44487    28385   -16102     
     Branches     4227        0    -4227     
   ==========================================
   - Hits        30020    17952   -12068     
   + Misses      14364    10433    -3931     
   + Partials      103        0     -103     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `?` | |
   | python | `63.24% <100.00%> (-0.74%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `83.80% <100.00%> (-3.95%)` | :arrow_down: |
   | [...uperset/databases/commands/importers/dispatcher.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy9kaXNwYXRjaGVyLnB5) | `80.64% <100.00%> (+1.33%)` | :arrow_up: |
   | [...perset/databases/commands/importers/v1/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy92MS9fX2luaXRfXy5weQ==) | `97.14% <100.00%> (+0.17%)` | :arrow_up: |
   | [superset/databases/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `95.58% <100.00%> (-4.42%)` | :arrow_down: |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `86.29% <100.00%> (-2.66%)` | :arrow_down: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `89.78% <100.00%> (+0.04%)` | :arrow_up: |
   | [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
   | [superset/databases/commands/create.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `32.65% <0.00%> (-59.19%)` | :arrow_down: |
   | [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `54.08% <0.00%> (-29.97%)` | :arrow_down: |
   | [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-22.81%)` | :arrow_down: |
   | ... and [506 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [9c69679...4e7a189](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] betodealmeida commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r535734483



##########
File path: superset-frontend/src/database/components/ImportModal/ImportModal.test.tsx
##########
@@ -0,0 +1,106 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import thunk from 'redux-thunk';
+import configureStore from 'redux-mock-store';
+import { styledMount as mount } from 'spec/helpers/theming';
+import { ReactWrapper } from 'enzyme';
+
+import ImportDatabaseModal from 'src/database/components/ImportModal';
+import Modal from 'src/common/components/Modal';
+
+const mockStore = configureStore([thunk]);
+const store = mockStore({});
+
+const requiredProps = {
+  addDangerToast: () => {},
+  addSuccessToast: () => {},
+  onDatabaseImport: () => {},
+  show: true,
+  onHide: () => {},
+};
+
+describe('ImportDatabaseModal', () => {
+  let wrapper: ReactWrapper;
+
+  beforeEach(() => {
+    wrapper = mount(<ImportDatabaseModal {...requiredProps} />, {
+      context: { store },
+    });
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('renders', () => {
+    expect(wrapper.find(ImportDatabaseModal)).toExist();
+  });
+
+  it('renders a Modal', () => {
+    expect(wrapper.find(Modal)).toExist();
+  });
+
+  it('renders "Import Database" header', () => {
+    expect(wrapper.find('h4').text()).toEqual('Import Database');
+  });
+
+  it('renders a label and a file input field', () => {
+    expect(wrapper.find('input[type="file"]')).toExist();
+    expect(wrapper.find('label')).toExist();
+  });
+
+  it('should attach the label to the input field', () => {
+    const id = 'databaseFile';
+    expect(wrapper.find('label').prop('htmlFor')).toBe(id);
+    expect(wrapper.find('input').prop('id')).toBe(id);
+  });
+
+  it('should render the close, import and cancel buttons', () => {
+    expect(wrapper.find('button')).toHaveLength(3);
+  });
+
+  it('should render the import button initially disabled', () => {
+    expect(wrapper.find('button[children="Import"]').prop('disabled')).toBe(
+      true,
+    );
+  });
+
+  it('should render the import button enabled when a file is selected', () => {
+    const file = new File([new ArrayBuffer(1)], 'database_export.zip');
+    wrapper.find('input').simulate('change', { target: { files: [file] } });
+
+    expect(wrapper.find('button[children="Import"]').prop('disabled')).toBe(
+      false,
+    );
+  });
+
+  it('should render password fields when needed for import', () => {
+    const wrapperWithPasswords = mount(
+      <ImportDatabaseModal
+        {...requiredProps}
+        passwordFields={['databases/examples.yaml']}
+      />,
+      {
+        context: { store },
+      },
+    );
+    expect(wrapperWithPasswords.find('input[type="password"]')).toExist();
+  });

Review comment:
       Talked with @eschutho, I'll try to add more tests in a future PR.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (91f3b14) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9c69679e14c2fa2b031c5c22b5cb95b640925176?el=desc) (9c69679) will **increase** coverage by `5.61%`.
   > The diff coverage is `45.78%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   + Coverage   67.48%   73.09%   +5.61%     
   ==========================================
     Files         916      466     -450     
     Lines       44487    16702   -27785     
     Branches     4227     4322      +95     
   ==========================================
   - Hits        30020    12208   -17812     
   + Misses      14364     4386    -9978     
   - Partials      103      108       +5     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.17% <31.57%> (-0.94%)` | :arrow_down: |
   | javascript | `63.05% <45.12%> (+0.13%)` | :arrow_up: |
   | python | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `61.90% <9.09%> (-6.63%)` | :arrow_down: |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `83.83% <41.66%> (-5.82%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `61.70% <61.70%> (ø)` | |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `72.00% <100.00%> (ø)` | |
   | [...et-frontend/src/SqlLab/reducers/getInitialState.js](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9yZWR1Y2Vycy9nZXRJbml0aWFsU3RhdGUuanM=) | `33.33% <0.00%> (-16.67%)` | :arrow_down: |
   | [superset-frontend/src/reduxUtils.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3JlZHV4VXRpbHMudHM=) | `70.88% <0.00%> (-8.87%)` | :arrow_down: |
   | [...rontend/src/SqlLab/components/TabbedSqlEditors.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1RhYmJlZFNxbEVkaXRvcnMuanN4) | `76.12% <0.00%> (-5.17%)` | :arrow_down: |
   | [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `60.25% <0.00%> (-4.28%)` | :arrow_down: |
   | [...rontend/src/SqlLab/components/SqlEditorLeftBar.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvckxlZnRCYXIuanN4) | `49.01% <0.00%> (-3.93%)` | :arrow_down: |
   | [superset-frontend/src/SqlLab/reducers/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9yZWR1Y2Vycy9zcWxMYWIuanM=) | `37.60% <0.00%> (-3.31%)` | :arrow_down: |
   | ... and [509 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [9c69679...4e7a189](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (90569fd) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9c69679e14c2fa2b031c5c22b5cb95b640925176?el=desc) (9c69679) will **decrease** coverage by `7.02%`.
   > The diff coverage is `50.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   - Coverage   67.48%   60.45%   -7.03%     
   ==========================================
     Files         916      881      -35     
     Lines       44487    43014    -1473     
     Branches     4227     3837     -390     
   ==========================================
   - Hits        30020    26005    -4015     
   - Misses      14364    17009    +2645     
   + Partials      103        0     -103     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.85% <35.29%> (-0.26%)` | :arrow_down: |
   | javascript | `?` | |
   | python | `63.42% <100.00%> (-0.57%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `54.21% <17.24%> (-14.32%)` | :arrow_down: |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `65.06% <36.36%> (-24.60%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `44.18% <44.18%> (ø)` | |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `69.11% <100.00%> (-2.89%)` | :arrow_down: |
   | [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `87.85% <100.00%> (+0.09%)` | :arrow_up: |
   | [...uperset/databases/commands/importers/dispatcher.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy9kaXNwYXRjaGVyLnB5) | `80.64% <100.00%> (+1.33%)` | :arrow_up: |
   | [...perset/databases/commands/importers/v1/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy92MS9fX2luaXRfXy5weQ==) | `97.14% <100.00%> (+0.17%)` | :arrow_up: |
   | [superset/databases/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.17% <100.00%> (-0.78%)` | :arrow_down: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `89.78% <100.00%> (+0.04%)` | :arrow_up: |
   | ... and [352 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [9c69679...90569fd](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] betodealmeida commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r534549578



##########
File path: superset-frontend/src/database/components/ImportModal/ImportModal.test.tsx
##########
@@ -0,0 +1,106 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import thunk from 'redux-thunk';
+import configureStore from 'redux-mock-store';
+import { styledMount as mount } from 'spec/helpers/theming';
+import { ReactWrapper } from 'enzyme';
+
+import ImportDatabaseModal from 'src/database/components/ImportModal';
+import Modal from 'src/common/components/Modal';
+
+const mockStore = configureStore([thunk]);
+const store = mockStore({});
+
+const requiredProps = {
+  addDangerToast: () => {},
+  addSuccessToast: () => {},
+  onDatabaseImport: () => {},
+  show: true,
+  onHide: () => {},
+};
+
+describe('ImportDatabaseModal', () => {
+  let wrapper: ReactWrapper;
+
+  beforeEach(() => {
+    wrapper = mount(<ImportDatabaseModal {...requiredProps} />, {
+      context: { store },

Review comment:
       Ah, good to know! Let me remove it.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] ktmud commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r535698248



##########
File path: superset-frontend/src/database/components/ImportModal/ImportModal.test.tsx
##########
@@ -0,0 +1,106 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import thunk from 'redux-thunk';
+import configureStore from 'redux-mock-store';
+import { styledMount as mount } from 'spec/helpers/theming';
+import { ReactWrapper } from 'enzyme';
+
+import ImportDatabaseModal from 'src/database/components/ImportModal';
+import Modal from 'src/common/components/Modal';
+
+const mockStore = configureStore([thunk]);
+const store = mockStore({});
+
+const requiredProps = {
+  addDangerToast: () => {},
+  addSuccessToast: () => {},
+  onDatabaseImport: () => {},
+  show: true,
+  onHide: () => {},
+};
+
+describe('ImportDatabaseModal', () => {
+  let wrapper: ReactWrapper;
+
+  beforeEach(() => {
+    wrapper = mount(<ImportDatabaseModal {...requiredProps} />, {
+      context: { store },
+    });
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('renders', () => {
+    expect(wrapper.find(ImportDatabaseModal)).toExist();
+  });
+
+  it('renders a Modal', () => {
+    expect(wrapper.find(Modal)).toExist();
+  });
+
+  it('renders "Import Database" header', () => {
+    expect(wrapper.find('h4').text()).toEqual('Import Database');
+  });
+
+  it('renders a label and a file input field', () => {
+    expect(wrapper.find('input[type="file"]')).toExist();
+    expect(wrapper.find('label')).toExist();
+  });
+
+  it('should attach the label to the input field', () => {
+    const id = 'databaseFile';
+    expect(wrapper.find('label').prop('htmlFor')).toBe(id);
+    expect(wrapper.find('input').prop('id')).toBe(id);
+  });
+
+  it('should render the close, import and cancel buttons', () => {
+    expect(wrapper.find('button')).toHaveLength(3);
+  });
+
+  it('should render the import button initially disabled', () => {
+    expect(wrapper.find('button[children="Import"]').prop('disabled')).toBe(
+      true,
+    );
+  });
+
+  it('should render the import button enabled when a file is selected', () => {
+    const file = new File([new ArrayBuffer(1)], 'database_export.zip');
+    wrapper.find('input').simulate('change', { target: { files: [file] } });
+
+    expect(wrapper.find('button[children="Import"]').prop('disabled')).toBe(
+      false,
+    );
+  });
+
+  it('should render password fields when needed for import', () => {
+    const wrapperWithPasswords = mount(
+      <ImportDatabaseModal
+        {...requiredProps}
+        passwordFields={['databases/examples.yaml']}
+      />,
+      {
+        context: { store },
+      },
+    );
+    expect(wrapperWithPasswords.find('input[type="password"]')).toExist();
+  });

Review comment:
       For custom hooks we may even use [react-hooks-testing-library](https://github.com/testing-library/react-hooks-testing-library). Feel free to add it to the infrastructure if you managed to get it work.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (90569fd) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9c69679e14c2fa2b031c5c22b5cb95b640925176?el=desc) (9c69679) will **decrease** coverage by `7.02%`.
   > The diff coverage is `50.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   - Coverage   67.48%   60.45%   -7.03%     
   ==========================================
     Files         916      881      -35     
     Lines       44487    43028    -1459     
     Branches     4227     3837     -390     
   ==========================================
   - Hits        30020    26011    -4009     
   - Misses      14364    17017    +2653     
   + Partials      103        0     -103     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.85% <35.29%> (-0.26%)` | :arrow_down: |
   | javascript | `?` | |
   | python | `63.41% <100.00%> (-0.58%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `54.21% <17.24%> (-14.32%)` | :arrow_down: |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `65.06% <36.36%> (-24.60%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `44.18% <44.18%> (ø)` | |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `69.11% <100.00%> (-2.89%)` | :arrow_down: |
   | [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `87.85% <100.00%> (+0.09%)` | :arrow_up: |
   | [...uperset/databases/commands/importers/dispatcher.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy9kaXNwYXRjaGVyLnB5) | `80.64% <100.00%> (+1.33%)` | :arrow_up: |
   | [...perset/databases/commands/importers/v1/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy92MS9fX2luaXRfXy5weQ==) | `97.14% <100.00%> (+0.17%)` | :arrow_up: |
   | [superset/databases/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.17% <100.00%> (-0.78%)` | :arrow_down: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `89.78% <100.00%> (+0.04%)` | :arrow_up: |
   | ... and [346 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [9c69679...90569fd](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (5fb717c) into [master](https://codecov.io/gh/apache/incubator-superset/commit/a7bba92469025a1f75149ff616af9dba24e81717?el=desc) (a7bba92) will **decrease** coverage by `1.03%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   - Coverage   64.45%   63.41%   -1.04%     
   ==========================================
     Files         937      465     -472     
     Lines       45319    28561   -16758     
     Branches     4317        0    -4317     
   ==========================================
   - Hits        29211    18113   -11098     
   + Misses      15948    10448    -5500     
   + Partials      160        0     -160     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `?` | |
   | python | `63.41% <100.00%> (-0.77%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `87.44% <100.00%> (-1.54%)` | :arrow_down: |
   | [...uperset/databases/commands/importers/dispatcher.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy9kaXNwYXRjaGVyLnB5) | `80.64% <100.00%> (+1.33%)` | :arrow_up: |
   | [...perset/databases/commands/importers/v1/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy92MS9fX2luaXRfXy5weQ==) | `97.14% <100.00%> (+0.17%)` | :arrow_up: |
   | [superset/databases/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `85.63% <100.00%> (-3.23%)` | :arrow_down: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `89.78% <100.00%> (+0.04%)` | :arrow_up: |
   | [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [superset/views/database/views.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.10% <0.00%> (-25.33%)` | :arrow_down: |
   | [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.59% <0.00%> (-12.25%)` | :arrow_down: |
   | [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `65.62% <0.00%> (-9.38%)` | :arrow_down: |
   | ... and [488 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [a7bba92...5fb717c](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (28a6489) into [master](https://codecov.io/gh/apache/incubator-superset/commit/a7bba92469025a1f75149ff616af9dba24e81717?el=desc) (a7bba92) will **decrease** coverage by `0.94%`.
   > The diff coverage is `60.16%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   - Coverage   64.45%   63.51%   -0.95%     
   ==========================================
     Files         937      938       +1     
     Lines       45319    45427     +108     
     Branches     4317     4358      +41     
   ==========================================
   - Hits        29211    28852     -359     
   - Misses      15948    16398     +450     
   - Partials      160      177      +17     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `62.91% <49.46%> (-0.08%)` | :arrow_down: |
   | python | `63.86% <100.00%> (-0.33%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `57.23% <20.00%> (-8.66%)` | :arrow_down: |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `75.75% <50.00%> (-3.56%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `65.30% <65.30%> (ø)` | |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `55.33% <100.00%> (ø)` | |
   | [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `89.06% <100.00%> (+0.08%)` | :arrow_up: |
   | [...uperset/databases/commands/importers/dispatcher.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy9kaXNwYXRjaGVyLnB5) | `80.64% <100.00%> (+1.33%)` | :arrow_up: |
   | [...perset/databases/commands/importers/v1/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy92MS9fX2luaXRfXy5weQ==) | `97.14% <100.00%> (+0.17%)` | :arrow_up: |
   | [superset/databases/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.61% <100.00%> (-0.25%)` | :arrow_down: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `89.78% <100.00%> (+0.04%)` | :arrow_up: |
   | ... and [53 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [a7bba92...5fb717c](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (1556c25) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9c69679e14c2fa2b031c5c22b5cb95b640925176?el=desc) (9c69679) will **decrease** coverage by `12.67%`.
   > The diff coverage is `31.57%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #11884       +/-   ##
   ===========================================
   - Coverage   67.48%   54.80%   -12.68%     
   ===========================================
     Files         916      420      -496     
     Lines       44487    14857    -29630     
     Branches     4227     3825      -402     
   ===========================================
   - Hits        30020     8142    -21878     
   + Misses      14364     6715     -7649     
   + Partials      103        0      -103     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.80% <31.57%> (-0.32%)` | :arrow_down: |
   | javascript | `?` | |
   | python | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `54.71% <9.09%> (-13.82%)` | :arrow_down: |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `65.06% <36.36%> (-24.60%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `39.02% <39.02%> (ø)` | |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `69.11% <100.00%> (-2.89%)` | :arrow_down: |
   | [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
   | [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
   | [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
   | ... and [783 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [9c69679...91f3b14](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] betodealmeida commented on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737005086


   cc: @eschutho 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (91f3b14) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9c69679e14c2fa2b031c5c22b5cb95b640925176?el=desc) (9c69679) will **increase** coverage by `5.94%`.
   > The diff coverage is `45.78%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   + Coverage   67.48%   73.42%   +5.94%     
   ==========================================
     Files         916      466     -450     
     Lines       44487    16702   -27785     
     Branches     4227     4322      +95     
   ==========================================
   - Hits        30020    12264   -17756     
   + Misses      14364     4334   -10030     
   - Partials      103      104       +1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.76% <31.57%> (-0.36%)` | :arrow_down: |
   | javascript | `63.05% <45.12%> (+0.13%)` | :arrow_up: |
   | python | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `61.90% <9.09%> (-6.63%)` | :arrow_down: |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `83.83% <41.66%> (-5.82%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `61.70% <61.70%> (ø)` | |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `72.00% <100.00%> (ø)` | |
   | [...src/dashboard/components/HeaderActionsDropdown.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlckFjdGlvbnNEcm9wZG93bi5qc3g=) | `70.27% <0.00%> (-3.26%)` | :arrow_down: |
   | [...d/src/dashboard/components/SliceHeaderControls.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1NsaWNlSGVhZGVyQ29udHJvbHMuanN4) | `61.64% <0.00%> (-2.54%)` | :arrow_down: |
   | [.../src/dashboard/components/RefreshIntervalModal.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1JlZnJlc2hJbnRlcnZhbE1vZGFsLmpzeA==) | `87.50% <0.00%> (-2.50%)` | :arrow_down: |
   | [...erset-frontend/src/components/DatabaseSelector.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGF0YWJhc2VTZWxlY3Rvci50c3g=) | `90.42% <0.00%> (-0.88%)` | :arrow_down: |
   | [superset-frontend/src/views/App.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0FwcC50c3g=) | `27.50% <0.00%> (-0.71%)` | :arrow_down: |
   | [...rc/explore/components/controls/AnnotationLayer.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Bbm5vdGF0aW9uTGF5ZXIuanN4) | `50.72% <0.00%> (-0.45%)` | :arrow_down: |
   | ... and [500 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [9c69679...4e7a189](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io commented on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (91f3b14) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9c69679e14c2fa2b031c5c22b5cb95b640925176?el=desc) (9c69679) will **decrease** coverage by `4.42%`.
   > The diff coverage is `45.12%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   - Coverage   67.48%   63.05%   -4.43%     
   ==========================================
     Files         916      466     -450     
     Lines       44487    16693   -27794     
     Branches     4227     4322      +95     
   ==========================================
   - Hits        30020    10526   -19494     
   + Misses      14364     5989    -8375     
   - Partials      103      178      +75     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `63.05% <45.12%> (+0.13%)` | :arrow_up: |
   | python | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `51.49% <9.52%> (-17.04%)` | :arrow_down: |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `73.73% <33.33%> (-15.92%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `61.70% <61.70%> (ø)` | |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `55.33% <100.00%> (-16.67%)` | :arrow_down: |
   | [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [654 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [9c69679...91f3b14](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] hughhhh commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r536839772



##########
File path: superset/databases/schemas.py
##########
@@ -429,3 +433,12 @@ class ImportV1DatabaseSchema(Schema):
     extra = fields.Nested(ImportV1DatabaseExtraSchema)
     uuid = fields.UUID(required=True)
     version = fields.String(required=True)
+
+    # pylint: disable=no-self-use, unused-argument
+    @validates_schema
+    def validate_password(self, data: Dict[str, Any], **kwargs: Any) -> None:
+        """If sqlalchemy_uri has a masked password, password is required"""
+        uri = data["sqlalchemy_uri"]

Review comment:
       will we always receive `sqlalchemy_uri` key from data?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (90569fd) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9c69679e14c2fa2b031c5c22b5cb95b640925176?el=desc) (9c69679) will **decrease** coverage by `7.06%`.
   > The diff coverage is `50.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   - Coverage   67.48%   60.41%   -7.07%     
   ==========================================
     Files         916      881      -35     
     Lines       44487    43014    -1473     
     Branches     4227     3837     -390     
   ==========================================
   - Hits        30020    25989    -4031     
   - Misses      14364    17025    +2661     
   + Partials      103        0     -103     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.85% <35.29%> (-0.26%)` | :arrow_down: |
   | javascript | `?` | |
   | python | `63.36% <100.00%> (-0.62%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `54.21% <17.24%> (-14.32%)` | :arrow_down: |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `65.06% <36.36%> (-24.60%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `44.18% <44.18%> (ø)` | |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `69.11% <100.00%> (-2.89%)` | :arrow_down: |
   | [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `87.85% <100.00%> (+0.09%)` | :arrow_up: |
   | [...uperset/databases/commands/importers/dispatcher.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy9kaXNwYXRjaGVyLnB5) | `80.64% <100.00%> (+1.33%)` | :arrow_up: |
   | [...perset/databases/commands/importers/v1/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy92MS9fX2luaXRfXy5weQ==) | `97.14% <100.00%> (+0.17%)` | :arrow_up: |
   | [superset/databases/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `87.36% <100.00%> (-1.59%)` | :arrow_down: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `89.78% <100.00%> (+0.04%)` | :arrow_up: |
   | ... and [354 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [9c69679...90569fd](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] betodealmeida commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r537680649



##########
File path: superset/databases/schemas.py
##########
@@ -429,3 +433,12 @@ class ImportV1DatabaseSchema(Schema):
     extra = fields.Nested(ImportV1DatabaseExtraSchema)
     uuid = fields.UUID(required=True)
     version = fields.String(required=True)
+
+    # pylint: disable=no-self-use, unused-argument
+    @validates_schema
+    def validate_password(self, data: Dict[str, Any], **kwargs: Any) -> None:
+        """If sqlalchemy_uri has a masked password, password is required"""
+        uri = data["sqlalchemy_uri"]

Review comment:
       Yes, if `sqlalchemy_uri` is not there the validator will raise an exception before calling this method, since we have:
   
   ```python
   sqlalchemy_uri = fields.String(required=True)
   ```




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (90569fd) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9c69679e14c2fa2b031c5c22b5cb95b640925176?el=desc) (9c69679) will **decrease** coverage by `7.25%`.
   > The diff coverage is `50.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   - Coverage   67.48%   60.22%   -7.26%     
   ==========================================
     Files         916      881      -35     
     Lines       44487    43014    -1473     
     Branches     4227     3837     -390     
   ==========================================
   - Hits        30020    25906    -4114     
   - Misses      14364    17108    +2744     
   + Partials      103        0     -103     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.85% <35.29%> (-0.26%)` | :arrow_down: |
   | javascript | `?` | |
   | python | `63.07% <100.00%> (-0.92%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `54.21% <17.24%> (-14.32%)` | :arrow_down: |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `65.06% <36.36%> (-24.60%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `44.18% <44.18%> (ø)` | |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `69.11% <100.00%> (-2.89%)` | :arrow_down: |
   | [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `82.59% <100.00%> (-5.17%)` | :arrow_down: |
   | [...uperset/databases/commands/importers/dispatcher.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy9kaXNwYXRjaGVyLnB5) | `80.64% <100.00%> (+1.33%)` | :arrow_up: |
   | [...perset/databases/commands/importers/v1/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy92MS9fX2luaXRfXy5weQ==) | `97.14% <100.00%> (+0.17%)` | :arrow_up: |
   | [superset/databases/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `95.58% <100.00%> (-4.42%)` | :arrow_down: |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `86.29% <100.00%> (-2.66%)` | :arrow_down: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `89.78% <100.00%> (+0.04%)` | :arrow_up: |
   | ... and [367 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [9c69679...90569fd](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] betodealmeida commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r537728471



##########
File path: superset-frontend/src/database/components/ImportModal/index.tsx
##########
@@ -0,0 +1,191 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { FunctionComponent, useEffect, useRef, useState } from 'react';
+import { t } from '@superset-ui/core';
+
+import Modal from 'src/common/components/Modal';
+import {
+  StyledIcon,
+  StyledInputContainer,
+} from 'src/views/CRUD/data/database/DatabaseModal';
+import { useImportResource } from 'src/views/CRUD/hooks';
+import { DatabaseObject } from 'src/views/CRUD/data/database/types';
+
+export interface ImportDatabaseModalProps {
+  addDangerToast: (msg: string) => void;
+  addSuccessToast: (msg: string) => void;
+  onDatabaseImport: () => void;
+  show: boolean;
+  onHide: () => void;
+  passwordFields?: string[];
+  setPasswordFields?: (passwordFields: string[]) => void;
+}
+
+const ImportDatabaseModal: FunctionComponent<ImportDatabaseModalProps> = ({
+  addDangerToast,
+  addSuccessToast,
+  onDatabaseImport,
+  show,
+  onHide,
+  passwordFields = [],
+  setPasswordFields = () => {},
+}) => {
+  const [uploadFile, setUploadFile] = useState<File | null>(null);
+  const [isHidden, setIsHidden] = useState<boolean>(true);
+  const [passwords, setPasswords] = useState<Record<string, string>>({});
+  const fileInputRef = useRef<HTMLInputElement>(null);
+
+  const clearModal = () => {
+    setUploadFile(null);
+    setPasswords({});
+    setPasswordFields([]);
+    if (fileInputRef && fileInputRef.current) {
+      fileInputRef.current.value = '';
+    }
+  };
+
+  const handleErrorMsg = (msg: string) => {
+    clearModal();
+    addDangerToast(msg);
+  };
+
+  const {
+    state: { passwordsNeeded },
+    importResource,
+  } = useImportResource<DatabaseObject>(
+    'database',
+    t('database'),
+    handleErrorMsg,
+  );
+
+  useEffect(() => {
+    setPasswordFields(passwordsNeeded);
+  }, [passwordsNeeded]);
+
+  // Functions
+  const hide = () => {
+    setIsHidden(true);
+    onHide();
+  };
+
+  const onUpload = () => {
+    if (uploadFile === null) {
+      return;
+    }
+
+    importResource(uploadFile, passwords).then(result => {
+      if (result) {
+        addSuccessToast(t('The databases have been imported'));
+        clearModal();
+        onDatabaseImport();
+      }
+    });
+  };
+
+  const changeFile = (event: React.ChangeEvent<HTMLInputElement>) => {
+    const { files } = event.target as HTMLInputElement;
+    setUploadFile((files && files[0]) || null);
+  };
+
+  const renderPasswordFields = () => {
+    if (passwordFields.length === 0) {
+      return null;
+    }
+
+    return (
+      <>
+        <h5>Passwords</h5>
+        <StyledInputContainer>
+          <div className="helper">
+            {t('Please provide the password for the databases below')}
+          </div>
+        </StyledInputContainer>
+        {passwordFields.map(fileName => (
+          <StyledInputContainer key={`password-for-${fileName}`}>
+            <div className="control-label">
+              {fileName}
+              <span className="required">*</span>
+            </div>
+            <input
+              name={`password-${fileName}`}
+              autoComplete="off"
+              type="password"
+              value={passwords[fileName]}
+              onChange={event =>
+                setPasswords({ ...passwords, [fileName]: event.target.value })

Review comment:
       I'll do that in https://github.com/apache/incubator-superset/pull/11936, so I only have to do it once.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] betodealmeida commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r534550530



##########
File path: superset-frontend/src/database/components/ImportModal/ImportModal.test.tsx
##########
@@ -0,0 +1,106 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import thunk from 'redux-thunk';
+import configureStore from 'redux-mock-store';
+import { styledMount as mount } from 'spec/helpers/theming';
+import { ReactWrapper } from 'enzyme';
+
+import ImportDatabaseModal from 'src/database/components/ImportModal';
+import Modal from 'src/common/components/Modal';
+
+const mockStore = configureStore([thunk]);
+const store = mockStore({});
+
+const requiredProps = {
+  addDangerToast: () => {},
+  addSuccessToast: () => {},
+  onDatabaseImport: () => {},
+  show: true,
+  onHide: () => {},
+};
+
+describe('ImportDatabaseModal', () => {
+  let wrapper: ReactWrapper;
+
+  beforeEach(() => {
+    wrapper = mount(<ImportDatabaseModal {...requiredProps} />, {
+      context: { store },
+    });
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('renders', () => {
+    expect(wrapper.find(ImportDatabaseModal)).toExist();
+  });
+
+  it('renders a Modal', () => {
+    expect(wrapper.find(Modal)).toExist();
+  });
+
+  it('renders "Import Database" header', () => {
+    expect(wrapper.find('h4').text()).toEqual('Import Database');
+  });
+
+  it('renders a label and a file input field', () => {
+    expect(wrapper.find('input[type="file"]')).toExist();
+    expect(wrapper.find('label')).toExist();
+  });
+
+  it('should attach the label to the input field', () => {
+    const id = 'databaseFile';
+    expect(wrapper.find('label').prop('htmlFor')).toBe(id);
+    expect(wrapper.find('input').prop('id')).toBe(id);
+  });
+
+  it('should render the close, import and cancel buttons', () => {
+    expect(wrapper.find('button')).toHaveLength(3);
+  });
+
+  it('should render the import button initially disabled', () => {
+    expect(wrapper.find('button[children="Import"]').prop('disabled')).toBe(
+      true,
+    );
+  });
+
+  it('should render the import button enabled when a file is selected', () => {
+    const file = new File([new ArrayBuffer(1)], 'database_export.zip');
+    wrapper.find('input').simulate('change', { target: { files: [file] } });
+
+    expect(wrapper.find('button[children="Import"]').prop('disabled')).toBe(
+      false,
+    );
+  });
+
+  it('should render password fields when needed for import', () => {
+    const wrapperWithPasswords = mount(
+      <ImportDatabaseModal
+        {...requiredProps}
+        passwordFields={['databases/examples.yaml']}
+      />,
+      {
+        context: { store },
+      },
+    );
+    expect(wrapperWithPasswords.find('input[type="password"]')).toExist();
+  });

Review comment:
       Yeah, let me take a look at that.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (90569fd) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9c69679e14c2fa2b031c5c22b5cb95b640925176?el=desc) (9c69679) will **decrease** coverage by `1.05%`.
   > The diff coverage is `52.85%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   - Coverage   67.48%   66.42%   -1.06%     
   ==========================================
     Files         916      932      +16     
     Lines       44487    46214    +1727     
     Branches     4227     4749     +522     
   ==========================================
   + Hits        30020    30696     +676     
   - Misses      14364    15414    +1050     
   - Partials      103      104       +1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.85% <35.29%> (-0.26%)` | :arrow_down: |
   | javascript | `62.83% <47.31%> (-0.10%)` | :arrow_down: |
   | python | `64.00% <100.00%> (+0.02%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `55.40% <19.23%> (-13.14%)` | :arrow_down: |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `83.83% <41.66%> (-5.82%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `65.30% <65.30%> (ø)` | |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `72.00% <100.00%> (ø)` | |
   | [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `87.85% <100.00%> (+0.09%)` | :arrow_up: |
   | [...uperset/databases/commands/importers/dispatcher.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy9kaXNwYXRjaGVyLnB5) | `80.64% <100.00%> (+1.33%)` | :arrow_up: |
   | [...perset/databases/commands/importers/v1/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy92MS9fX2luaXRfXy5weQ==) | `97.14% <100.00%> (+0.17%)` | :arrow_up: |
   | [superset/databases/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.97% <100.00%> (+0.02%)` | :arrow_up: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `89.78% <100.00%> (+0.04%)` | :arrow_up: |
   | ... and [60 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [9c69679...90569fd](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] eschutho commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r534445151



##########
File path: superset-frontend/src/database/components/ImportModal/ImportModal.test.tsx
##########
@@ -0,0 +1,106 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import thunk from 'redux-thunk';
+import configureStore from 'redux-mock-store';
+import { styledMount as mount } from 'spec/helpers/theming';
+import { ReactWrapper } from 'enzyme';
+
+import ImportDatabaseModal from 'src/database/components/ImportModal';
+import Modal from 'src/common/components/Modal';
+
+const mockStore = configureStore([thunk]);
+const store = mockStore({});
+
+const requiredProps = {
+  addDangerToast: () => {},
+  addSuccessToast: () => {},
+  onDatabaseImport: () => {},
+  show: true,
+  onHide: () => {},
+};
+
+describe('ImportDatabaseModal', () => {
+  let wrapper: ReactWrapper;
+
+  beforeEach(() => {
+    wrapper = mount(<ImportDatabaseModal {...requiredProps} />, {
+      context: { store },
+    });
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('renders', () => {
+    expect(wrapper.find(ImportDatabaseModal)).toExist();
+  });
+
+  it('renders a Modal', () => {
+    expect(wrapper.find(Modal)).toExist();
+  });
+
+  it('renders "Import Database" header', () => {
+    expect(wrapper.find('h4').text()).toEqual('Import Database');
+  });
+
+  it('renders a label and a file input field', () => {
+    expect(wrapper.find('input[type="file"]')).toExist();
+    expect(wrapper.find('label')).toExist();
+  });
+
+  it('should attach the label to the input field', () => {
+    const id = 'databaseFile';
+    expect(wrapper.find('label').prop('htmlFor')).toBe(id);
+    expect(wrapper.find('input').prop('id')).toBe(id);
+  });
+
+  it('should render the close, import and cancel buttons', () => {
+    expect(wrapper.find('button')).toHaveLength(3);
+  });
+
+  it('should render the import button initially disabled', () => {
+    expect(wrapper.find('button[children="Import"]').prop('disabled')).toBe(
+      true,
+    );
+  });
+
+  it('should render the import button enabled when a file is selected', () => {
+    const file = new File([new ArrayBuffer(1)], 'database_export.zip');
+    wrapper.find('input').simulate('change', { target: { files: [file] } });
+
+    expect(wrapper.find('button[children="Import"]').prop('disabled')).toBe(
+      false,
+    );
+  });
+
+  it('should render password fields when needed for import', () => {
+    const wrapperWithPasswords = mount(
+      <ImportDatabaseModal
+        {...requiredProps}
+        passwordFields={['databases/examples.yaml']}
+      />,
+      {
+        context: { store },
+      },
+    );
+    expect(wrapperWithPasswords.find('input[type="password"]')).toExist();
+  });

Review comment:
       where do you want to test some of the front end functional pieces? You could assert that prop function spies are being called and receiving the correct props. Maybe even `useSingleViewResource` although I haven't tried testing that yet. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] hughhhh commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r537716743



##########
File path: superset-frontend/src/database/components/ImportModal/index.tsx
##########
@@ -0,0 +1,191 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { FunctionComponent, useEffect, useRef, useState } from 'react';
+import { t } from '@superset-ui/core';
+
+import Modal from 'src/common/components/Modal';
+import {
+  StyledIcon,
+  StyledInputContainer,
+} from 'src/views/CRUD/data/database/DatabaseModal';
+import { useImportResource } from 'src/views/CRUD/hooks';
+import { DatabaseObject } from 'src/views/CRUD/data/database/types';
+
+export interface ImportDatabaseModalProps {
+  addDangerToast: (msg: string) => void;
+  addSuccessToast: (msg: string) => void;
+  onDatabaseImport: () => void;
+  show: boolean;
+  onHide: () => void;
+  passwordFields?: string[];
+  setPasswordFields?: (passwordFields: string[]) => void;
+}
+
+const ImportDatabaseModal: FunctionComponent<ImportDatabaseModalProps> = ({
+  addDangerToast,
+  addSuccessToast,
+  onDatabaseImport,
+  show,
+  onHide,
+  passwordFields = [],
+  setPasswordFields = () => {},
+}) => {
+  const [uploadFile, setUploadFile] = useState<File | null>(null);
+  const [isHidden, setIsHidden] = useState<boolean>(true);
+  const [passwords, setPasswords] = useState<Record<string, string>>({});
+  const fileInputRef = useRef<HTMLInputElement>(null);
+
+  const clearModal = () => {
+    setUploadFile(null);
+    setPasswords({});
+    setPasswordFields([]);
+    if (fileInputRef && fileInputRef.current) {
+      fileInputRef.current.value = '';
+    }
+  };
+
+  const handleErrorMsg = (msg: string) => {
+    clearModal();
+    addDangerToast(msg);
+  };
+
+  const {
+    state: { passwordsNeeded },
+    importResource,
+  } = useImportResource<DatabaseObject>(
+    'database',
+    t('database'),
+    handleErrorMsg,
+  );
+
+  useEffect(() => {
+    setPasswordFields(passwordsNeeded);
+  }, [passwordsNeeded]);
+
+  // Functions
+  const hide = () => {
+    setIsHidden(true);
+    onHide();
+  };
+
+  const onUpload = () => {
+    if (uploadFile === null) {
+      return;
+    }
+
+    importResource(uploadFile, passwords).then(result => {
+      if (result) {
+        addSuccessToast(t('The databases have been imported'));
+        clearModal();
+        onDatabaseImport();
+      }
+    });
+  };
+
+  const changeFile = (event: React.ChangeEvent<HTMLInputElement>) => {
+    const { files } = event.target as HTMLInputElement;
+    setUploadFile((files && files[0]) || null);
+  };
+
+  const renderPasswordFields = () => {
+    if (passwordFields.length === 0) {
+      return null;
+    }
+
+    return (
+      <>
+        <h5>Passwords</h5>
+        <StyledInputContainer>
+          <div className="helper">
+            {t('Please provide the password for the databases below')}
+          </div>
+        </StyledInputContainer>
+        {passwordFields.map(fileName => (
+          <StyledInputContainer key={`password-for-${fileName}`}>
+            <div className="control-label">
+              {fileName}
+              <span className="required">*</span>
+            </div>
+            <input
+              name={`password-${fileName}`}
+              autoComplete="off"
+              type="password"
+              value={passwords[fileName]}
+              onChange={event =>
+                setPasswords({ ...passwords, [fileName]: event.target.value })

Review comment:
       nit: put this in a handleStyleContainerInputChange function




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] betodealmeida commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r535727295



##########
File path: superset-frontend/src/views/CRUD/hooks.ts
##########
@@ -294,6 +305,55 @@ export function useSingleViewResource<D extends object = any>(
       });
   }, []);
 
+  const importResource = useCallback(
+    (bundle: File, databasePasswords: Record<string, string> = {}) => {
+      // Set loading state
+      updateState({
+        loading: true,
+      });
+
+      const formData = new FormData();
+      formData.append('formData', bundle);
+
+      /* The import bundle never contains database passwords; if required
+       * they should be provided by the user during import.
+       */
+      if (databasePasswords) {
+        formData.append('passwords', JSON.stringify(databasePasswords));
+      }
+
+      return SupersetClient.post({
+        endpoint: `/api/v1/${resourceName}/import/`,
+        body: formData,
+      })
+        .then(() => true)
+        .catch(response =>
+          getClientErrorObject(response).then(error => {
+            /* When importing a bundle, if all validation errors are because
+             * the databases need passwords we return a list of the database
+             * files so that the user can type in the passwords and resubmit
+             * the file.
+             */
+            const errMsg = error.message || error.error;
+            if (typeof errMsg !== 'string' && needsPassword(errMsg)) {
+              return Object.keys(errMsg);

Review comment:
       Thanks! I ended up implementing my own hook, since `useSingleViewResource` was not really appropriate in this case.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] betodealmeida commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r533909174



##########
File path: superset-frontend/src/views/CRUD/hooks.ts
##########
@@ -294,6 +305,55 @@ export function useSingleViewResource<D extends object = any>(
       });
   }, []);
 
+  const importResource = useCallback(
+    (bundle: File, databasePasswords: Record<string, string> = {}) => {
+      // Set loading state
+      updateState({
+        loading: true,
+      });
+
+      const formData = new FormData();
+      formData.append('formData', bundle);
+
+      /* The import bundle never contains database passwords; if required
+       * they should be provided by the user during import.
+       */
+      if (databasePasswords) {
+        formData.append('passwords', JSON.stringify(databasePasswords));
+      }
+
+      return SupersetClient.post({
+        endpoint: `/api/v1/${resourceName}/import/`,
+        body: formData,
+      })
+        .then(() => true)
+        .catch(response =>
+          getClientErrorObject(response).then(error => {
+            /* When importing a bundle, if all validation errors are because
+             * the databases need passwords we return a list of the database
+             * files so that the user can type in the passwords and resubmit
+             * the file.
+             */
+            const errMsg = error.message || error.error;
+            if (typeof errMsg !== 'string' && needsPassword(errMsg)) {
+              return Object.keys(errMsg);

Review comment:
       This feels a bit dirty: we return `true` if the import was successful, but `string[]` if passwords are needed. Any thoughts on how to make this better?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] hughhhh commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r537716743



##########
File path: superset-frontend/src/database/components/ImportModal/index.tsx
##########
@@ -0,0 +1,191 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { FunctionComponent, useEffect, useRef, useState } from 'react';
+import { t } from '@superset-ui/core';
+
+import Modal from 'src/common/components/Modal';
+import {
+  StyledIcon,
+  StyledInputContainer,
+} from 'src/views/CRUD/data/database/DatabaseModal';
+import { useImportResource } from 'src/views/CRUD/hooks';
+import { DatabaseObject } from 'src/views/CRUD/data/database/types';
+
+export interface ImportDatabaseModalProps {
+  addDangerToast: (msg: string) => void;
+  addSuccessToast: (msg: string) => void;
+  onDatabaseImport: () => void;
+  show: boolean;
+  onHide: () => void;
+  passwordFields?: string[];
+  setPasswordFields?: (passwordFields: string[]) => void;
+}
+
+const ImportDatabaseModal: FunctionComponent<ImportDatabaseModalProps> = ({
+  addDangerToast,
+  addSuccessToast,
+  onDatabaseImport,
+  show,
+  onHide,
+  passwordFields = [],
+  setPasswordFields = () => {},
+}) => {
+  const [uploadFile, setUploadFile] = useState<File | null>(null);
+  const [isHidden, setIsHidden] = useState<boolean>(true);
+  const [passwords, setPasswords] = useState<Record<string, string>>({});
+  const fileInputRef = useRef<HTMLInputElement>(null);
+
+  const clearModal = () => {
+    setUploadFile(null);
+    setPasswords({});
+    setPasswordFields([]);
+    if (fileInputRef && fileInputRef.current) {
+      fileInputRef.current.value = '';
+    }
+  };
+
+  const handleErrorMsg = (msg: string) => {
+    clearModal();
+    addDangerToast(msg);
+  };
+
+  const {
+    state: { passwordsNeeded },
+    importResource,
+  } = useImportResource<DatabaseObject>(
+    'database',
+    t('database'),
+    handleErrorMsg,
+  );
+
+  useEffect(() => {
+    setPasswordFields(passwordsNeeded);
+  }, [passwordsNeeded]);
+
+  // Functions
+  const hide = () => {
+    setIsHidden(true);
+    onHide();
+  };
+
+  const onUpload = () => {
+    if (uploadFile === null) {
+      return;
+    }
+
+    importResource(uploadFile, passwords).then(result => {
+      if (result) {
+        addSuccessToast(t('The databases have been imported'));
+        clearModal();
+        onDatabaseImport();
+      }
+    });
+  };
+
+  const changeFile = (event: React.ChangeEvent<HTMLInputElement>) => {
+    const { files } = event.target as HTMLInputElement;
+    setUploadFile((files && files[0]) || null);
+  };
+
+  const renderPasswordFields = () => {
+    if (passwordFields.length === 0) {
+      return null;
+    }
+
+    return (
+      <>
+        <h5>Passwords</h5>
+        <StyledInputContainer>
+          <div className="helper">
+            {t('Please provide the password for the databases below')}
+          </div>
+        </StyledInputContainer>
+        {passwordFields.map(fileName => (
+          <StyledInputContainer key={`password-for-${fileName}`}>
+            <div className="control-label">
+              {fileName}
+              <span className="required">*</span>
+            </div>
+            <input
+              name={`password-${fileName}`}
+              autoComplete="off"
+              type="password"
+              value={passwords[fileName]}
+              onChange={event =>
+                setPasswords({ ...passwords, [fileName]: event.target.value })

Review comment:
       put this in a handleStyleContainerInputChange function at the top for consistency




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] eschutho commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r534613826



##########
File path: superset-frontend/src/views/CRUD/hooks.ts
##########
@@ -294,6 +305,55 @@ export function useSingleViewResource<D extends object = any>(
       });
   }, []);
 
+  const importResource = useCallback(
+    (bundle: File, databasePasswords: Record<string, string> = {}) => {
+      // Set loading state
+      updateState({
+        loading: true,
+      });
+
+      const formData = new FormData();
+      formData.append('formData', bundle);
+
+      /* The import bundle never contains database passwords; if required
+       * they should be provided by the user during import.
+       */
+      if (databasePasswords) {
+        formData.append('passwords', JSON.stringify(databasePasswords));
+      }
+
+      return SupersetClient.post({
+        endpoint: `/api/v1/${resourceName}/import/`,
+        body: formData,
+      })
+        .then(() => true)
+        .catch(response =>
+          getClientErrorObject(response).then(error => {
+            /* When importing a bundle, if all validation errors are because
+             * the databases need passwords we return a list of the database
+             * files so that the user can type in the passwords and resubmit
+             * the file.
+             */
+            const errMsg = error.message || error.error;
+            if (typeof errMsg !== 'string' && needsPassword(errMsg)) {
+              return Object.keys(errMsg);

Review comment:
       it looks to me that the common usage is not to return something, but rather update state. In this case, you could have a state of errMsg for example, and then check to see that there are no errors.. so something like: 
   
   ```
   .then(
             () => {
               updateState({
                 errMsg: [],
               });
             },
             response => {
               getClientErrorObject(response).then(error => {
                 const errMsg = error.message || error.error;
                 if (typeof errMsg !== 'string' && needsPassword(errMsg)) {
                   updateState({ errMsg });
                   return;
                 }
                 handleErrorMsg( // you can update this callback to clear the modal too
                   t(
                     'An error occurred while importing %%s: %s',
                     resourceLabel,
                     JSON.stringify(errMsg),
                   ),
                 );
               });
             },
           )
   ```
   and then..
   ```
   importResource(uploadFile, passwords).then(() => {
         if (!errMsg.length) {
           // Success
           addSuccessToast(t('The databases have been imported'));
           clearModal();
           onDatabaseImport();
          return;
         if (errMsg.length) {
           // Need passwords
           setPasswordFields(errMsg);
         } 
       });
   ```
   just some ideas.. :)




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] eschutho commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r534442099



##########
File path: superset-frontend/src/database/components/ImportModal/ImportModal.test.tsx
##########
@@ -0,0 +1,106 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import thunk from 'redux-thunk';
+import configureStore from 'redux-mock-store';
+import { styledMount as mount } from 'spec/helpers/theming';
+import { ReactWrapper } from 'enzyme';
+
+import ImportDatabaseModal from 'src/database/components/ImportModal';
+import Modal from 'src/common/components/Modal';
+
+const mockStore = configureStore([thunk]);
+const store = mockStore({});
+
+const requiredProps = {
+  addDangerToast: () => {},
+  addSuccessToast: () => {},
+  onDatabaseImport: () => {},
+  show: true,
+  onHide: () => {},
+};
+
+describe('ImportDatabaseModal', () => {
+  let wrapper: ReactWrapper;
+
+  beforeEach(() => {
+    wrapper = mount(<ImportDatabaseModal {...requiredProps} />, {
+      context: { store },

Review comment:
       I tested it without passing a store, and it still passes if you want to remove redux-thunk. I'm not sure how much longer we'll be using that library.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] hughhhh commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r536839772



##########
File path: superset/databases/schemas.py
##########
@@ -429,3 +433,12 @@ class ImportV1DatabaseSchema(Schema):
     extra = fields.Nested(ImportV1DatabaseExtraSchema)
     uuid = fields.UUID(required=True)
     version = fields.String(required=True)
+
+    # pylint: disable=no-self-use, unused-argument
+    @validates_schema
+    def validate_password(self, data: Dict[str, Any], **kwargs: Any) -> None:
+        """If sqlalchemy_uri has a masked password, password is required"""
+        uri = data["sqlalchemy_uri"]

Review comment:
       will we always receive `sqlalchemy_uri` key from data? If not you can have a catch or use get with a blocking condition




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] hughhhh commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r537580308



##########
File path: superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx
##########
@@ -74,6 +75,21 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) {
   const [currentDatabase, setCurrentDatabase] = useState<DatabaseObject | null>(
     null,
   );
+  const [importingDatabase, showImportModal] = useState<boolean>(false);
+  const [passwordFields, setPasswordFields] = useState<string[]>([]);
+
+  function openDatabaseImportModal() {
+    showImportModal(true);
+  }
+
+  function closeDatabaseImportModal() {
+    showImportModal(false);
+  }

Review comment:
       ```suggestion
   const closeDatabaseImportModal = () => showImportModal(false)
   ```




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] hughhhh commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
hughhhh commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r537579358



##########
File path: superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx
##########
@@ -74,6 +75,21 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) {
   const [currentDatabase, setCurrentDatabase] = useState<DatabaseObject | null>(
     null,
   );
+  const [importingDatabase, showImportModal] = useState<boolean>(false);
+  const [passwordFields, setPasswordFields] = useState<string[]>([]);
+
+  function openDatabaseImportModal() {
+    showImportModal(true);
+  }

Review comment:
       ```suggestion
   const openDatabaseImportModal = () => (showImport(true));
   ```




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (5fb717c) into [master](https://codecov.io/gh/apache/incubator-superset/commit/a7bba92469025a1f75149ff616af9dba24e81717?el=desc) (a7bba92) will **decrease** coverage by `4.13%`.
   > The diff coverage is `51.81%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   - Coverage   64.45%   60.32%   -4.14%     
   ==========================================
     Files         937      891      -46     
     Lines       45319    43517    -1802     
     Branches     4317     3841     -476     
   ==========================================
   - Hits        29211    26252    -2959     
   - Misses      15948    17265    +1317     
   + Partials      160        0     -160     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.41% <37.64%> (+26.53%)` | :arrow_up: |
   | javascript | `?` | |
   | python | `63.42% <100.00%> (-0.77%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `60.40% <17.24%> (-5.49%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `44.18% <44.18%> (ø)` | |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `67.46% <54.54%> (-11.85%)` | :arrow_down: |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `69.11% <100.00%> (+13.78%)` | :arrow_up: |
   | [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `87.44% <100.00%> (-1.54%)` | :arrow_down: |
   | [...uperset/databases/commands/importers/dispatcher.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy9kaXNwYXRjaGVyLnB5) | `80.64% <100.00%> (+1.33%)` | :arrow_up: |
   | [...perset/databases/commands/importers/v1/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy92MS9fX2luaXRfXy5weQ==) | `97.14% <100.00%> (+0.17%)` | :arrow_up: |
   | [superset/databases/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `85.63% <100.00%> (-3.23%)` | :arrow_down: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `89.78% <100.00%> (+0.04%)` | :arrow_up: |
   | ... and [404 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [a7bba92...5fb717c](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] betodealmeida merged pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
betodealmeida merged pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (90569fd) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9c69679e14c2fa2b031c5c22b5cb95b640925176?el=desc) (9c69679) will **decrease** coverage by `1.42%`.
   > The diff coverage is `52.85%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   - Coverage   67.48%   66.05%   -1.43%     
   ==========================================
     Files         916      932      +16     
     Lines       44487    46214    +1727     
     Branches     4227     4749     +522     
   ==========================================
   + Hits        30020    30528     +508     
   - Misses      14364    15582    +1218     
   - Partials      103      104       +1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.85% <35.29%> (-0.26%)` | :arrow_down: |
   | javascript | `62.83% <47.31%> (-0.10%)` | :arrow_down: |
   | python | `63.41% <100.00%> (-0.58%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `55.40% <19.23%> (-13.14%)` | :arrow_down: |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `83.83% <41.66%> (-5.82%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `65.30% <65.30%> (ø)` | |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `72.00% <100.00%> (ø)` | |
   | [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `87.85% <100.00%> (+0.09%)` | :arrow_up: |
   | [...uperset/databases/commands/importers/dispatcher.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy9kaXNwYXRjaGVyLnB5) | `80.64% <100.00%> (+1.33%)` | :arrow_up: |
   | [...perset/databases/commands/importers/v1/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy92MS9fX2luaXRfXy5weQ==) | `97.14% <100.00%> (+0.17%)` | :arrow_up: |
   | [superset/databases/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.17% <100.00%> (-0.78%)` | :arrow_down: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `89.78% <100.00%> (+0.04%)` | :arrow_up: |
   | ... and [68 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [9c69679...90569fd](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] eschutho commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r534599989



##########
File path: superset-frontend/src/database/components/ImportModal/ImportModal.test.tsx
##########
@@ -0,0 +1,106 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import thunk from 'redux-thunk';
+import configureStore from 'redux-mock-store';
+import { styledMount as mount } from 'spec/helpers/theming';
+import { ReactWrapper } from 'enzyme';
+
+import ImportDatabaseModal from 'src/database/components/ImportModal';
+import Modal from 'src/common/components/Modal';
+
+const mockStore = configureStore([thunk]);
+const store = mockStore({});
+
+const requiredProps = {
+  addDangerToast: () => {},
+  addSuccessToast: () => {},
+  onDatabaseImport: () => {},
+  show: true,
+  onHide: () => {},
+};
+
+describe('ImportDatabaseModal', () => {
+  let wrapper: ReactWrapper;
+
+  beforeEach(() => {
+    wrapper = mount(<ImportDatabaseModal {...requiredProps} />, {
+      context: { store },
+    });
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('renders', () => {
+    expect(wrapper.find(ImportDatabaseModal)).toExist();
+  });
+
+  it('renders a Modal', () => {
+    expect(wrapper.find(Modal)).toExist();
+  });
+
+  it('renders "Import Database" header', () => {
+    expect(wrapper.find('h4').text()).toEqual('Import Database');
+  });
+
+  it('renders a label and a file input field', () => {
+    expect(wrapper.find('input[type="file"]')).toExist();
+    expect(wrapper.find('label')).toExist();
+  });
+
+  it('should attach the label to the input field', () => {
+    const id = 'databaseFile';
+    expect(wrapper.find('label').prop('htmlFor')).toBe(id);
+    expect(wrapper.find('input').prop('id')).toBe(id);
+  });
+
+  it('should render the close, import and cancel buttons', () => {
+    expect(wrapper.find('button')).toHaveLength(3);
+  });
+
+  it('should render the import button initially disabled', () => {
+    expect(wrapper.find('button[children="Import"]').prop('disabled')).toBe(
+      true,
+    );
+  });
+
+  it('should render the import button enabled when a file is selected', () => {
+    const file = new File([new ArrayBuffer(1)], 'database_export.zip');
+    wrapper.find('input').simulate('change', { target: { files: [file] } });
+
+    expect(wrapper.find('button[children="Import"]').prop('disabled')).toBe(
+      false,
+    );
+  });
+
+  it('should render password fields when needed for import', () => {
+    const wrapperWithPasswords = mount(
+      <ImportDatabaseModal
+        {...requiredProps}
+        passwordFields={['databases/examples.yaml']}
+      />,
+      {
+        context: { store },
+      },
+    );
+    expect(wrapperWithPasswords.find('input[type="password"]')).toExist();
+  });

Review comment:
       sounds like you're stuck in the battle between hooks and enzyme, and I agree about testing functional components being more complicated. Looks like @ktmud just landed the react-testing-library PR if you feel like giving that a shot. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (90569fd) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9c69679e14c2fa2b031c5c22b5cb95b640925176?el=desc) (9c69679) will **decrease** coverage by `13.20%`.
   > The diff coverage is `35.29%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #11884       +/-   ##
   ===========================================
   - Coverage   67.48%   54.27%   -13.21%     
   ===========================================
     Files         916      422      -494     
     Lines       44487    14893    -29594     
     Branches     4227     3837      -390     
   ===========================================
   - Hits        30020     8083    -21937     
   + Misses      14364     6810     -7554     
   + Partials      103        0      -103     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.27% <35.29%> (-0.85%)` | :arrow_down: |
   | javascript | `?` | |
   | python | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `54.21% <17.24%> (-14.32%)` | :arrow_down: |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `65.06% <36.36%> (-24.60%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `44.18% <44.18%> (ø)` | |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `69.11% <100.00%> (-2.89%)` | :arrow_down: |
   | [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
   | [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
   | [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
   | ... and [786 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [9c69679...90569fd](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (5fb717c) into [master](https://codecov.io/gh/apache/incubator-superset/commit/a7bba92469025a1f75149ff616af9dba24e81717?el=desc) (a7bba92) will **decrease** coverage by `3.93%`.
   > The diff coverage is `51.81%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   - Coverage   64.45%   60.52%   -3.94%     
   ==========================================
     Files         937      891      -46     
     Lines       45319    43517    -1802     
     Branches     4317     3841     -476     
   ==========================================
   - Hits        29211    26339    -2872     
   - Misses      15948    17178    +1230     
   + Partials      160        0     -160     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.99% <37.64%> (+27.11%)` | :arrow_up: |
   | javascript | `?` | |
   | python | `63.42% <100.00%> (-0.77%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `60.40% <17.24%> (-5.49%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `44.18% <44.18%> (ø)` | |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `67.46% <54.54%> (-11.85%)` | :arrow_down: |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `69.11% <100.00%> (+13.78%)` | :arrow_up: |
   | [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `87.44% <100.00%> (-1.54%)` | :arrow_down: |
   | [...uperset/databases/commands/importers/dispatcher.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy9kaXNwYXRjaGVyLnB5) | `80.64% <100.00%> (+1.33%)` | :arrow_up: |
   | [...perset/databases/commands/importers/v1/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy92MS9fX2luaXRfXy5weQ==) | `97.14% <100.00%> (+0.17%)` | :arrow_up: |
   | [superset/databases/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `85.63% <100.00%> (-3.23%)` | :arrow_down: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `89.78% <100.00%> (+0.04%)` | :arrow_up: |
   | ... and [405 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [a7bba92...5fb717c](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] betodealmeida commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r534783017



##########
File path: superset-frontend/src/database/components/ImportModal/index.tsx
##########
@@ -0,0 +1,185 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { FunctionComponent, useRef, useState } from 'react';
+import { t } from '@superset-ui/core';
+
+import Modal from 'src/common/components/Modal';
+import {
+  StyledIcon,
+  StyledInputContainer,
+} from 'src/views/CRUD/data/database/DatabaseModal';
+import { useSingleViewResource } from 'src/views/CRUD/hooks';
+import { DatabaseObject } from 'src/views/CRUD/data/database/types';
+
+export interface ImportDatabaseModalProps {
+  addDangerToast: (msg: string) => void;
+  addSuccessToast: (msg: string) => void;
+  onDatabaseImport: () => void;
+  show: boolean;
+  onHide: () => void;
+  passwordFields?: string[];
+  setPasswordFields?: (passwordFields: string[]) => void;
+}
+
+const ImportDatabaseModal: FunctionComponent<ImportDatabaseModalProps> = ({
+  addDangerToast,
+  addSuccessToast,
+  onDatabaseImport,
+  show,
+  onHide,
+  passwordFields = [],
+  setPasswordFields = () => {},
+}) => {
+  const [uploadFile, setUploadFile] = useState<File | null>(null);
+  const [isHidden, setIsHidden] = useState<boolean>(true);
+  const [passwords, setPasswords] = useState<Record<string, string>>({});
+  const fileInputRef = useRef<HTMLInputElement>(null);
+
+  const { importResource } = useSingleViewResource<DatabaseObject>(
+    'database',
+    t('database'),
+    addDangerToast,
+  );
+
+  // Functions
+  const hide = () => {
+    setIsHidden(true);
+    onHide();
+  };
+
+  const clearModal = () => {
+    setUploadFile(null);
+    setPasswordFields([]);
+    setPasswords({});
+    if (fileInputRef && fileInputRef.current) {
+      fileInputRef.current.value = '';
+    }
+  };
+
+  const onUpload = () => {
+    if (uploadFile === null) {
+      return;
+    }
+
+    importResource(uploadFile, passwords).then(result => {
+      if (result === true) {
+        // Success
+        addSuccessToast(t('The databases have been imported'));
+        clearModal();
+        onDatabaseImport();
+      } else if (result) {
+        // Need passwords
+        setPasswordFields(result);
+      } else {
+        // Failure
+        clearModal();
+      }
+    });
+  };
+
+  const changeFile = (event: React.ChangeEvent<HTMLInputElement>) => {
+    const { files } = event.target as HTMLInputElement;
+    setUploadFile((files && files[0]) || null);
+  };
+
+  const renderPasswordFields = () => {
+    if (passwordFields.length === 0) {
+      return null;
+    }
+
+    return (
+      <>
+        <h5>Passwords</h5>
+        <StyledInputContainer>
+          <div className="helper">
+            {t('Please provide the password for the databases below')}
+          </div>
+        </StyledInputContainer>
+        {passwordFields.map(fileName => (
+          <StyledInputContainer key={`password-for-${fileName}`}>
+            <div className="control-label">
+              {fileName}
+              <span className="required">*</span>
+            </div>
+            <input

Review comment:
       That would be good! How do I do that?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (1556c25) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9c69679e14c2fa2b031c5c22b5cb95b640925176?el=desc) (9c69679) will **decrease** coverage by `13.26%`.
   > The diff coverage is `31.57%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #11884       +/-   ##
   ===========================================
   - Coverage   67.48%   54.21%   -13.27%     
   ===========================================
     Files         916      420      -496     
     Lines       44487    14857    -29630     
     Branches     4227     3825      -402     
   ===========================================
   - Hits        30020     8055    -21965     
   + Misses      14364     6802     -7562     
   + Partials      103        0      -103     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.21% <31.57%> (-0.90%)` | :arrow_down: |
   | javascript | `?` | |
   | python | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `54.71% <9.09%> (-13.82%)` | :arrow_down: |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `65.06% <36.36%> (-24.60%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `39.02% <39.02%> (ø)` | |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `69.11% <100.00%> (-2.89%)` | :arrow_down: |
   | [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
   | [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
   | [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
   | ... and [784 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [9c69679...91f3b14](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] eschutho commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r534590830



##########
File path: superset-frontend/src/database/components/ImportModal/index.tsx
##########
@@ -0,0 +1,185 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { FunctionComponent, useRef, useState } from 'react';
+import { t } from '@superset-ui/core';
+
+import Modal from 'src/common/components/Modal';
+import {
+  StyledIcon,
+  StyledInputContainer,
+} from 'src/views/CRUD/data/database/DatabaseModal';
+import { useSingleViewResource } from 'src/views/CRUD/hooks';
+import { DatabaseObject } from 'src/views/CRUD/data/database/types';
+
+export interface ImportDatabaseModalProps {
+  addDangerToast: (msg: string) => void;
+  addSuccessToast: (msg: string) => void;
+  onDatabaseImport: () => void;
+  show: boolean;
+  onHide: () => void;
+  passwordFields?: string[];
+  setPasswordFields?: (passwordFields: string[]) => void;
+}
+
+const ImportDatabaseModal: FunctionComponent<ImportDatabaseModalProps> = ({
+  addDangerToast,
+  addSuccessToast,
+  onDatabaseImport,
+  show,
+  onHide,
+  passwordFields = [],
+  setPasswordFields = () => {},
+}) => {
+  const [uploadFile, setUploadFile] = useState<File | null>(null);
+  const [isHidden, setIsHidden] = useState<boolean>(true);
+  const [passwords, setPasswords] = useState<Record<string, string>>({});
+  const fileInputRef = useRef<HTMLInputElement>(null);
+
+  const { importResource } = useSingleViewResource<DatabaseObject>(
+    'database',
+    t('database'),
+    addDangerToast,
+  );
+
+  // Functions
+  const hide = () => {
+    setIsHidden(true);
+    onHide();
+  };
+
+  const clearModal = () => {
+    setUploadFile(null);
+    setPasswordFields([]);
+    setPasswords({});
+    if (fileInputRef && fileInputRef.current) {
+      fileInputRef.current.value = '';
+    }
+  };
+
+  const onUpload = () => {
+    if (uploadFile === null) {
+      return;
+    }
+
+    importResource(uploadFile, passwords).then(result => {
+      if (result === true) {
+        // Success
+        addSuccessToast(t('The databases have been imported'));
+        clearModal();
+        onDatabaseImport();
+      } else if (result) {
+        // Need passwords
+        setPasswordFields(result);
+      } else {
+        // Failure
+        clearModal();
+      }
+    });
+  };
+
+  const changeFile = (event: React.ChangeEvent<HTMLInputElement>) => {
+    const { files } = event.target as HTMLInputElement;
+    setUploadFile((files && files[0]) || null);
+  };
+
+  const renderPasswordFields = () => {
+    if (passwordFields.length === 0) {
+      return null;
+    }
+
+    return (
+      <>
+        <h5>Passwords</h5>
+        <StyledInputContainer>
+          <div className="helper">
+            {t('Please provide the password for the databases below')}
+          </div>
+        </StyledInputContainer>
+        {passwordFields.map(fileName => (
+          <StyledInputContainer key={`password-for-${fileName}`}>
+            <div className="control-label">
+              {fileName}
+              <span className="required">*</span>
+            </div>
+            <input

Review comment:
       do you want to turn off autocomplete on this just in case?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] betodealmeida commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r534556874



##########
File path: superset-frontend/src/database/components/ImportModal/ImportModal.test.tsx
##########
@@ -0,0 +1,106 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import thunk from 'redux-thunk';
+import configureStore from 'redux-mock-store';
+import { styledMount as mount } from 'spec/helpers/theming';
+import { ReactWrapper } from 'enzyme';
+
+import ImportDatabaseModal from 'src/database/components/ImportModal';
+import Modal from 'src/common/components/Modal';
+
+const mockStore = configureStore([thunk]);
+const store = mockStore({});
+
+const requiredProps = {
+  addDangerToast: () => {},
+  addSuccessToast: () => {},
+  onDatabaseImport: () => {},
+  show: true,
+  onHide: () => {},
+};
+
+describe('ImportDatabaseModal', () => {
+  let wrapper: ReactWrapper;
+
+  beforeEach(() => {
+    wrapper = mount(<ImportDatabaseModal {...requiredProps} />, {
+      context: { store },
+    });
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('renders', () => {
+    expect(wrapper.find(ImportDatabaseModal)).toExist();
+  });
+
+  it('renders a Modal', () => {
+    expect(wrapper.find(Modal)).toExist();
+  });
+
+  it('renders "Import Database" header', () => {
+    expect(wrapper.find('h4').text()).toEqual('Import Database');
+  });
+
+  it('renders a label and a file input field', () => {
+    expect(wrapper.find('input[type="file"]')).toExist();
+    expect(wrapper.find('label')).toExist();
+  });
+
+  it('should attach the label to the input field', () => {
+    const id = 'databaseFile';
+    expect(wrapper.find('label').prop('htmlFor')).toBe(id);
+    expect(wrapper.find('input').prop('id')).toBe(id);
+  });
+
+  it('should render the close, import and cancel buttons', () => {
+    expect(wrapper.find('button')).toHaveLength(3);
+  });
+
+  it('should render the import button initially disabled', () => {
+    expect(wrapper.find('button[children="Import"]').prop('disabled')).toBe(
+      true,
+    );
+  });
+
+  it('should render the import button enabled when a file is selected', () => {
+    const file = new File([new ArrayBuffer(1)], 'database_export.zip');
+    wrapper.find('input').simulate('change', { target: { files: [file] } });
+
+    expect(wrapper.find('button[children="Import"]').prop('disabled')).toBe(
+      false,
+    );
+  });
+
+  it('should render password fields when needed for import', () => {
+    const wrapperWithPasswords = mount(
+      <ImportDatabaseModal
+        {...requiredProps}
+        passwordFields={['databases/examples.yaml']}
+      />,
+      {
+        context: { store },
+      },
+    );
+    expect(wrapperWithPasswords.find('input[type="password"]')).toExist();
+  });

Review comment:
       Yesterday I tried testing the flow by mocking the network requests, but I wasn't able to mock `useState` correctly to replace `useSingleViewResource`. Mocking in functional components is more complicated, and from what I read people seemed to suggest testing only by changing the props and testing that it behaves as expected.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (5fb717c) into [master](https://codecov.io/gh/apache/incubator-superset/commit/a7bba92469025a1f75149ff616af9dba24e81717?el=desc) (a7bba92) will **decrease** coverage by `1.03%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   - Coverage   64.45%   63.42%   -1.04%     
   ==========================================
     Files         937      465     -472     
     Lines       45319    28563   -16756     
     Branches     4317        0    -4317     
   ==========================================
   - Hits        29211    18115   -11096     
   + Misses      15948    10448    -5500     
   + Partials      160        0     -160     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `?` | |
   | python | `63.42% <100.00%> (-0.77%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `87.44% <100.00%> (-1.54%)` | :arrow_down: |
   | [...uperset/databases/commands/importers/dispatcher.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy9kaXNwYXRjaGVyLnB5) | `80.64% <100.00%> (+1.33%)` | :arrow_up: |
   | [...perset/databases/commands/importers/v1/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy92MS9fX2luaXRfXy5weQ==) | `97.14% <100.00%> (+0.17%)` | :arrow_up: |
   | [superset/databases/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `85.63% <100.00%> (-3.23%)` | :arrow_down: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `89.78% <100.00%> (+0.04%)` | :arrow_up: |
   | [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [superset/views/database/views.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.30% <0.00%> (-25.14%)` | :arrow_down: |
   | [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.59% <0.00%> (-12.25%)` | :arrow_down: |
   | [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `65.62% <0.00%> (-9.38%)` | :arrow_down: |
   | ... and [487 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [a7bba92...5fb717c](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (90569fd) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9c69679e14c2fa2b031c5c22b5cb95b640925176?el=desc) (9c69679) will **decrease** coverage by `1.16%`.
   > The diff coverage is `52.85%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   - Coverage   67.48%   66.31%   -1.17%     
   ==========================================
     Files         916      932      +16     
     Lines       44487    46214    +1727     
     Branches     4227     4749     +522     
   ==========================================
   + Hits        30020    30649     +629     
   - Misses      14364    15461    +1097     
   - Partials      103      104       +1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.85% <35.29%> (-0.26%)` | :arrow_down: |
   | javascript | `62.83% <47.31%> (-0.10%)` | :arrow_down: |
   | python | `63.84% <100.00%> (-0.15%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `55.40% <19.23%> (-13.14%)` | :arrow_down: |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `83.83% <41.66%> (-5.82%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `65.30% <65.30%> (ø)` | |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `72.00% <100.00%> (ø)` | |
   | [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `87.85% <100.00%> (+0.09%)` | :arrow_up: |
   | [...uperset/databases/commands/importers/dispatcher.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy9kaXNwYXRjaGVyLnB5) | `80.64% <100.00%> (+1.33%)` | :arrow_up: |
   | [...perset/databases/commands/importers/v1/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy92MS9fX2luaXRfXy5weQ==) | `97.14% <100.00%> (+0.17%)` | :arrow_up: |
   | [superset/databases/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.70% <100.00%> (-0.24%)` | :arrow_down: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `89.78% <100.00%> (+0.04%)` | :arrow_up: |
   | ... and [66 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [9c69679...90569fd](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (90569fd) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9c69679e14c2fa2b031c5c22b5cb95b640925176?el=desc) (9c69679) will **decrease** coverage by `12.62%`.
   > The diff coverage is `35.29%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #11884       +/-   ##
   ===========================================
   - Coverage   67.48%   54.85%   -12.63%     
   ===========================================
     Files         916      422      -494     
     Lines       44487    14893    -29594     
     Branches     4227     3837      -390     
   ===========================================
   - Hits        30020     8170    -21850     
   + Misses      14364     6723     -7641     
   + Partials      103        0      -103     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.85% <35.29%> (-0.26%)` | :arrow_down: |
   | javascript | `?` | |
   | python | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `54.21% <17.24%> (-14.32%)` | :arrow_down: |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `65.06% <36.36%> (-24.60%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `44.18% <44.18%> (ø)` | |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `69.11% <100.00%> (-2.89%)` | :arrow_down: |
   | [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
   | [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
   | [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
   | ... and [785 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [9c69679...90569fd](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] eschutho commented on a change in pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#discussion_r535441472



##########
File path: superset-frontend/src/database/components/ImportModal/index.tsx
##########
@@ -0,0 +1,185 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { FunctionComponent, useRef, useState } from 'react';
+import { t } from '@superset-ui/core';
+
+import Modal from 'src/common/components/Modal';
+import {
+  StyledIcon,
+  StyledInputContainer,
+} from 'src/views/CRUD/data/database/DatabaseModal';
+import { useSingleViewResource } from 'src/views/CRUD/hooks';
+import { DatabaseObject } from 'src/views/CRUD/data/database/types';
+
+export interface ImportDatabaseModalProps {
+  addDangerToast: (msg: string) => void;
+  addSuccessToast: (msg: string) => void;
+  onDatabaseImport: () => void;
+  show: boolean;
+  onHide: () => void;
+  passwordFields?: string[];
+  setPasswordFields?: (passwordFields: string[]) => void;
+}
+
+const ImportDatabaseModal: FunctionComponent<ImportDatabaseModalProps> = ({
+  addDangerToast,
+  addSuccessToast,
+  onDatabaseImport,
+  show,
+  onHide,
+  passwordFields = [],
+  setPasswordFields = () => {},
+}) => {
+  const [uploadFile, setUploadFile] = useState<File | null>(null);
+  const [isHidden, setIsHidden] = useState<boolean>(true);
+  const [passwords, setPasswords] = useState<Record<string, string>>({});
+  const fileInputRef = useRef<HTMLInputElement>(null);
+
+  const { importResource } = useSingleViewResource<DatabaseObject>(
+    'database',
+    t('database'),
+    addDangerToast,
+  );
+
+  // Functions
+  const hide = () => {
+    setIsHidden(true);
+    onHide();
+  };
+
+  const clearModal = () => {
+    setUploadFile(null);
+    setPasswordFields([]);
+    setPasswords({});
+    if (fileInputRef && fileInputRef.current) {
+      fileInputRef.current.value = '';
+    }
+  };
+
+  const onUpload = () => {
+    if (uploadFile === null) {
+      return;
+    }
+
+    importResource(uploadFile, passwords).then(result => {
+      if (result === true) {
+        // Success
+        addSuccessToast(t('The databases have been imported'));
+        clearModal();
+        onDatabaseImport();
+      } else if (result) {
+        // Need passwords
+        setPasswordFields(result);
+      } else {
+        // Failure
+        clearModal();
+      }
+    });
+  };
+
+  const changeFile = (event: React.ChangeEvent<HTMLInputElement>) => {
+    const { files } = event.target as HTMLInputElement;
+    setUploadFile((files && files[0]) || null);
+  };
+
+  const renderPasswordFields = () => {
+    if (passwordFields.length === 0) {
+      return null;
+    }
+
+    return (
+      <>
+        <h5>Passwords</h5>
+        <StyledInputContainer>
+          <div className="helper">
+            {t('Please provide the password for the databases below')}
+          </div>
+        </StyledInputContainer>
+        {passwordFields.map(fileName => (
+          <StyledInputContainer key={`password-for-${fileName}`}>
+            <div className="control-label">
+              {fileName}
+              <span className="required">*</span>
+            </div>
+            <input

Review comment:
       adding a prop on the input tag `autocomplete="new-password"` should do the trick. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11884: feat: add modal to import databases

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11884:
URL: https://github.com/apache/incubator-superset/pull/11884#issuecomment-737008833


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=h1) Report
   > Merging [#11884](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=desc) (4e7a189) into [master](https://codecov.io/gh/apache/incubator-superset/commit/9c69679e14c2fa2b031c5c22b5cb95b640925176?el=desc) (9c69679) will **increase** coverage by `0.29%`.
   > The diff coverage is `58.71%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11884/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11884      +/-   ##
   ==========================================
   + Coverage   67.48%   67.77%   +0.29%     
   ==========================================
     Files         916      919       +3     
     Lines       44487    45546    +1059     
     Branches     4227     4245      +18     
   ==========================================
   + Hits        30020    30867     +847     
   - Misses      14364    14572     +208     
   - Partials      103      107       +4     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `54.40% <31.57%> (-0.72%)` | :arrow_down: |
   | javascript | `62.82% <45.78%> (-0.10%)` | :arrow_down: |
   | python | `64.81% <100.00%> (+0.82%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `60.24% <13.04%> (-8.30%)` | :arrow_down: |
   | [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | `83.83% <41.66%> (-5.82%)` | :arrow_down: |
   | [...tend/src/database/components/ImportModal/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFiYXNlL2NvbXBvbmVudHMvSW1wb3J0TW9kYWwvaW5kZXgudHN4) | `61.70% <61.70%> (ø)` | |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `72.00% <100.00%> (ø)` | |
   | [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `89.06% <100.00%> (+1.31%)` | :arrow_up: |
   | [...uperset/databases/commands/importers/dispatcher.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy9kaXNwYXRjaGVyLnB5) | `80.64% <100.00%> (+1.33%)` | :arrow_up: |
   | [...perset/databases/commands/importers/v1/\_\_init\_\_.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2ltcG9ydGVycy92MS9fX2luaXRfXy5weQ==) | `97.14% <100.00%> (+0.17%)` | :arrow_up: |
   | [superset/databases/schemas.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.97% <100.00%> (+0.02%)` | :arrow_up: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `89.78% <100.00%> (+0.04%)` | :arrow_up: |
   | ... and [41 more](https://codecov.io/gh/apache/incubator-superset/pull/11884/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=footer). Last update [9c69679...4e7a189](https://codecov.io/gh/apache/incubator-superset/pull/11884?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org