Commit 441498ec authored by Allen Lai's avatar Allen Lai Committed by Paul Slaughter

Replace Vue key=index in licenses list table

**Why?**

In Vue, lists are not re-rendered, but patched when new props come. This
can cause issues when items which are actually different share the same key.

- https://gitlab.com/gitlab-org/gitlab/issues/39126
parent 566b0e4a
...@@ -32,8 +32,8 @@ export default { ...@@ -32,8 +32,8 @@ export default {
<div> <div>
<div class="gl-responsive-table-row table-row-header text-2 bg-secondary-50 px-2" role="row"> <div class="gl-responsive-table-row table-row-header text-2 bg-secondary-50 px-2" role="row">
<div <div
v-for="(header, index) in tableHeaders" v-for="header in tableHeaders"
:key="index" :key="header.label"
class="table-section" class="table-section"
:class="header.className" :class="header.className"
role="rowheader" role="rowheader"
...@@ -43,8 +43,8 @@ export default { ...@@ -43,8 +43,8 @@ export default {
</div> </div>
<licenses-table-row <licenses-table-row
v-for="(license, index) in licenses" v-for="license in licenses"
:key="index" :key="license.key"
:license="license" :license="license"
:is-loading="isLoading" :is-loading="isLoading"
/> />
......
import * as types from './mutation_types'; import * as types from './mutation_types';
import { toLicenseObject } from '../../../utils/mappers';
export default { export default {
[types.SET_LICENSES_ENDPOINT](state, payload) { [types.SET_LICENSES_ENDPOINT](state, payload) {
...@@ -9,7 +10,7 @@ export default { ...@@ -9,7 +10,7 @@ export default {
state.errorLoading = false; state.errorLoading = false;
}, },
[types.RECEIVE_LICENSES_SUCCESS](state, { licenses, reportInfo, pageInfo }) { [types.RECEIVE_LICENSES_SUCCESS](state, { licenses, reportInfo, pageInfo }) {
state.licenses = licenses; state.licenses = licenses.map(toLicenseObject);
state.pageInfo = pageInfo; state.pageInfo = pageInfo;
state.isLoading = false; state.isLoading = false;
state.errorLoading = false; state.errorLoading = false;
......
import _ from 'underscore';
export const getLicenseKey = ({ id }) => {
if (id) {
return `id_${id}`;
}
return `client_${_.uniqueId()}`;
};
/**
* Maps an individual license response entity into the license object we'll store in our Vuex state
* @param {Object} license
*/
export const toLicenseObject = license => ({
...license,
key: getLicenseKey(license),
});
---
title: Generate appropriate Vue key for licenses list table
merge_request: 21566
author: allenlai18
type: other
import { createLocalVue, shallowMount } from '@vue/test-utils'; import { createLocalVue, shallowMount } from '@vue/test-utils';
import createStore from 'ee/project_licenses/store'; import createStore from 'ee/project_licenses/store';
import LicensesTable from 'ee/project_licenses/components/licenses_table.vue'; import LicensesTable from 'ee/project_licenses/components/licenses_table.vue';
import { toLicenseObject } from 'ee/project_licenses/utils/mappers';
import { LICENSE_LIST } from 'ee/project_licenses/store/constants'; import { LICENSE_LIST } from 'ee/project_licenses/store/constants';
import PaginatedLicensesTable from 'ee/project_licenses/components/paginated_licenses_table.vue'; import PaginatedLicensesTable from 'ee/project_licenses/components/paginated_licenses_table.vue';
import Pagination from '~/vue_shared/components/pagination_links.vue'; import Pagination from '~/vue_shared/components/pagination_links.vue';
import mockLicensesResponse from '../store/modules/list/data/mock_licenses'; import mockLicensesResponse from '../store/modules/list/data/mock_licenses';
jest.mock('underscore', () => ({
uniqueId: () => 'fakeUniqueId',
}));
describe('PaginatedLicensesTable component', () => { describe('PaginatedLicensesTable component', () => {
const localVue = createLocalVue(); const localVue = createLocalVue();
const namespace = LICENSE_LIST; const namespace = LICENSE_LIST;
...@@ -48,7 +53,7 @@ describe('PaginatedLicensesTable component', () => { ...@@ -48,7 +53,7 @@ describe('PaginatedLicensesTable component', () => {
it('passes the correct props to the licenses table', () => { it('passes the correct props to the licenses table', () => {
expectComponentWithProps(LicensesTable, { expectComponentWithProps(LicensesTable, {
licenses: mockLicensesResponse.licenses, licenses: mockLicensesResponse.licenses.map(toLicenseObject),
isLoading: store.state[namespace].isLoading, isLoading: store.state[namespace].isLoading,
}); });
}); });
......
import * as types from 'ee/project_licenses/store/modules/list/mutation_types'; import * as types from 'ee/project_licenses/store/modules/list/mutation_types';
import mutations from 'ee/project_licenses/store/modules/list/mutations'; import mutations from 'ee/project_licenses/store/modules/list/mutations';
import { toLicenseObject } from 'ee/project_licenses/utils/mappers';
import getInitialState from 'ee/project_licenses/store/modules/list/state'; import getInitialState from 'ee/project_licenses/store/modules/list/state';
import { REPORT_STATUS } from 'ee/project_licenses/store/modules/list/constants'; import { REPORT_STATUS } from 'ee/project_licenses/store/modules/list/constants';
import { TEST_HOST } from 'helpers/test_constants'; import { TEST_HOST } from 'helpers/test_constants';
...@@ -31,7 +32,7 @@ describe('Licenses mutations', () => { ...@@ -31,7 +32,7 @@ describe('Licenses mutations', () => {
}); });
describe(types.RECEIVE_LICENSES_SUCCESS, () => { describe(types.RECEIVE_LICENSES_SUCCESS, () => {
const licenses = []; const licenses = [{ id: 1 }, { id: 2 }];
const pageInfo = {}; const pageInfo = {};
const reportInfo = { const reportInfo = {
status: REPORT_STATUS.jobFailed, status: REPORT_STATUS.jobFailed,
...@@ -45,7 +46,7 @@ describe('Licenses mutations', () => { ...@@ -45,7 +46,7 @@ describe('Licenses mutations', () => {
it('correctly mutates the state', () => { it('correctly mutates the state', () => {
expect(state.isLoading).toBe(false); expect(state.isLoading).toBe(false);
expect(state.errorLoading).toBe(false); expect(state.errorLoading).toBe(false);
expect(state.licenses).toBe(licenses); expect(state.licenses).toEqual(licenses.map(toLicenseObject));
expect(state.pageInfo).toBe(pageInfo); expect(state.pageInfo).toBe(pageInfo);
expect(state.initialized).toBe(true); expect(state.initialized).toBe(true);
expect(state.reportInfo).toEqual({ expect(state.reportInfo).toEqual({
......
import { toLicenseObject } from 'ee/project_licenses/utils/mappers';
const UNIQUE_ID = 'fakeUniqueId';
jest.mock('underscore', () => ({
uniqueId: () => 'fakeUniqueId',
}));
describe('ee/project_licenses/utils/mappers', () => {
describe('toLicenseObject', () => {
describe.each`
input | key
${{ id: 1, name: 'TEST' }} | ${'id_1'}
${{ id: 2001, name: 'TEST' }} | ${'id_2001'}
${{ name: 'TEST' }} | ${`client_${UNIQUE_ID}`}
`('with object $input', ({ input, key }) => {
it(`sets the key to ${key}`, () => {
const result = toLicenseObject(input);
expect(result).toEqual({
...input,
key,
});
});
});
});
});
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment