]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-11687 Remove multiselection of issues
authorJeremy Davis <jeremy.davis@sonarsource.com>
Thu, 21 Feb 2019 16:15:22 +0000 (17:15 +0100)
committerSonarTech <sonartech@sonarsource.com>
Wed, 27 Feb 2019 19:20:50 +0000 (20:20 +0100)
server/sonar-web/src/main/js/apps/issues/components/App.tsx
server/sonar-web/src/main/js/apps/issues/components/IssuesList.tsx
server/sonar-web/src/main/js/apps/issues/components/ListItem.tsx
server/sonar-web/src/main/js/apps/issues/components/__tests__/App-test.tsx
server/sonar-web/src/main/js/components/issue/Issue.css
server/sonar-web/src/main/js/components/issue/Issue.tsx
server/sonar-web/src/main/js/components/issue/IssueView.tsx

index 60c699186af81bdece1b3cf005689f496cca9c9d..f79427acc78e9b527520b3589f76002e1cd7b930 100644 (file)
@@ -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<Props, State> {
 
   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<Props, State> {
 
     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<Props, State> {
     });
   };
 
-  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) => {
index 15736ca2b1d5c6fb954338e98c365d95e783c56b..da62b8a436c807c83dadecccdb0276357d5a8404 100644 (file)
@@ -28,7 +28,7 @@ interface Props {
   issues: T.Issue[];
   onFilterChange: (changes: Partial<Query>) => 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;
index 10c9984972b6f8b2a0b8383d0bebf824dbef12f8..2ac011d118aa0d4a3efda8da81269e66c0e4d9c5 100644 (file)
@@ -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<Query>) => void;
   onPopupToggle: (issue: string, popupName: string, open?: boolean) => void;
index 6742f296c867c5c886e0f2d0ab736a1070ced8b0..1c27ca4366e8469632bb15e8010f0dbf5af37216 100644 (file)
 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<App['props']> = {}) {
   return shallow<App>(
     <App
index b9004dc6c0f6e78cb583acd3cb5795a56584e411..35af21af2c9460e89a92e9ad4fb45fd346c1350c 100644 (file)
 }
 
 .issue-with-checkbox .issue-checkbox-container {
-  display: block;
+  display: flex;
+  justify-content: center;
+  align-items: center;
 }
 
 .issue-checkbox-container {
   background-color: rgba(0, 0, 0, 0.05);
 }
 
-.issue-checkbox {
-  position: absolute;
-  top: 50%;
-  left: 50%;
-  margin: -8px 0 0 -8px;
-}
-
 .issue:not(.selected) .location-index {
   background-color: #ccc;
 }
index ace21e0ab87d3e5f3ee05e432388088439c0b3e1..841c2f31a322d73cc6742d6478e8f2d1f59f597f 100644 (file)
@@ -31,7 +31,7 @@ interface Props {
   displayLocationsLink?: boolean;
   issue: T.Issue;
   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;
   onPopupToggle: (issue: string, popupName: string, open?: boolean) => void;
@@ -97,9 +97,9 @@ export default class Issue extends React.PureComponent<Props> {
       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;
index f30cf4c490fb4b1830d3a91d5da5922c8fc765ec..0526cd87d6c09946f83f4586c76e207e2b597f1b 100644 (file)
  */
 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<Props> {
-  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<Props> {
             </div>
           )}
         {hasCheckbox && (
-          <a className="js-toggle issue-checkbox-container" href="#" onClick={this.handleCheck}>
-            <i
-              className={classNames('issue-checkbox', 'icon-checkbox', {
-                'icon-checkbox-checked': this.props.checked
-              })}
+          <>
+            <Checkbox
+              checked={this.props.checked || false}
+              className="issue-checkbox-container"
+              onCheck={this.handleCheck}
             />
-          </a>
+          </>
         )}
       </div>
     );