Only send changed attributes when PATCHing app settings (#997)

frontend/kolide/helpers.js

@@ -1,4 +1,4 @@
-import { flatMap, kebabCase, pick } from 'lodash';
+import { flatMap, kebabCase, pick, size } from 'lodash';
 import md5 from 'js-md5';
 
 const ORG_INFO_ATTRS = ['org_name', 'org_logo_url'];
@@ -33,17 +33,21 @@ const filterTarget = (targetType) => {
 };
 
 export const formatConfigDataForServer = (config) => {
-  const orgInfoAttrs = ['org_logo_url', 'org_name'];
-  const serverSettingsAttrs = ['kolide_server_url'];
-  const smtpSettingsAttrs = [
+  const orgInfoAttrs = pick(config, ['org_logo_url', 'org_name']);
+  const serverSettingsAttrs = pick(config, ['kolide_server_url']);
+  const smtpSettingsAttrs = pick(config, [
     'authentication_method', 'authentication_type', 'email_enabled', 'enable_ssl_tls',
     'enable_start_tls', 'password', 'port', 'sender_address', 'server', 'user_name', 'verify_ssl_certs',
-  ];
+  ]);
+
+  const orgInfo = size(orgInfoAttrs) && { org_info: orgInfoAttrs };
+  const serverSettings = size(serverSettingsAttrs) && { server_settings: serverSettingsAttrs };
+  const smtpSettings = size(smtpSettingsAttrs) && { smtp_settings: smtpSettingsAttrs };
 
   return {
-    org_info: pick(config, orgInfoAttrs),
-    server_settings: pick(config, serverSettingsAttrs),
-    smtp_settings: pick(config, smtpSettingsAttrs),
+    ...orgInfo,
+    ...serverSettings,
+    ...smtpSettings,
   };
 };
 

frontend/kolide/helpers.tests.js

@@ -39,6 +39,15 @@ describe('Kolide API - helpers', () => {
 
     it('splits config into categories for the server', () => {
       expect(formatConfigDataForServer(config)).toEqual(omit(configStub, ['smtp_settings.configured']));
+      expect(formatConfigDataForServer({ org_name: 'The Gnar Co' })).toEqual({
+        org_info: { org_name: 'The Gnar Co' },
+      });
+      expect(
+        formatConfigDataForServer({ org_name: 'The Gnar Co', kolide_server_url: 'https://example.com' })
+      ).toEqual({
+        org_info: { org_name: 'The Gnar Co' },
+        server_settings: { kolide_server_url: 'https://example.com' },
+      });
     });
   });
 

frontend/kolide/index.js

@@ -1,3 +1,5 @@
+import { get } from 'lodash';
+
 import { appendTargetTypeToTargets } from 'redux/nodes/entities/targets/helpers';
 import Base from 'kolide/base';
 import endpoints from 'kolide/endpoints';
@@ -377,7 +379,10 @@ class Kolide extends Base {
   updateConfig = (formData) => {
     const { CONFIG } = endpoints;
     const configData = helpers.formatConfigDataForServer(formData);
-    configData.smtp_settings.port = parseInt(configData.smtp_settings.port, 10);
+
+    if (get(configData, 'smtp_settings.port')) {
+      configData.smtp_settings.port = parseInt(configData.smtp_settings.port, 10);
+    }
 
     return this.authenticatedPatch(this.endpoint(CONFIG), JSON.stringify(configData));
   }

frontend/pages/Admin/AppSettingsPage/AppSettingsPage.jsx

@@ -4,6 +4,7 @@ import { size } from 'lodash';
 
 import AppConfigForm from 'components/forms/admin/AppConfigForm';
 import configInterface from 'interfaces/config';
+import deepDifference from 'utilities/deep_difference';
 import { renderFlash } from 'redux/nodes/notifications/actions';
 import SmtpWarning from 'components/SmtpWarning';
 import { updateConfig } from 'redux/nodes/app/actions';
@@ -30,15 +31,22 @@ class AppSettingsPage extends Component {
   }
 
   onFormSubmit = (formData) => {
-    const { dispatch } = this.props;
+    const { appConfig, dispatch } = this.props;
+    const diff = deepDifference(formData, appConfig);
 
-    dispatch(updateConfig(formData))
+    dispatch(updateConfig(diff))
       .then(() => {
         dispatch(renderFlash('success', 'Settings updated!'));
 
         return false;
       })
-      .catch(() => false);
+      .catch((errors) => {
+        if (errors.base) {
+          dispatch(renderFlash('error', errors.base));
+        }
+
+        return false;
+      });
 
     return false;
   }

frontend/redux/nodes/app/actions.js

@@ -39,7 +39,7 @@ export const getConfig = () => {
 
         dispatch(configFailure(formattedErrors));
 
-        throw error;
+        throw formattedErrors;
       });
   };
 };
@@ -60,7 +60,7 @@ export const updateConfig = (configData) => {
 
         dispatch(configFailure(formattedErrors));
 
-        throw error;
+        throw formattedErrors;
       });
   };
 };

frontend/utilities/deep_difference/deep_difference.tests.js

@@ -0,0 +1,45 @@
+import expect from 'expect';
+
+import deepDifference from 'utilities/deep_difference';
+
+describe('deepDifference - utility', () => {
+  it('returns the difference for 2 un-nested objects', () => {
+    const obj1 = { id: 1, first_name: 'Joe', last_name: 'Smith' };
+    const obj2 = { id: 1, first_name: 'Joe', last_name: 'Smyth' };
+
+    expect(deepDifference(obj1, obj2)).toEqual({ last_name: 'Smith' });
+    expect(deepDifference(obj2, obj1)).toEqual({ last_name: 'Smyth' });
+  });
+
+  it('returns the difference for 2 nested objects', () => {
+    const obj1 = {
+      profile: { id: 1, first_name: 'Joe', last_name: 'Smith' },
+      preferences: { email: true, push: false },
+    };
+    const obj2 = {
+      profile: { id: 1, first_name: 'Joe', last_name: 'Smyth' },
+      preferences: { email: false, push: false },
+    };
+
+    expect(deepDifference(obj1, obj2)).toEqual({
+      profile: { last_name: 'Smith' },
+      preferences: { email: true },
+    });
+
+    expect(deepDifference(obj2, obj1)).toEqual({
+      profile: { last_name: 'Smyth' },
+      preferences: { email: false },
+    });
+  });
+
+  it('returns the difference for 1 nested object and 1 non-nested object', () => {
+    const obj1 = {
+      profile: { id: 1, first_name: 'Joe', last_name: 'Smith' },
+      preferences: { email: true, push: false },
+    };
+    const obj2 = { profile: 'my profile', preferences: 'my preferences' };
+
+    expect(deepDifference(obj1, obj2)).toEqual(obj1);
+    expect(deepDifference(obj2, obj1)).toEqual(obj2);
+  });
+});

frontend/utilities/deep_difference/index.js

@@ -0,0 +1,19 @@
+import { isObject, map } from 'lodash';
+
+const deepDifference = (obj1, obj2) => {
+  const result = {};
+
+  map(obj1, (value, key) => {
+    const obj2Value = obj2[key];
+
+    if (isObject(value) && isObject(obj2Value)) {
+      result[key] = deepDifference(value, obj2Value);
+    } else if (value !== obj2Value) {
+      result[key] = value;
+    }
+  });
+
+  return result;
+};
+
+export default deepDifference;