Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor dashboard code #1876

Closed
3 tasks
bajiat opened this issue Nov 4, 2016 · 4 comments
Closed
3 tasks

Refactor dashboard code #1876

bajiat opened this issue Nov 4, 2016 · 4 comments
Assignees
Milestone

Comments

@bajiat
Copy link
Contributor

bajiat commented Nov 4, 2016

Break the dashboard code into submodules. At the same time, ensure that the charts will work with multiproxy.

Definition of done

  • Instance functions are moved to own js files
  • Use ES import for chart functions
  • Make sure all charts work and are still interactive

Notes from exploratory session

List of things to refactor in Dashboard code:

Current structure

The Dashboard code has the following structure:

  • client/
    • charts/
      • bar/ ...
      • filtering/ ...
      • line/ ...
      • overview/ ...
      • row/ ...
      • statistics/ ...
      • table/ ...
      • charts.html
      • charts.js
      • charts.less
    • lib/
    • dashboard.html
    • dashboard.js
    • dashboard.less

Complexity

The main areas of complexity are within charts.js and dashboard.js.

charts.js

What is in the charts.js file? What does it do?

The charts.js file manages

  • chartData
  • currentData.loadingState
  • api frontend prefixes
  • filtered chart data
  • add/remove loading placeholders (DOM)
  • parsed chart data
  • rendering charts
  • table data

Methods

The dashboardCharts template has the following instance methods:

  • parseChartData
  • renderCharts
  • getTableData
  • updateDataTable
  • updateLineChart
  • filterData
  • updateStatisticsData
  • getRequestsCount
  • getAverageResponseTime
  • getResponseRate
  • getUniqueUsersCount

Some of these template methods could be moved into the templates where they are directly used. Then, the 'chartData' would just be passed directly into each chart component, or we could even make a request specifically for the chart component.

Event handlers

The dashboardCharts has the following event handlers:

  • 'change api-frontend-prefix-form' : sets a list of frontend prefixes from UI widget
  • 'click tick-hour' - updates a CSS class, sets the timestamp format for analytics data
  • 'click tick-day' - sets or removes a CSS class, sets timestamp format for analytics data
  • 'click tick-week' - sets or removes a CSS class, sets timestamp format for analytics data
  • 'click tick-month' - sets or removes a CSS class, sets timestamp format for analytics data

It may be possible to combine, or even remove, the tick (day, week, month) handlers and move the result methods to tickButtons template

dashboard.js

What is in the dashboard.js file? What does it do?

Methods

The dashboard template has the following instance methods:

  • checkElasticsearch
  • getChartData
  • getElasticSearchQuery

Desired structure

We would like to structure the dashboard code in a simpler manner, such as:

  • client/
    • bar/ ...
    • filtering/ ...
    • line/ ...
    • overview/ ...
    • row/ ...
    • statistics/ ...
    • table/ ...
  • server/
@bajiat
Copy link
Contributor Author

bajiat commented Nov 7, 2016

@marla-singer Could you co-pilot this task, i.e. mentor @brylie when he is working with modularizing?

@marla-singer
Copy link
Contributor

@bajiat yes, sure.

@bajiat bajiat added this to the Sprint 35 milestone Nov 7, 2016
@bajiat bajiat added ready and removed planning labels Nov 7, 2016
@brylie
Copy link
Contributor

brylie commented Nov 9, 2016

After meeting with @marla-singer, we are suggesting that:

  • data handling code and rendering code should be separate
  • rendering logic should be closer to the chart templates
  • data handling might be achieved by server methods, rather than on the client
  • rely on Elasticsearch to do the work, such as aggregations
  • we should avoid imperative DOM manipulation (such as adding classes or placeholders)
    • rather, do things declaratively, such as in template helpers

@philippeluickx also suggested that each chart template could trigger a server side request for only the data it needs. That would mean that multiple requests could be made in parallel, and each chart would render as its data became available.

Ping @bajiat @frenchbread @jykae @NNN

@philippeluickx
Copy link
Contributor

I think also from the "perceived performance" point of view, it's always nice to see some results already. So even if I only see the first graph and the others are still loading, that will feel faster than having everything shown at the same time.
Question of course is if the gains from using parallel queries is lost by overheard?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants