diff options
author | 7PH <benjamin.raymond@sonarsource.com> | 2023-11-21 15:43:04 +0100 |
---|---|---|
committer | sonartech <sonartech@sonarsource.com> | 2023-11-22 20:02:41 +0000 |
commit | 17072178fa9a64aa99fde55cf3efeae75593a84e (patch) | |
tree | eda8e94b2fc11ce5a68ef9643a7b0f8058fd2e78 /server/sonar-web/src/main/js/apps/issues | |
parent | 5f10187fc52c4b5cebbab5fb0cc9daa8c51697b2 (diff) | |
download | sonarqube-17072178fa9a64aa99fde55cf3efeae75593a84e.tar.gz sonarqube-17072178fa9a64aa99fde55cf3efeae75593a84e.zip |
SONAR-20975 Change IssuesPage issue loading logic to always be to able fetch open issue
Diffstat (limited to 'server/sonar-web/src/main/js/apps/issues')
5 files changed, 127 insertions, 130 deletions
diff --git a/server/sonar-web/src/main/js/apps/issues/__tests__/IssueApp-it.tsx b/server/sonar-web/src/main/js/apps/issues/__tests__/IssueApp-it.tsx index 3d0555f1af1..c610490fa88 100644 --- a/server/sonar-web/src/main/js/apps/issues/__tests__/IssueApp-it.tsx +++ b/server/sonar-web/src/main/js/apps/issues/__tests__/IssueApp-it.tsx @@ -64,6 +64,14 @@ beforeEach(() => { }); describe('issue app', () => { + it('should always be able to render the open issue', async () => { + renderProjectIssuesApp('project/issues?issueStatuses=CONFIRMED&open=issue2&id=myproject&why=1'); + + expect(await ui.conciseIssueTotal.find()).toHaveTextContent('3'); + expect(ui.conciseIssueItem4.get()).toBeInTheDocument(); + expect(ui.conciseIssueItem2.get()).toBeInTheDocument(); + }); + it('should navigate to Why is this an issue tab', async () => { renderProjectIssuesApp('project/issues?issues=issue2&open=issue2&id=myproject&why=1'); diff --git a/server/sonar-web/src/main/js/apps/issues/__tests__/IssuesApp-it.tsx b/server/sonar-web/src/main/js/apps/issues/__tests__/IssuesApp-it.tsx index 23152f04b84..6c4457bbeae 100644 --- a/server/sonar-web/src/main/js/apps/issues/__tests__/IssuesApp-it.tsx +++ b/server/sonar-web/src/main/js/apps/issues/__tests__/IssuesApp-it.tsx @@ -233,14 +233,14 @@ describe('issues app', () => { renderIssueApp(); expect(await ui.issueItems.findAll()).toHaveLength(7); - expect(ui.issueItem8.query()).not.toBeInTheDocument(); + expect(ui.issueItem9.query()).not.toBeInTheDocument(); await act(async () => { await user.click(screen.getByRole('button', { name: 'show_more' })); }); - expect(ui.issueItems.getAll()).toHaveLength(10); - expect(ui.issueItem8.get()).toBeInTheDocument(); + expect(ui.issueItems.getAll()).toHaveLength(9); + expect(ui.issueItem9.get()).toBeInTheDocument(); }); it('should be able to select issues for bulk change', async () => { diff --git a/server/sonar-web/src/main/js/apps/issues/components/IssuesApp.tsx b/server/sonar-web/src/main/js/apps/issues/components/IssuesApp.tsx index 768480535f2..2a3f7907d4a 100644 --- a/server/sonar-web/src/main/js/apps/issues/components/IssuesApp.tsx +++ b/server/sonar-web/src/main/js/apps/issues/components/IssuesApp.tsx @@ -121,7 +121,6 @@ interface Props extends WithIndexationContextProps { } export interface State { bulkChangeModal: boolean; - cannotShowOpenIssue?: boolean; checkAll?: boolean; checked: string[]; effortTotal?: number; @@ -150,7 +149,8 @@ export interface State { selectedLocationIndex?: number; } -const MAX_INITAL_FETCH = 1000; +// When opening a specific issue, number of issues to fetch through pagination before loading it specifically +const MAX_INITAL_FETCH = 400; const VARIANTS_FACET = 'codeVariants'; const ISSUES_PAGE_SIZE = 100; @@ -469,32 +469,24 @@ export class App extends React.PureComponent<Props, State> { createdAfterIncludesTime = () => Boolean(this.props.location.query.createdAfter?.includes('T')); - fetchIssuesHelper = (query: RawQuery) => { + fetchIssuesHelper = async (query: RawQuery) => { if (this.props.component?.needIssueSync) { - return listIssues({ - ...query, - }).then((response) => { - const { components, issues, rules } = response; - - const parsedIssues = issues.map((issue) => - parseIssueFromResponse(issue, components, undefined, rules), - ); - - return { ...response, issues: parsedIssues } as FetchIssuesPromise; - }); + const response = await listIssues(query); + const parsedIssues = response.issues.map((issue) => + parseIssueFromResponse(issue, response.components, undefined, response.rules), + ); + return { ...response, issues: parsedIssues } as FetchIssuesPromise; } - return searchIssues({ + const response = await searchIssues({ ...query, additionalFields: '_all', timeZone: Intl.DateTimeFormat().resolvedOptions().timeZone, - }).then((response) => { - const parsedIssues = response.issues.map((issue) => - parseIssueFromResponse(issue, response.components, response.users, response.rules), - ); - - return { ...response, issues: parsedIssues } as FetchIssuesPromise; }); + const parsedIssues = response.issues.map((issue) => + parseIssueFromResponse(issue, response.components, response.users, response.rules), + ); + return { ...response, issues: parsedIssues } as FetchIssuesPromise; }; fetchIssues = ( @@ -549,123 +541,131 @@ export class App extends React.PureComponent<Props, State> { return this.fetchIssuesHelper(parameters); }; - fetchFirstIssues(firstRequest: boolean) { + async fetchFirstIssues(firstRequest: boolean) { const prevQuery = this.props.location.query; const openIssueKey = getOpen(this.props.location.query); - let fetchPromise; this.setState({ checked: [], loading: true }); + let response: FetchIssuesPromise; if (openIssueKey !== undefined) { - fetchPromise = this.fetchIssuesUntil(1, (pageIssues: Issue[], paging: Paging) => { - if ( - paging.total <= paging.pageIndex * paging.pageSize || - paging.pageIndex * paging.pageSize >= MAX_INITAL_FETCH - ) { - return true; - } - - return pageIssues.some((issue) => issue.key === openIssueKey); - }); + response = await this.fetchIssuesUntil(openIssueKey); } else { - fetchPromise = this.fetchIssues({}, true, firstRequest); + response = await this.fetchIssues({}, true, firstRequest); } - return fetchPromise.then(this.parseFirstIssues(firstRequest, openIssueKey, prevQuery), () => { + try { + return this.parseFirstIssues(response, firstRequest, prevQuery); + } catch (error) { if (this.mounted && areQueriesEqual(prevQuery, this.props.location.query)) { this.setState({ loading: false }); } - return []; - }); + } } - parseFirstIssues = - (firstRequest: boolean, openIssueKey: string | undefined, prevQuery: RawQuery) => - ({ effortTotal, facets, issues, paging, ...other }: FetchIssuesPromise) => { - if (this.mounted && areQueriesEqual(prevQuery, this.props.location.query)) { - const openIssue = getOpenIssue(this.props, issues); - let selected: string | undefined = undefined; + parseFirstIssues = (response: FetchIssuesPromise, firstRequest: boolean, prevQuery: RawQuery) => { + const { effortTotal, facets, issues, paging, ...other } = response; - if (issues.length > 0) { - selected = openIssue ? openIssue.key : issues[0].key; - } + if (this.mounted && areQueriesEqual(prevQuery, this.props.location.query)) { + const openIssue = getOpenIssue(this.props, issues); + let selected: string | undefined = undefined; - this.setState(({ showVariantsFilter }) => ({ - cannotShowOpenIssue: Boolean(openIssueKey) && !openIssue, - effortTotal, - facets: parseFacets(facets), - issues, - loading: false, - locationsNavigator: true, - openIssue, - paging, - referencedComponentsById: keyBy(other.components, 'uuid'), - referencedComponentsByKey: keyBy(other.components, 'key'), - referencedLanguages: keyBy(other.languages, 'key'), - referencedRules: keyBy(other.rules, 'key'), - referencedUsers: keyBy(other.users, 'login'), - selected, - selectedFlowIndex: 0, - selectedLocationIndex: undefined, - showVariantsFilter: firstRequest - ? Boolean(facets?.find((f) => f.property === VARIANTS_FACET)?.values.length) - : showVariantsFilter, - })); + if (issues.length > 0) { + selected = openIssue ? openIssue.key : issues[0].key; } - return issues; - }; + this.setState(({ showVariantsFilter }) => ({ + effortTotal, + facets: parseFacets(facets), + issues, + loading: false, + locationsNavigator: true, + openIssue, + paging, + referencedComponentsById: keyBy(other.components, 'uuid'), + referencedComponentsByKey: keyBy(other.components, 'key'), + referencedLanguages: keyBy(other.languages, 'key'), + referencedRules: keyBy(other.rules, 'key'), + referencedUsers: keyBy(other.users, 'login'), + selected, + selectedFlowIndex: 0, + selectedLocationIndex: undefined, + showVariantsFilter: firstRequest + ? Boolean(facets?.find((f) => f.property === VARIANTS_FACET)?.values.length) + : showVariantsFilter, + })); + } - fetchIssuesPage = (p: number) => { - return this.fetchIssues({ p }); + return issues; }; - fetchIssuesUntil = ( - page: number, - done: (pageIssues: Issue[], paging: Paging) => boolean, - ): Promise<FetchIssuesPromise> => { - const recursiveFetch = (p: number, prevIssues: Issue[]): Promise<FetchIssuesPromise> => { - return this.fetchIssuesPage(p).then(({ issues: pageIssues, paging, ...other }) => { - const issues = [...prevIssues, ...pageIssues]; - - // eslint-disable-next-line promise/no-callback-in-promise - return done(pageIssues, paging) - ? { issues, paging, ...other } - : recursiveFetch(p + 1, issues); - }); - }; + fetchIssuesUntil = async (issueKey: string): Promise<FetchIssuesPromise> => { + let issueOfInterest: Issue | undefined; + const allIssues: Issue[] = []; + + // Try and find issue of interest in the first pages of issues. Stop if we find it. + let currentPage = 1; + let lastResponse: FetchIssuesPromise; + do { + // eslint-disable-next-line no-await-in-loop + const response = await this.fetchIssues({ p: currentPage }); + allIssues.push(...response.issues); + lastResponse = response; + + issueOfInterest = response.issues.find((issue) => issue.key === issueKey); + if (issueOfInterest) { + return { ...response, issues: allIssues }; + } + + currentPage++; + } while (currentPage <= MAX_INITAL_FETCH / ISSUES_PAGE_SIZE); - return recursiveFetch(page, []); + // If we could not find the issue, we load it specifically and prepend it to the list + const { + issues: [issue], + } = await this.fetchIssuesHelper({ issues: issueKey }); + return { + ...lastResponse, + issues: [issue, ...allIssues], + // Use last paging object we got from the previous requests and patch it with this issue + paging: { + ...lastResponse.paging, + total: lastResponse.paging.total + 1, + }, + }; }; - fetchMoreIssues = () => { + fetchMoreIssues = async () => { const { paging } = this.state; if (!paging) { - return Promise.reject(); + throw new Error('Paging is not defined'); } - const p = paging.pageIndex + 1; - this.setState({ checkAll: false, loadingMore: true }); - return this.fetchIssuesPage(p).then( - (response) => { - if (this.mounted) { - this.setState((state) => ({ - issues: [...state.issues, ...response.issues], - loadingMore: false, - paging: response.paging, - })); - } - }, - () => { - if (this.mounted) { - this.setState({ loadingMore: false }); - } - }, - ); + try { + const response = await this.fetchIssues({ p: paging.pageIndex + 1 }); + + // In some cases, we can get an issue that we already have in the list as the first issue + // When this happens, we filter it out + // @see this.fetchIssuesUntil + const firstIssueKey = response.issues[0]?.key; + response.issues = response.issues.filter((issue) => issue.key !== firstIssueKey); + + if (this.mounted) { + this.setState((state) => ({ + issues: [...state.issues, ...response.issues], + loadingMore: false, + paging: response.paging, + })); + } + } catch (error) { + if (this.mounted) { + this.setState({ loadingMore: false }); + } + } }; fetchFacet = (facet: string) => { @@ -1082,7 +1082,8 @@ export class App extends React.PureComponent<Props, State> { {({ top }) => ( <nav aria-label={openIssue ? translate('list_of_issues') : translate('filters')} - className="it__issues-nav-bar sw-overflow-y-auto issue-filters-list" + data-testid="issues-nav-bar" + className="issues-nav-bar sw-overflow-y-auto issue-filters-list" style={{ height: `calc((100vh - ${top}px) - 60px)` }} // 60px (footer) > <div className="sw-w-[300px] lg:sw-w-[390px]"> @@ -1210,16 +1211,8 @@ export class App extends React.PureComponent<Props, State> { } renderPage() { - const { - cannotShowOpenIssue, - openRuleDetails, - checkAll, - issues, - loading, - openIssue, - paging, - loadingRule, - } = this.state; + const { openRuleDetails, checkAll, issues, loading, openIssue, paging, loadingRule } = + this.state; return ( <ScreenPositionHelper> @@ -1286,15 +1279,6 @@ export class App extends React.PureComponent<Props, State> { </div> )} - {cannotShowOpenIssue && (!paging || paging.total > 0) && ( - <FlagMessage className="sw-mb-4" variant="warning"> - {translateWithParameters( - 'issues.cannot_open_issue_max_initial_X_fetched', - MAX_INITAL_FETCH, - )} - </FlagMessage> - )} - {this.renderList()} </Spinner> </div> diff --git a/server/sonar-web/src/main/js/apps/issues/issues-subnavigation/SubnavigationIssue.tsx b/server/sonar-web/src/main/js/apps/issues/issues-subnavigation/SubnavigationIssue.tsx index 5587252136f..284f3872755 100644 --- a/server/sonar-web/src/main/js/apps/issues/issues-subnavigation/SubnavigationIssue.tsx +++ b/server/sonar-web/src/main/js/apps/issues/issues-subnavigation/SubnavigationIssue.tsx @@ -49,7 +49,7 @@ export default function SubnavigationIssue(props: ConciseIssueProps) { React.useEffect(() => { if (selected && element.current) { - const parent = document.querySelector('nav.it__issues-nav-bar') as HTMLMenuElement; + const parent = document.querySelector('nav.issues-nav-bar') as HTMLMenuElement; const rect = parent.getBoundingClientRect(); const offset = element.current.offsetTop - rect.height / HALF_DIVIDER + rect.top / HALF_DIVIDER; diff --git a/server/sonar-web/src/main/js/apps/issues/test-utils.tsx b/server/sonar-web/src/main/js/apps/issues/test-utils.tsx index 342c7dc7f4e..4ac409fb53d 100644 --- a/server/sonar-web/src/main/js/apps/issues/test-utils.tsx +++ b/server/sonar-web/src/main/js/apps/issues/test-utils.tsx @@ -63,8 +63,13 @@ export const ui = { issueItem6: byRole('region', { name: 'Second issue' }), issueItem7: byRole('region', { name: 'Issue with tags' }), issueItem8: byRole('region', { name: 'Issue on page 2' }), + issueItem9: byRole('region', { name: 'Issue inside folderA' }), projectIssueItem6: byRole('button', { name: 'Second issue', exact: false }), + conciseIssueTotal: byTestId('page-counter-total'), + conciseIssueItem2: byTestId('issues-nav-bar').byRole('button', { name: 'Fix that' }), + conciseIssueItem4: byTestId('issues-nav-bar').byRole('button', { name: 'Issue with tags' }), + assigneeFacet: byRole('button', { name: 'issues.facet.assignees' }), authorFacet: byRole('button', { name: 'issues.facet.authors' }), codeVariantsFacet: byRole('button', { name: 'issues.facet.codeVariants' }), |