Browse Source

SONAR-20975 Change IssuesPage issue loading logic to always be to able fetch open issue

tags/10.4.0.87286
7PH 6 months ago
parent
commit
17072178fa

+ 6
- 0
server/sonar-web/src/main/js/api/mocks/IssuesServiceMock.ts View File

@@ -462,6 +462,12 @@ export default class IssuesServiceMock {
return item.issue.codeVariants.some(
(codeVariant) => query.codeVariants?.split(',').includes(codeVariant),
);
})
.filter((item) => {
if (!query.issues) {
return true;
}
return query.issues.split(',').includes(item.issue.key);
});

// Splice list items according to paging using a fixed page size

+ 2
- 1
server/sonar-web/src/main/js/api/mocks/data/issues.ts View File

@@ -387,6 +387,7 @@ export function mockIssuesList(baseComponentKey = PARENT_COMPONENT_KEY): IssueDa
project: 'org.project2',
assignee: 'email1@sonarsource.com',
author: 'email3@sonarsource.com',
issueStatus: IssueStatus.Confirmed,
}),
snippets: keyBy(
[
@@ -438,7 +439,7 @@ export function mockIssuesList(baseComponentKey = PARENT_COMPONENT_KEY): IssueDa
key: ISSUE_1103,
component: `${baseComponentKey}:${ISSUE_TO_FILES[ISSUE_1103][0]}`,
creationDate: '2022-01-15T09:36:01+0100',
message: 'Issue inside folderA',
message: 'Issue 2 inside folderA',
type: IssueType.CodeSmell,
rule: ISSUE_TO_RULE[ISSUE_1103],
textRange: {

+ 8
- 0
server/sonar-web/src/main/js/apps/issues/__tests__/IssueApp-it.tsx View File

@@ -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');


+ 3
- 3
server/sonar-web/src/main/js/apps/issues/__tests__/IssuesApp-it.tsx View File

@@ -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 () => {

+ 110
- 126
server/sonar-web/src/main/js/apps/issues/components/IssuesApp.tsx View File

@@ -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>

+ 1
- 1
server/sonar-web/src/main/js/apps/issues/issues-subnavigation/SubnavigationIssue.tsx View File

@@ -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;

+ 5
- 0
server/sonar-web/src/main/js/apps/issues/test-utils.tsx View File

@@ -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' }),

+ 1
- 1
server/sonar-web/src/main/js/components/common/PageCounter.tsx View File

@@ -33,7 +33,7 @@ export default function PageCounter({ className, current, label, total }: PageCo
<div className={className}>
<strong className="sw-mr-1">
{current !== undefined && formatMeasure(current + 1, MetricType.Integer) + ' / '}
<span className="it__page-counter-total">{formatMeasure(total, MetricType.Integer)}</span>
<span data-testid="page-counter-total">{formatMeasure(total, MetricType.Integer)}</span>
</strong>
{label}
</div>

+ 0
- 1
sonar-core/src/main/resources/org/sonar/l10n/core.properties View File

@@ -1097,7 +1097,6 @@ issue.this_issue_involves_x_code_locations=This issue involves {0} code location
issue.this_flow_involves_x_code_locations=This flow involves {0} code location(s)
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.cannot_open_issue_max_initial_X_fetched=Cannot open selected issue, as it's not part of the initial {0} loaded issues.
issues.loading_issues=Loading issues
issues.return_to_list=Return to List
issues.bulk_change_X_issues=Bulk Change {0} Issue(s)

Loading…
Cancel
Save