Add RedisTimeseries plugin #312

GnaneshKunal · 2022-02-21T12:20:37Z

No description provided.

rsergeenko · 2022-02-28T07:41:14Z

redisinsight/ui/src/packages/redistimeseries-app/src/App.tsx

+import Chart from './components/Chart/Chart'
+import ChartResultView from './components/Chart/ChartResultView'
+
+import { TimeSeries, YAxisConfig, ChartConfig, AxisScale, GraphMode } from './components/Chart/interfaces'


Please, move interface after imports.

rsergeenko · 2022-02-28T07:41:39Z

redisinsight/ui/src/packages/redistimeseries-app/src/App.tsx

+/* eslint-disable react/jsx-filename-extension */
+import React, {useState} from 'react'
+
+


Please, remove one empty line

rsergeenko · 2022-02-28T07:41:59Z

redisinsight/ui/src/packages/redistimeseries-app/src/App.tsx

+const App = (props: Props) => {
+  const { command = '', result: [{ response = '', status = '' } = {}] = [] } = props
+
+  const data= response1.result


Please, add space after data

rsergeenko · 2022-02-28T07:43:18Z

redisinsight/ui/src/packages/redistimeseries-app/src/App.tsx

+    *         chartConfig={chartConfig}
+    *         data={data as any}
+    *     />
+    * ) */


Please, remove comment blocks

rsergeenko · 2022-02-28T07:43:25Z

redisinsight/ui/src/packages/redistimeseries-app/src/App.tsx

+    * }
+
+    * return <TableResult query={command} result={result} /> */
+}


The same.
Also will be good to handle fail status

rsergeenko · 2022-02-28T08:02:11Z

redisinsight/ui/src/packages/redistimeseries-app/src/components/Chart/ChartResultView.tsx

+import ChartConfigForm from './ChartConfigForm'
+import { TimeSeries, YAxisConfig, ChartConfig, AxisScale, GraphMode } from './interfaces'
+import Chart from './Chart'
+import { EuiSpacer } from '@elastic/eui'


Please, sort imports

rsergeenko · 2022-02-28T08:03:50Z

redisinsight/ui/src/packages/redistimeseries-app/src/components/Chart/ChartResultView.tsx

+      </div>
+    )
+  }
+}


Could you please refactor this component to function component

rsergeenko · 2022-02-28T08:05:44Z

redisinsight/ui/src/packages/redistimeseries-app/src/components/Chart/Chart.tsx

+export const TIMESERIES_HISTORY_CONTAINER_NAME = 'REDIS_TIMESERIES'
+
+export const AUTO_UPDATE_TIMER_DEFAULT_VALUE = 5000 // time in milliseconds
+export const AUTO_UPDATE_NUM_SAMPLES_DEFAULT_VALUE = 50


import these constants from './constants'

rsergeenko · 2022-02-28T08:07:14Z

redisinsight/ui/src/packages/redistimeseries-app/src/main.tsx

+  check: EuiIconCheck,
+})
+
+import { EuiProvider } from '@elastic/eui';


Please, sort imports

rsergeenko · 2022-02-28T08:07:36Z

redisinsight/ui/src/packages/redistimeseries-app/src/main.tsx

+import { icon as EuiIconArrowRight } from '@elastic/eui/es/components/icon/assets/arrow_right';
+import { icon as EuiIconArrowDown } from '@elastic/eui/es/components/icon/assets/arrow_down';
+import { icon as EuiIconCross } from '@elastic/eui/es/components/icon/assets/cross';
+import { icon as EuiIconCheck } from '@elastic/eui/es/components/icon/assets/check';


please, remove semicolons

rsergeenko

Also, could you please, check the plugin for responsive design?

* add redistimeseries plugin to app * update plugin dir path

- Remove semicolons, dead code and more

merge main

…es-plugin

egor-zalenski · 2022-03-15T15:24:03Z

redisinsight/ui/src/packages/redistimeseries-app/src/components/Chart/Chart.tsx

+import React, { useRef, useEffect } from 'react'
+import Plotly, { Layout } from 'plotly.js-dist-min'
+import { Legend, LayoutAxis, PlotData } from 'plotly.js'
+import moment from 'moment'


moment.js is depricated
Could you replace it to date-fns?

egor-zalenski · 2022-03-15T15:25:46Z

redisinsight/ui/src/packages/redistimeseries-app/src/components/Chart/Chart.tsx

