Merge branch '216134_04-geo-settings-form-actions' into 'master'

Geo Settings Form - Validations and Save

See merge request gitlab-org/gitlab!35672

ee/app/assets/javascripts/api.js

@@ -355,4 +355,9 @@ export default {
     const url = Api.buildUrl(this.applicationSettingsPath);
     return axios.get(url);
   },
+
+  updateApplicationSettings(data) {
+    const url = Api.buildUrl(this.applicationSettingsPath);
+    return axios.put(url, data);
+  },
 };

ee/app/assets/javascripts/geo_settings/components/geo_settings_form.vue

 <script>
-import { mapState } from 'vuex';
+import { mapState, mapActions, mapGetters } from 'vuex';
 import { GlFormGroup, GlFormInput, GlButton } from '@gitlab/ui';
+import { mapComputed } from '~/vuex_shared/bindings';
 import { visitUrl } from '~/lib/utils/url_utility';
+import { validateTimeout, validateAllowedIp } from '../validations';
+import { FORM_VALIDATION_FIELDS } from '../constants';
 
 export default {
   name: 'GeoSettingsForm',
@@ -11,14 +14,30 @@ export default {
     GlButton,
   },
   computed: {
-    // The real connection between vuex and the component will be implemented in
-    // a later MR, this feature is anyhow behind feature flag
-    ...mapState(['timeout', 'allowedIp']),
+    ...mapState(['formErrors']),
+    ...mapGetters(['formHasError']),
+    ...mapComputed([
+      { key: 'timeout', updateFn: 'setTimeout' },
+      { key: 'allowedIp', updateFn: 'setAllowedIp' },
+    ]),
   },
   methods: {
+    ...mapActions(['updateGeoSettings', 'setFormError']),
     redirect() {
       visitUrl('/admin/geo/nodes');
     },
+    checkTimeout() {
+      this.setFormError({
+        key: FORM_VALIDATION_FIELDS.TIMEOUT,
+        error: validateTimeout(this.timeout),
+      });
+    },
+    checkAllowedIp() {
+      this.setFormError({
+        key: FORM_VALIDATION_FIELDS.ALLOWED_IP,
+        error: validateAllowedIp(this.allowedIp),
+      });
+    },
   },
 };
 </script>
