From: Jeremy Davis Date: Thu, 21 Feb 2019 16:15:22 +0000 (+0100) Subject: SONAR-11687 Remove multiselection of issues X-Git-Tag: 7.7~124 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=38d1fb46dece0d6c767108f1c02759a4df6216bf;p=sonarqube.git SONAR-11687 Remove multiselection of issues --- diff --git a/server/sonar-web/src/main/js/apps/issues/components/App.tsx b/server/sonar-web/src/main/js/apps/issues/components/App.tsx index 60c699186af..f79427acc78 100644 --- a/server/sonar-web/src/main/js/apps/issues/components/App.tsx +++ b/server/sonar-web/src/main/js/apps/issues/components/App.tsx @@ -21,7 +21,7 @@ import * as React from 'react'; import { FormattedMessage } from 'react-intl'; import * as key from 'keymaster'; import Helmet from 'react-helmet'; -import { keyBy, omit, union, without } from 'lodash'; +import { keyBy, omit, without } from 'lodash'; import BulkChangeModal, { MAX_PAGE_SIZE } from './BulkChangeModal'; import ComponentBreadcrumbs from './ComponentBreadcrumbs'; import IssuesList from './IssuesList'; @@ -116,7 +116,6 @@ export interface State { effortTotal?: number; facets: { [facet: string]: Facet }; issues: T.Issue[]; - lastChecked?: string; loading: boolean; loadingFacets: { [key: string]: boolean }; loadingMore: boolean; @@ -493,20 +492,27 @@ export class App extends React.PureComponent { fetchIssuesUntil = ( p: number, - done: (issues: T.Issue[], paging: T.Paging) => boolean + done: (lastIssue: T.Issue, paging: T.Paging) => boolean ): Promise<{ issues: T.Issue[]; paging: T.Paging }> => { - return this.fetchIssuesPage(p).then(response => { - const { issues, paging } = response; - - return done(issues, paging) - ? { issues, paging } - : this.fetchIssuesUntil(p + 1, done).then(nextResponse => { - return { - issues: [...issues, ...nextResponse.issues], - paging: nextResponse.paging - }; - }); - }); + const recursiveFetch = ( + p: number, + issues: T.Issue[] + ): Promise<{ issues: T.Issue[]; paging: T.Paging }> => { + return this.fetchIssuesPage(p) + .then(response => { + return { + issues: [...issues, ...response.issues], + paging: response.paging + }; + }) + .then(({ issues, paging }) => { + return done(issues[issues.length - 1], paging) + ? { issues, paging } + : recursiveFetch(p + 1, issues); + }); + }; + + return recursiveFetch(p, []); }; fetchMoreIssues = () => { @@ -546,18 +552,17 @@ export class App extends React.PureComponent { const isSameComponent = (issue: T.Issue) => issue.component === openIssue.component; - const done = (issues: T.Issue[], paging: T.Paging) => { + const done = (lastIssue: T.Issue, paging: T.Paging) => { if (paging.total <= paging.pageIndex * paging.pageSize) { return true; } - const lastIssue = issues[issues.length - 1]; if (lastIssue.component !== openIssue.component) { return true; } return lastIssue.textRange !== undefined && lastIssue.textRange.endLine > to; }; - if (done(issues, paging)) { + if (done(issues[issues.length - 1], paging)) { return Promise.resolve(issues.filter(isSameComponent)); } @@ -737,37 +742,13 @@ export class App extends React.PureComponent { }); }; - handleIssueCheck = (issue: string, event: { shiftKey?: boolean }) => { - // Selecting multiple issues with shift+click - const { lastChecked } = this.state; - if (event.shiftKey && lastChecked) { - this.setState(state => { - const issueKeys = state.issues.map(issue => issue.key); - const currentIssueIndex = issueKeys.indexOf(issue); - const lastSelectedIndex = issueKeys.indexOf(lastChecked); - const shouldCheck = state.checked.includes(lastChecked); - let { checked } = state; - if (currentIssueIndex < 0) { - return null; - } - const start = Math.min(currentIssueIndex, lastSelectedIndex); - const end = Math.max(currentIssueIndex, lastSelectedIndex); - for (let i = start; i < end + 1; i++) { - checked = shouldCheck - ? union(checked, [state.issues[i].key]) - : without(checked, state.issues[i].key); - } - return { checkAll: false, checked }; - }); - } else { - this.setState(state => ({ - checkAll: false, - lastChecked: issue, - checked: state.checked.includes(issue) - ? without(state.checked, issue) - : [...state.checked, issue] - })); - } + handleIssueCheck = (issue: string) => { + this.setState(state => ({ + checkAll: false, + checked: state.checked.includes(issue) + ? without(state.checked, issue) + : [...state.checked, issue] + })); }; handleIssueChange = (issue: T.Issue) => { diff --git a/server/sonar-web/src/main/js/apps/issues/components/IssuesList.tsx b/server/sonar-web/src/main/js/apps/issues/components/IssuesList.tsx index 15736ca2b1d..da62b8a436c 100644 --- a/server/sonar-web/src/main/js/apps/issues/components/IssuesList.tsx +++ b/server/sonar-web/src/main/js/apps/issues/components/IssuesList.tsx @@ -28,7 +28,7 @@ interface Props { issues: T.Issue[]; onFilterChange: (changes: Partial) => void; onIssueChange: (issue: T.Issue) => void; - onIssueCheck: ((issueKey: string, event: { shiftKey?: boolean }) => void) | undefined; + onIssueCheck: ((issueKey: string) => void) | undefined; onIssueClick: (issueKey: string) => void; onPopupToggle: (issue: string, popupName: string, open?: boolean) => void; openPopup: { issue: string; name: string } | undefined; diff --git a/server/sonar-web/src/main/js/apps/issues/components/ListItem.tsx b/server/sonar-web/src/main/js/apps/issues/components/ListItem.tsx index 10c9984972b..2ac011d118a 100644 --- a/server/sonar-web/src/main/js/apps/issues/components/ListItem.tsx +++ b/server/sonar-web/src/main/js/apps/issues/components/ListItem.tsx @@ -28,7 +28,7 @@ interface Props { component: T.Component | undefined; issue: T.Issue; onChange: (issue: T.Issue) => void; - onCheck: ((issueKey: string, event: { shiftKey?: boolean }) => void) | undefined; + onCheck: ((issueKey: string) => void) | undefined; onClick: (issueKey: string) => void; onFilterChange: (changes: Partial) => void; onPopupToggle: (issue: string, popupName: string, open?: boolean) => void; diff --git a/server/sonar-web/src/main/js/apps/issues/components/__tests__/App-test.tsx b/server/sonar-web/src/main/js/apps/issues/components/__tests__/App-test.tsx index 6742f296c86..1c27ca4366e 100644 --- a/server/sonar-web/src/main/js/apps/issues/components/__tests__/App-test.tsx +++ b/server/sonar-web/src/main/js/apps/issues/components/__tests__/App-test.tsx @@ -20,7 +20,12 @@ import * as React from 'react'; import { shallow } from 'enzyme'; import { App } from '../App'; -import { mockCurrentUser, mockRouter } from '../../../../helpers/testMocks'; +import { + mockCurrentUser, + mockRouter, + mockIssue, + mockLocation +} from '../../../../helpers/testMocks'; import { waitAndUpdate } from '../../../../helpers/testUtils'; const ISSUES = [ @@ -32,9 +37,6 @@ const ISSUES = [ const FACETS = [{ property: 'severities', values: [{ val: 'MINOR', count: 4 }] }]; const PAGING = { pageIndex: 1, pageSize: 100, total: 4 }; -const eventNoShiftKey = { shiftKey: false } as MouseEvent; -const eventWithShiftKey = { shiftKey: true } as MouseEvent; - const referencedComponent = { key: 'foo-key', name: 'bar', organization: 'John', uuid: 'foo-uuid' }; it('should render a list of issue', async () => { @@ -45,46 +47,14 @@ it('should render a list of issue', async () => { expect(wrapper.state().referencedComponentsByKey).toEqual({ 'foo-key': referencedComponent }); }); -it('should be able to check/uncheck a group of issues with the Shift key', async () => { - const wrapper = shallowRender(); - await waitAndUpdate(wrapper); - expect(wrapper.state().issues.length).toBe(4); - - const instance = wrapper.instance(); - instance.handleIssueCheck('foo', eventNoShiftKey); - expect(wrapper.state().checked.length).toBe(1); - - instance.handleIssueCheck('fourth', eventWithShiftKey); - expect(wrapper.state().checked.length).toBe(4); - - instance.handleIssueCheck('third', eventNoShiftKey); - expect(wrapper.state().checked.length).toBe(3); - - instance.handleIssueCheck('foo', eventWithShiftKey); - expect(wrapper.state().checked.length).toBe(1); -}); - -it('should avoid non-existing keys', async () => { - const wrapper = shallowRender(); - await waitAndUpdate(wrapper); - expect(wrapper.state().issues.length).toBe(4); - - const instance = wrapper.instance(); - instance.handleIssueCheck('foo', eventNoShiftKey); - expect(wrapper.state().checked.length).toBe(1); - - instance.handleIssueCheck('non-existing-key', eventWithShiftKey); - expect(wrapper.state().checked.length).toBe(1); -}); - it('should be able to uncheck all issue with global checkbox', async () => { const wrapper = shallowRender(); await waitAndUpdate(wrapper); expect(wrapper.state().issues.length).toBe(4); const instance = wrapper.instance(); - instance.handleIssueCheck('foo', eventNoShiftKey); - instance.handleIssueCheck('bar', eventNoShiftKey); + instance.handleIssueCheck('foo'); + instance.handleIssueCheck('bar'); expect(wrapper.state().checked.length).toBe(2); instance.handleCheckAll(false); @@ -146,6 +116,73 @@ it('should check max 500 issues', async () => { expect(wrapper.find('#issues-bulk-change')).toMatchSnapshot(); }); +it('should fetch issues for component', async () => { + const wrapper = shallowRender({ + fetchIssues: fetchIssuesMockFactory(), + location: mockLocation({ + query: { open: '0' } + }) + }); + const instance = wrapper.instance(); + await waitAndUpdate(wrapper); + expect(wrapper.state('issues')).toHaveLength(2); + + await instance.fetchIssuesForComponent('', 0, 30); + expect(wrapper.state('issues')).toHaveLength(6); +}); + +it('should fetch issues until defined', async () => { + const mockDone = (_lastIssue: T.Issue, paging: T.Paging) => + paging.total <= paging.pageIndex * paging.pageSize; + + const wrapper = shallowRender({ + fetchIssues: fetchIssuesMockFactory(), + location: mockLocation({ + query: { open: '0' } + }) + }); + const instance = wrapper.instance(); + await waitAndUpdate(wrapper); + + const result = await instance.fetchIssuesUntil(1, mockDone); + expect(result.issues).toHaveLength(6); + expect(result.paging.pageIndex).toBe(3); +}); + +function fetchIssuesMockFactory(keyCount = 0, lineCount = 1) { + return jest.fn().mockImplementation(({ p }: any) => + Promise.resolve({ + components: [referencedComponent], + effortTotal: 1, + facets: FACETS, + issues: [ + mockIssue(false, { + key: '' + keyCount++, + textRange: { + startLine: lineCount++, + endLine: lineCount, + startOffset: 0, + endOffset: 15 + } + }), + mockIssue(false, { + key: '' + keyCount++, + textRange: { + startLine: lineCount++, + endLine: lineCount, + startOffset: 0, + endOffset: 15 + } + }) + ], + languages: [], + paging: { pageIndex: p || 1, pageSize: 2, total: 6 }, + rules: [], + users: [] + }) + ); +} + function shallowRender(props: Partial = {}) { return shallow( void; - onCheck?: (issue: string, event: { shiftKey?: boolean }) => void; + onCheck?: (issue: string) => void; onClick: (issueKey: string) => void; onFilter?: (property: string, issue: T.Issue) => void; onPopupToggle: (issue: string, popupName: string, open?: boolean) => void; @@ -97,9 +97,9 @@ export default class Issue extends React.PureComponent { this.togglePopup('edit-tags'); return false; }); - key('space', 'issues', (event: KeyboardEvent) => { + key('space', 'issues', () => { if (this.props.onCheck) { - this.props.onCheck(this.props.issue.key, event); + this.props.onCheck(this.props.issue.key); return false; } return undefined; diff --git a/server/sonar-web/src/main/js/components/issue/IssueView.tsx b/server/sonar-web/src/main/js/components/issue/IssueView.tsx index f30cf4c490f..0526cd87d6c 100644 --- a/server/sonar-web/src/main/js/components/issue/IssueView.tsx +++ b/server/sonar-web/src/main/js/components/issue/IssueView.tsx @@ -19,10 +19,11 @@ */ import * as React from 'react'; import classNames from 'classnames'; +import { updateIssue } from './actions'; import IssueTitleBar from './components/IssueTitleBar'; import IssueActionsBar from './components/IssueActionsBar'; import IssueCommentLine from './components/IssueCommentLine'; -import { updateIssue } from './actions'; +import Checkbox from '../controls/Checkbox'; import { deleteIssueComment, editIssueComment } from '../../api/issues'; interface Props { @@ -34,7 +35,7 @@ interface Props { issue: T.Issue; onAssign: (login: string) => void; onChange: (issue: T.Issue) => void; - onCheck?: (issue: string, event: { shiftKey?: boolean }) => void; + onCheck?: (issue: string) => void; onClick: (issueKey: string) => void; onFilter?: (property: string, issue: T.Issue) => void; selected: boolean; @@ -42,10 +43,9 @@ interface Props { } export default class IssueView extends React.PureComponent { - handleCheck = (event: React.MouseEvent) => { - event.preventDefault(); + handleCheck = (_checked: boolean) => { if (this.props.onCheck) { - this.props.onCheck(this.props.issue.key, event); + this.props.onCheck(this.props.issue.key); } }; @@ -111,13 +111,13 @@ export default class IssueView extends React.PureComponent { )} {hasCheckbox && ( - - + - + )} );