Fixed review comments for Import/Export Servers.

refs #4803

docs/en_US/import_export_servers.rst

@@ -24,8 +24,9 @@ menu bar to click on the *Import/Export Servers* option.
 * Use the *Filename* field to select the JSON file to import servers or create the
   new file in case of Export where the servers to be exported in the JSON format.
 
-* Use the *Replace existing servers?* field to specify whether to replace the
+* Use the *Remove all the existing servers?* field to specify whether to remove all the
-  existing servers or not. This field is applicable only in case of Import Servers.
+  existing servers or not before importing the new selected servers.
+  This field is applicable only in case of Import Servers.
 
 Click the *Next* button to continue, or the *X* button to close the wizard.
 
@@ -41,18 +42,11 @@ Click the *Next* button to continue, or the *X* button to close the wizard.
     :alt: Import/Export Servers step three page
     :align: center
 
-Check the summary of the imported/exported servers on the Summary page.
+Check the summary of the servers that are going to be imported/exported on the
+Summary page.
 
 Click the *Finish* button to close the wizard.
 
-.. image:: images/import_export_servers_refresh_tree.png
-    :alt: Import/Export Servers Tree Refresh
-    :align: center
-
-In case of importing the server above confirmation box will be popped up to
-confirm whether to refresh the browser tree or later.
-
-
 Using 'setup.py' command line script
 ####################################
 

web/pgadmin/misc/file_manager/static/js/utility.js

@@ -712,7 +712,7 @@ define([
 
     var loading_icon_url = url_for(
       'static', {
-        'filename': 'js/generated/load-root.gif',
+        'filename': 'img/loading.gif',
       }
     );
 

web/pgadmin/misc/file_manager/templates/file_manager/index.html

@@ -58,7 +58,7 @@
     </div>
     <div class="fileinfo flex-grow-1 d-flex">
         <span class="activity">
-        <img src="{{ url_for('static', filename='js/generated/img/load-root.gif') }}"
+        <img src="{{ url_for('static', filename='img/loading.gif') }}"
             alt="{{ _('Loading...') }}">
         </span>
         <div class="file_listing"></div>

web/pgadmin/tools/import_export_servers/__init__.py

@@ -23,7 +23,7 @@ from pgadmin.utils.constants import MIMETYPE_APP_JS
 from pgadmin.utils.ajax import make_json_response, internal_server_error
 from pgadmin.model import ServerGroup, Server
 from pgadmin.utils import clear_database_servers, dump_database_servers,\
-    load_database_servers
+    load_database_servers, validate_json_data
 
 MODULE_NAME = 'import_export_servers'
 
@@ -131,6 +131,12 @@ def load_servers():
         try:
             with open(filename, 'r') as j:
                 data = json.loads(j.read())
+
+                # Validate the json file and data
+                errmsg = validate_json_data(data, False)
+                if errmsg is not None:
+                    return internal_server_error(errmsg)
+
                 if 'Servers' in data:
                     for server in data["Servers"]:
                         obj = data["Servers"][server]
@@ -184,18 +190,17 @@ def save():
 
     status = False
     errmsg = None
-    summary_data = []
     if data['type'] == 'export':
-        status, errmsg, summary_data = \
+        status, errmsg = \
             dump_database_servers(data['filename'], data['selected_sever_ids'])
     elif data['type'] == 'import':
         # Clear all the existing servers
         if 'replace_servers' in data and data['replace_servers']:
             clear_database_servers()
-        status, errmsg, summary_data = \
+        status, errmsg = \
             load_database_servers(data['filename'], data['selected_sever_ids'])
 
     if not status:
         return internal_server_error(errmsg)
 
-    return make_json_response(success=1, data=summary_data)
+    return make_json_response(success=1)

web/pgadmin/tools/import_export_servers/static/js/ImportExportServers.jsx

@@ -15,7 +15,7 @@ import { Box, Paper} from '@material-ui/core';
 import { makeStyles } from '@material-ui/core/styles';
 import Wizard from '../../../../static/js/helpers/wizard/Wizard';
 import WizardStep from '../../../../static/js/helpers/wizard/WizardStep';
-import { FormFooterMessage, MESSAGE_TYPE } from '../../../../static/js/components/FormComponents';
+import { FormFooterMessage, MESSAGE_TYPE, FormNote } from '../../../../static/js/components/FormComponents';
 import SchemaView from '../../../../static/js/SchemaView';
 import Loader from 'sources/components/Loader';
 import ImportExportSelectionSchema from './import_export_selection.ui';
@@ -46,6 +46,9 @@ const useStyles = makeStyles(() =>
       flexGrow: 1,
       minHeight: 0,
       overflow: 'auto',
+    },
+    noteContainer: {
+      marginTop: '5px',
     }
   }),
 );
