summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFilipa Lacerda <filipa@gitlab.com>2018-08-01 15:57:05 +0100
committerFilipa Lacerda <filipa@gitlab.com>2018-08-02 17:59:25 +0100
commit15511ed14f595b30de90f32b3751c8599550e4c7 (patch)
treef1d69b6510ebcdc5d1c2b5c4007f693871c053f5
parentd40ef4c83e90823221a66c746f414faf7da442e8 (diff)
downloadgitlab-ce-15511ed14f595b30de90f32b3751c8599550e4c7.tar.gz
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
-rw-r--r--app/assets/javascripts/reports/components/grouped_test_reports_app.vue22
-rw-r--r--app/assets/javascripts/reports/components/modal.vue4
-rw-r--r--app/assets/javascripts/reports/components/test_issue_body.vue6
-rw-r--r--app/assets/javascripts/reports/store/actions.js16
-rw-r--r--app/assets/javascripts/reports/store/getters.js12
-rw-r--r--app/assets/javascripts/reports/store/mutations.js8
-rw-r--r--app/assets/javascripts/reports/store/state.js2
-rw-r--r--app/assets/javascripts/reports/store/utils.js9
-rw-r--r--app/assets/javascripts/vue_shared/components/code_block.vue4
-rw-r--r--app/assets/stylesheets/framework/blocks.scss9
-rw-r--r--app/assets/stylesheets/framework/common.scss2
-rw-r--r--spec/javascripts/reports/components/grouped_test_reports_app_spec.js20
-rw-r--r--spec/javascripts/reports/components/test_issue_body_spec.js1
-rw-r--r--spec/javascripts/reports/store/mutations_spec.js14
14 files changed, 67 insertions, 62 deletions
diff --git a/app/assets/javascripts/reports/components/grouped_test_reports_app.vue b/app/assets/javascripts/reports/components/grouped_test_reports_app.vue
index d2e72e72421..0fc84b4552a 100644
--- a/app/assets/javascripts/reports/components/grouped_test_reports_app.vue
+++ b/app/assets/javascripts/reports/components/grouped_test_reports_app.vue
@@ -1,5 +1,5 @@
<script>
- import { mapActions, mapGetters } from 'vuex';
+ import { mapActions, mapGetters, mapState } from 'vuex';
import { s__ } from '~/locale';
import { componentNames } from '~/vue_shared/components/reports/issue_body';
import ReportSection from '~/vue_shared/components/reports/report_section.vue';
@@ -26,27 +26,29 @@
},
componentNames,
computed: {
- ...mapGetters([
+ ...mapState([
'reports',
- 'summaryStatus',
'isLoading',
'hasError',
- 'summaryCounts',
- 'modalTitle',
- 'modalData',
- 'isCreatingNewIssue',
+ 'summary',
+ ]),
+ ...mapState({
+ modalTitle: state => state.modal.title || '',
+ modalData: state => state.modal.data || {},
+ }),
+ ...mapGetters([
+ 'summaryStatus',
]),
-
groupedSummaryText() {
if (this.isLoading) {
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 summaryTextBuilder(s__('Reports|Test summary'), this.summaryCounts);
+ return summaryTextBuilder(s__('Reports|Test summary'), this.summary);
},
},
created() {
diff --git a/app/assets/javascripts/reports/components/modal.vue b/app/assets/javascripts/reports/components/modal.vue
index 6257ebf71fc..b2133714858 100644
--- a/app/assets/javascripts/reports/components/modal.vue
+++ b/app/assets/javascripts/reports/components/modal.vue
@@ -36,9 +36,9 @@
:key="index"
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 }}:
- </label>
+ </strong>
<div class="col-sm-10 text-secondary">
<code-block
diff --git a/app/assets/javascripts/reports/components/test_issue_body.vue b/app/assets/javascripts/reports/components/test_issue_body.vue
index 13b7fdf58bc..cd443a49b52 100644
--- a/app/assets/javascripts/reports/components/test_issue_body.vue
+++ b/app/assets/javascripts/reports/components/test_issue_body.vue
@@ -21,10 +21,6 @@
},
methods: {
...mapActions(['openModal']),
- handleIssueClick() {
- const { issue, status, openModal } = this;
- openModal({ issue, status });
- },
},
};
</script>
@@ -34,7 +30,7 @@
<button
type="button"
class="btn-link btn-blank text-left break-link vulnerability-name-button"
- @click="handleIssueClick()"
+ @click="openModal({ issue })"
>
<div
v-if="isNew"
diff --git a/app/assets/javascripts/reports/store/actions.js b/app/assets/javascripts/reports/store/actions.js
index edbf860ecc6..e8f6b53cea0 100644
--- a/app/assets/javascripts/reports/store/actions.js
+++ b/app/assets/javascripts/reports/store/actions.js
@@ -3,6 +3,7 @@ import $ from 'jquery';
import axios from '../../lib/utils/axios_utils';
import Poll from '../../lib/utils/poll';
import * as types from './mutation_types';
+import httpStatusCodes from '../../lib/utils/http_status';
export const setEndpoint = ({ commit }, endpoint) => commit(types.SET_ENDPOINT, endpoint);
@@ -42,12 +43,17 @@ export const fetchReports = ({ state, dispatch }) => {
},
data: state.endpoint,
method: 'getReports',
- successCallback: ({ data }) => dispatch('receiveReportsSuccess', data),
+ successCallback: (response) => dispatch('receiveReportsSuccess', response),
errorCallback: () => dispatch('receiveReportsError'),
});
if (!Visibility.hidden()) {
eTagPoll.makeRequest();
+ } else {
+ axios
+ .get(state.endpoint)
+ .then((response) => dispatch('receiveReportsSuccess', response))
+ .catch(() => dispatch('receiveReportsError'));
}
Visibility.change(() => {
@@ -59,8 +65,12 @@ export const fetchReports = ({ state, dispatch }) => {
});
};
-export const receiveReportsSuccess = ({ commit }, response) =>
- commit(types.RECEIVE_REPORTS_SUCCESS, response);
+export const receiveReportsSuccess = ({ commit }, 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);
diff --git a/app/assets/javascripts/reports/store/getters.js b/app/assets/javascripts/reports/store/getters.js
index 5fbafa201b5..95266194acb 100644
--- a/app/assets/javascripts/reports/store/getters.js
+++ b/app/assets/javascripts/reports/store/getters.js
@@ -1,19 +1,11 @@
-import { LOADING, ERROR, SUCCESS } 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;
+import { LOADING, ERROR, SUCCESS, STATUS_FAILED } from '../constants';
export const summaryStatus = state => {
if (state.isLoading) {
return LOADING;
}
- if (state.hasError) {
+ if (state.hasError || state.status === STATUS_FAILED) {
return ERROR;
}
diff --git a/app/assets/javascripts/reports/store/mutations.js b/app/assets/javascripts/reports/store/mutations.js
index 6b900356bfc..e806d120b51 100644
--- a/app/assets/javascripts/reports/store/mutations.js
+++ b/app/assets/javascripts/reports/store/mutations.js
@@ -23,11 +23,17 @@ export default {
[types.RECEIVE_REPORTS_ERROR](state) {
state.isLoading = false;
state.hasError = true;
+
state.reports = [];
+ state.summary = {
+ total: 0,
+ resolved: 0,
+ failed: 0,
+ };
+ state.status = null;
},
[types.SET_ISSUE_MODAL_DATA](state, payload) {
state.modal.title = payload.issue.name;
- state.modal.status = payload.status;
Object.keys(payload.issue).forEach((key) => {
if (Object.prototype.hasOwnProperty.call(state.modal.data, key)) {
diff --git a/app/assets/javascripts/reports/store/state.js b/app/assets/javascripts/reports/store/state.js
index fd1819dbfea..4cab2e27a16 100644
--- a/app/assets/javascripts/reports/store/state.js
+++ b/app/assets/javascripts/reports/store/state.js
@@ -34,8 +34,6 @@ export default () => ({
modal: {
title: null,
- status: null,
-
data: {
class: {
value: null,
diff --git a/app/assets/javascripts/reports/store/utils.js b/app/assets/javascripts/reports/store/utils.js
index caa4039af72..35632218269 100644
--- a/app/assets/javascripts/reports/store/utils.js
+++ b/app/assets/javascripts/reports/store/utils.js
@@ -17,7 +17,8 @@ const textBuilder = results => {
? n__('%d fixed test result', '%d fixed test results', resolved)
: 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 (resolved) {
@@ -30,19 +31,17 @@ const textBuilder = results => {
}
} else if (resolved) {
resultsString = resolvedString;
- } else {
- resultsString = s__('Reports|no changed test results');
}
return `${resultsString} ${totalString}`;
};
-export const summaryTextBuilder = (name = '', results) => {
+export const summaryTextBuilder = (name = '', results = {}) => {
const resultsString = textBuilder(results);
return `${name} contained ${resultsString}`;
};
-export const reportTextBuilder = (name = '', results) => {
+export const reportTextBuilder = (name = '', results = {}) => {
const resultsString = textBuilder(results);
return `${name} found ${resultsString}`;
};
diff --git a/app/assets/javascripts/vue_shared/components/code_block.vue b/app/assets/javascripts/vue_shared/components/code_block.vue
index 30920a2a86d..3cca7a86bef 100644
--- a/app/assets/javascripts/vue_shared/components/code_block.vue
+++ b/app/assets/javascripts/vue_shared/components/code_block.vue
@@ -10,7 +10,7 @@ export default {
};
</script>
<template>
- <pre class="code-block build-trace-rounded">
- <code class="bash">{{ code }}</code>
+ <pre class="code-block rounded">
+ <code class="d-block">{{ code }}</code>
</pre>
</template>
diff --git a/app/assets/stylesheets/framework/blocks.scss b/app/assets/stylesheets/framework/blocks.scss
index 12fc307130a..7145a76db6d 100644
--- a/app/assets/stylesheets/framework/blocks.scss
+++ b/app/assets/stylesheets/framework/blocks.scss
@@ -360,7 +360,6 @@
white-space: pre;
overflow-x: auto;
font-size: 12px;
- border-radius: 0;
border: 0;
padding: $grid-size;
@@ -368,12 +367,4 @@
background-color: inherit;
padding: inherit;
}
-
- .bash {
- display: block;
- }
-
- &.build-trace-rounded {
- border-radius: $border-radius-base;
- }
}
diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss
index 3825cbf4b96..c9865610b78 100644
--- a/app/assets/stylesheets/framework/common.scss
+++ b/app/assets/stylesheets/framework/common.scss
@@ -469,4 +469,4 @@ img.emoji {
.inline { display: inline-block; }
.center { text-align: center; }
.vertical-align-middle { vertical-align: middle; }
-.flex-align-self-center { align-self: center; } \ No newline at end of file
+.flex-align-self-center { align-self: center; }
diff --git a/spec/javascripts/reports/components/grouped_test_reports_app_spec.js b/spec/javascripts/reports/components/grouped_test_reports_app_spec.js
index 6ca03620ca3..d86e565036c 100644
--- a/spec/javascripts/reports/components/grouped_test_reports_app_spec.js
+++ b/spec/javascripts/reports/components/grouped_test_reports_app_spec.js
@@ -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', () => {
beforeEach(() => {
mock.onGet('test_results.json').reply(200, newFailedTestReports, {});
diff --git a/spec/javascripts/reports/components/test_issue_body_spec.js b/spec/javascripts/reports/components/test_issue_body_spec.js
index 242eed114d9..0ea81f714e7 100644
--- a/spec/javascripts/reports/components/test_issue_body_spec.js
+++ b/spec/javascripts/reports/components/test_issue_body_spec.js
@@ -31,7 +31,6 @@ describe('Test Issue body', () => {
vm.$el.querySelector('button').click();
expect(vm.openModal).toHaveBeenCalledWith({
issue: commonProps.issue,
- status: commonProps.status,
});
});
});
diff --git a/spec/javascripts/reports/store/mutations_spec.js b/spec/javascripts/reports/store/mutations_spec.js
index c3e2bb8e00c..8f99d2675a5 100644
--- a/spec/javascripts/reports/store/mutations_spec.js
+++ b/spec/javascripts/reports/store/mutations_spec.js
@@ -43,24 +43,21 @@ describe('Reports Store Mutations', () => {
{
name: 'StringHelper#concatenate when a is git and b is lab returns summary',
execution_time: 0.0092435,
- system_output:
- 'Failure/Error: is_expected.to eq(\'gitlab\')',
+ system_output: "Failure/Error: is_expected.to eq('gitlab')",
},
],
resolved_failures: [
{
name: 'StringHelper#concatenate when a is git and b is lab returns summary',
execution_time: 0.009235,
- system_output:
- 'Failure/Error: is_expected.to eq(\'gitlab\')',
+ system_output: "Failure/Error: is_expected.to eq('gitlab')",
},
],
existing_failures: [
{
name: 'StringHelper#concatenate when a is git and b is lab returns summary',
execution_time: 1232.08,
- system_output:
- 'Failure/Error: is_expected.to eq(\'gitlab\')',
+ system_output: "Failure/Error: is_expected.to eq('gitlab')",
},
],
},
@@ -108,7 +105,6 @@ describe('Reports Store Mutations', () => {
beforeEach(() => {
mutations[types.SET_ISSUE_MODAL_DATA](stateCopy, {
issue,
- status: 'failed',
});
});
@@ -116,10 +112,6 @@ describe('Reports Store Mutations', () => {
expect(stateCopy.modal.title).toEqual(issue.name);
});
- it('should set modal status', () => {
- expect(stateCopy.modal.status).toEqual('failed');
- });
-
it('should set modal data', () => {
expect(stateCopy.modal.data.execution_time.value).toEqual(issue.execution_time);
expect(stateCopy.modal.data.system_output.value).toEqual(issue.system_output);