Skip to content

Refactor of usage dashboard (moved logic from JS to Ruby) - #2336

Merged
xsrust merged 7 commits into
developmentfrom
issue2083
Jan 28, 2020
Merged

Refactor of usage dashboard (moved logic from JS to Ruby)#2336
xsrust merged 7 commits into
developmentfrom
issue2083

Conversation

@briri

@briri briri commented Dec 23, 2019

Copy link
Copy Markdown
Contributor

Fixes #2083, #2116, #2119

Decided to refactor the usage dashboard code while in fixing these tickets. The discrepancies we were seeing with the numbers were due to the fact that some of the code was calling the API and the others were pulling data from the stat table (which they should all be doing).

  • Broke the large index.html.erb file into smaller partials
  • Moved as much logic from the JS to the usage_controller.rb
  • Added a policy file for the controller
  • Moved the generic chart rendering JS into a utils/charts.js file
  • Removed ajax calls from JS and replaced with Rails pattern to render a js.erb instead.
  • Changed buttons to links with the new 'opens in a new window' accessibility feature
  • Removed references to and need for API token in the usage dashboard
  • Added tests for all new code

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

rubocop

app/helpers/usage_helper.rb|50 col 33| Lint/UnusedBlockArgument: Unused block argument - k. If it's necessary, use _ or _k as an argument name to indicate that it won't be used. (https://rubystyle.guide#underscore-unused-vars)
app/helpers/usage_helper.rb|55 col 3| Metrics/AbcSize: Assignment Branch Condition size for plans_per_template_ranges is too high. [<0, 20, 0> 20/15] (http://c2.com/cgi/wiki?AbcMetric, https://en.wikipedia.org/wiki/ABC_Software_Metric)
app/helpers/usage_helper.rb|57 col 10| Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://rubystyle.guide#consistent-string-literals)
app/helpers/usage_helper.rb|58 col 10| Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://rubystyle.guide#consistent-string-literals)
app/helpers/usage_helper.rb|59 col 10| Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://rubystyle.guide#consistent-string-literals)
app/helpers/usage_helper.rb|60 col 10| Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://rubystyle.guide#consistent-string-literals)
app/helpers/usage_helper.rb|61 col 10| Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://rubystyle.guide#consistent-string-literals)
app/helpers/usage_helper.rb|70 col 45| Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://rubystyle.guide#consistent-string-literals)
app/helpers/usage_helper.rb|71 col 46| Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://rubystyle.guide#consistent-string-literals)
app/helpers/usage_helper.rb|77 col 31| Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://rubystyle.guide#consistent-string-literals)
app/models/stat.rb|37 col 17| Style/SymbolArray: Use %i or %I for an array of symbols. (https://rubystyle.guide#percent-i)
app/models/stat_created_plan.rb|24 col 8| Style/RedundantSelf: Redundant self detected. (https://rubystyle.guide#no-self-unless-required)
app/models/stat_created_plan.rb|24 col 33| Style/RedundantSelf: Redundant self detected. (https://rubystyle.guide#no-self-unless-required)
app/models/stat_created_plan.rb|31 col 3| Lint/ToJSON: #to_json requires an optional argument to be parsable via JSON.generate(obj).
app/policies/usage_policy.rb|3 col 21| Style/StructInheritance: Don't extend an instance initialized by Struct.new. Use a block to customize the struct. (https://rubystyle.guide#no-extend-struct-new)
app/policies/usage_policy.rb|4 col 1| Layout/EmptyLinesAroundClassBody: Empty line missing at class body beginning. (https://rubystyle.guide#empty-lines-around-bodies)
app/policies/usage_policy.rb|6 col 24| Lint/UnusedMethodArgument: Unused method argument - usage. If it's necessary, use _ or _usage as an argument name to indicate that it won't be used. (https://rubystyle.guide#underscore-unused-vars)
app/policies/usage_policy.rb|7 col 5| Layout/EmptyLineAfterGuardClause: Add empty line after guard clause.

Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/helpers/usage_helper.rb Outdated
Comment thread app/helpers/usage_helper.rb Outdated
Comment thread app/helpers/usage_helper.rb Outdated
Comment thread app/helpers/usage_helper.rb Outdated
Comment thread app/helpers/usage_helper.rb Outdated
Comment thread app/helpers/usage_helper.rb Outdated
Comment thread app/helpers/usage_helper.rb Outdated
Comment thread app/helpers/usage_helper.rb Outdated
Comment thread app/helpers/usage_helper.rb Outdated
Comment thread app/models/stat.rb Outdated
Comment thread app/models/stat_created_plan.rb Outdated
Comment thread app/policies/usage_policy.rb Outdated
Comment thread app/policies/usage_policy.rb Outdated
Comment thread app/policies/usage_policy.rb Outdated
Comment thread app/policies/usage_policy.rb Outdated
Comment thread app/helpers/usage_helper.rb Outdated
Comment thread app/models/stat_created_plan.rb Outdated
Comment thread app/policies/usage_policy.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
@briri briri changed the title Refactor of usage dashboard Dec 23, 2019
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/controllers/usage_controller.rb Outdated
Comment thread app/helpers/usage_helper.rb Outdated
Comment thread app/helpers/usage_helper.rb Outdated
Comment thread spec/controllers/usage_controller_spec.rb Outdated
Comment thread spec/policies/usage_policy_spec.rb Outdated
Comment thread spec/policies/usage_policy_spec.rb Outdated
Comment thread spec/policies/usage_policy_spec.rb Outdated
Comment thread spec/policies/usage_policy_spec.rb Outdated
Comment thread spec/policies/usage_policy_spec.rb Outdated
briri added 5 commits January 3, 2020 12:07
…ils instead of JS

finished up changes to usage

fixed eslinter issues

fixed rubocop issues

fixed additional rubocop issues

fixed issue with filter

fixed issue with totals

removed old controller test and added new usage_controller tests

added tests for new usage_helper

added usage_policy tests and pundit test helper

fixed some rubocop complaints

fixed up remaining rubocop issues
@briri briri changed the title DO NOT MERGE YET: Refactor of usage dashboard Jan 3, 2020
@raycarrick-ed raycarrick-ed self-assigned this Jan 20, 2020

@xsrust xsrust left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the cleanup here @briri

I think moving the logic over to ruby will make this a lot more maintainable for our current team.

@briri

briri commented Jan 22, 2020

Copy link
Copy Markdown
Contributor Author

made some changes based on @raycarrick-ed feedback in #2083.

  • Removed the class that was making the 'opens in a new window' message appear with the download buttons
  • Moved the Org selection box of the filter down to its own row
Comment thread spec/helpers/usage_helper_spec.rb Outdated
Comment thread spec/helpers/usage_helper_spec.rb Outdated
@xsrust
xsrust merged commit b009374 into development Jan 28, 2020
@briri
briri deleted the issue2083 branch February 13, 2020 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants