From: Wouter Admiraal Date: Tue, 8 Jan 2019 10:19:29 +0000 (+0100) Subject: SONAR-11609 Update the Issues bulkchange action X-Git-Tag: 7.7~147 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=92c3efb4212370af7c19514136bdbc1051148210;p=sonarqube.git SONAR-11609 Update the Issues bulkchange action --- 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 5c6db560471..60c699186af 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 @@ -18,10 +18,11 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ 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 BulkChangeModal from './BulkChangeModal'; +import BulkChangeModal, { MAX_PAGE_SIZE } from './BulkChangeModal'; import ComponentBreadcrumbs from './ComponentBreadcrumbs'; import IssuesList from './IssuesList'; import IssuesSourceViewer from './IssuesSourceViewer'; @@ -51,11 +52,10 @@ import { STANDARDS, ReferencedRule } from '../utils'; +import { Alert } from '../../../components/ui/Alert'; import { Button } from '../../../components/ui/buttons'; import Checkbox from '../../../components/controls/Checkbox'; import DeferredSpinner from '../../../components/common/DeferredSpinner'; -import Dropdown from '../../../components/controls/Dropdown'; -import DropdownIcon from '../../../components/icons-components/DropdownIcon'; import EmptySearch from '../../../components/common/EmptySearch'; import FiltersHeader from '../../../components/common/FiltersHeader'; import handleRequiredAuthentication from '../../../app/utils/handleRequiredAuthentication'; @@ -110,7 +110,8 @@ interface Props { } export interface State { - bulkChange?: 'all' | 'selected'; + bulkChangeModal: boolean; + checkAll?: boolean; checked: string[]; effortTotal?: number; facets: { [facet: string]: Facet }; @@ -144,6 +145,7 @@ export class App extends React.PureComponent { constructor(props: Props) { super(props); this.state = { + bulkChangeModal: false, checked: [], facets: {}, issues: [], @@ -210,6 +212,7 @@ export class App extends React.PureComponent { areMyIssuesSelected(prevQuery) !== areMyIssuesSelected(query) ) { this.fetchFirstIssues(); + this.setState({ checkAll: false }); } else if ( !this.state.openIssue && (prevState.selected !== this.state.selected || prevState.openIssue) @@ -515,7 +518,7 @@ export class App extends React.PureComponent { const p = paging.pageIndex + 1; - this.setState({ loadingMore: true }); + this.setState({ checkAll: false, loadingMore: true }); this.fetchIssuesPage(p).then( response => { if (this.mounted) { @@ -623,6 +626,21 @@ export class App extends React.PureComponent { return Promise.resolve({ issues, paging }); }; + getButtonLabel = (checked: string[], checkAll?: boolean, paging?: T.Paging) => { + if (checked.length > 0) { + let count; + if (checkAll && paging) { + count = paging.total > MAX_PAGE_SIZE ? MAX_PAGE_SIZE : paging.total; + } else { + count = Math.min(checked.length, MAX_PAGE_SIZE); + } + + return translateWithParameters('issues.bulk_change_X_issues', count); + } else { + return translate('bulk_change'); + } + }; + handleFilterChange = (changes: Partial) => { this.setState({ loading: true }); this.props.router.push({ @@ -739,10 +757,11 @@ export class App extends React.PureComponent { ? union(checked, [state.issues[i].key]) : without(checked, state.issues[i].key); } - return { checked }; + return { checkAll: false, checked }; }); } else { this.setState(state => ({ + checkAll: false, lastChecked: issue, checked: state.checked.includes(issue) ? without(state.checked, issue) @@ -757,29 +776,20 @@ export class App extends React.PureComponent { })); }; - openBulkChange = (mode: 'all' | 'selected') => { - this.setState({ bulkChange: mode }); + handleOpenBulkChange = () => { key.setScope('issues-bulk-change'); + this.setState({ bulkChangeModal: true }); }; - closeBulkChange = () => { + handleCloseBulkChange = () => { key.setScope('issues'); - this.setState({ bulkChange: undefined }); - }; - - handleBulkChangeClick = () => { - this.openBulkChange('all'); - }; - - handleBulkChangeSelectedClick = (event: React.MouseEvent) => { - event.preventDefault(); - event.currentTarget.blur(); - this.openBulkChange('selected'); + this.setState({ bulkChangeModal: false }); }; handleBulkChangeDone = () => { + this.setState({ checkAll: false }); this.fetchFirstIssues(); - this.closeBulkChange(); + this.handleCloseBulkChange(); }; handleReload = () => { @@ -812,11 +822,14 @@ export class App extends React.PureComponent { this.setState(actions.selectPreviousLocation); }; - onCheckAll = (checked: boolean) => { + handleCheckAll = (checked: boolean) => { if (checked) { - this.setState(state => ({ checked: state.issues.map(issue => issue.key) })); + this.setState(state => ({ + checkAll: true, + checked: state.issues.map(issue => issue.key) + })); } else { - this.setState({ checked: [] }); + this.setState({ checkAll: false, checked: [] }); } }; @@ -834,7 +847,7 @@ export class App extends React.PureComponent { renderBulkChange(openIssue: T.Issue | undefined) { const { component, currentUser } = this.props; - const { bulkChange, checked, paging, issues } = this.state; + const { checkAll, bulkChangeModal, checked, issues, paging } = this.state; const isAllChecked = checked.length > 0 && issues.length === checked.length; const thirdState = checked.length > 0 && !isAllChecked; @@ -851,45 +864,22 @@ export class App extends React.PureComponent { className="spacer-right vertical-middle" disabled={issues.length === 0} id="issues-selection" - onCheck={this.onCheckAll} + onCheck={this.handleCheckAll} thirdState={thirdState} /> - {checked.length > 0 ? ( - -
  • - - {translateWithParameters('issues.bulk_change', paging ? paging.total : 0)} - -
    • -
  • - - {translateWithParameters('issues.bulk_change_selected', checked.length)} - -
    • - - }> - -     - ) : ( - - )} - {bulkChange && ( + + + {bulkChangeModal && ( @@ -1073,7 +1063,7 @@ export class App extends React.PureComponent { } renderPage() { - const { loading, openIssue } = this.state; + const { checkAll, loading, openIssue, paging } = this.state; return (
    {openIssue ? ( @@ -1089,7 +1079,20 @@ export class App extends React.PureComponent { selectedLocationIndex={this.state.selectedLocationIndex} /> ) : ( - {this.renderList()} + + {checkAll && + paging && + paging.total > MAX_PAGE_SIZE && ( + + {MAX_PAGE_SIZE} }} + /> + + )} + {this.renderList()} + )}
    ); diff --git a/server/sonar-web/src/main/js/apps/issues/components/BulkChangeModal.tsx b/server/sonar-web/src/main/js/apps/issues/components/BulkChangeModal.tsx index 9578cbbaa56..361fb238886 100644 --- a/server/sonar-web/src/main/js/apps/issues/components/BulkChangeModal.tsx +++ b/server/sonar-web/src/main/js/apps/issues/components/BulkChangeModal.tsx @@ -18,6 +18,7 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ import * as React from 'react'; +import { FormattedMessage } from 'react-intl'; import { pickBy, sortBy } from 'lodash'; import { searchAssignees } from '../utils'; import { searchIssueTags, bulkChangeIssues } from '../../../api/issues'; @@ -30,7 +31,7 @@ import Select from '../../../components/controls/Select'; import HelpTooltip from '../../../components/controls/HelpTooltip'; import SeverityHelper from '../../../components/shared/SeverityHelper'; import Avatar from '../../../components/ui/Avatar'; -import { SubmitButton } from '../../../components/ui/buttons'; +import { SubmitButton, ResetButtonLink } from '../../../components/ui/buttons'; import IssueTypeIcon from '../../../components/ui/IssueTypeIcon'; import { translate, translateWithParameters } from '../../../helpers/l10n'; import { Alert } from '../../../components/ui/Alert'; @@ -85,6 +86,8 @@ const AssigneeSelect = SearchSelect as AssigneeSelectType; type TagSelectType = new () => SearchSelect; const TagSelect = SearchSelect as TagSelectType; +export const MAX_PAGE_SIZE = 500; + export default class BulkChangeModal extends React.PureComponent { mounted = false; @@ -104,13 +107,17 @@ export default class BulkChangeModal extends React.PureComponent { this.loadIssues(), searchIssueTags({ organization: this.state.organization }) ]).then( - ([issues, tags]) => { + ([{ issues, paging }, tags]) => { if (this.mounted) { + if (issues.length > MAX_PAGE_SIZE) { + issues = issues.slice(0, MAX_PAGE_SIZE); + } + this.setState({ initialTags: tags.map(tag => ({ label: tag, value: tag })), - issues: issues.issues, + issues, loading: false, - paging: issues.paging + paging }); } }, @@ -122,7 +129,9 @@ export default class BulkChangeModal extends React.PureComponent { this.mounted = false; } - loadIssues = () => this.props.fetchIssues({ additionalFields: 'actions,transitions', ps: 250 }); + loadIssues = () => { + return this.props.fetchIssues({ additionalFields: 'actions,transitions', ps: MAX_PAGE_SIZE }); + }; getDefaultAssignee = () => { const { currentUser } = this.props; @@ -149,12 +158,6 @@ export default class BulkChangeModal extends React.PureComponent { return options; }; - handleCloseClick = (event: React.MouseEvent) => { - event.preventDefault(); - event.currentTarget.blur(); - this.props.onClose(); - }; - handleAssigneeSearch = (query: string) => { return searchAssignees(query, this.state.organization).then(({ results }) => results.map(r => ({ avatar: r.avatar, label: r.name, value: r.login })) @@ -250,12 +253,6 @@ export default class BulkChangeModal extends React.PureComponent { })); } - renderCancelButton = () => ( - - {translate('cancel')} - - ); - renderLoading = () => (
    @@ -266,7 +263,9 @@ export default class BulkChangeModal extends React.PureComponent {
    -
    {this.renderCancelButton()}
    +
    + {translate('cancel')} +
    ); @@ -496,7 +495,7 @@ export default class BulkChangeModal extends React.PureComponent { renderForm = () => { const { issues, paging, submitting } = this.state; - const limitReached = paging !== undefined && paging.total > paging.pageIndex * paging.pageSize; + const limitReached = paging && paging.total > MAX_PAGE_SIZE; return (
    @@ -507,7 +506,11 @@ export default class BulkChangeModal extends React.PureComponent {
    {limitReached && ( - {translateWithParameters('issue_bulk_change.max_issues_reached', issues.length)} + {MAX_PAGE_SIZE} }} + /> )} @@ -529,7 +532,7 @@ export default class BulkChangeModal extends React.PureComponent { {translate('apply')} - {this.renderCancelButton()} + {translate('cancel')}
    ); 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 e8bcd5e0323..6742f296c86 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,52 +20,25 @@ import * as React from 'react'; import { shallow } from 'enzyme'; import { App } from '../App'; +import { mockCurrentUser, mockRouter } from '../../../../helpers/testMocks'; import { waitAndUpdate } from '../../../../helpers/testUtils'; -const issues = [ +const ISSUES = [ { key: 'foo' } as T.Issue, { key: 'bar' } as T.Issue, { key: 'third' } as T.Issue, { key: 'fourth' } as T.Issue ]; -const facets = [{ property: 'severities', values: [{ val: 'MINOR', count: 4 }] }]; -const paging = { pageIndex: 1, pageSize: 100, total: 4 }; +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' }; -const PROPS = { - branch: { isMain: true, name: 'master' }, - currentUser: { - isLoggedIn: true, - avatar: 'foo', - email: 'forr@bar.com', - login: 'JohnDoe', - name: 'John Doe' - }, - component: { breadcrumbs: [], key: 'foo', name: 'bar', organization: 'John', qualifier: 'Doe' }, - location: { pathname: '/issues', query: {} }, - fetchIssues: () => - Promise.resolve({ - components: [referencedComponent], - effortTotal: 1, - facets, - issues, - languages: [], - paging, - rules: [], - users: [] - }), - onBranchesChange: () => {}, - onSonarCloud: false, - organization: { key: 'foo' }, - router: { push: jest.fn(), replace: jest.fn() }, - userOrganizations: [] -}; it('should render a list of issue', async () => { - const wrapper = shallow(); + const wrapper = shallowRender(); await waitAndUpdate(wrapper); expect(wrapper.state().issues.length).toBe(4); expect(wrapper.state().referencedComponentsById).toEqual({ 'foo-uuid': referencedComponent }); @@ -73,7 +46,7 @@ it('should render a list of issue', async () => { }); it('should be able to check/uncheck a group of issues with the Shift key', async () => { - const wrapper = shallow(); + const wrapper = shallowRender(); await waitAndUpdate(wrapper); expect(wrapper.state().issues.length).toBe(4); @@ -92,7 +65,7 @@ it('should be able to check/uncheck a group of issues with the Shift key', async }); it('should avoid non-existing keys', async () => { - const wrapper = shallow(); + const wrapper = shallowRender(); await waitAndUpdate(wrapper); expect(wrapper.state().issues.length).toBe(4); @@ -105,7 +78,7 @@ it('should avoid non-existing keys', async () => { }); it('should be able to uncheck all issue with global checkbox', async () => { - const wrapper = shallow(); + const wrapper = shallowRender(); await waitAndUpdate(wrapper); expect(wrapper.state().issues.length).toBe(4); @@ -114,16 +87,92 @@ it('should be able to uncheck all issue with global checkbox', async () => { instance.handleIssueCheck('bar', eventNoShiftKey); expect(wrapper.state().checked.length).toBe(2); - instance.onCheckAll(false); + instance.handleCheckAll(false); expect(wrapper.state().checked.length).toBe(0); }); it('should be able to check all issue with global checkbox', async () => { - const wrapper = shallow(); + const wrapper = shallowRender(); await waitAndUpdate(wrapper); const instance = wrapper.instance(); expect(wrapper.state().checked.length).toBe(0); - instance.onCheckAll(true); + instance.handleCheckAll(true); expect(wrapper.state().checked.length).toBe(wrapper.state().issues.length); }); + +it('should check all issues, even the ones that are not visible', async () => { + const wrapper = shallowRender({ + fetchIssues: jest.fn().mockResolvedValue({ + components: [referencedComponent], + effortTotal: 1, + facets: FACETS, + issues: ISSUES, + languages: [], + paging: { pageIndex: 1, pageSize: 100, total: 250 }, + rules: [], + users: [] + }) + }); + const instance = wrapper.instance(); + await waitAndUpdate(wrapper); + + // Checking all issues should show the correct count in the Bulk Change button. + instance.handleCheckAll(true); + waitAndUpdate(wrapper); + expect(wrapper.find('#issues-bulk-change')).toMatchSnapshot(); +}); + +it('should check max 500 issues', async () => { + const wrapper = shallowRender({ + fetchIssues: jest.fn().mockResolvedValue({ + components: [referencedComponent], + effortTotal: 1, + facets: FACETS, + issues: ISSUES, + languages: [], + paging: { pageIndex: 1, pageSize: 100, total: 1000 }, + rules: [], + users: [] + }) + }); + const instance = wrapper.instance(); + await waitAndUpdate(wrapper); + + // Checking all issues should show 500 in the Bulk Change button, and display + // a warning. + instance.handleCheckAll(true); + waitAndUpdate(wrapper); + expect(wrapper.find('#issues-bulk-change')).toMatchSnapshot(); +}); + +function shallowRender(props: Partial = {}) { + return shallow( + {}} + organization={{ key: 'foo' }} + router={mockRouter()} + userOrganizations={[]} + {...props} + /> + ); +} diff --git a/server/sonar-web/src/main/js/apps/issues/components/__tests__/BulkChangeModal-test.tsx b/server/sonar-web/src/main/js/apps/issues/components/__tests__/BulkChangeModal-test.tsx index fd1ca320e29..39494a70beb 100644 --- a/server/sonar-web/src/main/js/apps/issues/components/__tests__/BulkChangeModal-test.tsx +++ b/server/sonar-web/src/main/js/apps/issues/components/__tests__/BulkChangeModal-test.tsx @@ -19,13 +19,20 @@ */ import * as React from 'react'; import { shallow } from 'enzyme'; -import BulkChangeModal from '../BulkChangeModal'; +import BulkChangeModal, { MAX_PAGE_SIZE } from '../BulkChangeModal'; +import { mockIssue } from '../../../../helpers/testMocks'; import { waitAndUpdate } from '../../../../helpers/testUtils'; jest.mock('../../../../api/issues', () => ({ searchIssueTags: () => Promise.resolve([undefined, []]) })); +jest.mock('../BulkChangeModal', () => { + const mock = require.requireActual('../BulkChangeModal'); + mock.MAX_PAGE_SIZE = 1; + return mock; +}); + it('should display error message when no issues available', async () => { const wrapper = getWrapper([]); await waitAndUpdate(wrapper); @@ -33,36 +40,23 @@ it('should display error message when no issues available', async () => { }); it('should display form when issues are present', async () => { - const wrapper = getWrapper([ - { - actions: [], - component: 'foo', - componentLongName: 'foo', - componentQualifier: 'foo', - componentUuid: 'foo', - creationDate: 'foo', - key: 'foo', - flows: [], - fromHotspot: false, - message: 'foo', - organization: 'foo', - project: 'foo', - projectName: 'foo', - projectOrganization: 'foo', - projectKey: 'foo', - rule: 'foo', - ruleName: 'foo', - secondaryLocations: [], - severity: 'foo', - status: 'foo', - transitions: [], - type: 'BUG' - } - ]); + const wrapper = getWrapper([mockIssue()]); await waitAndUpdate(wrapper); expect(wrapper).toMatchSnapshot(); }); +it('should display warning when too many issues are passed', async () => { + const issues: T.Issue[] = []; + for (let i = MAX_PAGE_SIZE + 1; i > 0; i--) { + issues.push(mockIssue()); + } + + const wrapper = getWrapper(issues); + await waitAndUpdate(wrapper); + expect(wrapper.find('h2')).toMatchSnapshot(); + expect(wrapper.find('Alert')).toMatchSnapshot(); +}); + const getWrapper = (issues: T.Issue[]) => { return shallow( { Promise.resolve({ issues, paging: { - pageIndex: 0, - pageSize: 0, - total: 0 + pageIndex: issues.length, + pageSize: issues.length, + total: issues.length } }) } diff --git a/server/sonar-web/src/main/js/apps/issues/components/__tests__/__snapshots__/App-test.tsx.snap b/server/sonar-web/src/main/js/apps/issues/components/__tests__/__snapshots__/App-test.tsx.snap new file mode 100644 index 00000000000..0aa0114a24b --- /dev/null +++ b/server/sonar-web/src/main/js/apps/issues/components/__tests__/__snapshots__/App-test.tsx.snap @@ -0,0 +1,21 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`should check all issues, even the ones that are not visible 1`] = ` + +`; + +exports[`should check max 500 issues 1`] = ` + +`; diff --git a/server/sonar-web/src/main/js/apps/issues/components/__tests__/__snapshots__/BulkChangeModal-test.tsx.snap b/server/sonar-web/src/main/js/apps/issues/components/__tests__/__snapshots__/BulkChangeModal-test.tsx.snap index 6ecd67855be..e45da7a29a6 100644 --- a/server/sonar-web/src/main/js/apps/issues/components/__tests__/__snapshots__/BulkChangeModal-test.tsx.snap +++ b/server/sonar-web/src/main/js/apps/issues/components/__tests__/__snapshots__/BulkChangeModal-test.tsx.snap @@ -34,13 +34,11 @@ exports[`should display error message when no issues available 1`] = ` > apply - cancel - + @@ -90,14 +88,36 @@ exports[`should display form when issues are present 1`] = ` > apply - cancel - + `; + +exports[`should display warning when too many issues are passed 1`] = ` +

    + issue_bulk_change.form.title.1 +

    +`; + +exports[`should display warning when too many issues are passed 2`] = ` + + + 1 + , + } + } + /> + +`; diff --git a/server/sonar-web/src/main/js/apps/issues/styles.css b/server/sonar-web/src/main/js/apps/issues/styles.css index 80de331b327..ff12f912ecc 100644 --- a/server/sonar-web/src/main/js/apps/issues/styles.css +++ b/server/sonar-web/src/main/js/apps/issues/styles.css @@ -25,13 +25,6 @@ margin-right: 2px; } -.concise-issues-list-header, -.concise-issues-list-header-inner { -} - -.concise-issues-list-header { -} - .concise-issues-list-header-inner { width: 260px; text-align: center; diff --git a/server/sonar-web/src/main/js/helpers/testMocks.ts b/server/sonar-web/src/main/js/helpers/testMocks.ts index 4a7fd47182b..04df471571e 100644 --- a/server/sonar-web/src/main/js/helpers/testMocks.ts +++ b/server/sonar-web/src/main/js/helpers/testMocks.ts @@ -71,6 +71,34 @@ export function mockEvent(overrides = {}) { } as any; } +export function mockIssue(overrides = {}): T.Issue { + return { + actions: [], + component: 'my-component', + componentLongName: 'My Component', + componentQualifier: 'my-component', + componentUuid: 'uuid', + creationDate: 'date', + key: 'foo', + flows: [], + fromHotspot: false, + message: 'Message', + organization: 'foo', + project: 'my-project', + projectName: 'My Project', + projectOrganization: 'org', + projectKey: 'key', + rule: 'rule', + ruleName: 'Rule', + secondaryLocations: [], + severity: 'severity', + status: 'status', + transitions: [], + type: 'BUG', + ...overrides + }; +} + export function mockLocation(overrides: Partial = {}): Location { return { action: 'PUSH', diff --git a/sonar-core/src/main/resources/org/sonar/l10n/core.properties b/sonar-core/src/main/resources/org/sonar/l10n/core.properties index f11a0baa6a3..8427e8ecd10 100644 --- a/sonar-core/src/main/resources/org/sonar/l10n/core.properties +++ b/sonar-core/src/main/resources/org/sonar/l10n/core.properties @@ -665,8 +665,7 @@ issue.this_issue_involves_x_code_locations=This issue involves {0} code location issue.from_external_rule_engine=Issue detected by an external rule engine: {0} issue.external_issue_description=This is external rule {0}. No details are available. issues.return_to_list=Return to List -issues.bulk_change=All Issues ({0}) -issues.bulk_change_selected=Selected Issues ({0}) +issues.bulk_change_X_issues=Bulk Change {0} Issue(s) issues.issues=issues issues.to_select_issues=to select issues issues.to_navigate=to navigate @@ -745,7 +744,7 @@ issues.facet.cwe=CWE issue_bulk_change.form.title=Change {0} issues issue_bulk_change.comment.help=This comment will be applied only to issues that will effectively be modified -issue_bulk_change.max_issues_reached=As too many issues have been selected, only the first {0} issues will be updated. +issue_bulk_change.max_issues_reached=There are more issues available than can be treated by a single bulk action. Your changes will only be applied to the first {max} issues. issue_bulk_change.x_issues={0} issues issue_bulk_change.no_match=There is no issue matching your filter selection