@@ -29,19 +48,32 @@ export default {
       :label="__('Connection timeout')"
       label-for="settings-timeout-field"
       :description="__('Time in seconds')"
+      :state="Boolean(formErrors.timeout)"
+      :invalid-feedback="formErrors.timeout"
     >
-      <gl-form-input id="settings-timeout-field" v-model="timeout" class="col-sm-2" type="number" />
+      <gl-form-input
+        id="settings-timeout-field"
+        v-model="timeout"
+        class="col-sm-2"
+        type="number"
+        :class="{ 'is-invalid': Boolean(formErrors.timeout) }"
+        @blur="checkTimeout"
+      />
     </gl-form-group>
     <gl-form-group
       :label="__('Allowed Geo IP')"
       label-for="settings-allowed-ip-field"
       :description="__('Comma-separated, e.g. \'1.1.1.1, 2.2.2.0/24\'')"
+      :state="Boolean(formErrors.allowedIp)"
+      :invalid-feedback="formErrors.allowedIp"
     >
       <gl-form-input
         id="settings-allowed-ip-field"
         v-model="allowedIp"
         class="col-sm-6"
         type="text"
+        :class="{ 'is-invalid': Boolean(formErrors.allowedIp) }"
+        @blur="checkAllowedIp"
       />
     </gl-form-group>
     <section
@@ -51,6 +83,8 @@ export default {
         data-testid="settingsSaveButton"
         data-qa-selector="add_node_button"
         variant="success"
+        :disabled="formHasError"
+        @click="updateGeoSettings"
         >{{ __('Save changes') }}</gl-button
       >
       <gl-button data-testid="settingsCancelButton" class="gl-ml-auto" @click="redirect">{{

ee/app/assets/javascripts/geo_settings/constants.js

 export const DEFAULT_TIMEOUT = 10;
 export const DEFAULT_ALLOWED_IP = '0.0.0.0/0, ::/0';
+
+export const FORM_VALIDATION_FIELDS = {
+  TIMEOUT: 'timeout',
+  ALLOWED_IP: 'allowedIp',
+};

ee/app/assets/javascripts/geo_settings/store/actions.js

@@ -3,7 +3,6 @@ import createFlash from '~/flash';
 import { __ } from '~/locale';
 import * as types from './mutation_types';
 
-// eslint-disable-next-line import/prefer-default-export
 export const fetchGeoSettings = ({ commit }) => {
   commit(types.REQUEST_GEO_SETTINGS);
   Api.getApplicationSettings()
@@ -18,3 +17,33 @@ export const fetchGeoSettings = ({ commit }) => {
       commit(types.RECEIVE_GEO_SETTINGS_ERROR);
     });
 };
+
+export const updateGeoSettings = ({ commit, state }) => {
+  commit(types.REQUEST_UPDATE_GEO_SETTINGS);
+  Api.updateApplicationSettings({
+    geo_status_timeout: state.timeout,
+    geo_node_allowed_ips: state.allowedIp,
+  })
+    .then(({ data }) => {
+      commit(types.RECEIVE_UPDATE_GEO_SETTINGS_SUCCESS, {
+        timeout: data.geo_status_timeout,
+        allowedIp: data.geo_node_allowed_ips,
+      });
+    })
+    .catch(() => {
+      createFlash(__('There was an error updating the Geo Settings'));
+      commit(types.RECEIVE_UPDATE_GEO_SETTINGS_ERROR);
+    });
+};
+
+export const setTimeout = ({ commit }, { timeout }) => {
+  commit(types.SET_TIMEOUT, timeout);
+};
+
+export const setAllowedIp = ({ commit }, { allowedIp }) => {
+  commit(types.SET_ALLOWED_IP, allowedIp);
+};
+
+export const setFormError = ({ commit }, { key, error }) => {
+  commit(types.SET_FORM_ERROR, { key, error });
+};

ee/app/assets/javascripts/geo_settings/store/getters.js

+// eslint-disable-next-line import/prefer-default-export
+export const formHasError = state =>
+  Object.keys(state.formErrors)
+    .map(key => state.formErrors[key])
+    .some(val => Boolean(val));

ee/app/assets/javascripts/geo_settings/store/index.js

 import Vue from 'vue';
 import Vuex from 'vuex';
 import * as actions from './actions';
+import * as getters from './getters';
 import mutations from './mutations';
 import createState from './state';
 
@@ -10,5 +11,6 @@ export default () =>
   new Vuex.Store({
     actions,
     mutations,
+    getters,
     state: createState(),
   });

ee/app/assets/javascripts/geo_settings/store/mutation_types.js

 export const REQUEST_GEO_SETTINGS = 'REQUEST_GEO_SETTINGS';
 export const RECEIVE_GEO_SETTINGS_SUCCESS = 'RECEIVE_GEO_SETTINGS_SUCCESS';
 export const RECEIVE_GEO_SETTINGS_ERROR = 'RECEIVE_GEO_SETTINGS_ERROR';
+
+export const REQUEST_UPDATE_GEO_SETTINGS = 'REQUEST_UPDATE_GEO_SETTINGS';
+export const RECEIVE_UPDATE_GEO_SETTINGS_SUCCESS = 'RECEIVE_UPDATE_GEO_SETTINGS_SUCCESS';
+export const RECEIVE_UPDATE_GEO_SETTINGS_ERROR = 'RECEIVE_UPDATE_GEO_SETTINGS_ERROR';
+
+export const SET_TIMEOUT = 'SET_TIMEOUT';
+export const SET_ALLOWED_IP = 'SET_ALLOWED_IP';
+export const SET_FORM_ERROR = 'SET_FORM_ERROR';

ee/app/assets/javascripts/geo_settings/store/mutations.js

@@ -15,4 +15,26 @@ export default {
     state.timeout = DEFAULT_TIMEOUT;
     state.allowedIp = DEFAULT_ALLOWED_IP;
   },
+  [types.REQUEST_UPDATE_GEO_SETTINGS](state) {
+    state.isLoading = true;
+  },
+  [types.RECEIVE_UPDATE_GEO_SETTINGS_SUCCESS](state, { timeout, allowedIp }) {
+    state.isLoading = false;
+    state.timeout = timeout;
+    state.allowedIp = allowedIp;
+  },
+  [types.RECEIVE_UPDATE_GEO_SETTINGS_ERROR](state) {
+    state.isLoading = false;
+    state.timeout = DEFAULT_TIMEOUT;
+    state.allowedIp = DEFAULT_ALLOWED_IP;
+  },
+  [types.SET_TIMEOUT](state, timeout) {
+    state.timeout = timeout;
+  },
+  [types.SET_ALLOWED_IP](state, allowedIp) {
+    state.allowedIp = allowedIp;
+  },
+  [types.SET_FORM_ERROR](state, { key, error }) {
+    state.formErrors[key] = error;
+  },
 };

ee/app/assets/javascripts/geo_settings/store/state.js

-import { DEFAULT_TIMEOUT, DEFAULT_ALLOWED_IP } from '../constants';
+import { DEFAULT_TIMEOUT, DEFAULT_ALLOWED_IP, FORM_VALIDATION_FIELDS } from '../constants';
 
 export default () => ({
   isLoading: false,
   timeout: DEFAULT_TIMEOUT,
   allowedIp: DEFAULT_ALLOWED_IP,
+  formErrors: Object.keys(FORM_VALIDATION_FIELDS)
+    .map(key => FORM_VALIDATION_FIELDS[key])
+    .reduce((acc, cur) => ({ ...acc, [cur]: '' }), {}),
 });

ee/app/assets/javascripts/geo_settings/validations.js

+import ipaddr from 'ipaddr.js';
+import { s__ } from '~/locale';
+
+const validateAddress = address => {
+  try {
+    // Checks if Valid IPv4/IPv6 (CIDR) - Throws if not
+    return Boolean(ipaddr.parseCIDR(address));
+  } catch (e) {
+    // Checks if Valid IPv4/IPv6 (Non-CIDR) - Does not Throw
+    return ipaddr.isValid(address);
+  }
+};
+
+const validateIP = data => {
+  let addresses = data.replace(/\s/g, '').split(',');
+
+  addresses = addresses.map(address => validateAddress(address));
+
+  return !addresses.some(a => !a);
+};
+
+export const validateTimeout = data => {
+  if (!data && data !== 0) {
+    return s__("Geo|Connection timeout can't be blank");
+  } else if (data && Number.isNaN(Number(data))) {
+    return s__('Geo|Connection timeout must be a number');
+  } else if (data < 1 || data > 120) {
+    return s__('Geo|Connection timeout should be between 1-120');
+  }
+
+  return '';
+};
+
+export const validateAllowedIp = data => {
+  if (!data) {
+    return s__("Geo|Allowed Geo IP can't be blank");
+  } else if (data.length > 255) {
+    return s__('Geo|Allowed Geo IP should be between 1 and 255 characters');
+  } else if (!validateIP(data)) {
+    return s__('Geo|Allowed Geo IP should contain valid IP addresses');
+  }
+
+  return '';
+};

ee/spec/frontend/api_spec.js

@@ -842,10 +842,11 @@ describe('Api', () => {
     });
   });
 