@@ -62,27 +65,48 @@ export default function ImportExportServers() {
   const [selectedServers, setSelectedServers] = React.useState([]);
   const [summaryData, setSummaryData] = React.useState([]);
   const [summaryText, setSummaryText] = React.useState('');
+  const [noteText, setNoteText] = React.useState('');
   const api = getApiInstance();
 
   const onSave = () => {
-    if (selectionFormData.imp_exp == 'i') {
+    let post_data = {'filename': selectionFormData.filename},
-      Notify.confirm(
+      save_url = url_for('import_export_servers.save');
-        gettext('Browser tree refresh required'),
+
-        gettext('A browser tree refresh is required. Do you wish to refresh the tree?'),
+    if (selectionFormData.imp_exp == 'e') {
-        function() {
+      post_data['type'] = 'export';
-          pgAdmin.Browser.tree.destroy({
+      post_data['selected_sever_ids'] = selectedServers;
-            success: function() {
+      api.post(save_url, post_data)
-              pgAdmin.Browser.initializeBrowserTree(pgAdmin.Browser);
+        .then(() => {
-              return true;
+          Notify.alert(gettext('Export Servers'), gettext('The selected servers were exported successfully.'));
-            },
+        })
-          });
+        .catch((err) => {
-        },
+          Notify.alert(err.response.data.errormsg);
-        function() {
+        });
-          return true;
+    } else if (selectionFormData.imp_exp == 'i') {
-        },
+      // Remove the random number added to create unique tree item,
-        gettext('Refresh'),
+      let selected_sever_ids = [];
-        gettext('Later')
+      selectedServers.forEach((id) => {
-      );
+        selected_sever_ids.push(id.split('_')[0]);
+      });
+
+      post_data['type'] = 'import';
+      post_data['selected_sever_ids'] = selected_sever_ids;
+      post_data['replace_servers'] = selectionFormData.replace_servers;
+
+      api.post(save_url, post_data)
+        .then(() => {
+          pgAdmin.Browser.tree.destroy();
+
+          let msg = gettext('The selected servers were imported successfully.');
+          if (selectionFormData.replace_servers) {
+            msg = gettext('The existing server groups and servers were removed, and the selected servers were imported successfully.');
+          }
+
+          Notify.alert(gettext('Import Servers'), msg);
+        })
+        .catch((err) => {
+          Notify.alert(err.response.data.errormsg);
+        });
     }
  
     Alertify.importExportWizardDialog().close();
@@ -109,45 +133,30 @@ export default function ImportExportServers() {
   const wizardStepChange= (data) => {
     switch (data.currentStep) {
     case 2: {
-      let post_data = {'filename': selectionFormData.filename},
+      let sumData = [],
-        save_url = url_for('import_export_servers.save');
+        serverSerialNumber = 0;
-      if (selectionFormData.imp_exp == 'e') {
+      serverData.forEach((server_group) => {
-        setLoaderText('Exporting Server Groups/Servers ...');
+        server_group.children.forEach((server) =>{
-        setSummaryText('Exported following Server Groups/Servers:');
+          selectedServers.forEach((id) => {
-
+            if (server.value == id) {
-        post_data['type'] = 'export';
+              serverSerialNumber = serverSerialNumber + 1;
-        post_data['selected_sever_ids'] = selectedServers;
+              sumData.push({'srno': serverSerialNumber,
-        api.post(save_url, post_data)
+                'server_group':server_group.label, 'server': server.label});
-          .then(res => {
+            }
-            setLoaderText('');
-            setSummaryData(res.data.data);
-          })
-          .catch((err) => {
-            setLoaderText('');
-            setErrMsg(err.response.data.errormsg);
           });
-      } else if (selectionFormData.imp_exp == 'i') {
-        setLoaderText('Importing Server Groups/Servers ...');
-        setSummaryText('Imported following Server Groups/Servers:');
-        // Remove the random number added to create unique tree item,
-        let selected_sever_ids = [];
-        selectedServers.forEach((id) => {
-          selected_sever_ids.push(id.split('_')[0]);
         });
-
+      });
-        post_data['type'] = 'import';
+      setSummaryData(sumData);
-        post_data['selected_sever_ids'] = selected_sever_ids;
+      if (selectionFormData.imp_exp == 'e') {
-        post_data['replace_servers'] = selectionFormData.replace_servers;
+        setSummaryText('The following servers will be exported. Click the Finish button to complete the export process.');
-
+        setNoteText('');
-        api.post(save_url, post_data)
+      } else if (selectionFormData.imp_exp == 'i') {
-          .then(res => {
+        setSummaryText('The following servers will be imported. Click the Finish button to complete the import process.');
-            setLoaderText('');
+        if (selectionFormData.replace_servers) {
-            setSummaryData(res.data.data);
+          setNoteText('All existing server groups and servers will be removed before the servers above are imported. On a successful import process, the browser tree will be refreshed.');
-          })
+        } else {
-          .catch((err) => {
+          setNoteText('On a successful import process, the browser tree will be refreshed.');
-            setLoaderText('');
+        }
-            setErrMsg(err.response.data.errormsg);
-          });
       }
       break;
     }
@@ -252,6 +261,8 @@ export default function ImportExportServers() {
               </tbody>
             </table>
           </Paper>
+          {selectionFormData.imp_exp == 'i' && 
+          <FormNote className={classes.noteContainer} text={gettext(noteText)}/> }
         </WizardStep>
       </Wizard>
     </Box>

web/pgadmin/tools/import_export_servers/static/js/import_export_selection.ui.js

@@ -65,8 +65,9 @@ export default class ImportExportSelectionSchema extends BaseUISchema {
       helpMessage: gettext('Supports only JSON format.')
     }, {
       id: 'replace_servers',
-      label: gettext('Replace existing servers?'),
+      label: gettext('Remove all the existing servers?'),
       type: 'switch', deps: ['imp_exp'],
+      helpMessage: gettext('If this option is turned on then pgAdmin will remove all the existing database servers and then import the selected servers. This setting is applicable only while importing the servers.'),
       depChange: (state)=> {
         if (state.imp_exp == 'e') {
           state.replace_servers = false;

web/pgadmin/utils/__init__.py

@@ -397,7 +397,7 @@ def dump_database_servers(output_file, selected_servers,
     """
     user = _does_user_exist(dump_user, from_setup)
     if user is None:
-        return False, USER_NOT_FOUND % dump_user, []
+        return False, USER_NOT_FOUND % dump_user
 
     user_id = user.id
     # Dict to collect the output
@@ -408,7 +408,6 @@ def dump_database_servers(output_file, selected_servers,
     # Dump servers
     servers = Server.query.filter_by(user_id=user_id).all()
     server_dict = {}
-    dump_servers = []
     for server in servers:
         if selected_servers is None or str(server.id) in selected_servers:
             # Get the group name
@@ -446,9 +445,6 @@ def dump_database_servers(output_file, selected_servers,
                       server.tunnel_authentication)
 
             servers_dumped = servers_dumped + 1
-            dump_servers.append({'srno': servers_dumped,
-                                 'server_group': group_name,
-                                 'server': server.name})
 
             server_dict[servers_dumped] = attr_dict
 
@@ -473,13 +469,14 @@ def dump_database_servers(output_file, selected_servers,
           (servers_dumped, output_file)
     print(msg)
 
-    return True, msg, dump_servers
+    return True, msg
 
 
-def _validate_servers_data(data, is_admin):
+def validate_json_data(data, is_admin):
     """
     Used internally by load_servers to validate servers data.
     :param data: servers data
+    :param is_admin:
     :return: error message if any
     """
     skip_servers = []
@@ -503,6 +500,11 @@ def _validate_servers_data(data, is_admin):
                         (attrib, server))
             return None
 
+        def check_is_integer(value):
+            if not isinstance(value, int):
+                return "Port must be integer for server '%s'" % server
+            return None
+
         for attrib in ("Group", "Name"):
             errmsg = check_attrib(attrib)
             if errmsg:
@@ -515,6 +517,10 @@ def _validate_servers_data(data, is_admin):
                 errmsg = check_attrib(attrib)
                 if errmsg:
                     return errmsg
+                if attrib == 'Port':
+                    errmsg = check_is_integer(obj[attrib])
+                    if errmsg:
+                        return errmsg
 
         for attrib in ("SSLMode", "MaintenanceDB"):
             errmsg = check_attrib(attrib)
@@ -549,7 +555,7 @@ def load_database_servers(input_file, selected_servers,
 
     user = _does_user_exist(load_user, from_setup)
     if user is None:
-        return False, USER_NOT_FOUND % load_user, []
+        return False, USER_NOT_FOUND % load_user
 
     user_id = user.id
     # Counters
@@ -560,12 +566,11 @@ def load_database_servers(input_file, selected_servers,
     groups = ServerGroup.query.filter_by(user_id=user_id)
 
     # Validate server data
-    error_msg = _validate_servers_data(data, user.has_role("Administrator"))
+    error_msg = validate_json_data(data, user.has_role("Administrator"))
     if error_msg is not None and from_setup:
         print(ADD_SERVERS_MSG % (groups_added, servers_added))
         return _handle_error(error_msg, from_setup)
 
-    load_servers = []
     for server in data["Servers"]:
         if selected_servers is None or str(server) in selected_servers:
             obj = data["Servers"][server]
@@ -662,14 +667,11 @@ def load_database_servers(input_file, selected_servers,
                                      (new_server.name, e), from_setup)
 
             servers_added = servers_added + 1
-            load_servers.append({'srno': servers_added,
-                                 'server_group': obj["Group"],
-                                 'server': obj["Name"]})
 
     msg = ADD_SERVERS_MSG % (groups_added, servers_added)
     print(msg)
 
-    return True, msg, load_servers
+    return True, msg
 
 
 def clear_database_servers(load_user=current_user, from_setup=False):
@@ -733,7 +735,7 @@ def _handle_error(error_msg, from_setup):
         print(error_msg)
         sys.exit(1)
 
-    return False, error_msg, []
+    return False, error_msg
 
 
 # Shortcut configuration for Accesskey