+
+const isDarkTheme = document.body.classList.contains('theme_DARK')
+
+export default function Chart(props: any) {


Could you add an interface for props?

egor-zalenski · 2022-03-15T15:27:11Z

redisinsight/ui/src/packages/redistimeseries-app/src/components/Chart/Chart.tsx

+      { displayModeBar: false, autosizable: true, responsive: true, setBackground: () => 'transparent', },
+    )
+    chartContainer.current.on('plotly_hover', function (eventdata) {
+      var points = eventdata.points[0]


Please change var to const
and at the line below

egor-zalenski · 2022-03-15T15:29:07Z

redisinsight/ui/src/packages/redistimeseries-app/src/components/Chart/Chart.tsx

+        })),
+        Object.keys((chartContainer.current)._fullLayout._plots))
+    })
+    chartContainer.current.on('plotly_relayout', function (eventdata) {


could you add an interface for eventdata?
And for others chartContainer.current handlers

egor-zalenski · 2022-03-15T15:30:12Z

redisinsight/ui/src/packages/redistimeseries-app/src/components/Chart/Chart.tsx

+        })),
+        Object.keys((chartContainer.current)._fullLayout._plots))
+    })
+    chartContainer.current.on('plotly_relayout', function (eventdata) {


please move 'plotly_relayout', 'plotly_hover' and 'plotly_doubleclick' to enum

egor-zalenski · 2022-03-15T15:38:37Z

redisinsight/ui/src/packages/redistimeseries-app/src/components/Chart/ChartResultView.tsx

+  chartState: LAYOUT_STATE
+}
+
+export default class ChartResultView extends React.Component<ChartResultViewProps, ChartResultViewState> {


Please rewrite React.Component to Functional Component

egor-zalenski · 2022-03-15T15:40:09Z

redisinsight/ui/src/packages/redistimeseries-app/src/components/Chart/utils.ts

@@ -0,0 +1,264 @@
+export const WATERMARK_OPTIONS = {


Please move all constants from this file to constants.ts file

egor-zalenski · 2022-03-15T15:45:29Z

redisinsight/ui/src/packages/redistimeseries-app/src/components/Chart/utils.ts

+}
+
+function getMimeTypeOfDataURI(uri: DataURI) {
+    return uri.split(/[:;,]/g)[1]


Could you move RegExp to constants?
and please add Optional chaining operator before [1], this is not safety
uri.split(...).?[1]

egor-zalenski · 2022-03-15T15:48:38Z

redisinsight/ui/src/packages/redistimeseries-app/src/components/Chart/utils.ts

+  }
+}
+
+export function hexToRGBA(hex: string, opacity: number): string {


Please rewrite this function. It's hard to read

egor-zalenski · 2022-03-15T15:50:26Z

redisinsight/ui/src/packages/redistimeseries-app/src/main.tsx

+  const { command = '', data: result = [] } = props
+  render(
+    <App command={command} result={result} />,
+    document.getElementById('app'))


please move second round bracket to the next line

egor-zalenski · 2022-03-16T07:55:54Z

Also could you add Unit tests for utils?
The better way is like in this test.
Create an array with input and output, and then compare them

https://github.com/RedisInsight/RedisInsight/blob/main/redisinsight/ui/src/utils/tests/formatTypes.spec.ts

…eadcode

egor-zalenski · 2022-03-18T15:12:13Z

@GnaneshKunal
We have to merge this pr to main without fix pr comments for release.
Please fix all comments in the nearest feature

# Conflicts: # scripts/build-statics.cmd # scripts/build-statics.sh

Add timeseries plugin

f7da277

rsergeenko reviewed Feb 28, 2022

View reviewed changes

rsergeenko requested a review from egor-zalenski February 28, 2022 08:09

rsergeenko assigned GnaneshKunal Feb 28, 2022

egor-zalenski and others added 9 commits February 28, 2022 18:43

Build/timeseries (#336)

bbbf922

* add redistimeseries plugin to app * update plugin dir path

- Load plugin on RANGE, REVRANGE and MREVRANGE.

5904275

- Remove semicolons, dead code and more

Merge pull request #337 from RedisInsight/main

8928b01

merge main

Use 'Graphik' font

065cf7c

Merge remote-tracking branch 'origin/timeseries-plugin' into timeseri…

603c03e

…es-plugin

Prettify error messages

d1ed9b0

Parse non-multi range queries differently

79d40a6

main.tsx is the main source file

ca8bebc

Add range slider, config menu new styles, right y axis hover fix

3ddc2f5

egor-zalenski reviewed Mar 15, 2022

View reviewed changes

GnaneshKunal added 3 commits March 17, 2022 15:45

Add new form design

de313db

Add button group padding and font weight fix

de4984f

Use date-fns, add types, move interfaces to interfaces file, remove d…

7d67428

…eadcode

Merge branch 'main' into timeseries-plugin

c4bb694

# Conflicts: # scripts/build-statics.cmd # scripts/build-statics.sh

egor-zalenski approved these changes Mar 18, 2022

View reviewed changes

egor-zalenski merged commit 5a94334 into main Mar 18, 2022

egor-zalenski deleted the timeseries-plugin branch March 18, 2022 15:14

GnaneshKunal mentioned this pull request Mar 31, 2022

[RedisTimeseries] Convert class components to functional, add tests for utils, refactor hex to rgba #479

Merged

		/* eslint-disable react/jsx-filename-extension */
		import React, {useState} from 'react'


		const isDarkTheme = document.body.classList.contains('theme_DARK')

		export default function Chart(props: any) {

Add RedisTimeseries plugin #312

Add RedisTimeseries plugin #312

Uh oh!

Conversation

GnaneshKunal commented Feb 21, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rsergeenko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

egor-zalenski commented Mar 16, 2022

Uh oh!

egor-zalenski commented Mar 18, 2022

Uh oh!

Uh oh!