Skip to content

Commit

Permalink
Fix Counter, Leaderboard and TimeSeries bugs (#73)
Browse files Browse the repository at this point in the history
* fix: fixes to TimeSeries component

* fix: Leaderboard fixes

* fix: counter fixes

* fix: static mode bugs

* refactor: remove unnecessary import

* refactor: use ?? + remove unnecessary typeof

* refactor: comment all console.error
  • Loading branch information
felipecadavid committed Jun 15, 2023
1 parent 1641430 commit 2751f40
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 50 deletions.
2 changes: 1 addition & 1 deletion packages/core/components/src/ErrorBoundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export class ErrorBoundary extends Component<Props, State> {
}

componentDidCatch(error: Error, errorInfo: React.ErrorInfo) {
console.error(error, errorInfo)
// console.error(error, errorInfo) we will set logs as a feature later
}

render() {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/release/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ async function main(): Promise<void> {
}
}
} catch (error) {
console.error(error)
// console.error(error) we will set logs as a feature later
process.exit(1)
}
}
Expand Down
49 changes: 34 additions & 15 deletions packages/react/counter/src/Counter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ export function Counter(props: CounterProps) {
const [hasError, setHasError] = React.useState(false)
const [isLoading, setIsLoading] = React.useState(false)

const filtersString = JSON.stringify(query?.filters || [])

/**
* Fetches the counter data
* when the user doesn't provide
Expand All @@ -64,14 +66,21 @@ export function Counter(props: CounterProps) {
try {
setIsLoading(true)

const filters = JSON.parse(filtersString)

const response = await request(
PROPEL_GRAPHQL_API_ENDPOINT,
CounterDocument,
{
uniqueName: query?.metric,
counterInput: {
timeRange: query?.timeRange,
filters: query?.filters,
timeRange: {
relative: query?.timeRange?.relative ?? null,
n: query?.timeRange?.n ?? null,
start: query?.timeRange?.start ?? null,
stop: query?.timeRange?.stop ?? null
},
filters,
propeller: query?.propeller
}
},
Expand All @@ -90,20 +99,29 @@ export function Counter(props: CounterProps) {
} finally {
setIsLoading(false)
}
}, [query])
}, [
query?.metric,
query?.accessToken,
query?.timeRange?.n,
query?.timeRange?.relative,
query?.timeRange?.start,
query?.timeRange?.stop,
query?.propeller,
filtersString
])

React.useEffect(() => {
function handlePropsMismatch() {
if (isStatic && !value) {
console.error('InvalidPropsError: You must pass either `value` or `query` props')
// console.error('InvalidPropsError: You must pass either `value` or `query` props') we will set logs as a feature later
setHasError(true)
return
}

if (!isStatic && (!query?.accessToken || !query?.metric || !query?.timeRange)) {
console.error(
'InvalidPropsError: When opting for fetching data you must pass at least `accessToken`, `metric` and `timeRange` in the `query` prop'
)
// console.error(
// 'InvalidPropsError: When opting for fetching data you must pass at least `accessToken`, `metric` and `timeRange` in the `query` prop'
// ) we will set logs as a feature later
setHasError(true)
return
}
Expand All @@ -118,18 +136,19 @@ export function Counter(props: CounterProps) {
async function setup() {
if (isStatic && value) {
setDataValue(value)
return
}

const fetchedValue = await fetchData()
if (!isStatic) {
const fetchedValue = await fetchData()

if (!fetchedValue) {
setHasError(true)
console.error(`QueryError: Your metric ${query?.metric} returned undefined or a \`null\` value.`)
return
}
if (fetchedValue === undefined) {
setHasError(true)
// console.error(`QueryError: Your metric ${query?.metric} returned undefined.`) we will set logs as a feature later
return
}

setDataValue(fetchedValue)
setDataValue(fetchedValue)
}
}

setup()
Expand Down
6 changes: 5 additions & 1 deletion packages/react/counter/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,14 @@ const getValue = (options: getValueOptions) => {
const { value, localize } = options

const isInteger = Number.isInteger(parseFloat(value))
const isNull = value === null

if (isInteger) {
return localize ? parseInt(value).toLocaleString() : parseInt(value)
}
if (isNull) {
return '-'
}

return localize
? parseFloat(parseFloat(value).toFixed(2).toLocaleString()).toLocaleString()
Expand All @@ -25,7 +29,7 @@ export const getValueWithPrefixAndSufix = (params: {
}) => {
const { prefix, value, sufix, localize } = params

if (!value) return
if (value === undefined) return

return (prefix ? prefix : '') + getValue({ value, localize }) + (sufix ? sufix : '')
}
56 changes: 33 additions & 23 deletions packages/react/leaderboard/src/Leaderboard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ export function Leaderboard(props: LeaderboardProps) {
const idRef = React.useRef(idCounter++)
const id = `leaderboard-${idRef.current}`

const filtersString = JSON.stringify(query?.filters || [])
const dimensionsString = JSON.stringify(query?.dimensions || [])

/**
* The html node where the chart will render
*/
Expand Down Expand Up @@ -196,18 +199,26 @@ export function Leaderboard(props: LeaderboardProps) {
try {
setIsLoading(true)

const dimensions = JSON.parse(dimensionsString)
const filters = JSON.parse(filtersString)

const response = await request(
PROPEL_GRAPHQL_API_ENDPOINT,
LeaderboardDocument,
{
uniqueName: query?.metric,
leaderboardInput: {
timeRange: query?.timeRange,
filters: query?.filters,
filters,
propeller: query?.propeller,
sort: query?.sort,
rowLimit: query?.rowLimit,
dimensions: query?.dimensions
dimensions,
timeRange: {
relative: query?.timeRange?.relative ?? null,
n: query?.timeRange?.n ?? null,
start: query?.timeRange?.start ?? null,
stop: query?.timeRange?.stop ?? null
}
}
},
{
Expand All @@ -228,35 +239,38 @@ export function Leaderboard(props: LeaderboardProps) {
}
}, [
query?.accessToken,
query?.dimensions,
query?.filters,
dimensionsString,
filtersString,
query?.metric,
query?.propeller,
query?.rowLimit,
query?.sort,
query?.timeRange
query?.timeRange?.n,
query?.timeRange?.relative,
query?.timeRange?.start,
query?.timeRange?.stop
])

React.useEffect(() => {
function handlePropsMismatch() {
if (isStatic && !headers && !rows) {
console.error('InvalidPropsError: You must pass either `headers` and `rows` or `query` props')
// console.error('InvalidPropsError: You must pass either `headers` and `rows` or `query` props') we will set logs as a feature later
setHasError(true)
return
}

if (isStatic && (!headers || !rows)) {
console.error('InvalidPropsError: When passing the data via props you must pass both `headers` and `rows`')
// console.error('InvalidPropsError: When passing the data via props you must pass both `headers` and `rows`') we will set logs as a feature later
setHasError(true)
return
}
if (
!isStatic &&
(!query.accessToken || !query.metric || !query.timeRange || !query.dimensions || !query.rowLimit)
) {
console.error(
'InvalidPropsError: When opting for fetching data you must pass at least `accessToken`, `metric`, `dimensions`, `rowLimit` and `timeRange` in the `query` prop'
)
// console.error(
// 'InvalidPropsError: When opting for fetching data you must pass at least `accessToken`, `metric`, `dimensions`, `rowLimit` and `timeRange` in the `query` prop'
// ) we will set logs as a feature later
setHasError(true)
return
}
Expand All @@ -275,17 +289,7 @@ export function Leaderboard(props: LeaderboardProps) {
if (!isStatic) {
fetchChartData()
}
}, [
isStatic,
query?.timeRange,
query?.filters,
query?.propeller,
query?.sort,
query?.rowLimit,
query?.dimensions,
query?.rowLimit,
fetchData
])
}, [isStatic, fetchData])

React.useEffect(() => {
if (isStatic) {
Expand All @@ -302,7 +306,7 @@ export function Leaderboard(props: LeaderboardProps) {
React.useEffect(() => {
try {
if (variant !== 'bar' && variant !== 'table') {
console.error('InvalidPropsError: `variant` prop must be either `bar` or `table`')
// console.error('InvalidPropsError: `variant` prop must be either `bar` or `table`') we will set logs as a feature later
throw new Error('InvalidPropsError')
}
setHasError(false)
Expand All @@ -317,6 +321,12 @@ export function Leaderboard(props: LeaderboardProps) {
}
}, [])

React.useEffect(() => {
if (variant === 'table') {
destroyChart()
}
}, [variant])

if (isLoading || loading) {
destroyChart()
return <Loader styles={styles} />
Expand Down
37 changes: 28 additions & 9 deletions packages/react/time-series/src/TimeSeries.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ export function TimeSeries(props: TimeSeriesProps) {
const idRef = React.useRef(idCounter++)
const id = `time-series-${idRef.current}`

const filtersString = JSON.stringify(query?.filters || [])

/**
* The html node where the chart will render
*/
Expand Down Expand Up @@ -258,15 +260,22 @@ export function TimeSeries(props: TimeSeriesProps) {
try {
setIsLoading(true)

const filters = JSON.parse(filtersString)

const response = await request(
PROPEL_GRAPHQL_API_ENDPOINT,
TimeSeriesDocument,
{
uniqueName: query?.metric,
timeSeriesInput: {
timeRange: query?.timeRange,
timeRange: {
relative: query?.timeRange?.relative ?? null,
n: query?.timeRange?.n ?? null,
start: query?.timeRange?.start ?? null,
stop: query?.timeRange?.stop ?? null
},
granularity,
filters: query?.filters,
filters: filters,
propeller: query?.propeller
}
},
Expand All @@ -286,25 +295,35 @@ export function TimeSeries(props: TimeSeriesProps) {
} finally {
setIsLoading(false)
}
}, [granularity, query?.accessToken, query?.filters, query?.metric, query?.propeller, query?.timeRange])
}, [
granularity,
query?.accessToken,
filtersString,
query?.metric,
query?.propeller,
query?.timeRange?.n,
query?.timeRange?.relative,
query?.timeRange?.start,
query?.timeRange?.stop
])

React.useEffect(() => {
function handlePropsMismatch() {
if (isStatic && !labels && !values) {
console.error('InvalidPropsError: You must pass either `labels` and `values` or `query` props')
// console.error('InvalidPropsError: You must pass either `labels` and `values` or `query` props') we will set logs as a feature later
setHasError(true)
return
}

if (isStatic && (!labels || !values)) {
console.error('InvalidPropsError: When passing the data via props you must pass both `labels` and `values`')
// console.error('InvalidPropsError: When passing the data via props you must pass both `labels` and `values`') we will set logs as a feature later
setHasError(true)
return
}
if (!isStatic && (!query.accessToken || !query.metric || !query.timeRange)) {
console.error(
'InvalidPropsError: When opting for fetching data you must pass at least `accessToken`, `metric` and `timeRange` in the `query` prop'
)
// console.error(
// 'InvalidPropsError: When opting for fetching data you must pass at least `accessToken`, `metric` and `timeRange` in the `query` prop'
// ) we will set logs as a feature later
setHasError(true)
return
}
Expand All @@ -324,7 +343,7 @@ export function TimeSeries(props: TimeSeriesProps) {
if (!isStatic) {
fetchChartData()
}
}, [isStatic, query?.timeRange, query?.filters, query?.propeller, query?.granularity, query?.accessToken, fetchData])
}, [isStatic, fetchData])

React.useEffect(() => {
if (isStatic) {
Expand Down

0 comments on commit 2751f40

Please sign in to comment.