Commit 15511ed1 authored by Filipa Lacerda's avatar Filipa Lacerda

Changes after review:

- Cleans up CSS to use common classes
- Removes getters to use mapState instead
- Makes the first request even when tab is not visible
- Show loading state in 204
parent d40ef4c8
<script> <script>
import { mapActions, mapGetters } from 'vuex'; import { mapActions, mapGetters, mapState } from 'vuex';
import { s__ } from '~/locale'; import { s__ } from '~/locale';
import { componentNames } from '~/vue_shared/components/reports/issue_body'; import { componentNames } from '~/vue_shared/components/reports/issue_body';
import ReportSection from '~/vue_shared/components/reports/report_section.vue'; import ReportSection from '~/vue_shared/components/reports/report_section.vue';
...@@ -26,27 +26,29 @@ ...@@ -26,27 +26,29 @@
}, },
componentNames, componentNames,
computed: { computed: {
...mapGetters([ ...mapState([
'reports', 'reports',
'summaryStatus',
'isLoading', 'isLoading',
'hasError', 'hasError',
'summaryCounts', 'summary',
'modalTitle', ]),
'modalData', ...mapState({
'isCreatingNewIssue', modalTitle: state => state.modal.title || '',
modalData: state => state.modal.data || {},
}),
...mapGetters([
'summaryStatus',
]), ]),
groupedSummaryText() { groupedSummaryText() {
if (this.isLoading) { if (this.isLoading) {
return s__('Reports|Test summary results are being parsed'); return s__('Reports|Test summary results are being parsed');
} }
if (this.hasError || !this.summaryCounts) { if (this.hasError) {
return s__('Reports|Test summary failed loading results'); return s__('Reports|Test summary failed loading results');
} }
return summaryTextBuilder(s__('Reports|Test summary'), this.summaryCounts); return summaryTextBuilder(s__('Reports|Test summary'), this.summary);
}, },
}, },
created() { created() {
......
...@@ -36,9 +36,9 @@ ...@@ -36,9 +36,9 @@
:key="index" :key="index"
class="row prepend-top-10 append-bottom-10" class="row prepend-top-10 append-bottom-10"
> >
<label class="col-sm-2 text-right font-weight-bold"> <strong class="col-sm-2 text-right">
{{ field.text }}: {{ field.text }}:
</label> </strong>
<div class="col-sm-10 text-secondary"> <div class="col-sm-10 text-secondary">
<code-block <code-block
......
...@@ -21,10 +21,6 @@ ...@@ -21,10 +21,6 @@
}, },
methods: { methods: {
...mapActions(['openModal']), ...mapActions(['openModal']),
handleIssueClick() {
const { issue, status, openModal } = this;
openModal({ issue, status });
},
}, },
}; };
</script> </script>
...@@ -34,7 +30,7 @@ ...@@ -34,7 +30,7 @@
<button <button
type="button" type="button"
class="btn-link btn-blank text-left break-link vulnerability-name-button" class="btn-link btn-blank text-left break-link vulnerability-name-button"
@click="handleIssueClick()" @click="openModal({ issue })"
> >
<div <div
v-if="isNew" v-if="isNew"
......
...@@ -3,6 +3,7 @@ import $ from 'jquery'; ...@@ -3,6 +3,7 @@ import $ from 'jquery';
import axios from '../../lib/utils/axios_utils'; import axios from '../../lib/utils/axios_utils';
import Poll from '../../lib/utils/poll'; import Poll from '../../lib/utils/poll';
import * as types from './mutation_types'; import * as types from './mutation_types';
import httpStatusCodes from '../../lib/utils/http_status';
export const setEndpoint = ({ commit }, endpoint) => commit(types.SET_ENDPOINT, endpoint); export const setEndpoint = ({ commit }, endpoint) => commit(types.SET_ENDPOINT, endpoint);
...@@ -42,12 +43,17 @@ export const fetchReports = ({ state, dispatch }) => { ...@@ -42,12 +43,17 @@ export const fetchReports = ({ state, dispatch }) => {
}, },
data: state.endpoint, data: state.endpoint,
method: 'getReports', method: 'getReports',
successCallback: ({ data }) => dispatch('receiveReportsSuccess', data), successCallback: (response) => dispatch('receiveReportsSuccess', response),
errorCallback: () => dispatch('receiveReportsError'), errorCallback: () => dispatch('receiveReportsError'),
}); });
if (!Visibility.hidden()) { if (!Visibility.hidden()) {
eTagPoll.makeRequest(); eTagPoll.makeRequest();
} else {
axios
.get(state.endpoint)
.then((response) => dispatch('receiveReportsSuccess', response))
.catch(() => dispatch('receiveReportsError'));
} }
Visibility.change(() => { Visibility.change(() => {
...@@ -59,8 +65,12 @@ export const fetchReports = ({ state, dispatch }) => { ...@@ -59,8 +65,12 @@ export const fetchReports = ({ state, dispatch }) => {
}); });
}; };
export const receiveReportsSuccess = ({ commit }, response) => export const receiveReportsSuccess = ({ commit }, response) => {
commit(types.RECEIVE_REPORTS_SUCCESS, response); // With 204 we keep polling and don't update the state
if (response.status === httpStatusCodes.OK) {
commit(types.RECEIVE_REPORTS_SUCCESS, response.data);
}
};
export const receiveReportsError = ({ commit }) => commit(types.RECEIVE_REPORTS_ERROR); export const receiveReportsError = ({ commit }) => commit(types.RECEIVE_REPORTS_ERROR);
......
import { LOADING, ERROR, SUCCESS } from '../constants'; import { LOADING, ERROR, SUCCESS, STATUS_FAILED } from '../constants';
export const reports = state => state.reports;
export const summaryCounts = state => state.summary;
export const isLoading = state => state.isLoading;
export const hasError = state => state.hasError;
export const modalTitle = state => state.modal.title || '';
export const modalData = state => state.modal.data || {};
export const isCreatingNewIssue = state => state.modal.isLoading;
export const summaryStatus = state => { export const summaryStatus = state => {
if (state.isLoading) { if (state.isLoading) {
return LOADING; return LOADING;
} }
if (state.hasError) { if (state.hasError || state.status === STATUS_FAILED) {
return ERROR; return ERROR;
} }
......
...@@ -23,11 +23,17 @@ export default { ...@@ -23,11 +23,17 @@ export default {
[types.RECEIVE_REPORTS_ERROR](state) { [types.RECEIVE_REPORTS_ERROR](state) {
state.isLoading = false; state.isLoading = false;
state.hasError = true; state.hasError = true;
state.reports = []; state.reports = [];
state.summary = {
total: 0,
resolved: 0,
failed: 0,
};
state.status = null;
}, },
[types.SET_ISSUE_MODAL_DATA](state, payload) { [types.SET_ISSUE_MODAL_DATA](state, payload) {
state.modal.title = payload.issue.name; state.modal.title = payload.issue.name;
state.modal.status = payload.status;
Object.keys(payload.issue).forEach((key) => { Object.keys(payload.issue).forEach((key) => {
if (Object.prototype.hasOwnProperty.call(state.modal.data, key)) { if (Object.prototype.hasOwnProperty.call(state.modal.data, key)) {
......
...@@ -34,8 +34,6 @@ export default () => ({ ...@@ -34,8 +34,6 @@ export default () => ({
modal: { modal: {
title: null, title: null,
status: null,
data: { data: {
class: { class: {
value: null, value: null,
......
...@@ -17,7 +17,8 @@ const textBuilder = results => { ...@@ -17,7 +17,8 @@ const textBuilder = results => {
? n__('%d fixed test result', '%d fixed test results', resolved) ? n__('%d fixed test result', '%d fixed test results', resolved)
: null; : null;
const totalString = total ? n__('out of %d total test', 'out of %d total tests', total) : null; const totalString = total ? n__('out of %d total test', 'out of %d total tests', total) : null;
let resultsString;
let resultsString = s__('Reports|no changed test results');
if (failed) { if (failed) {
if (resolved) { if (resolved) {
...@@ -30,19 +31,17 @@ const textBuilder = results => { ...@@ -30,19 +31,17 @@ const textBuilder = results => {
} }
} else if (resolved) { } else if (resolved) {
resultsString = resolvedString; resultsString = resolvedString;
} else {
resultsString = s__('Reports|no changed test results');
} }
return `${resultsString} ${totalString}`; return `${resultsString} ${totalString}`;
}; };
export const summaryTextBuilder = (name = '', results) => { export const summaryTextBuilder = (name = '', results = {}) => {
const resultsString = textBuilder(results); const resultsString = textBuilder(results);
return `${name} contained ${resultsString}`; return `${name} contained ${resultsString}`;
}; };
export const reportTextBuilder = (name = '', results) => { export const reportTextBuilder = (name = '', results = {}) => {
const resultsString = textBuilder(results); const resultsString = textBuilder(results);
return `${name} found ${resultsString}`; return `${name} found ${resultsString}`;
}; };
......
...@@ -10,7 +10,7 @@ export default { ...@@ -10,7 +10,7 @@ export default {
}; };
</script> </script>
<template> <template>
<pre class="code-block build-trace-rounded"> <pre class="code-block rounded">
<code class="bash">{{ code }}</code> <code class="d-block">{{ code }}</code>
</pre> </pre>
</template> </template>
...@@ -360,7 +360,6 @@ ...@@ -360,7 +360,6 @@
white-space: pre; white-space: pre;
overflow-x: auto; overflow-x: auto;
font-size: 12px; font-size: 12px;
border-radius: 0;
border: 0; border: 0;
padding: $grid-size; padding: $grid-size;
...@@ -368,12 +367,4 @@ ...@@ -368,12 +367,4 @@
background-color: inherit; background-color: inherit;
padding: inherit; padding: inherit;
} }
.bash {
display: block;
}
&.build-trace-rounded {
border-radius: $border-radius-base;
}
} }
...@@ -469,4 +469,4 @@ img.emoji { ...@@ -469,4 +469,4 @@ img.emoji {
.inline { display: inline-block; } .inline { display: inline-block; }
.center { text-align: center; } .center { text-align: center; }
.vertical-align-middle { vertical-align: middle; } .vertical-align-middle { vertical-align: middle; }
.flex-align-self-center { align-self: center; } .flex-align-self-center { align-self: center; }
\ No newline at end of file
...@@ -49,6 +49,26 @@ describe('Grouped Test Reports App', () => { ...@@ -49,6 +49,26 @@ describe('Grouped Test Reports App', () => {
}); });
}); });
describe('with 204 result', () => {
beforeEach(() => {
mock.onGet('test_results.json').reply(204, {}, {});
vm = mountComponent(Component, {
endpoint: 'test_results.json',
});
});
it('renders success summary text', done => {
setTimeout(() => {
expect(vm.$el.querySelector('.fa-spinner')).not.toBeNull();
expect(vm.$el.querySelector('.js-code-text').textContent.trim()).toEqual(
'Test summary results are being parsed',
);
done();
}, 0);
});
});
describe('with new failed result', () => { describe('with new failed result', () => {
beforeEach(() => { beforeEach(() => {
mock.onGet('test_results.json').reply(200, newFailedTestReports, {}); mock.onGet('test_results.json').reply(200, newFailedTestReports, {});
......
...@@ -31,7 +31,6 @@ describe('Test Issue body', () => { ...@@ -31,7 +31,6 @@ describe('Test Issue body', () => {
vm.$el.querySelector('button').click(); vm.$el.querySelector('button').click();
expect(vm.openModal).toHaveBeenCalledWith({ expect(vm.openModal).toHaveBeenCalledWith({
issue: commonProps.issue, issue: commonProps.issue,
status: commonProps.status,
}); });
}); });
}); });
......
...@@ -43,24 +43,21 @@ describe('Reports Store Mutations', () => { ...@@ -43,24 +43,21 @@ describe('Reports Store Mutations', () => {
{ {
name: 'StringHelper#concatenate when a is git and b is lab returns summary', name: 'StringHelper#concatenate when a is git and b is lab returns summary',
execution_time: 0.0092435, execution_time: 0.0092435,
system_output: system_output: "Failure/Error: is_expected.to eq('gitlab')",
'Failure/Error: is_expected.to eq(\'gitlab\')',
}, },
], ],
resolved_failures: [ resolved_failures: [
{ {
name: 'StringHelper#concatenate when a is git and b is lab returns summary', name: 'StringHelper#concatenate when a is git and b is lab returns summary',
execution_time: 0.009235, execution_time: 0.009235,
system_output: system_output: "Failure/Error: is_expected.to eq('gitlab')",
'Failure/Error: is_expected.to eq(\'gitlab\')',
}, },
], ],
existing_failures: [ existing_failures: [
{ {
name: 'StringHelper#concatenate when a is git and b is lab returns summary', name: 'StringHelper#concatenate when a is git and b is lab returns summary',
execution_time: 1232.08, execution_time: 1232.08,
system_output: system_output: "Failure/Error: is_expected.to eq('gitlab')",
'Failure/Error: is_expected.to eq(\'gitlab\')',
}, },
], ],
}, },
...@@ -108,7 +105,6 @@ describe('Reports Store Mutations', () => { ...@@ -108,7 +105,6 @@ describe('Reports Store Mutations', () => {
beforeEach(() => { beforeEach(() => {
mutations[types.SET_ISSUE_MODAL_DATA](stateCopy, { mutations[types.SET_ISSUE_MODAL_DATA](stateCopy, {
issue, issue,
status: 'failed',
}); });
}); });
...@@ -116,10 +112,6 @@ describe('Reports Store Mutations', () => { ...@@ -116,10 +112,6 @@ describe('Reports Store Mutations', () => {
expect(stateCopy.modal.title).toEqual(issue.name); expect(stateCopy.modal.title).toEqual(issue.name);
}); });
it('should set modal status', () => {
expect(stateCopy.modal.status).toEqual('failed');
});
it('should set modal data', () => { it('should set modal data', () => {
expect(stateCopy.modal.data.execution_time.value).toEqual(issue.execution_time); expect(stateCopy.modal.data.execution_time.value).toEqual(issue.execution_time);
expect(stateCopy.modal.data.system_output.value).toEqual(issue.system_output); expect(stateCopy.modal.data.system_output.value).toEqual(issue.system_output);
......
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