Add support of non existent in a timeframe data-points (#21288)

* Add support of non-existent in time-frame data-points

* Add story for line chart with missing data
This commit is contained in:
Vova Kulikov 2021-06-01 19:40:29 +03:00 committed by GitHub
parent 7555eda981
commit f8bed47e3e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 266 additions and 34 deletions

View File

@ -1,5 +1,5 @@
import { Settings, SettingsCascade } from '@sourcegraph/shared/out/src/settings/settings'
import { IOrg } from '@sourcegraph/shared/src/graphql/schema'
import { Settings, SettingsCascade } from '@sourcegraph/shared/src/settings/settings'
import { authUser } from '@sourcegraph/web/src/search/panels/utils'
import { AuthenticatedUser } from '../../auth'

View File

@ -4,27 +4,55 @@ import { defer } from 'rxjs'
import { retry } from 'rxjs/operators'
import type { LineChartContent } from 'sourcegraph'
import { EMPTY_DATA_POINT_VALUE } from '../../../../views/ChartViewContent/charts/line/constants'
import { fetchRawSearchInsightResults, fetchSearchInsightCommits } from '../requests/fetch-search-insight'
import { SearchInsightSettings } from '../types'
interface RepoCommit {
date: Date
commit: string
repo: string
}
interface InsightSeriesData {
date: number
[seriesName: string]: number
}
/**
* This logic is a copy of fetch logic of search-based code insight extension.
* See https://github.com/sourcegraph/sourcegraph-search-insights/blob/master/src/search-insights.ts
* In order to have live preview for creation UI we had to copy this logic from
* extension.
*
* */
export async function getSearchInsightContent(insight: SearchInsightSettings): Promise<LineChartContent<any, string>> {
const step = insight.step || { days: 1 }
const { repositories: repos } = insight
const dates = getDaysToQuery(step)
// Get commits to search for each day
const data: InsightSeriesData[] = []
for (const date of dates) {
const dataIndex = dates.indexOf(date)
// Initialize data series object by all dates.
data[dataIndex] = {
date: date.getTime(),
// Initialize all series with EMPTY_DATA_POINT_VALUE
...Object.fromEntries(insight.series.map(series => [series.name, EMPTY_DATA_POINT_VALUE])),
}
}
// Get commits to search for each day.
const repoCommits = (
await Promise.all(
repos.map(async repo => (await determineCommitsToSearch(dates, repo)).map(commit => ({ repo, ...commit })))
)
).flat()
)
.flat()
// For commit which we couldn't find we should not run search API request.
// Instead of it we will use just EMPTY_DATA_POINT_VALUE
.filter(commitData => commitData.commit !== null) as RepoCommit[]
const searchQueries = insight.series.flatMap(({ query, name }) =>
repoCommits.map(({ date, repo, commit }) => ({
@ -35,35 +63,36 @@ export async function getSearchInsightContent(insight: SearchInsightSettings): P
query: `repo:^${escapeRegExp(repo)}$@${commit} ${query} count:99999`,
}))
)
const rawSearchResults = await defer(() => fetchRawSearchInsightResults(searchQueries.map(search => search.query)))
// The bulk search may timeout, but a retry is then likely faster because caches are warm
// The bulk search may timeout, but a retry is then likely faster
// because caches are warm
.pipe(retry(3))
.toPromise()
const searchResults = Object.entries(rawSearchResults).map(([field, result]) => {
const index = +field.slice('search'.length)
const query = searchQueries[index]
return { ...query, result }
})
const data: {
date: number
[seriesName: string]: number
}[] = []
// Merge initial data and search API data
for (const { name, date, result } of searchResults) {
const dataKey = name
const dataIndex = dates.indexOf(date)
const object =
data[dataIndex] ??
(data[dataIndex] = {
date: date.getTime(),
// Initialize all series to 0
...Object.fromEntries(insight.series.map(series => [series.name, 0])),
})
// Sum across repos
const object = data[dataIndex]
const countForRepo = result?.results.matchCount
object[dataKey] += countForRepo ?? 0
// If we got some data that means for this data points we got
// a valid commit in a git history therefore we need to write
// some data to this series.
if (object[dataKey] === EMPTY_DATA_POINT_VALUE) {
object[dataKey] = countForRepo ?? 0
} else {
object[dataKey] += countForRepo ?? 0
}
}
return {
@ -95,7 +124,12 @@ export async function getSearchInsightContent(insight: SearchInsightSettings): P
}
}
async function determineCommitsToSearch(dates: Date[], repo: string): Promise<{ date: Date; commit: string }[]> {
interface SearchCommit {
date: Date
commit: string | null
}
async function determineCommitsToSearch(dates: Date[], repo: string): Promise<SearchCommit[]> {
const commitQueries = dates.map(date => {
const before = formatISO(date)
return `repo:^${escapeRegExp(repo)}$ type:commit before:${before} count:1`
@ -103,25 +137,29 @@ async function determineCommitsToSearch(dates: Date[], repo: string): Promise<{
const commitResults = await fetchSearchInsightCommits(commitQueries).toPromise()
const commitOids = Object.entries(commitResults).map(([name, search], index) => {
return Object.entries(commitResults).map(([name, search], index) => {
const index_ = +name.slice('search'.length)
const date = dates[index_]
if (index_ !== index) {
throw new Error(`Expected field ${name} to be at index ${index_} of object keys`)
}
if (search.results.results.length === 0) {
throw new Error(`No result for ${commitQueries[index_]}`)
console.warn(`No result for ${commitQueries[index_]}`)
return { commit: null, date }
}
const commit = (search?.results.results[0]).commit
// Sanity check
const commitDate = commit.committer && new Date(commit.committer.date)
const date = dates[index_]
if (!commitDate) {
throw new Error(`Expected commit to have committer: \`${commit.oid}\``)
}
if (isAfter(commitDate, date)) {
throw new Error(
`Expected commit \`${commit.oid}\` to be before ${formatISO(date)}, but was after: ${formatISO(
@ -132,8 +170,6 @@ async function determineCommitsToSearch(dates: Date[], repo: string): Promise<{
return { commit: commit.oid, date }
})
return commitOids
}
const NUMBER_OF_CHART_POINTS = 7

View File

@ -69,6 +69,53 @@ add('Line chart', () => (
/>
))
add('Line chart with missing data', () => (
<ChartViewContent
{...commonProps}
content={{
chart: 'line',
data: [
{ x: 1588965700286 - 4 * 24 * 60 * 60 * 1000, a: null, b: null },
{ x: 1588965700286 - 3 * 24 * 60 * 60 * 1000, a: null, b: null },
{ x: 1588965700286 - 2 * 24 * 60 * 60 * 1000, a: 94, b: 200 },
{ x: 1588965700286 - 1 * 24 * 60 * 60 * 1000, a: 134, b: 190 },
{ x: 1588965700286, a: 123, b: 170 },
],
series: [
{
dataKey: 'a',
name: 'A metric',
stroke: 'var(--warning)',
linkURLs: [
'#A:1st_data_point',
'#A:2nd_data_point',
'#A:3rd_data_point',
'#A:4th_data_point',
'#A:5th_data_point',
],
},
{
dataKey: 'b',
name: 'B metric',
stroke: 'var(--warning)',
linkURLs: [
'#B:1st_data_point',
'#B:2nd_data_point',
'#B:3rd_data_point',
'#B:4th_data_point',
'#B:5th_data_point',
],
},
],
xAxis: {
dataKey: 'x',
scale: 'time',
type: 'number',
},
}}
/>
))
add('Bar chart', () => (
<ChartViewContent
{...commonProps}

View File

@ -1,4 +0,0 @@
/**
* Default value for line color in case if we didn't get color for line from content config.
*/
export const DEFAULT_LINE_STROKE = 'var(--color-bg-5)'

View File

@ -12,13 +12,14 @@ import React, { ReactElement, useCallback, useMemo, useState, MouseEvent, useRef
import { noop } from 'rxjs'
import { LineChartContent as LineChartContentType } from 'sourcegraph'
import { DEFAULT_LINE_STROKE } from '../colors'
import { DEFAULT_LINE_STROKE } from '../constants'
import { generateAccessors } from '../helpers/generate-accessors'
import { usePointerEventEmitters } from '../helpers/use-event-emitters'
import { useScales } from '../helpers/use-scales'
import { onDatumZoneClick } from '../types'
import { ActiveDatum, GlyphContent } from './GlyphContent'
import { NonActiveBackground } from './NonActiveBackground'
import { dateTickFormatter, numberFormatter, Tick, getTickXProps, getTickYProps } from './TickComponent'
import { TooltipContent } from './TooltipContent'
@ -35,7 +36,7 @@ const SCALES_CONFIG = {
type: 'linear' as const,
nice: true,
zero: false,
clamp: true,
clamp: false,
},
}
@ -233,6 +234,7 @@ export function LineChartContent<Datum extends object>(props: LineChartContentPr
onPointerUp={handlePointerUp}
accessibilityLabel="Line chart content"
>
<NonActiveBackground data={sortedData} accessors={accessors} series={series} />
<Group aria-label="Chart axes">
{/* eslint-disable-next-line jsx-a11y/aria-role */}
<Group role="graphics-axis" aria-orientation="horizontal" aria-label="Y axis: number">

View File

@ -0,0 +1,107 @@
import { PatternLines } from '@visx/pattern'
import { DataContext } from '@visx/xychart'
import React, { ReactElement, useContext, useMemo } from 'react'
import { LineChartContent } from 'sourcegraph'
import { Accessors, YAccessor } from '../types'
const patternId = 'xy-chart-pattern'
function getFirstNonNullablePoint<Datum>(data: Datum[], accessor: YAccessor<Datum>): Datum | undefined {
return data.find(datum => accessor(datum) !== null)
}
export interface NonActiveBackgroundProps<Datum extends object, Key extends keyof Datum> {
data: Datum[]
series: LineChartContent<Datum, keyof Datum>['series']
accessors: Accessors<Datum, Key>
}
/**
* Displays custom pattern background for area where we don't have any data points.
* Example:
*
* `````````````````` 10
* ``````````````````
* `````````````````` 9
* ``````````````````
* `````````````````` 8
* ``````````````````
* `````````````````` 7
* ``````````````````
* `````````````````` 6
* ``````````````````
* `````````````````` 5
*
* Where ` is a non-active background
* */
export function NonActiveBackground<Datum extends object>(
props: NonActiveBackgroundProps<Datum, keyof Datum>
): ReactElement | null {
const { data, series, accessors } = props
const { theme, margin, width, height, innerHeight, xScale } = useContext(DataContext)
const backgroundWidth = useMemo(() => {
// For non active background's width we need to find first non nullable element
const firstNonNullablePoints: (Datum | undefined)[] = []
for (const line of series) {
const lineKey = line.dataKey
const lineYAccessor = accessors.y[lineKey]
firstNonNullablePoints.push(getFirstNonNullablePoint(data, lineYAccessor))
}
const lastNullablePointX = firstNonNullablePoints.reduce((xCoord, datum) => {
// In case if the first non nullable element is the first element
// of data that means we don't need to render non active background.
if (!datum || datum === data[0]) {
return null
}
// Get x point from datum by x accessor
return accessors.x(datum)
}, null as Date | number | null)
// If we didn't find any non-nullable elements we don't need to render
// non-active background.
if (!lastNullablePointX) {
return 0
}
// Convert x value of first non nullable point to reals svg coordinate
const xValue = xScale?.(lastNullablePointX) ?? 0
return +xValue
}, [data, series, accessors, xScale])
// Early return values not available in context or we don't need render
// non active background.
if (!backgroundWidth || !width || !height || !margin || !theme) {
return null
}
return (
<>
<PatternLines
id={patternId}
width={16}
height={16}
orientation={['diagonal']}
stroke={theme?.gridStyles?.stroke}
strokeWidth={1}
/>
<rect x={0} y={0} width={width} height={height} fill="transparent" />
<rect
x={margin.left}
y={margin.top}
width={backgroundWidth - margin.left}
height={innerHeight}
fill={`url(#${patternId})`}
fillOpacity={0.3}
/>
</>
)
}

View File

@ -3,7 +3,7 @@ import classNames from 'classnames'
import React, { ReactElement } from 'react'
import { LineChartContent } from 'sourcegraph'
import { DEFAULT_LINE_STROKE } from '../colors'
import { DEFAULT_LINE_STROKE } from '../constants'
import { Accessors } from '../types'
export interface TooltipContentProps<Datum extends object> extends RenderTooltipParams<Datum> {

View File

@ -0,0 +1,28 @@
/**
* Default value for line color in case if we didn't get color for line from content config.
*/
export const DEFAULT_LINE_STROKE = 'var(--color-bg-5)'
/**
* Visx xy-chart supports data series with missing. To show the
* points but not the very beginning of the chart we should use
* this default empty value. See example below points that have
* EMPTY_DATA_POINT_VALUE value haven't been rendered instead of
* that we rendered non active background (``` area)
*
*
* `````````````````` 10 ```````` 10
* `````````````````` ````````
* `````````````````` 9 ```````` 9
* `````````````````` ````````
* `````````````````` 8 ```````` 8
* `````````````````` ````````
* `````````````````` 7 ```````` 7
* `````````````````` ````````
* `````````````````` 6 ```````` 6
* `````````````````` ````````
* `````````````````` 5 ```````` 5
*
*
*/
export const EMPTY_DATA_POINT_VALUE = null

View File

@ -1,5 +1,6 @@
import { ChartAxis } from 'sourcegraph'
import { EMPTY_DATA_POINT_VALUE } from '../constants'
import { Accessors } from '../types'
/**
@ -26,7 +27,9 @@ export function generateAccessors<Datum extends object>(
// fix that when we will have a value type for LineChartContent<D> generic
const key = (dataKey as unknown) as keyof Datum
accessors[key] = data => +data[dataKey]
// If we get EMPTY_DATA_POINT_VALUE we should omit the '+' number casting
accessors[key] = data =>
data[dataKey] === EMPTY_DATA_POINT_VALUE ? EMPTY_DATA_POINT_VALUE : +data[dataKey]
return accessors
}, {} as Record<keyof Datum, (data: Datum) => any>),

View File

@ -43,7 +43,7 @@ interface UseScalesOutput {
}
/**
* Hook to generate d3 scales according chart configuration.
* Hook to generate d3 scales according to chart configuration.
*/
export function useScales<Datum>(props: UseScalesProps<Datum>): UseScalesOutput {
const { config, accessors, width, height, data } = props

View File

@ -1,9 +1,11 @@
import { MouseEvent } from 'react'
export type YAccessor<Datum> = (data: Datum) => any
/** Accessors map for getting values for x and y axes from datum object */
export interface Accessors<Datum, Key extends keyof Datum> {
x: (d: Datum) => Date | number
y: Record<Key, (data: Datum) => any>
y: Record<Key, YAccessor<Datum>>
}
export interface DatumZoneClickEvent {

View File

@ -315,6 +315,7 @@
"@visx/grid": "^1.7.0",
"@visx/group": "^1.7.0",
"@visx/mock-data": "^1.7.0",
"@visx/pattern": "^1.7.0",
"@visx/scale": "^1.7.0",
"@visx/xychart": "^1.7.3",
"bloomfilter": "^0.0.18",

View File

@ -5050,6 +5050,16 @@
"@types/d3-random" "^2.2.0"
d3-random "^2.2.2"
"@visx/pattern@^1.7.0":
version "1.7.0"
resolved "https://registry.npmjs.org/@visx/pattern/-/pattern-1.7.0.tgz#4b4392b976f57592d836b2055dc606aa7b0192d9"
integrity sha512-uIrGDtm7NqDfZZneRZ/DRdZA0nli/13sIUcxQ0oRL70b6ul+epNGiBVR8pvrgMhZ+SW3b05xhfEDnfnl2CqFQw==
dependencies:
"@types/classnames" "^2.2.9"
"@types/react" "*"
classnames "^2.2.5"
prop-types "^15.5.10"
"@visx/point@1.7.0":
version "1.7.0"
resolved "https://registry.npmjs.org/@visx/point/-/point-1.7.0.tgz#1df3c3425eae464f498473bcdda2fcae05c8ecbe"