You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2019/04/11 05:04:03 UTC

[GitHub] [nifi] ijokarumawak commented on a change in pull request #3401: NIFI-6160 NIFI-6170 Apply config error handling convention

ijokarumawak commented on a change in pull request #3401: NIFI-6160 NIFI-6170 Apply config error handling convention
URL: https://github.com/apache/nifi/pull/3401#discussion_r274258601
 
 

 ##########
 File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-settings.js
 ##########
 @@ -116,35 +143,42 @@
     var saveSettings = function (version) {
         // marshal the configuration details
         var configuration = marshalConfiguration();
-        var entity = {
-            'revision': nfClient.getRevision({
-                'revision': {
-                    'version': version
-                }
-            }),
-            'disconnectedNodeAcknowledged': nfStorage.isDisconnectionAcknowledged(),
-            'component': configuration
-        };
+        // ensure settings are valid as far as we can tell
+        if (validateSettings(configuration)) {
+            var entity = {
+                'revision': nfClient.getRevision({
+                    'revision': {
+                        'version': version
+                    }
+                }),
+                'disconnectedNodeAcknowledged': nfStorage.isDisconnectionAcknowledged(),
+                'component': configuration
+            };
 
-        // save the new configuration details
-        $.ajax({
-            type: 'PUT',
-            url: config.urls.controllerConfig,
-            data: JSON.stringify(entity),
-            dataType: 'json',
-            contentType: 'application/json'
-        }).done(function (response) {
-            // close the settings dialog
-            nfDialog.showOkDialog({
-                headerText: 'Settings',
-                dialogContent: 'Settings successfully applied.'
-            });
+            // save the new configuration details
+            $.ajax({
+                type: 'PUT',
+                url: config.urls.controllerConfig,
+                data: JSON.stringify(entity),
+                dataType: 'json',
+                contentType: 'application/json'
+            }).done(function (response) {
+                // close the settings dialog
+                nfDialog.showOkDialog({
+                    headerText: 'Settings',
+                    dialogContent: 'Settings successfully applied.'
+                });
 
-            // register the click listener for the save button
-            $('#settings-save').off('click').on('click', function () {
-                saveSettings(response.revision.version);
-            });
-        }).fail(nfErrorHandler.handleConfigurationUpdateAjaxError);
+                // register the click listener for the save button
+                $('#settings-save').off('click').on('click', function () {
+                    saveSettings(response.revision.version);
+                });
+            }).fail(nfErrorHandler.handleConfigurationUpdateAjaxError);
+        } else {
+            return $.Deferred(function (deferred) {
 
 Review comment:
   Thanks for the catch! Yeah, I felt it a bit strange when I wrote that part, too.. I used nf-controller-service.js `saveControllerService` as a reference where returned ajax instance is used to add additional `done()` callback function.
   https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-controller-service.js#L1573
   
   We don't need such function chains here. I will remove the else block.

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


With regards,
Apache Git Services