Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { Modal } from 'uiSrc/components/base/display/modal'
import { UploadWarningBanner } from 'uiSrc/components/upload-warning/styles'

export interface Props {
loading: boolean
loading?: boolean
disabled: boolean
onReset: () => void
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, { useCallback, useEffect } from 'react'
import { useDispatch, useSelector } from 'react-redux'
import { useParams } from 'react-router-dom'

import styled from 'styled-components'
import {
getPipelineStatusAction,
rdiPipelineActionSelector,
Expand All @@ -23,13 +24,12 @@ import {
} from 'uiSrc/slices/interfaces'

import { FlexItem, Row } from 'uiSrc/components/base/layout/flex'
import { Theme } from 'uiSrc/components/base/theme/types'
import DeployPipelineButton from '../buttons/deploy-pipeline-button'
import ResetPipelineButton from '../buttons/reset-pipeline-button'
import RdiConfigFileActionMenu from '../rdi-config-file-action-menu'
import StopPipelineButton from '../buttons/stop-pipeline-button'
import StartPipelineButton from '../buttons/start-pipeline-button/StartPipelineButton'
import styled from 'styled-components'
import { Theme } from 'uiSrc/components/base/theme/types'

const VerticalDelimiter = styled(FlexItem)`
border: ${({ theme }: { theme: Theme }) => theme.components.appBar.separator};
Expand Down Expand Up @@ -183,11 +183,7 @@ const PipelineActions = ({ collectorStatus, pipelineStatus }: Props) => {
) : null}
</FlexItem>
<FlexItem>
<DeployPipelineButton
loading={deployLoading}
disabled={disabled}
onReset={resetPipeline}
/>
<DeployPipelineButton disabled={disabled} onReset={resetPipeline} />
</FlexItem>
<FlexItem>
<RdiConfigFileActionMenu />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import { useHistory, useLocation, useParams } from 'react-router-dom'
import { Nullable } from 'uiSrc/utils'
import { PageNames, Pages } from 'uiSrc/constants'
import { Title } from 'uiSrc/components/base/text'
import { Loader } from 'uiSrc/components/base/display'
import { Col } from 'uiSrc/components/base/layout/flex'
import { LoadingContent } from 'uiSrc/components/base'
import { RdiPipelineTabs } from 'uiSrc/slices/interfaces/rdi'
import { rdiPipelineSelector } from 'uiSrc/slices/rdi/pipeline'

Expand Down Expand Up @@ -55,7 +55,7 @@ const Navigation = () => {
Pipeline management
</Title>

{loading && <Loader size="xl" />}
{loading && <LoadingContent lines={4} />}

{!loading && (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ describe('Config', () => {
expect(screen.getByTestId('rdi-config-loading')).toBeInTheDocument()
})

it('should render loader on btn', () => {
it('should not render loader on btn for pipeline loading', () => {
const rdiPipelineSelectorMock = jest.fn().mockReturnValue({
loading: true,
})
Expand All @@ -251,10 +251,11 @@ describe('Config', () => {

// check is btn has loader
const child = getByTestId('rdi-test-connection-btn')
expect(child.querySelector('svg')).toBeTruthy()
const icon = child.querySelector('svg')
expect(icon).not.toBeInTheDocument()
})

it('should render loader on btn', () => {
it('should render loader on btn when testing connection', () => {
const rdiTestConnectionsSelectorMock = jest.fn().mockReturnValue({
loading: true,
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,8 @@ const Config = () => {
<Row grow={false} justify="end">
<PrimaryButton
onClick={testConnections}
loading={testingConnections || pipelineLoading}
loading={testingConnections}
disabled={pipelineLoading}
aria-labelledby="test target connections"
data-testid="rdi-test-connection-btn"
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ jest.mock('uiSrc/slices/rdi/statistics', () => ({
loading: false,
results: {
status: 'success',
data: CONNECTIONS_DATA
data: CONNECTIONS_DATA,
},
}),
}))
Expand Down Expand Up @@ -151,7 +151,7 @@ describe('StatisticsPage', () => {
loading: false,
results: {
status: null,
data: CONNECTIONS_DATA
data: CONNECTIONS_DATA,
},
})
render(<StatisticsPage />)
Expand All @@ -170,12 +170,14 @@ describe('StatisticsPage', () => {
expect(screen.getByTestId('empty-pipeline')).toBeInTheDocument()
})

it('renders statistics sections when status is success and data exists', () => {
it('renders statistics sections when status is success and data exists', async () => {
render(<StatisticsPage />)

// Check that statistics sections are rendered instead of empty state
expect(screen.queryByTestId('empty-pipeline')).not.toBeInTheDocument()
expect(screen.getByTestId('processing-performance-info-refresh-btn')).toBeInTheDocument()
expect(
await screen.findByTestId('processing-performance-info-refresh-btn'),
).toBeInTheDocument()
})

it('should call proper telemetry on page view', () => {
Expand All @@ -189,11 +191,11 @@ describe('StatisticsPage', () => {
})
})

it('should call proper telemetry event when refresh is clicked for processing performance section', () => {
it('should call proper telemetry event when refresh is clicked for processing performance section', async () => {
render(<StatisticsPage />)

fireEvent.click(
screen.getByTestId('processing-performance-info-refresh-btn'),
await screen.findByTestId('processing-performance-info-refresh-btn'),
)

expect(sendEventTelemetry).toBeCalledWith({
Expand Down Expand Up @@ -238,7 +240,9 @@ describe('StatisticsPage', () => {

const testid = 'processing-performance-info'

await userEvent.click(screen.getByTestId(`${testid}-auto-refresh-config-btn`))
await userEvent.click(
await screen.findByTestId(`${testid}-auto-refresh-config-btn`),
)
await waitForRiPopoverVisible()
await userEvent.click(screen.getByTestId(`${testid}-auto-refresh-switch`)) // disabled

Expand All @@ -258,7 +262,9 @@ describe('StatisticsPage', () => {

const testid = 'processing-performance-info'

await userEvent.click(screen.getByTestId(`${testid}-auto-refresh-config-btn`))
await userEvent.click(
await screen.findByTestId(`${testid}-auto-refresh-config-btn`),
)
await waitForRiPopoverVisible()
await userEvent.click(screen.getByTestId(`${testid}-auto-refresh-switch`)) // disabled
await userEvent.click(screen.getByTestId(`${testid}-auto-refresh-switch`)) // enabled
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import styled from 'styled-components'
import { Col } from 'uiSrc/components/base/layout/flex'

export const Container = styled(Col)`
overflow: auto;
position: relative;
`
export const ContentWrapper = styled(Col)`
padding: ${({ theme }) => theme.core.space.space200};
`

export const LoadingState = styled(Col)`
z-index: 2;
`
100 changes: 50 additions & 50 deletions redisinsight/ui/src/pages/rdi/statistics/StatisticsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,17 @@ import { formatLongName, Nullable, setTitle } from 'uiSrc/utils'
import { setLastPageContext } from 'uiSrc/slices/app/context'
import { PageNames } from 'uiSrc/constants'
import { Loader } from 'uiSrc/components/base/display'
import { IRdiStatistics, RdiPipelineStatus } from 'uiSrc/slices/interfaces'
import { type IRdiStatistics, RdiPipelineStatus } from 'uiSrc/slices/interfaces'

import { FlexItem, Row } from 'uiSrc/components/base/layout/flex'
import { AutoRefresh } from 'uiSrc/components'
import Clients from './clients'
import DataStreams from './data-streams'
import Empty from './empty'
import ProcessingPerformance from './processing-performance'
import Status from './status'
import TargetConnections from './target-connections'

import styles from './styles.module.scss'
import { Col, FlexItem, Row } from 'uiSrc/components/base/layout/flex'
import { AutoRefresh } from 'uiSrc/components'
import * as S from './StatisticsPage.styles'

const shouldShowStatistics = (data: Nullable<IRdiStatistics>) =>
data?.status === RdiPipelineStatus.Success && !!data?.data
Expand Down Expand Up @@ -123,60 +122,61 @@ const StatisticsPage = () => {

const { data: statisticsData } = statisticsResults


return (
<div className={styles.pageContainer}>
<Col gap="xxl" style={{ padding: 16 }}>
<S.Container>
<S.ContentWrapper gap="xxl">
{pageLoading && (
<div className={styles.cover}>
<S.LoadingState centered>
<Loader size="xl" />
</div>
</S.LoadingState>
)}
{!shouldShowStatistics(statisticsResults) ? (
// TODO add loader
<Empty rdiInstanceId={rdiInstanceId} />
) : (
<>
<Row justify="end">
<FlexItem>
<AutoRefresh
postfix="processing-performance-info"
displayText
loading={isStatisticsLoading}
lastRefreshTime={lastRefreshTime}
enableAutoRefreshDefault
testid="processing-performance-info"
onRefresh={() => {
setLastRefreshTime(Date.now())
onRefresh('processing_performance')
}}
onRefreshClicked={() =>
onRefreshClicked('processing_performance')
}
onEnableAutoRefresh={(
enableAutoRefresh: boolean,
refreshRate: string,
) =>
onChangeAutoRefresh(
'processing_performance',
enableAutoRefresh,
refreshRate,
)
}
/>
</FlexItem>
</Row>
<Status data={statisticsData.rdiPipelineStatus} />
<ProcessingPerformance
data={statisticsData.processingPerformance}
/>
<TargetConnections data={statisticsData.connections} />
<DataStreams data={statisticsData.dataStreams} />
<Clients data={statisticsData.clients} />
</>
!pageLoading && (
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Empty component renders alongside loader during initial load

The Empty component renders during initial page load alongside the loader because it's not gated by pageLoading. When statisticsResults is null (initial state), shouldShowStatistics returns false, so Empty renders even while pageLoading is true. The statistics content was correctly gated with !pageLoading &&, but the Empty branch wasn't given the same treatment. This creates inconsistent behavior where users see both the loader and empty state simultaneously during initial load.

Additional Locations (1)

Fix in Cursor Fix in Web

<>
<Row justify="end">
<FlexItem>
<AutoRefresh
postfix="processing-performance-info"
displayText
loading={isStatisticsLoading}
lastRefreshTime={lastRefreshTime}
enableAutoRefreshDefault
testid="processing-performance-info"
onRefresh={() => {
setLastRefreshTime(Date.now())
onRefresh('processing_performance')
}}
onRefreshClicked={() =>
onRefreshClicked('processing_performance')
}
onEnableAutoRefresh={(
enableAutoRefresh: boolean,
refreshRate: string,
) =>
onChangeAutoRefresh(
'processing_performance',
enableAutoRefresh,
refreshRate,
)
}
/>
</FlexItem>
</Row>
<Status data={statisticsData.rdiPipelineStatus} />
<ProcessingPerformance
data={statisticsData.processingPerformance}
/>
<TargetConnections data={statisticsData.connections} />
<DataStreams data={statisticsData.dataStreams} />
<Clients data={statisticsData.clients} />
</>
)
)}
</Col>
</div>
</S.ContentWrapper>
</S.Container>
)
}

Expand Down
20 changes: 0 additions & 20 deletions redisinsight/ui/src/pages/rdi/statistics/styles.module.scss

This file was deleted.

Loading