From efd6ebcbb8330deec1b88dd65e8f54f729d5eab0 Mon Sep 17 00:00:00 2001 From: Zack Cuddy <zcuddy@gitlab.com> Date: Fri, 3 Mar 2023 20:55:24 +0000 Subject: [PATCH] Geo Sites - Remove primary version duplication This change removes the HTML data being passed that houses the Primary Site Version Data. We already fetch this data from the API and don't need a seperate data source for this anymore. --- .../details/geo_node_core_details.vue | 12 ++--- ee/app/assets/javascripts/geo_nodes/index.js | 4 +- .../javascripts/geo_nodes/store/getters.js | 7 +++ .../javascripts/geo_nodes/store/index.js | 9 +--- .../javascripts/geo_nodes/store/state.js | 4 +- ee/app/helpers/ee/geo_helper.rb | 11 ---- .../details/geo_node_core_details_spec.js | 31 ++++++----- ee/spec/frontend/geo_nodes/mock_data.js | 5 -- .../frontend/geo_nodes/store/actions_spec.js | 3 -- .../frontend/geo_nodes/store/getters_spec.js | 51 +++++++++++++++++-- .../geo_nodes/store/mutations_spec.js | 4 +- 11 files changed, 85 insertions(+), 56 deletions(-) diff --git a/ee/app/assets/javascripts/geo_nodes/components/details/geo_node_core_details.vue b/ee/app/assets/javascripts/geo_nodes/components/details/geo_node_core_details.vue index a8dfe1870685b..88565a37519d7 100644 --- a/ee/app/assets/javascripts/geo_nodes/components/details/geo_node_core_details.vue +++ b/ee/app/assets/javascripts/geo_nodes/components/details/geo_node_core_details.vue @@ -1,6 +1,6 @@ <script> import { GlLink, GlIcon } from '@gitlab/ui'; -import { mapState } from 'vuex'; +import { mapGetters } from 'vuex'; import { __, s__ } from '~/locale'; export default { @@ -22,17 +22,15 @@ export default { }, }, computed: { - ...mapState(['primaryVersion', 'primaryRevision']), + ...mapGetters(['nodeHasVersionMismatch']), nodeVersion() { if (!this.node.version || !this.node.revision) { return this.$options.i18n.unknown; } return `${this.node.version} (${this.node.revision})`; }, - versionMismatch() { - return ( - this.node.version !== this.primaryVersion || this.node.revision !== this.primaryRevision - ); + hasMismatchVersion() { + return this.nodeHasVersionMismatch(this.node.id); }, }, }; @@ -61,7 +59,7 @@ export default { <div class="gl-display-flex gl-flex-direction-column gl-lg-mt-5"> <span>{{ $options.i18n.gitlabVersion }}</span> <span - :class="{ 'gl-text-red-500': versionMismatch }" + :class="{ 'gl-text-red-500': hasMismatchVersion }" class="gl-font-weight-bold" data-testid="node-version" > diff --git a/ee/app/assets/javascripts/geo_nodes/index.js b/ee/app/assets/javascripts/geo_nodes/index.js index c67602d36f9bd..ee2265ef6e4d1 100644 --- a/ee/app/assets/javascripts/geo_nodes/index.js +++ b/ee/app/assets/javascripts/geo_nodes/index.js @@ -14,7 +14,7 @@ export const initGeoNodes = () => { return false; } - const { primaryVersion, primaryRevision, newNodeUrl, geoNodesEmptyStateSvg } = el.dataset; + const { newNodeUrl, geoNodesEmptyStateSvg } = el.dataset; const searchFilter = getParameterByName('search') || ''; let { replicableTypes } = el.dataset; @@ -22,7 +22,7 @@ export const initGeoNodes = () => { return new Vue({ el, - store: createStore({ primaryVersion, primaryRevision, replicableTypes, searchFilter }), + store: createStore({ replicableTypes, searchFilter }), provide: { geoNodesEmptyStateSvg, }, diff --git a/ee/app/assets/javascripts/geo_nodes/store/getters.js b/ee/app/assets/javascripts/geo_nodes/store/getters.js index 1f776ee7e6ae7..51750e5078e46 100644 --- a/ee/app/assets/javascripts/geo_nodes/store/getters.js +++ b/ee/app/assets/javascripts/geo_nodes/store/getters.js @@ -118,3 +118,10 @@ export const filteredNodes = (state) => { export const countNodesForStatus = (state) => (status) => { return state.nodes.filter(filterByStatus(status)).length; }; + +export const nodeHasVersionMismatch = (state) => (id) => { + const node = state.nodes.find((n) => n.id === id); + const primaryNode = state.nodes.find((n) => n.primary); + + return node?.version !== primaryNode?.version || node?.revision !== primaryNode?.revision; +}; diff --git a/ee/app/assets/javascripts/geo_nodes/store/index.js b/ee/app/assets/javascripts/geo_nodes/store/index.js index e85067ad8e403..b1606ce2f1fde 100644 --- a/ee/app/assets/javascripts/geo_nodes/store/index.js +++ b/ee/app/assets/javascripts/geo_nodes/store/index.js @@ -7,16 +7,11 @@ import createState from './state'; Vue.use(Vuex); -export const getStoreConfig = ({ - primaryVersion, - primaryRevision, - replicableTypes, - searchFilter = '', -}) => ({ +export const getStoreConfig = ({ replicableTypes, searchFilter = '' }) => ({ actions, getters, mutations, - state: createState({ primaryVersion, primaryRevision, replicableTypes, searchFilter }), + state: createState({ replicableTypes, searchFilter }), }); const createStore = (config) => new Vuex.Store(getStoreConfig(config)); diff --git a/ee/app/assets/javascripts/geo_nodes/store/state.js b/ee/app/assets/javascripts/geo_nodes/store/state.js index 3cbbe3d523ec9..5476ca7129469 100644 --- a/ee/app/assets/javascripts/geo_nodes/store/state.js +++ b/ee/app/assets/javascripts/geo_nodes/store/state.js @@ -1,6 +1,4 @@ -const createState = ({ primaryVersion, primaryRevision, replicableTypes, searchFilter }) => ({ - primaryVersion, - primaryRevision, +const createState = ({ replicableTypes, searchFilter }) => ({ replicableTypes, searchFilter, statusFilter: null, diff --git a/ee/app/helpers/ee/geo_helper.rb b/ee/app/helpers/ee/geo_helper.rb index d534aa74dd6e6..daf5de38a5032 100644 --- a/ee/app/helpers/ee/geo_helper.rb +++ b/ee/app/helpers/ee/geo_helper.rb @@ -17,18 +17,7 @@ def self.current_node_human_status end def node_vue_list_properties - version, revision = - if ::Gitlab::Geo.primary? - [::Gitlab::VERSION, ::Gitlab.revision] - else - status = ::Gitlab::Geo.primary_node&.status - - [status&.version, status&.revision] - end - { - primary_version: version.to_s, - primary_revision: revision.to_s, replicable_types: replicable_types.to_json, new_node_url: new_admin_geo_node_path, geo_nodes_empty_state_svg: image_path("illustrations/empty-state/geo-empty.svg") diff --git a/ee/spec/frontend/geo_nodes/components/details/geo_node_core_details_spec.js b/ee/spec/frontend/geo_nodes/components/details/geo_node_core_details_spec.js index bcc653d0c13f9..acbbb62b5a9a7 100644 --- a/ee/spec/frontend/geo_nodes/components/details/geo_node_core_details_spec.js +++ b/ee/spec/frontend/geo_nodes/components/details/geo_node_core_details_spec.js @@ -4,7 +4,6 @@ import Vuex from 'vuex'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import GeoNodeCoreDetails from 'ee/geo_nodes/components/details/geo_node_core_details.vue'; import { - MOCK_PRIMARY_VERSION, MOCK_REPLICABLE_TYPES, MOCK_PRIMARY_NODE, MOCK_SECONDARY_NODE, @@ -19,13 +18,18 @@ describe('GeoNodeCoreDetails', () => { node: MOCK_PRIMARY_NODE, }; - const createComponent = (initialState, props) => { + const defaultGetters = { + nodeHasVersionMismatch: () => () => false, + }; + + const createComponent = (props, getters) => { const store = new Vuex.Store({ state: { - primaryVersion: MOCK_PRIMARY_VERSION.version, - primaryRevision: MOCK_PRIMARY_VERSION.revision, replicableTypes: MOCK_REPLICABLE_TYPES, - ...initialState, + }, + getters: { + ...defaultGetters, + ...getters, }, }); @@ -66,7 +70,7 @@ describe('GeoNodeCoreDetails', () => { ${MOCK_SECONDARY_NODE} `('internal URL', ({ node }) => { beforeEach(() => { - createComponent(null, { node }); + createComponent({ node }); }); describe(`when primary is ${node.primary}`, () => { @@ -78,14 +82,17 @@ describe('GeoNodeCoreDetails', () => { describe('node version', () => { describe.each` - currentNode | versionText | versionMismatch - ${{ version: MOCK_PRIMARY_VERSION.version, revision: MOCK_PRIMARY_VERSION.revision }} | ${`${MOCK_PRIMARY_VERSION.version} (${MOCK_PRIMARY_VERSION.revision})`} | ${false} - ${{ version: 'asdf', revision: MOCK_PRIMARY_VERSION.revision }} | ${`asdf (${MOCK_PRIMARY_VERSION.revision})`} | ${true} - ${{ version: MOCK_PRIMARY_VERSION.version, revision: 'asdf' }} | ${`${MOCK_PRIMARY_VERSION.version} (asdf)`} | ${true} - ${{ version: null, revision: null }} | ${'Unknown'} | ${true} + currentNode | versionText | versionMismatch + ${{ version: MOCK_PRIMARY_NODE.version, revision: MOCK_PRIMARY_NODE.revision }} | ${`${MOCK_PRIMARY_NODE.version} (${MOCK_PRIMARY_NODE.revision})`} | ${false} + ${{ version: 'asdf', revision: MOCK_PRIMARY_NODE.revision }} | ${`asdf (${MOCK_PRIMARY_NODE.revision})`} | ${true} + ${{ version: MOCK_PRIMARY_NODE.version, revision: 'asdf' }} | ${`${MOCK_PRIMARY_NODE.version} (asdf)`} | ${true} + ${{ version: null, revision: null }} | ${'Unknown'} | ${true} `(`conditionally`, ({ currentNode, versionText, versionMismatch }) => { beforeEach(() => { - createComponent(null, { node: { ...MOCK_PRIMARY_NODE, ...currentNode } }); + createComponent( + { node: { ...MOCK_PRIMARY_NODE, ...currentNode } }, + { nodeHasVersionMismatch: () => () => versionMismatch }, + ); }); describe(`when version mismatch is ${versionMismatch} and current node version is ${versionText}`, () => { diff --git a/ee/spec/frontend/geo_nodes/mock_data.js b/ee/spec/frontend/geo_nodes/mock_data.js index 41c5cc2a94e8c..2199165f726a3 100644 --- a/ee/spec/frontend/geo_nodes/mock_data.js +++ b/ee/spec/frontend/geo_nodes/mock_data.js @@ -2,11 +2,6 @@ export const MOCK_NEW_NODE_URL = 'http://localhost:3000/admin/geo/sites/new'; export const MOCK_EMPTY_STATE_SVG = 'illustrations/empty-state/geo-empty.svg'; -export const MOCK_PRIMARY_VERSION = { - version: '10.4.0-pre', - revision: 'b93c51849b', -}; - export const MOCK_REPLICABLE_TYPES = [ { dataType: 'repository', diff --git a/ee/spec/frontend/geo_nodes/store/actions_spec.js b/ee/spec/frontend/geo_nodes/store/actions_spec.js index a106d044a33c8..483ab0ebef5b6 100644 --- a/ee/spec/frontend/geo_nodes/store/actions_spec.js +++ b/ee/spec/frontend/geo_nodes/store/actions_spec.js @@ -7,7 +7,6 @@ import { createAlert } from '~/flash'; import axios from '~/lib/utils/axios_utils'; import { HTTP_STATUS_INTERNAL_SERVER_ERROR, HTTP_STATUS_OK } from '~/lib/utils/http_status'; import { - MOCK_PRIMARY_VERSION, MOCK_REPLICABLE_TYPES, MOCK_NODES, MOCK_NODES_RES, @@ -22,8 +21,6 @@ describe('GeoNodes Store Actions', () => { beforeEach(() => { state = createState({ - primaryVersion: MOCK_PRIMARY_VERSION.version, - primaryRevision: MOCK_PRIMARY_VERSION.revision, replicableTypes: MOCK_REPLICABLE_TYPES, }); mock = new MockAdapter(axios); diff --git a/ee/spec/frontend/geo_nodes/store/getters_spec.js b/ee/spec/frontend/geo_nodes/store/getters_spec.js index cdc52733f1417..c8b6a8cb9676d 100644 --- a/ee/spec/frontend/geo_nodes/store/getters_spec.js +++ b/ee/spec/frontend/geo_nodes/store/getters_spec.js @@ -1,7 +1,6 @@ import * as getters from 'ee/geo_nodes/store/getters'; import createState from 'ee/geo_nodes/store/state'; import { - MOCK_PRIMARY_VERSION, MOCK_REPLICABLE_TYPES, MOCK_NODES, MOCK_PRIMARY_VERIFICATION_INFO, @@ -18,8 +17,6 @@ describe('GeoNodes Store Getters', () => { beforeEach(() => { state = createState({ - primaryVersion: MOCK_PRIMARY_VERSION.version, - primaryRevision: MOCK_PRIMARY_VERSION.revision, replicableTypes: MOCK_REPLICABLE_TYPES, }); }); @@ -157,4 +154,52 @@ describe('GeoNodes Store Getters', () => { }, ); }); + + describe('nodeHasVersionMismatch', () => { + const NODE_ID = '9'; + + describe.each` + node | nodeHasVersionMismatch + ${{ id: NODE_ID, version: '1.0.0', revision: 'asdf' }} | ${true} + ${{ id: NODE_ID, version: MOCK_PRIMARY_NODE.version, revision: 'asdf' }} | ${true} + ${{ id: NODE_ID, version: '1.0.0', revision: MOCK_PRIMARY_NODE.revision }} | ${true} + ${{ id: NODE_ID, version: MOCK_PRIMARY_NODE.version, revision: MOCK_PRIMARY_NODE.revision }} | ${false} + `('when primary site exists', ({ node, nodeHasVersionMismatch }) => { + describe(`when node version: ${node.version} (${node.revision}) and primary node version: ${MOCK_PRIMARY_NODE.version} (${MOCK_PRIMARY_NODE.revision}) version mismatch is ${nodeHasVersionMismatch}`, () => { + beforeEach(() => { + state.nodes = [node, MOCK_PRIMARY_NODE]; + }); + + it(`should return ${nodeHasVersionMismatch}`, () => { + expect(getters.nodeHasVersionMismatch(state)(NODE_ID)).toBe(nodeHasVersionMismatch); + }); + }); + }); + + describe('when passed in site does not exist', () => { + beforeEach(() => { + state.nodes = [MOCK_PRIMARY_NODE]; + }); + + it('should return true', () => { + expect(getters.nodeHasVersionMismatch(state)(NODE_ID)).toBe(true); + }); + }); + + describe('when primary site does not exist', () => { + const node = { + id: NODE_ID, + version: MOCK_PRIMARY_NODE.version, + revision: MOCK_PRIMARY_NODE.revision, + }; + + beforeEach(() => { + state.nodes = [node]; + }); + + it('should return true', () => { + expect(getters.nodeHasVersionMismatch(state)(NODE_ID)).toBe(true); + }); + }); + }); }); diff --git a/ee/spec/frontend/geo_nodes/store/mutations_spec.js b/ee/spec/frontend/geo_nodes/store/mutations_spec.js index 233f7366e59bf..00071bf65b293 100644 --- a/ee/spec/frontend/geo_nodes/store/mutations_spec.js +++ b/ee/spec/frontend/geo_nodes/store/mutations_spec.js @@ -1,14 +1,12 @@ import * as types from 'ee/geo_nodes/store/mutation_types'; import mutations from 'ee/geo_nodes/store/mutations'; import createState from 'ee/geo_nodes/store/state'; -import { MOCK_PRIMARY_VERSION, MOCK_REPLICABLE_TYPES, MOCK_NODES } from '../mock_data'; +import { MOCK_REPLICABLE_TYPES, MOCK_NODES } from '../mock_data'; describe('GeoNodes Store Mutations', () => { let state; beforeEach(() => { state = createState({ - primaryVersion: MOCK_PRIMARY_VERSION.version, - primaryRevision: MOCK_PRIMARY_VERSION.revision, replicableTypes: MOCK_REPLICABLE_TYPES, }); }); -- GitLab