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:
[email protected]
With regards,
Apache Git Services