-  describe('getApplicationSettings', () => {
+  describe('Application Settings', () => {
     const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/application/settings`;
-    const apiResponse = { mock_setting: 1, mock_setting2: 2 };
+    const apiResponse = { mock_setting: 1, mock_setting2: 2, mock_setting3: 3 };
 
+    describe('getApplicationSettings', () => {
       it('fetches applications settings', () => {
         jest.spyOn(Api, 'buildUrl').mockReturnValue(expectedUrl);
         jest.spyOn(axios, 'get');
@@ -857,4 +858,20 @@ describe('Api', () => {
         });
       });
     });
+
+    describe('updateApplicationSettings', () => {
+      const mockReq = { mock_setting: 10 };
+
+      it('updates applications settings', () => {
+        jest.spyOn(Api, 'buildUrl').mockReturnValue(expectedUrl);
+        jest.spyOn(axios, 'put');
+        mock.onPut(expectedUrl).replyOnce(201, apiResponse);
+
+        return Api.updateApplicationSettings(mockReq).then(({ data }) => {
+          expect(data).toEqual(apiResponse);
+          expect(axios.put).toHaveBeenCalledWith(expectedUrl, mockReq);
+        });
+      });
+    });
+  });
 });

ee/spec/frontend/geo_settings/components/geo_settings_form_spec.js

 import Vuex from 'vuex';
-import { createLocalVue, shallowMount } from '@vue/test-utils';
+import { createLocalVue, mount } from '@vue/test-utils';
 import { visitUrl } from '~/lib/utils/url_utility';
-import store from 'ee/geo_settings/store';
+import initStore from 'ee/geo_settings/store';
+import * as types from 'ee/geo_settings/store/mutation_types';
 import GeoSettingsForm from 'ee/geo_settings/components/geo_settings_form.vue';
+import { STRING_OVER_255 } from '../mock_data';
 
 const localVue = createLocalVue();
 localVue.use(Vuex);
@@ -13,9 +15,14 @@ jest.mock('~/lib/utils/url_utility', () => ({
 
 describe('GeoSettingsForm', () => {
   let wrapper;
+  let store;
+
+  const createStore = () => {
+    store = initStore();
+  };
 
   const createComponent = () => {
-    wrapper = shallowMount(GeoSettingsForm, {
+    wrapper = mount(GeoSettingsForm, {
       store,
     });
   };
@@ -26,10 +33,13 @@ describe('GeoSettingsForm', () => {
 
   const findGeoSettingsTimeoutField = () => wrapper.find('#settings-timeout-field');
   const findGeoSettingsAllowedIpField = () => wrapper.find('#settings-allowed-ip-field');
-  const findSettingsCancelButton = () => wrapper.find('[data-testid="settingsCancelButton"]');
+  const findGeoSettingsSaveButton = () => wrapper.find('[data-testid="settingsSaveButton"]');
+  const findGeoSettingsCancelButton = () => wrapper.find('[data-testid="settingsCancelButton"]');
+  const findErrorMessage = () => wrapper.find('.invalid-feedback');
 
   describe('template', () => {
     beforeEach(() => {
+      createStore();
       createComponent();
     });
 
@@ -40,18 +50,100 @@ describe('GeoSettingsForm', () => {
     it('renders Geo Node Form Url Field', () => {
       expect(findGeoSettingsAllowedIpField().exists()).toBe(true);
     });
+
+    describe('Save Button', () => {
+      describe('with errors on form', () => {
+        beforeEach(() => {
+          store.commit(types.SET_FORM_ERROR, {
+            key: 'timeout',
+            error: 'error',
+          });
+        });
+
+        it('disables button', () => {
+          expect(findGeoSettingsSaveButton().attributes('disabled')).toBeTruthy();
+        });
+      });
+
+      describe('with no errors on form', () => {
+        it('does not disable button', () => {
+          expect(findGeoSettingsSaveButton().attributes('disabled')).toBeFalsy();
+        });
+      });
+    });
   });
 
   describe('methods', () => {
-    describe('redirect', () => {
     beforeEach(() => {
+      createStore();
+      jest.spyOn(store, 'dispatch').mockImplementation();
       createComponent();
     });
 
-      it('calls visitUrl when cancel is clicked', () => {
-        findSettingsCancelButton().vm.$emit('click');
+    describe('save button', () => {
+      it('calls updateGeoSettings when clicked', () => {
+        findGeoSettingsSaveButton().vm.$emit('click');
+        expect(store.dispatch).toHaveBeenCalledWith('updateGeoSettings');
+      });
+    });
+
+    describe('cancel button', () => {
+      it('calls visitUrl when clicked', () => {
+        findGeoSettingsCancelButton().vm.$emit('click');
         expect(visitUrl).toHaveBeenCalledWith('/admin/geo/nodes');
       });
     });
   });
+
+  describe('errors', () => {
+    describe.each`
+      data    | showError | errorMessage
+      ${null} | ${true}   | ${"Connection timeout can't be blank"}
+      ${''}   | ${true}   | ${"Connection timeout can't be blank"}
+      ${0}    | ${true}   | ${'Connection timeout should be between 1-120'}
+      ${121}  | ${true}   | ${'Connection timeout should be between 1-120'}
+      ${10}   | ${false}  | ${null}
+    `(`Timeout Field`, ({ data, showError, errorMessage }) => {
+      beforeEach(() => {
+        createStore();
+        createComponent();
+        findGeoSettingsTimeoutField().vm.$emit('input', data);
+        findGeoSettingsTimeoutField().trigger('blur');
+      });
+
+      it(`${showError ? 'shows' : 'hides'} error when data is ${data}`, () => {
+        expect(findGeoSettingsTimeoutField().classes('is-invalid')).toBe(showError);
+        if (showError) {
+          expect(findErrorMessage().text()).toBe(errorMessage);
+        }
+      });
+    });
+
+    describe.each`
+      data               | showError | errorMessage
+      ${null}            | ${true}   | ${"Allowed Geo IP can't be blank"}
+      ${''}              | ${true}   | ${"Allowed Geo IP can't be blank"}
+      ${STRING_OVER_255} | ${true}   | ${'Allowed Geo IP should be between 1 and 255 characters'}
+      ${'asdf'}          | ${true}   | ${'Allowed Geo IP should contain valid IP addresses'}
+      ${'1.1.1.1, asdf'} | ${true}   | ${'Allowed Geo IP should contain valid IP addresses'}
+      ${'asdf, 1.1.1.1'} | ${true}   | ${'Allowed Geo IP should contain valid IP addresses'}
+      ${'1.1.1.1'}       | ${false}  | ${null}
+      ${'::/0'}          | ${false}  | ${null}
+      ${'1.1.1.1, ::/0'} | ${false}  | ${null}
+    `(`Allowed Geo IP Field`, ({ data, showError, errorMessage }) => {
+      beforeEach(() => {
+        createStore();
+        createComponent();
+        findGeoSettingsAllowedIpField().vm.$emit('input', data);
+        findGeoSettingsAllowedIpField().trigger('blur');
+      });
+
+      it(`${showError ? 'shows' : 'hides'} error when data is ${data}`, () => {
+        expect(findGeoSettingsAllowedIpField().classes('is-invalid')).toBe(showError);
+        if (showError) {
+          expect(findErrorMessage().text()).toBe(errorMessage);
+        }
+      });
+    });
+  });
 });

ee/spec/frontend/geo_settings/mock_data.js

@@ -7,3 +7,5 @@ export const MOCK_BASIC_SETTINGS_DATA = {
   timeout: MOCK_APPLICATION_SETTINGS_FETCH_RESPONSE.geo_status_timeout,
   allowedIp: MOCK_APPLICATION_SETTINGS_FETCH_RESPONSE.geo_node_allowed_ips,
 };
+
+export const STRING_OVER_255 = new Array(257).join('a');

ee/spec/frontend/geo_settings/store/actions_spec.js

+import MockAdapter from 'axios-mock-adapter';
 import testAction from 'helpers/vuex_action_helper';
 import flash from '~/flash';
-import Api from 'ee/api';
+import axios from '~/lib/utils/axios_utils';
 import * as actions from 'ee/geo_settings/store/actions';
 import * as types from 'ee/geo_settings/store/mutation_types';
 import state from 'ee/geo_settings/store/state';
@@ -9,45 +10,50 @@ import { MOCK_BASIC_SETTINGS_DATA, MOCK_APPLICATION_SETTINGS_FETCH_RESPONSE } fr
 jest.mock('~/flash');
 
 describe('GeoSettings Store Actions', () => {
-  describe('fetchGeoSettings', () => {
-    describe('on success', () => {
+  let mock;
+
+  const noCallback = () => {};
+  const flashCallback = () => {
+    expect(flash).toHaveBeenCalledTimes(1);
+    flash.mockClear();
+  };
+
   beforeEach(() => {
-        jest
-          .spyOn(Api, 'getApplicationSettings')
-          .mockResolvedValue({ data: MOCK_APPLICATION_SETTINGS_FETCH_RESPONSE });
+    mock = new MockAdapter(axios);
+  });
+
+  afterEach(() => {
+    mock.restore();
   });
 
-      it('should commit the request and success actions', done => {
-        testAction(
-          actions.fetchGeoSettings,
-          {},
-          state,
-          [
-            { type: types.REQUEST_GEO_SETTINGS },
-            { type: types.RECEIVE_GEO_SETTINGS_SUCCESS, payload: MOCK_BASIC_SETTINGS_DATA },
-          ],
-          [],
-          done,
-        );
+  describe.each`
+    action                  | data                                                 | mutationName            | mutationCall                                                                   | callback
+    ${actions.setTimeout}   | ${{ timeout: MOCK_BASIC_SETTINGS_DATA.timeout }}     | ${types.SET_TIMEOUT}    | ${{ type: types.SET_TIMEOUT, payload: MOCK_BASIC_SETTINGS_DATA.timeout }}      | ${noCallback}
+    ${actions.setAllowedIp} | ${{ allowedIp: MOCK_BASIC_SETTINGS_DATA.allowedIp }} | ${types.SET_ALLOWED_IP} | ${{ type: types.SET_ALLOWED_IP, payload: MOCK_BASIC_SETTINGS_DATA.allowedIp }} | ${noCallback}
+    ${actions.setFormError} | ${{ key: 'timeout', error: 'error' }}                | ${types.SET_FORM_ERROR} | ${{ type: types.SET_FORM_ERROR, payload: { key: 'timeout', error: 'error' } }} | ${noCallback}
+  `(`non-axios calls`, ({ action, data, mutationName, mutationCall, callback }) => {
+    describe(action.name, () => {
+      it(`should commit mutation ${mutationName}`, () => {
+        return testAction(action, data, state, [mutationCall], []).then(() => callback());
+      });
     });
   });
 
-    describe('on error', () => {
+  describe.each`
+    action                       | axiosMock                                                                        | type         | mutationCalls                                                                                                                            | callback
+    ${actions.fetchGeoSettings}  | ${{ method: 'onGet', code: 200, res: MOCK_APPLICATION_SETTINGS_FETCH_RESPONSE }} | ${'success'} | ${[{ type: types.REQUEST_GEO_SETTINGS }, { type: types.RECEIVE_GEO_SETTINGS_SUCCESS, payload: MOCK_BASIC_SETTINGS_DATA }]}               | ${noCallback}
+    ${actions.fetchGeoSettings}  | ${{ method: 'onGet', code: 500, res: null }}                                     | ${'error'}   | ${[{ type: types.REQUEST_GEO_SETTINGS }, { type: types.RECEIVE_GEO_SETTINGS_ERROR }]}                                                    | ${flashCallback}
+    ${actions.updateGeoSettings} | ${{ method: 'onPut', code: 200, res: MOCK_APPLICATION_SETTINGS_FETCH_RESPONSE }} | ${'success'} | ${[{ type: types.REQUEST_UPDATE_GEO_SETTINGS }, { type: types.RECEIVE_UPDATE_GEO_SETTINGS_SUCCESS, payload: MOCK_BASIC_SETTINGS_DATA }]} | ${noCallback}
+    ${actions.updateGeoSettings} | ${{ method: 'onPut', code: 500, res: null }}                                     | ${'error'}   | ${[{ type: types.REQUEST_UPDATE_GEO_SETTINGS }, { type: types.RECEIVE_UPDATE_GEO_SETTINGS_ERROR }]}                                      | ${flashCallback}
+  `(`axios calls`, ({ action, axiosMock, type, mutationCalls, callback }) => {
+    describe(action.name, () => {
+      describe(`on ${type}`, () => {
         beforeEach(() => {
-        jest.spyOn(Api, 'getApplicationSettings').mockRejectedValue(new Error(500));
+          mock[axiosMock.method]().replyOnce(axiosMock.code, axiosMock.res);
+        });
+        it(`should dispatch the correct mutations`, () => {
+          return testAction(action, null, state, mutationCalls, []).then(() => callback());
         });
-
-      it('should commit the request and error actions', () => {
-        testAction(
-          actions.fetchGeoSettings,
-          {},
-          state,
-          [{ type: types.REQUEST_GEO_SETTINGS }, { type: types.RECEIVE_GEO_SETTINGS_ERROR }],
-          [],
-          () => {
-            expect(flash).toHaveBeenCalledTimes(1);
-          },
-        );
       });
     });
   });

ee/spec/frontend/geo_settings/store/getters_spec.js

+import * as getters from 'ee/geo_settings/store/getters';
+import createState from 'ee/geo_settings/store/state';
+
+describe('Geo Settings Store Getters', () => {
+  let state;
+
+  beforeEach(() => {
+    state = createState();
+  });
+
+  describe('formHasError', () => {
+    it('with error returns true', () => {
+      state.formErrors.timeout = 'Error';
+
+      expect(getters.formHasError(state)).toBeTruthy();
+    });
+
+    it('without error returns false', () => {
+      state.formErrors.timeout = '';
+
+      expect(getters.formHasError(state)).toBeFalsy();
+    });
+  });
+});

ee/spec/frontend/geo_settings/store/mutations_spec.js

@@ -11,55 +11,88 @@ describe('GeoSettings Store Mutations', () => {
     state = createState();
   });
 
-  describe('REQUEST_GEO_SETTINGS', () => {
-    it('sets isLoading to true', () => {
-      mutations[types.REQUEST_GEO_SETTINGS](state);
-
-      expect(state.isLoading).toEqual(true);
+  describe.each`
+    mutation                                   | data                        | loadingBefore | loadingAfter
+    ${types.REQUEST_GEO_SETTINGS}              | ${null}                     | ${false}      | ${true}
+    ${types.RECEIVE_GEO_SETTINGS_SUCCESS}      | ${MOCK_BASIC_SETTINGS_DATA} | ${true}       | ${false}
+    ${types.RECEIVE_GEO_SETTINGS_ERROR}        | ${null}                     | ${true}       | ${false}
+    ${types.REQUEST_UPDATE_GEO_SETTINGS}       | ${null}                     | ${false}      | ${true}
+    ${types.RECEIVE_UPDATE_GEO_SETTINGS_ERROR} | ${null}                     | ${true}       | ${false}
+  `(`Loading Mutations: `, ({ mutation, data, loadingBefore, loadingAfter }) => {
+    describe(`${mutation}`, () => {
+      it(`sets isLoading to ${loadingAfter}`, () => {
+        state.isLoading = loadingBefore;
+
+        mutations[mutation](state, data);
+        expect(state.isLoading).toEqual(loadingAfter);
+      });
     });
   });
 
   describe('RECEIVE_GEO_SETTINGS_SUCCESS', () => {
-    const mockData = MOCK_BASIC_SETTINGS_DATA;
+    it('sets timeout and allowedIp array with data', () => {
+      mutations[types.RECEIVE_GEO_SETTINGS_SUCCESS](state, MOCK_BASIC_SETTINGS_DATA);
 
+      expect(state.timeout).toBe(MOCK_BASIC_SETTINGS_DATA.timeout);
+      expect(state.allowedIp).toBe(MOCK_BASIC_SETTINGS_DATA.allowedIp);
+    });
+  });
+
+  describe('RECEIVE_GEO_SETTINGS_ERROR', () => {
     beforeEach(() => {
-      state.isLoading = true;
+      state.timeout = MOCK_BASIC_SETTINGS_DATA.timeout;
+      state.allowedIp = MOCK_BASIC_SETTINGS_DATA.allowedIp;
     });
 
-    it('sets isLoading to false', () => {
-      mutations[types.RECEIVE_GEO_SETTINGS_SUCCESS](state, mockData);
+    it('resets timeout and allowedIp array', () => {
+      mutations[types.RECEIVE_GEO_SETTINGS_ERROR](state);
 
-      expect(state.isLoading).toEqual(false);
+      expect(state.timeout).toBe(DEFAULT_TIMEOUT);
+      expect(state.allowedIp).toBe(DEFAULT_ALLOWED_IP);
+    });
   });
 
+  describe('RECEIVE_UPDATE_GEO_SETTINGS_SUCCESS', () => {
     it('sets timeout and allowedIp array with data', () => {
-      mutations[types.RECEIVE_GEO_SETTINGS_SUCCESS](state, mockData);
+      mutations[types.RECEIVE_UPDATE_GEO_SETTINGS_SUCCESS](state, MOCK_BASIC_SETTINGS_DATA);
 
-      expect(state.timeout).toBe(mockData.timeout);
-      expect(state.allowedIp).toBe(mockData.allowedIp);
+      expect(state.timeout).toBe(MOCK_BASIC_SETTINGS_DATA.timeout);
+      expect(state.allowedIp).toBe(MOCK_BASIC_SETTINGS_DATA.allowedIp);
     });
   });
 
-  describe('RECEIVE_GEO_SETTINGS_ERROR', () => {
-    const mockData = MOCK_BASIC_SETTINGS_DATA;
-
+  describe('RECEIVE_UPDATE_GEO_SETTINGS_ERROR', () => {
     beforeEach(() => {
-      state.isLoading = true;
-      state.timeout = mockData.timeout;
-      state.allowedIp = mockData.allowedIp;
-    });
-
-    it('sets isLoading to false', () => {
-      mutations[types.RECEIVE_GEO_SETTINGS_ERROR](state);
-
-      expect(state.isLoading).toEqual(false);
+      state.timeout = MOCK_BASIC_SETTINGS_DATA.timeout;
+      state.allowedIp = MOCK_BASIC_SETTINGS_DATA.allowedIp;
     });
 
     it('resets timeout and allowedIp array', () => {
-      mutations[types.RECEIVE_GEO_SETTINGS_ERROR](state);
+      mutations[types.RECEIVE_UPDATE_GEO_SETTINGS_ERROR](state);
 
       expect(state.timeout).toBe(DEFAULT_TIMEOUT);
       expect(state.allowedIp).toBe(DEFAULT_ALLOWED_IP);
     });
   });
+
+  describe('SET_TIMEOUT', () => {
+    it('sets error for field', () => {
+      mutations[types.SET_TIMEOUT](state, 1);
+      expect(state.timeout).toBe(1);
+    });
+  });
+
+  describe('SET_ALLOWED_IP', () => {
+    it('sets error for field', () => {
+      mutations[types.SET_ALLOWED_IP](state, '0.0.0.0');
+      expect(state.allowedIp).toBe('0.0.0.0');
+    });
+  });
+
+  describe('SET_FORM_ERROR', () => {
+    it('sets error for field', () => {
+      mutations[types.SET_FORM_ERROR](state, { key: 'timeout', error: 'error' });
+      expect(state.formErrors.timeout).toBe('error');
+    });
+  });
 });

ee/spec/frontend/geo_settings/validations_spec.js

+import { validateTimeout, validateAllowedIp } from 'ee/geo_settings/validations';
+import { STRING_OVER_255 } from './mock_data';
+
+describe('Geo Settings Validations', () => {
+  let res = '';
+
+  describe.each`
+    data      | errorMessage
+    ${null}   | ${"Connection timeout can't be blank"}
+    ${''}     | ${"Connection timeout can't be blank"}
+    ${'asdf'} | ${'Connection timeout must be a number'}
+    ${0}      | ${'Connection timeout should be between 1-120'}
+    ${121}    | ${'Connection timeout should be between 1-120'}
+    ${10}     | ${''}
+  `(`validateTimeout`, ({ data, errorMessage }) => {
+    beforeEach(() => {
+      res = validateTimeout(data);
+    });
+
+    it(`return ${errorMessage} when data is ${data}`, () => {
+      expect(res).toBe(errorMessage);
+    });
+  });
+
+  describe.each`
+    data               | errorMessage
+    ${null}            | ${"Allowed Geo IP can't be blank"}
+    ${''}              | ${"Allowed Geo IP can't be blank"}
+    ${STRING_OVER_255} | ${'Allowed Geo IP should be between 1 and 255 characters'}
+    ${'asdf'}          | ${'Allowed Geo IP should contain valid IP addresses'}
+    ${'1.1.1.1, asdf'} | ${'Allowed Geo IP should contain valid IP addresses'}
+    ${'asdf, 1.1.1.1'} | ${'Allowed Geo IP should contain valid IP addresses'}
+    ${'1.1.1.1'}       | ${''}
+    ${'::/0'}          | ${''}
+    ${'1.1.1.1, ::/0'} | ${''}
+  `(`validateAllowedIp`, ({ data, errorMessage }) => {
+    beforeEach(() => {
+      res = validateAllowedIp(data);
+    });
+
+    it(`return ${errorMessage} when data is ${data}`, () => {
+      expect(res).toBe(errorMessage);
+    });
+  });
+});

locale/gitlab.pot

@@ -10666,6 +10666,24 @@ msgstr ""
 msgid "Geo|All projects are being scheduled for reverify"
 msgstr ""
 
+msgid "Geo|Allowed Geo IP can't be blank"
+msgstr ""
+
+msgid "Geo|Allowed Geo IP should be between 1 and 255 characters"
+msgstr ""
+
+msgid "Geo|Allowed Geo IP should contain valid IP addresses"
+msgstr ""
+
+msgid "Geo|Connection timeout can't be blank"
+msgstr ""
+
+msgid "Geo|Connection timeout must be a number"
+msgstr ""
+
+msgid "Geo|Connection timeout should be between 1-120"
+msgstr ""
+
 msgid "Geo|Could not remove tracking entry for an existing project."
 msgstr ""
 
@@ -23425,6 +23443,9 @@ msgstr ""
 msgid "There was an error trying to validate your query"
 msgstr ""
 
+msgid "There was an error updating the Geo Settings"
+msgstr ""
+
 msgid "There was an error updating the dashboard, branch name is invalid."
 msgstr ""
 

package.json

@@ -91,6 +91,7 @@
     "graphql-tag": "^2.10.1",
     "immer": "^5.2.1",
     "imports-loader": "^0.8.0",
+    "ipaddr.js": "^1.9.1",
     "jed": "^1.1.1",
     "jest-transform-graphql": "^2.1.0",
     "jquery": "^3.4.1",

yarn.lock

@@ -6130,11 +6130,16 @@ ip@^1.1.0, ip@^1.1.5:
   resolved "https://registry.yarnpkg.com/ip/-/ip-1.1.5.tgz#bdded70114290828c0a039e72ef25f5aaec4354a"
   integrity sha1-vd7XARQpCCjAoDnnLvJfWq7ENUo=
 
-ipaddr.js@1.9.0, ipaddr.js@^1.9.0:
+ipaddr.js@1.9.0:
   version "1.9.0"
   resolved "https://registry.yarnpkg.com/ipaddr.js/-/ipaddr.js-1.9.0.tgz#37df74e430a0e47550fe54a2defe30d8acd95f65"
   integrity sha512-M4Sjn6N/+O6/IXSJseKqHoFc+5FdGJ22sXqnjTpdZweHK64MzEPAyQZyEU3R/KRv2GLoa7nNtg/C2Ev6m7z+eA==
 
+ipaddr.js@^1.9.0, ipaddr.js@^1.9.1:
+  version "1.9.1"
+  resolved "https://registry.yarnpkg.com/ipaddr.js/-/ipaddr.js-1.9.1.tgz#bff38543eeb8984825079ff3a2a8e6cbd46781b3"
+  integrity sha512-0KI/607xoxSToH7GjN1FfSbLoU0+btTicjsQSWQlh/hZykN8KpmMf7uYwPW3R+akZ6R/w18ZlXSHBYXiYUPO3g==
+
 is-absolute-url@^3.0.3:
   version "3.0.3"
   resolved "https://registry.yarnpkg.com/is-absolute-url/-/is-absolute-url-3.0.3.tgz#96c6a22b6a23929b11ea0afb1836c36ad4a5d698"