Commit fbee16a3 authored by samdbeckham's avatar samdbeckham

Adds changes from @kushalpandya review

- improves tests with beforeEach and stricter value checks
- improves actions to reduce additional desctructuring
- uses vue.$options to improve the report_type_popover i18n
parent bb879b2b
<script> <script>
import { GlPopover } from '@gitlab/ui'; import { GlPopover } from '@gitlab/ui';
import { s__ } from '~/locale';
import Icon from '~/vue_shared/components/icon.vue'; import Icon from '~/vue_shared/components/icon.vue';
export default { export default {
...@@ -13,6 +14,7 @@ export default { ...@@ -13,6 +14,7 @@ export default {
required: true, required: true,
}, },
}, },
linkTitle: s__('Security Reports|Security dashboard documentation'),
}; };
</script> </script>
...@@ -33,12 +35,10 @@ export default { ...@@ -33,12 +35,10 @@ export default {
v-if="dashboardDocumentation" v-if="dashboardDocumentation"
target="_blank" target="_blank"
rel="noopener noreferrer" rel="noopener noreferrer"
:title="s__('Security Reports|Security dashboard documentation')" :title="$options.linkTitle"
:href="dashboardDocumentation" :href="dashboardDocumentation"
> >
<span class="vertical-align-middle">{{ <span class="vertical-align-middle">{{ $options.linkTitle }}</span>
s__('Security Reports|Security dashboard documentation')
}}</span>
<icon name="external-link" :size="16" class="vertical-align-middle" /> <icon name="external-link" :size="16" class="vertical-align-middle" />
</a> </a>
</gl-popover> </gl-popover>
......
...@@ -12,9 +12,8 @@ export const fetchProjects = ({ state, dispatch }) => { ...@@ -12,9 +12,8 @@ export const fetchProjects = ({ state, dispatch }) => {
method: 'GET', method: 'GET',
url: state.projectsEndpoint, url: state.projectsEndpoint,
}) })
.then(response => { .then(({ data }) => {
const { data } = response; dispatch('receiveProjectsSuccess', { projects: data });
dispatch('receiveProjectsSuccess', { data });
}) })
.catch(() => { .catch(() => {
dispatch('receiveProjectsError'); dispatch('receiveProjectsError');
...@@ -25,9 +24,7 @@ export const requestProjects = ({ commit }) => { ...@@ -25,9 +24,7 @@ export const requestProjects = ({ commit }) => {
commit(types.REQUEST_PROJECTS); commit(types.REQUEST_PROJECTS);
}; };
export const receiveProjectsSuccess = ({ commit }, { data }) => { export const receiveProjectsSuccess = ({ commit }, { projects }) => {
const projects = data;
commit(types.RECEIVE_PROJECTS_SUCCESS, { projects }); commit(types.RECEIVE_PROJECTS_SUCCESS, { projects });
}; };
......
...@@ -17,10 +17,14 @@ describe('filters module getters', () => { ...@@ -17,10 +17,14 @@ describe('filters module getters', () => {
getFilterIds, getFilterIds,
}; };
}; };
let state;
beforeEach(() => {
state = createState();
});
describe('getFilter', () => { describe('getFilter', () => {
it('should return the type filter information', () => { it('should return the type filter information', () => {
const state = createState();
const typeFilter = getters.getFilter(state)('report_type'); const typeFilter = getters.getFilter(state)('report_type');
expect(typeFilter.name).toEqual('Report type'); expect(typeFilter.name).toEqual('Report type');
...@@ -30,7 +34,6 @@ describe('filters module getters', () => { ...@@ -30,7 +34,6 @@ describe('filters module getters', () => {
describe('getSelectedOptions', () => { describe('getSelectedOptions', () => {
describe('with one selected option', () => { describe('with one selected option', () => {
it('should return "All" as the selected option', () => { it('should return "All" as the selected option', () => {
const state = createState();
const selectedOptions = getters.getSelectedOptions(state, mockedGetters(state))( const selectedOptions = getters.getSelectedOptions(state, mockedGetters(state))(
'report_type', 'report_type',
); );
...@@ -42,7 +45,7 @@ describe('filters module getters', () => { ...@@ -42,7 +45,7 @@ describe('filters module getters', () => {
describe('with multiple selected options', () => { describe('with multiple selected options', () => {
it('should return both "High" and "Critical" ', () => { it('should return both "High" and "Critical" ', () => {
const state = { state = {
filters: [ filters: [
{ {
id: 'severity', id: 'severity',
...@@ -59,7 +62,6 @@ describe('filters module getters', () => { ...@@ -59,7 +62,6 @@ describe('filters module getters', () => {
describe('getSelectedOptionIds', () => { describe('getSelectedOptionIds', () => {
it('should return "one" as the selcted dummy ID', () => { it('should return "one" as the selcted dummy ID', () => {
const state = createState();
const dummyFilter = { const dummyFilter = {
id: 'dummy', id: 'dummy',
options: [{ id: 'one', selected: true }, { id: 'anotherone', selected: false }], options: [{ id: 'one', selected: true }, { id: 'anotherone', selected: false }],
...@@ -74,7 +76,6 @@ describe('filters module getters', () => { ...@@ -74,7 +76,6 @@ describe('filters module getters', () => {
describe('getSelectedOptionNames', () => { describe('getSelectedOptionNames', () => {
it('should return "All" as the selected option', () => { it('should return "All" as the selected option', () => {
const state = createState();
const selectedOptionNames = getters.getSelectedOptionNames(state, mockedGetters(state))( const selectedOptionNames = getters.getSelectedOptionNames(state, mockedGetters(state))(
'severity', 'severity',
); );
...@@ -83,7 +84,7 @@ describe('filters module getters', () => { ...@@ -83,7 +84,7 @@ describe('filters module getters', () => {
}); });
it('should return the correct message when multiple filters are selected', () => { it('should return the correct message when multiple filters are selected', () => {
const state = { state = {
filters: [ filters: [
{ {
id: 'severity', id: 'severity',
...@@ -101,14 +102,12 @@ describe('filters module getters', () => { ...@@ -101,14 +102,12 @@ describe('filters module getters', () => {
describe('activeFilters', () => { describe('activeFilters', () => {
it('should return no severity filters', () => { it('should return no severity filters', () => {
const state = createState();
const activeFilters = getters.activeFilters(state, mockedGetters(state)); const activeFilters = getters.activeFilters(state, mockedGetters(state));
expect(activeFilters.severity).toHaveLength(0); expect(activeFilters.severity).toHaveLength(0);
}); });
it('should return multiple dummy filters"', () => { it('should return multiple dummy filters"', () => {
const state = createState();
const dummyFilter = { const dummyFilter = {
id: 'dummy', id: 'dummy',
options: [{ id: 'one', selected: true }, { id: 'anotherone', selected: true }], options: [{ id: 'one', selected: true }, { id: 'anotherone', selected: true }],
......
...@@ -41,7 +41,7 @@ describe('projects actions', () => { ...@@ -41,7 +41,7 @@ describe('projects actions', () => {
{ type: 'requestProjects' }, { type: 'requestProjects' },
{ {
type: 'receiveProjectsSuccess', type: 'receiveProjectsSuccess',
payload: { data }, payload: { projects: data },
}, },
], ],
done, done,
...@@ -73,7 +73,7 @@ describe('projects actions', () => { ...@@ -73,7 +73,7 @@ describe('projects actions', () => {
testAction( testAction(
actions.receiveProjectsSuccess, actions.receiveProjectsSuccess,
{ data }, { projects: data },
state, state,
[ [
{ {
......
...@@ -27,11 +27,11 @@ describe('projects module mutations', () => { ...@@ -27,11 +27,11 @@ describe('projects module mutations', () => {
}); });
it('should set `isLoadingProjects` to `true`', () => { it('should set `isLoadingProjects` to `true`', () => {
expect(state.isLoadingProjects).toBeTruthy(); expect(state.isLoadingProjects).toBe(true);
}); });
it('should set `errorLoadingProjects` to `false`', () => { it('should set `errorLoadingProjects` to `false`', () => {
expect(state.errorLoadingProjects).toBeFalsy(); expect(state.errorLoadingProjects).toBe(false);
}); });
}); });
...@@ -48,7 +48,7 @@ describe('projects module mutations', () => { ...@@ -48,7 +48,7 @@ describe('projects module mutations', () => {
}); });
it('should set `isLoadingProjects` to `false`', () => { it('should set `isLoadingProjects` to `false`', () => {
expect(state.isLoadingProjects).toBeFalsy(); expect(state.isLoadingProjects).toBe(false);
}); });
it('should set `pageInfo`', () => { it('should set `pageInfo`', () => {
...@@ -66,7 +66,7 @@ describe('projects module mutations', () => { ...@@ -66,7 +66,7 @@ describe('projects module mutations', () => {
mutations[types.RECEIVE_PROJECTS_ERROR](state); mutations[types.RECEIVE_PROJECTS_ERROR](state);
expect(state.isLoadingProjects).toBeFalsy(); expect(state.isLoadingProjects).toBe(false);
}); });
}); });
}); });
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