Commit 4711ac94 authored by Rémy Coutable's avatar Rémy Coutable

Merge remote-tracking branch 'origin/master' into ce-to-ee-2018-07-18

Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parents 7f6eeb70 d91899d5
......@@ -77,7 +77,8 @@ export default {
diffViewType: state => state.diffs.diffViewType,
diffFiles: state => state.diffs.diffFiles,
}),
...mapGetters(['isLoggedIn', 'discussionsByLineCode']),
...mapGetters(['isLoggedIn']),
...mapGetters('diffs', ['discussionsByLineCode']),
lineHref() {
return this.lineCode ? `#${this.lineCode}` : '#';
},
......
......@@ -26,13 +26,16 @@ export default {
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
...mapGetters(['discussionsByLineCode']),
...mapGetters('diffs', ['discussionsByLineCode']),
discussions() {
return this.discussionsByLineCode[this.line.lineCode] || [];
},
className() {
return this.discussions.length ? '' : 'js-temp-notes-holder';
},
hasCommentForm() {
return this.diffLineCommentForms[this.line.lineCode];
},
},
};
</script>
......@@ -53,8 +56,14 @@ export default {
:discussions="discussions"
/>
<diff-line-note-form
<<<<<<< HEAD
v-if="diffLineCommentForms[line.lineCode]"
:diff-file-hash="diffFileHash"
=======
v-if="hasCommentForm"
:diff-file="diffFile"
:diff-lines="diffLines"
>>>>>>> origin/master
:line="line"
:note-target-line="line"
/>
......
......@@ -20,8 +20,7 @@ export default {
},
},
computed: {
...mapGetters('diffs', ['commitId']),
...mapGetters(['discussionsByLineCode']),
...mapGetters('diffs', ['commitId', 'discussionsByLineCode']),
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
......
......@@ -26,7 +26,7 @@ export default {
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
...mapGetters(['discussionsByLineCode']),
...mapGetters('diffs', ['discussionsByLineCode']),
leftLineCode() {
return this.line.left.lineCode;
},
......
......@@ -21,8 +21,7 @@ export default {
},
},
computed: {
...mapGetters('diffs', ['commitId']),
...mapGetters(['discussionsByLineCode']),
...mapGetters('diffs', ['commitId', 'discussionsByLineCode']),
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
......
import _ from 'underscore';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '../constants';
import { getDiffRefsByLineCode } from './utils';
export const isParallelView = state => state.diffViewType === PARALLEL_DIFF_VIEW_TYPE;
......@@ -56,6 +58,44 @@ export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) =
discussion.diff_discussion && _.isEqual(discussion.diff_file.file_hash, diff.fileHash),
) || [];
/**
* Returns an Object with discussions by their diff line code
* To avoid rendering outdated discussions on the Changes tab we should do a bunch of SHA
* comparisions. `note.position.formatter` have the current version diff refs but
* `note.original_position.formatter` will have the first version's diff refs.
* If line diff refs matches with one of them, we should render it as a discussion on Changes tab.
*
* @param {Object} diff
* @returns {Array}
*/
export const discussionsByLineCode = (state, getters, rootState, rootGetters) => {
const diffRefsByLineCode = getDiffRefsByLineCode(state.diffFiles);
return rootGetters.discussions.reduce((acc, note) => {
const isDiffDiscussion = note.diff_discussion;
const hasLineCode = note.line_code;
const isResolvable = note.resolvable;
const diffRefs = diffRefsByLineCode[note.line_code];
if (isDiffDiscussion && hasLineCode && isResolvable && diffRefs) {
const refs = convertObjectPropsToCamelCase(note.position.formatter);
const originalRefs = convertObjectPropsToCamelCase(note.original_position.formatter);
if (_.isEqual(refs, diffRefs) || _.isEqual(originalRefs, diffRefs)) {
const lineCode = note.line_code;
if (acc[lineCode]) {
acc[lineCode].push(note);
} else {
acc[lineCode] = [note];
}
}
}
return acc;
}, {});
};
// prevent babel-plugin-rewire from generating an invalid default during karma∂ tests
export const getDiffFileByHash = state => fileHash =>
state.diffFiles.find(file => file.fileHash === fileHash);
......
......@@ -173,3 +173,24 @@ export function trimFirstCharOfLineContent(line = {}) {
return parsedLine;
}
export function getDiffRefsByLineCode(diffFiles) {
return diffFiles.reduce((acc, diffFile) => {
const { baseSha, headSha, startSha } = diffFile.diffRefs;
const { newPath, oldPath } = diffFile;
// We can only use highlightedDiffLines to create the map of diff lines because
// highlightedDiffLines will also include every parallel diff line in it.
if (diffFile.highlightedDiffLines) {
diffFile.highlightedDiffLines.forEach(line => {
const { lineCode, oldLine, newLine } = line;
if (lineCode) {
acc[lineCode] = { baseSha, headSha, startSha, newPath, oldPath, oldLine, newLine };
}
});
}
return acc;
}, {});
}
......@@ -28,18 +28,6 @@ export const notesById = state =>
return acc;
}, {});
export const discussionsByLineCode = state =>
state.discussions.reduce((acc, note) => {
if (note.diff_discussion && note.line_code && note.resolvable) {
// For context about line notes: there might be multiple notes with the same line code
const items = acc[note.line_code] || [];
items.push(note);
Object.assign(acc, { [note.line_code]: items });
}
return acc;
}, {});
export const noteableType = state => {
const { ISSUE_NOTEABLE_TYPE, MERGE_REQUEST_NOTEABLE_TYPE, EPIC_NOTEABLE_TYPE } = constants;
......
......@@ -5,7 +5,7 @@ import { inserted } from '~/feature_highlight/feature_highlight_helper';
import { mouseenter, debouncedMouseleave, togglePopover } from '~/shared/popover';
export default {
name: 'SecurityReportsHelpPopover',
name: 'ReportsHelpPopover',
components: {
Icon,
},
......
<script>
import IssuesBlock from './report_issues.vue';
import SastContainerInfo from './sast_container_info.vue';
import { SAST_CONTAINER } from '../store/constants';
import IssuesBlock from '~/vue_shared/components/reports/report_issues.vue';
import SastContainerInfo from 'ee/vue_shared/security_reports/components/sast_container_info.vue';
import { SAST_CONTAINER } from 'ee/vue_shared/security_reports//store/constants';
/**
* Renders block of issues
......
<script>
import Icon from '~/vue_shared/components/icon.vue';
import PerformanceIssue from 'ee/vue_merge_request_widget/components/performance_issue_body.vue';
import CodequalityIssue from 'ee/vue_merge_request_widget/components/codequality_issue_body.vue';
import LicenseIssue from 'ee/vue_merge_request_widget/components/license_issue_body.vue';
import SastIssue from './sast_issue_body.vue';
import SastContainerIssue from './sast_container_issue_body.vue';
import DastIssue from './dast_issue_body.vue';
import { SAST, DAST, SAST_CONTAINER } from '../store/constants';
import SastIssue from 'ee/vue_shared/security_reports/components/sast_issue_body.vue';
import SastContainerIssue from 'ee/vue_shared/security_reports/components/sast_container_issue_body.vue';
import DastIssue from 'ee/vue_shared/security_reports/components/dast_issue_body.vue';
import { SAST, DAST, SAST_CONTAINER } from 'ee/vue_shared/security_reports/store/constants';
export default {
name: 'ReportIssues',
......
......@@ -3,7 +3,10 @@ import { __ } from '~/locale';
import StatusIcon from '~/vue_merge_request_widget/components/mr_widget_status_icon.vue';
import IssuesList from './issues_list.vue';
import Popover from './help_popover.vue';
import { LOADING, ERROR, SUCCESS } from '../store/constants';
const LOADING = 'LOADING';
const ERROR = 'ERROR';
const SUCCESS = 'SUCCESS';
export default {
name: 'ReportSection',
......
......@@ -4,11 +4,15 @@ import LoadingIcon from '~/vue_shared/components/loading_icon.vue';
import Popover from './help_popover.vue';
/**
* Renders the summary row for each security report
* Renders the summary row for each report
*
* Used both in MR widget and Pipeline's view for:
* - Unit tests reports
* - Security reports
*/
export default {
name: 'SecuritySummaryRow',
name: 'ReportSummaryRow',
components: {
CiIcon,
LoadingIcon,
......
......@@ -4,6 +4,7 @@ class DiscussionEntity < Grape::Entity
expose :id, :reply_id
expose :position, if: -> (d, _) { d.diff_discussion? && !d.legacy_diff_discussion? }
expose :original_position, if: -> (d, _) { d.diff_discussion? && !d.legacy_diff_discussion? }
expose :line_code, if: -> (d, _) { d.diff_discussion? }
expose :expanded?, as: :expanded
expose :active?, as: :active, if: -> (d, _) { d.diff_discussion? }
......
---
title: Fix showing outdated discussions on Changes tab
merge_request: 20445
author:
type: fixed
......@@ -3,7 +3,7 @@
* Renders Code quality body text
* Fixed: [name] in [link]:[line]
*/
import ReportLink from 'ee/vue_shared/security_reports/components/report_link.vue';
import ReportLink from '~/vue_shared/components/reports/report_link.vue';
export default {
name: 'CodequalityIssueBody',
......
......@@ -3,7 +3,7 @@
* Renders Perfomance issue body text
* [name] :[score] [symbol] [delta] in [link]
*/
import ReportLink from 'ee/vue_shared/security_reports/components/report_link.vue';
import ReportLink from '~/vue_shared/components/reports/report_link.vue';
export default {
name: 'PerformanceIssueBody',
......
<script>
import ReportSection from 'ee/vue_shared/security_reports/components/report_section.vue';
import ReportSection from '~/vue_shared/components/reports/report_section.vue';
import GroupedSecurityReportsApp from 'ee/vue_shared/security_reports/grouped_security_reports_app.vue';
import reportsMixin from 'ee/vue_shared/security_reports/mixins/reports_mixin';
......
......@@ -4,7 +4,7 @@
* [severity] ([confidence]): [name]
*/
import ModalOpenName from './modal_open_name.vue';
import ModalOpenName from '~/vue_shared/components/reports/modal_open_name.vue';
export default {
name: 'SastIssueBody',
......
......@@ -3,8 +3,8 @@
* Renders SAST CONTAINER body text
* [priority]: [name] in [link]:[line]
*/
import ReportLink from './report_link.vue';
import ModalOpenName from './modal_open_name.vue';
import ReportLink from '~/vue_shared/components/reports/report_link.vue';
import ModalOpenName from '~/vue_shared/components/reports/modal_open_name.vue';
export default {
name: 'SastContainerIssueBody',
......
......@@ -3,8 +3,8 @@
* Renders SAST body text
* [severity] ([confidence]): [name] in [link] : [line]
*/
import ReportLink from './report_link.vue';
import ModalOpenName from './modal_open_name.vue';
import ReportLink from '~/vue_shared/components/reports/report_link.vue';
import ModalOpenName from '~/vue_shared/components/reports/modal_open_name.vue';
export default {
name: 'SastIssueBody',
......
<script>
import { mapActions, mapState, mapGetters } from 'vuex';
import { SAST, DAST, SAST_CONTAINER } from './store/constants';
import createStore from './store';
import ReportSection from './components/report_section.vue';
import SummaryRow from './components/summary_row.vue';
import IssuesList from './components/issues_list.vue';
import ReportSection from '~/vue_shared/components/reports/report_section.vue';
import SummaryRow from '~/vue_shared/components/reports/summary_row.vue';
import IssuesList from '~/vue_shared/components/reports/issues_list.vue';
import IssueModal from './components/modal.vue';
import { SAST, DAST, SAST_CONTAINER } from './store/constants';
import securityReportsMixin from './mixins/security_report_mixin';
import createStore from './store';
export default {
store: createStore(),
......
......@@ -2,8 +2,8 @@
import { mapActions, mapState } from 'vuex';
import { s__, sprintf, n__ } from '~/locale';
import createFlash from '~/flash';
import ReportSection from '~/vue_shared/components/reports/report_section.vue';
import { SAST, DAST, SAST_CONTAINER } from './store/constants';
import ReportSection from './components/report_section.vue';
import IssueModal from './components/modal.vue';
import mixin from './mixins/security_report_mixin';
import reportsMixin from './mixins/reports_mixin';
......
......@@ -18,10 +18,12 @@ describe('DiffLineGutterContent', () => {
};
const setDiscussions = component => {
component.$store.dispatch('setInitialNotes', getDiscussionsMockData());
component.$store.commit('diffs/SET_DIFF_DATA', { diffFiles: [getDiffFileMock()] });
};
const resetDiscussions = component => {
component.$store.dispatch('setInitialNotes', []);
component.$store.commit('diffs/SET_DIFF_DATA', {});
};
describe('computed', () => {
......
......@@ -33,6 +33,7 @@ describe('InlineDiffView', () => {
it('should render discussions', done => {
const el = component.$el;
component.$store.dispatch('setInitialNotes', getDiscussionsMockData());
component.$store.commit('diffs/SET_DIFF_DATA', { diffFiles: [getDiffFileMock()] });
Vue.nextTick(() => {
expect(el.querySelectorAll('.notes_holder').length).toEqual(1);
......
......@@ -12,6 +12,17 @@ export default {
head_sha: 'c48ee0d1bf3b30453f5b32250ce03134beaa6d13',
},
},
original_position: {
formatter: {
old_line: null,
new_line: 2,
old_path: 'CHANGELOG',
new_path: 'CHANGELOG',
base_sha: 'e63f41fe459e62e1228fcef60d7189127aeba95a',
start_sha: 'd9eaefe5a676b820c57ff18cf5b68316025f7962',
head_sha: 'c48ee0d1bf3b30453f5b32250ce03134beaa6d13',
},
},
line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_2',
expanded: true,
notes: [
......
......@@ -2,6 +2,7 @@ import * as getters from '~/diffs/store/getters';
import state from '~/diffs/store/modules/diff_state';
import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants';
import discussion from '../mock_data/diff_discussions';
import diffFile from '../mock_data/diff_file';
describe('Diffs Module Getters', () => {
let localState;
......@@ -185,6 +186,7 @@ describe('Diffs Module Getters', () => {
});
});
<<<<<<< HEAD
describe('getDiffFileByHash', () => {
it('returns file by hash', () => {
const fileA = {
......@@ -201,6 +203,39 @@ describe('Diffs Module Getters', () => {
it('returns null if no matching file is found', () => {
localState.diffFiles = [];
expect(getters.getDiffFileByHash(localState)('123')).toBeUndefined();
=======
describe('discussionsByLineCode', () => {
let mockState;
beforeEach(() => {
mockState = { diffFiles: [diffFile] };
});
it('should return a map of diff lines with their line codes', () => {
const mockGetters = { discussions: [discussionMock] };
const map = getters.discussionsByLineCode(mockState, {}, {}, mockGetters);
expect(map['1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_2']).toBeDefined();
expect(Object.keys(map).length).toEqual(1);
});
it('should have the diff discussion on the map if the original position matches', () => {
discussionMock.position.formatter.base_sha = 'ff9200';
const mockGetters = { discussions: [discussionMock] };
const map = getters.discussionsByLineCode(mockState, {}, {}, mockGetters);
expect(map['1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_2']).toBeDefined();
expect(Object.keys(map).length).toEqual(1);
});
it('should not add an outdated diff discussion to the returned map', () => {
discussionMock.position.formatter.base_sha = 'ff9200';
discussionMock.original_position.formatter.base_sha = 'ff9200';
const mockGetters = { discussions: [discussionMock] };
const map = getters.discussionsByLineCode(mockState, {}, {}, mockGetters);
expect(Object.keys(map).length).toEqual(0);
>>>>>>> origin/master
});
});
});
......@@ -207,4 +207,24 @@ describe('DiffsStoreUtils', () => {
expect(utils.trimFirstCharOfLineContent()).toEqual({});
});
});
describe('getDiffRefsByLineCode', () => {
it('should return diffRefs for all highlightedDiffLines', () => {
const diffFile = getDiffFileMock();
const map = utils.getDiffRefsByLineCode([diffFile]);
const { highlightedDiffLines } = diffFile;
const lineCodeCount = highlightedDiffLines.reduce(
(acc, line) => (line.lineCode ? acc + 1 : acc),
0,
);
const { baseSha, headSha, startSha } = diffFile.diffRefs;
const targetLine = map[highlightedDiffLines[4].lineCode];
expect(Object.keys(map).length).toEqual(lineCodeCount);
expect(targetLine.baseSha).toEqual(baseSha);
expect(targetLine.headSha).toEqual(headSha);
expect(targetLine.startSha).toEqual(startSha);
});
});
});
import Vue from 'vue';
import component from 'ee/vue_shared/security_reports/components/modal_open_name.vue';
import store from 'ee/vue_shared/security_reports/store';
import Vuex from 'vuex';
import component from '~/vue_shared/components/reports/modal_open_name.vue';
import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import { parsedDast } from '../mock_data';
describe('Modal open name', () => {
const Component = Vue.extend(component);
let vm;
const store = new Vuex.Store({
actions: {
openModal: () => {},
},
state: {},
mutations: {},
});
beforeEach(() => {
vm = mountComponentWithStore(Component, {
store,
props: {
issue: parsedDast[0],
issue: {
title: 'Issue',
},
status: 'failed',
},
});
......@@ -23,7 +32,7 @@ describe('Modal open name', () => {
});
it('renders the issue name', () => {
expect(vm.$el.textContent.trim()).toEqual(parsedDast[0].name);
expect(vm.$el.textContent.trim()).toEqual('Issue');
});
it('calls openModal actions when button is clicked', () => {
......
import Vue from 'vue';
import reportIssues from 'ee/vue_shared/security_reports/components/report_issues.vue';
import reportIssues from '~/vue_shared/components/reports/report_issues.vue';
import store from 'ee/vue_shared/security_reports/store';
import mountComponent, { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import {
......@@ -9,7 +9,7 @@ import {
sastParsedIssues,
dockerReportParsed,
parsedDast,
} from '../mock_data';
} from 'spec/vue_shared/security_reports/mock_data';
describe('Report issues', () => {
let vm;
......
import Vue from 'vue';
import component from 'ee/vue_shared/security_reports/components/report_link.vue';
import component from '~/vue_shared/components/reports/report_link.vue';
import mountComponent from '../../../helpers/vue_mount_component_helper';
describe('report link', () => {
......
import Vue from 'vue';
import reportSection from 'ee/vue_shared/security_reports/components/report_section.vue';
import reportSection from '~/vue_shared/components/reports/report_section.vue';
import mountComponent from 'spec/helpers/vue_mount_component_helper';
import { codequalityParsedIssues } from 'spec/vue_mr_widget/mock_data';
describe('Report section', () => {
let vm;
const ReportSection = Vue.extend(reportSection);
const resolvedIssues = [
{
name: 'Insecure Dependency',
fingerprint: 'ca2e59451e98ae60ba2f54e3857c50e5',
path: 'Gemfile.lock',
line: 12,
urlPath: 'foo/Gemfile.lock',
},
];
afterEach(() => {
vm.$destroy();
});
......@@ -19,7 +28,7 @@ describe('Report section', () => {
loadingText: 'Loading codeclimate report',
errorText: 'foo',
successText: 'Code quality improved on 1 point and degraded on 1 point',
resolvedIssues: codequalityParsedIssues,
resolvedIssues,
hasIssues: false,
alwaysOpen: false,
});
......@@ -77,6 +86,7 @@ describe('Report section', () => {
});
});
});
describe('when it is loading', () => {
it('should render loading indicator', () => {
vm = mountComponent(ReportSection, {
......@@ -99,7 +109,7 @@ describe('Report section', () => {
loadingText: 'Loading codeclimate report',
errorText: 'foo',
successText: 'Code quality improved on 1 point and degraded on 1 point',
resolvedIssues: codequalityParsedIssues,
resolvedIssues,
hasIssues: true,
});
});
......@@ -110,7 +120,7 @@ describe('Report section', () => {
);
expect(vm.$el.querySelectorAll('.js-mr-code-resolved-issues li').length).toEqual(
codequalityParsedIssues.length,
resolvedIssues.length,
);
});
......
import Vue from 'vue';
import component from 'ee/vue_shared/security_reports/components/summary_row.vue';
import component from '~/vue_shared/components/reports/summary_row.vue';
import mountComponent from 'spec/helpers/vue_mount_component_helper';
describe('Summary row', () => {
......
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