mcgilman 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_r274101632
##########
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:
It does appear this function currently returns anything so I'm not sure this
else clause is needed. I checked where `saveSettings` is invoked from and it
appears they are not expecting a return value. If we want to update this part
of this function's signature, then we'd need to update the `if` block of the
conditional to also return something.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
