From 06b7782cb15956b002d16f09be6f423a6c3fca7d Mon Sep 17 00:00:00 2001 From: Ambroise C Date: Thu, 18 Jan 2024 14:06:21 +0100 Subject: [PATCH] SONAR-21330 Fix Tutorials page behavior --- .../js/app/components/ComponentContainer.tsx | 77 +++++++--- .../__tests__/ComponentContainer-test.tsx | 144 +++++++++++++++++- .../tutorials/TutorialSelectionRenderer.tsx | 26 +--- .../__tests__/TutorialSelection-it.tsx | 7 +- 4 files changed, 206 insertions(+), 48 deletions(-) diff --git a/server/sonar-web/src/main/js/app/components/ComponentContainer.tsx b/server/sonar-web/src/main/js/app/components/ComponentContainer.tsx index d21f6d24ce9..829359c6353 100644 --- a/server/sonar-web/src/main/js/app/components/ComponentContainer.tsx +++ b/server/sonar-web/src/main/js/app/components/ComponentContainer.tsx @@ -30,7 +30,7 @@ import { getComponentNavigation } from '../../api/navigation'; import { useLocation, useRouter } from '../../components/hoc/withRouter'; import { translateWithParameters } from '../../helpers/l10n'; import { HttpStatus } from '../../helpers/request'; -import { getPortfolioUrl } from '../../helpers/urls'; +import { getPortfolioUrl, getProjectUrl, getPullRequestUrl } from '../../helpers/urls'; import { useBranchesQuery } from '../../queries/branch'; import { ProjectAlmBindingConfigurationErrors } from '../../types/alm-settings'; import { Branch } from '../../types/branch-like'; @@ -65,11 +65,13 @@ function ComponentContainer({ hasFeature }: Readonly const [projectBindingErrors, setProjectBindingErrors] = React.useState(); const [loading, setLoading] = React.useState(true); - const [isPending, setPending] = React.useState(false); + const [isPending, setIsPending] = React.useState(false); const { data: { branchLike } = {}, isFetching } = useBranchesQuery( fixedInPullRequest ? component : undefined, ); + const isInTutorials = pathname.includes('tutorials'); + const fetchComponent = React.useCallback( async (branchName?: string) => { // Only show loader if we're changing components @@ -102,20 +104,25 @@ function ComponentContainer({ hasFeature }: Readonly async (componentKey: string) => { try { const { current, queue } = await getTasksForComponent(componentKey); - const newCurrentTask = getCurrentTask(current, branch, pullRequest); - const pendingTasks = getPendingTasksForBranchLike(queue, branch, pullRequest); + const newCurrentTask = getCurrentTask(current, branch, pullRequest, isInTutorials); + const pendingTasks = getReportRelatedPendingTasks( + queue, + branch, + pullRequest, + isInTutorials, + ); const newTasksInProgress = getInProgressTasks(pendingTasks); const isPending = pendingTasks.some((task) => task.status === TaskStatuses.Pending); - setPending(isPending); + setIsPending(isPending); setCurrentTask(newCurrentTask); setTasksInProgress(newTasksInProgress); } catch { // noop } }, - [branch, pullRequest], + [branch, isInTutorials, pullRequest], ); const fetchProjectBindingErrors = React.useCallback( @@ -163,7 +170,7 @@ function ComponentContainer({ hasFeature }: Readonly }, [component, fetchStatus, fetchProjectBindingErrors]); // Refetch status when tasks in progress/current task have changed - // Or refetch component based on computeShouldFetchComponent + // Or refetch component based on computeHasUpdatedTasks React.useEffect(() => { // Stop here if tasks are not fetched yet if (!tasksInProgress) { @@ -171,7 +178,7 @@ function ComponentContainer({ hasFeature }: Readonly } const tasks = tasksInProgress ?? []; - const shouldFetchComponent = computeShouldFetchComponent( + const hasUpdatedTasks = computeHasUpdatedTasks( oldTasksInProgress.current, tasks, oldCurrentTask.current, @@ -179,20 +186,38 @@ function ComponentContainer({ hasFeature }: Readonly component, ); - if (needsAnotherCheck(shouldFetchComponent, component, tasks)) { - // Refresh the status as long as there is tasks in progress or no analysis + if (isInTutorials && hasUpdatedTasks) { + const { branch: branchName, pullRequest: pullRequestKey } = currentTask ?? tasks[0]; + const url = + pullRequestKey !== undefined + ? getPullRequestUrl(key, pullRequestKey) + : getProjectUrl(key, branchName); + router.replace(url); + } + + if (needsAnotherCheck(hasUpdatedTasks, component, tasks)) { + // Refresh the status as long as there are tasks in progress or no analysis window.clearTimeout(watchStatusTimer.current); watchStatusTimer.current = window.setTimeout(() => { fetchStatus(component?.key ?? ''); }, FETCH_STATUS_WAIT_TIME); - } else if (shouldFetchComponent) { + } else if (hasUpdatedTasks) { fetchComponent(); } oldCurrentTask.current = currentTask; oldTasksInProgress.current = tasks; - }, [tasksInProgress, currentTask, component, fetchComponent, fetchStatus]); + }, [ + component, + currentTask, + fetchComponent, + fetchStatus, + isInTutorials, + key, + router, + tasksInProgress, + ]); // Refetch component when a new branch is analyzed React.useEffect(() => { @@ -292,12 +317,12 @@ function addQualifier(component: Component) { } function needsAnotherCheck( - shouldFetchComponent: boolean, + hasUpdatedTasks: boolean, component: Component | undefined, newTasksInProgress: Task[], ) { return ( - !shouldFetchComponent && component && (newTasksInProgress.length > 0 || !component.analysisDate) + !hasUpdatedTasks && component && (newTasksInProgress.length > 0 || !component.analysisDate) ); } @@ -317,19 +342,33 @@ export function isSameBranch( return branch === task.branch; } -function getCurrentTask(current?: Task, branch?: string, pullRequest?: string) { +function getCurrentTask( + current?: Task, + branch?: string, + pullRequest?: string, + canBeDifferentBranchLike = false, +) { if (!current || !isReportRelatedTask(current)) { return undefined; } - return current.status === TaskStatuses.Failed || isSameBranch(current, branch, pullRequest) + return current.status === TaskStatuses.Failed || + canBeDifferentBranchLike || + isSameBranch(current, branch, pullRequest) ? current : undefined; } -function getPendingTasksForBranchLike(pendingTasks: Task[], branch?: string, pullRequest?: string) { +function getReportRelatedPendingTasks( + pendingTasks: Task[], + branch?: string, + pullRequest?: string, + canBeDifferentBranchLike = false, +) { return pendingTasks.filter( - (task) => isReportRelatedTask(task) && isSameBranch(task, branch, pullRequest), + (task) => + isReportRelatedTask(task) && + (canBeDifferentBranchLike || isSameBranch(task, branch, pullRequest)), ); } @@ -341,7 +380,7 @@ function isReportRelatedTask(task: Task) { return [TaskTypes.AppRefresh, TaskTypes.Report, TaskTypes.ViewRefresh].includes(task.type); } -function computeShouldFetchComponent( +function computeHasUpdatedTasks( tasksInProgress: Task[] | undefined, newTasksInProgress: Task[], currentTask: Task | undefined, diff --git a/server/sonar-web/src/main/js/app/components/__tests__/ComponentContainer-test.tsx b/server/sonar-web/src/main/js/app/components/__tests__/ComponentContainer-test.tsx index 8f1d7bf9281..573998a779f 100644 --- a/server/sonar-web/src/main/js/app/components/__tests__/ComponentContainer-test.tsx +++ b/server/sonar-web/src/main/js/app/components/__tests__/ComponentContainer-test.tsx @@ -25,6 +25,7 @@ import { validateProjectAlmBinding } from '../../../api/alm-settings'; import { getTasksForComponent } from '../../../api/ce'; import { getComponentData } from '../../../api/components'; import { getComponentNavigation } from '../../../api/navigation'; +import * as withRouter from '../../../components/hoc/withRouter'; import { mockProjectAlmBindingConfigurationErrors } from '../../../helpers/mocks/alm-settings'; import { mockBranch, mockPullRequest } from '../../../helpers/mocks/branch-like'; import { mockComponent } from '../../../helpers/mocks/component'; @@ -32,7 +33,8 @@ import { mockTask } from '../../../helpers/mocks/tasks'; import { HttpStatus } from '../../../helpers/request'; import { renderAppRoutes, renderComponent } from '../../../helpers/testReactTestingUtils'; import { byRole, byText } from '../../../helpers/testSelector'; -import { ComponentQualifier } from '../../../types/component'; +import { getProjectUrl, getPullRequestUrl } from '../../../helpers/urls'; +import { ComponentQualifier, Visibility } from '../../../types/component'; import { TaskStatuses, TaskTypes } from '../../../types/tasks'; import handleRequiredAuthorization from '../../utils/handleRequiredAuthorization'; import ComponentContainer, { isSameBranch } from '../ComponentContainer'; @@ -70,6 +72,11 @@ jest.mock('../../utils/handleRequiredAuthorization', () => ({ default: jest.fn(), })); +jest.mock('../../../components/hoc/withRouter', () => ({ + __esModule: true, + ...jest.requireActual('../../../components/hoc/withRouter'), +})); + const ui = { projectTitle: byRole('link', { name: 'Project' }), projectText: byText('project'), @@ -427,6 +434,141 @@ it('isSameBranch util returns expected result', () => { expect(isSameBranch(mockTask({ pullRequest: 'pr' }), undefined, 'pr')).toBe(true); }); +describe('tutorials', () => { + beforeEach(() => { + jest.useFakeTimers({ advanceTimers: true }); + }); + afterEach(() => { + jest.runOnlyPendingTimers(); + jest.useRealTimers(); + }); + + it('should redirect to project main branch dashboard from tutorials when receiving new related scan report', async () => { + const componentKey = 'foo-component'; + jest.mocked(getComponentData).mockResolvedValue({ + ancestors: [], + component: { + key: componentKey, + name: 'component name', + qualifier: ComponentQualifier.Project, + visibility: Visibility.Public, + }, + }); + jest + .mocked(getTasksForComponent) + .mockResolvedValueOnce({ queue: [] }) + .mockResolvedValue({ + queue: [{ status: TaskStatuses.InProgress, type: TaskTypes.Report }], + } as unknown as Awaited>); + + const mockedReplace = jest.fn(); + jest.spyOn(withRouter, 'useRouter').mockReturnValue({ + replace: mockedReplace, + push: jest.fn(), + }); + + renderComponentContainer( + { hasFeature: jest.fn().mockReturnValue(true) }, + `tutorials?id=${componentKey}`, + '/', + ); + + await waitFor(() => expect(getTasksForComponent).toHaveBeenCalledTimes(1)); + + act(() => jest.runOnlyPendingTimers()); + + expect(mockedReplace).not.toHaveBeenCalled(); + await waitFor(() => expect(getTasksForComponent).toHaveBeenCalledTimes(2)); + await waitFor(() => expect(mockedReplace).toHaveBeenCalledWith(getProjectUrl(componentKey))); + }); + + it('should redirect to project branch dashboard from tutorials when receiving new related scan report', async () => { + const componentKey = 'foo-component'; + const branchName = 'fooBranch'; + jest.mocked(getComponentData).mockResolvedValue({ + ancestors: [], + component: { + key: componentKey, + name: 'component name', + qualifier: ComponentQualifier.Project, + visibility: Visibility.Public, + }, + }); + jest + .mocked(getTasksForComponent) + .mockResolvedValueOnce({ queue: [] }) + .mockResolvedValue({ + queue: [{ branch: branchName, status: TaskStatuses.InProgress, type: TaskTypes.Report }], + } as unknown as Awaited>); + + const mockedReplace = jest.fn(); + jest.spyOn(withRouter, 'useRouter').mockReturnValue({ + replace: mockedReplace, + push: jest.fn(), + }); + + renderComponentContainer( + { hasFeature: jest.fn().mockReturnValue(true) }, + `tutorials?id=${componentKey}`, + '/', + ); + + await waitFor(() => expect(getTasksForComponent).toHaveBeenCalledTimes(1)); + + act(() => jest.runOnlyPendingTimers()); + + expect(mockedReplace).not.toHaveBeenCalled(); + await waitFor(() => expect(getTasksForComponent).toHaveBeenCalledTimes(2)); + await waitFor(() => + expect(mockedReplace).toHaveBeenCalledWith(getProjectUrl(componentKey, branchName)), + ); + }); + + it('should redirect to project pull request dashboard from tutorials when receiving new related scan report', async () => { + const componentKey = 'foo-component'; + const pullRequestKey = 'fooPR'; + jest.mocked(getComponentData).mockResolvedValue({ + ancestors: [], + component: { + key: componentKey, + name: 'component name', + qualifier: ComponentQualifier.Project, + visibility: Visibility.Public, + }, + }); + jest + .mocked(getTasksForComponent) + .mockResolvedValueOnce({ queue: [] }) + .mockResolvedValue({ + queue: [ + { pullRequest: pullRequestKey, status: TaskStatuses.InProgress, type: TaskTypes.Report }, + ], + } as unknown as Awaited>); + + const mockedReplace = jest.fn(); + jest.spyOn(withRouter, 'useRouter').mockReturnValue({ + replace: mockedReplace, + push: jest.fn(), + }); + + renderComponentContainer( + { hasFeature: jest.fn().mockReturnValue(true) }, + `tutorials?id=${componentKey}`, + '/', + ); + + await waitFor(() => expect(getTasksForComponent).toHaveBeenCalledTimes(1)); + + act(() => jest.runOnlyPendingTimers()); + + expect(mockedReplace).not.toHaveBeenCalled(); + await waitFor(() => expect(getTasksForComponent).toHaveBeenCalledTimes(2)); + await waitFor(() => + expect(mockedReplace).toHaveBeenCalledWith(getPullRequestUrl(componentKey, pullRequestKey)), + ); + }); +}); + function renderComponentContainerAsComponent(props: Partial = {}) { return renderComponent( <> diff --git a/server/sonar-web/src/main/js/components/tutorials/TutorialSelectionRenderer.tsx b/server/sonar-web/src/main/js/components/tutorials/TutorialSelectionRenderer.tsx index ef2ce0803e6..07b28dccafd 100644 --- a/server/sonar-web/src/main/js/components/tutorials/TutorialSelectionRenderer.tsx +++ b/server/sonar-web/src/main/js/components/tutorials/TutorialSelectionRenderer.tsx @@ -28,14 +28,13 @@ import { Title, } from 'design-system'; import * as React from 'react'; -import { useNavigate } from 'react-router-dom'; -import { getBranchLikeQuery, isMainBranch } from '../../helpers/branch-like'; +import { isMainBranch } from '../../helpers/branch-like'; import { translate } from '../../helpers/l10n'; import { getBaseUrl } from '../../helpers/system'; import { getProjectTutorialLocation } from '../../helpers/urls'; import { useBranchesQuery } from '../../queries/branch'; import { AlmKeys, AlmSettingsInstance, ProjectAlmBindingResponse } from '../../types/alm-settings'; -import { BranchLike, MainBranch } from '../../types/branch-like'; +import { MainBranch } from '../../types/branch-like'; import { Component } from '../../types/types'; import { LoggedInUser } from '../../types/users'; import { Alert } from '../ui/Alert'; @@ -82,8 +81,6 @@ function renderAlm(mode: TutorialModes, project: string, icon?: React.ReactNode) ); } -const CHECKING_NEW_BRANCH = 5_000; - export default function TutorialSelectionRenderer(props: TutorialSelectionRendererProps) { const { almBinding, @@ -97,24 +94,7 @@ export default function TutorialSelectionRenderer(props: TutorialSelectionRender willRefreshAutomatically, } = props; - const { data: { branchLikes } = { branchLikes: [] as BranchLike[] } } = useBranchesQuery( - component, - CHECKING_NEW_BRANCH, - ); - - const navigate = useNavigate(); - - const firstAnalysedBranch = branchLikes.find((b) => b.analysisDate !== undefined); - - if (firstAnalysedBranch) { - navigate({ - pathname: '/dashboard', - search: new URLSearchParams({ - id: component.key, - ...getBranchLikeQuery(firstAnalysedBranch), - }).toString(), - }); - } + const { data: { branchLikes } = { branchLikes: [] } } = useBranchesQuery(component); const mainBranchName = (branchLikes.find((b) => isMainBranch(b)) as MainBranch | undefined)?.name || diff --git a/server/sonar-web/src/main/js/components/tutorials/__tests__/TutorialSelection-it.tsx b/server/sonar-web/src/main/js/components/tutorials/__tests__/TutorialSelection-it.tsx index 5124f8bfb1f..4814621ae0d 100644 --- a/server/sonar-web/src/main/js/components/tutorials/__tests__/TutorialSelection-it.tsx +++ b/server/sonar-web/src/main/js/components/tutorials/__tests__/TutorialSelection-it.tsx @@ -23,7 +23,6 @@ import { UserEvent } from '@testing-library/user-event/dist/types/setup/setup'; import * as React from 'react'; import { getScannableProjects } from '../../../api/components'; import AlmSettingsServiceMock from '../../../api/mocks/AlmSettingsServiceMock'; -import BranchesServiceMock from '../../../api/mocks/BranchesServiceMock'; import SettingsServiceMock from '../../../api/mocks/SettingsServiceMock'; import UserTokensMock from '../../../api/mocks/UserTokensMock'; import { mockComponent } from '../../../helpers/mocks/component'; @@ -38,6 +37,8 @@ import { SettingsKey } from '../../../types/settings'; import TutorialSelection from '../TutorialSelection'; import { TutorialModes } from '../types'; +jest.mock('../../../api/branches'); + jest.mock('../../../helpers/urls', () => ({ ...jest.requireActual('../../../helpers/urls'), getHostUrl: jest.fn().mockReturnValue('http://host.url'), @@ -50,24 +51,20 @@ jest.mock('../../../api/components', () => ({ let settingsMock: SettingsServiceMock; let tokenMock: UserTokensMock; let almMock: AlmSettingsServiceMock; -let branchesMock: BranchesServiceMock; beforeAll(() => { settingsMock = new SettingsServiceMock(); tokenMock = new UserTokensMock(); almMock = new AlmSettingsServiceMock(); - branchesMock = new BranchesServiceMock(); }); afterEach(() => { tokenMock.reset(); settingsMock.reset(); almMock.reset(); - branchesMock.reset(); }); beforeEach(() => { - branchesMock.emptyBranchesAndPullRequest(); jest.clearAllMocks(); }); -- 2.39.5