Commit Graph

12 Commits

Author SHA1 Message Date
Javi Martín
3fcac3a9d8 Sort data by date when building graphs
This doesn't affect the end result because all collections used the same
order, but it makes debugging easier.
2024-05-09 14:28:33 +02:00
Javi Martín
86e131df26 Make it more obvious that we're using dates in charts
The `shared_keys` and `k` variable names could mean anything, so it
wasn't clear what these variables contained.
2024-05-09 14:28:32 +02:00
Javi Martín
d44cff630d Extract method to build a data source 2024-05-09 14:28:32 +02:00
Javi Martín
14454bdd45 Include data in chart tags instead of using AJAX
Now that we've moved the logic to generate the events data to a model,
and we've got access to the model in the component rendering the chart,
we can render the data inside the chart instead of doing an extra AJAX
request to get the same data.

Originally, this was problaby done this way so the page wouldn't take
several seconds to load while preparing the data for the chart when
there are thousands of dates being displayed. With an AJAX call, the
page would load as fast as usual, and then the chart would render after
a few seconds. However, we can have an even better performance
improvement in this scenario if we use a Set instead of an Array. The
method `Array#include?`, which we were calling for every date in the
data, is much slower that `Set#merge`. So now both the page and the
chart load as fast as expected.

We could also use something like:

```
def add
  (...)
  shared_keys.push(*collection.keys)
end

def build
  (...)
  shared_keys.uniq.each do |k|
  (...)
end

def shared_keys
  @shared_keys ||= []
end
```

Or other approaches to avoid using `Array#include?`. The performance
would be similar to the one we get when using `Set`. We're using a `Set`
because it makes more obvious that `shared_keys` is supposed to contain
unique elements.

We've had some tests failing in the past due to these AJAX requests
being triggered automatically during the tests and no expectations
checking the requests have finished, so now we're reducing the amount of
flaky tests.
2024-05-09 14:28:32 +02:00
Javi Martín
57ef380379 Add and apply Layout/ExtraSpacing rubocop rule 2019-10-24 18:11:58 +02:00
Javi Martín
db97f9d08c Add and apply rubocop rules for empty lines
We were very inconsistent regarding these rules.

Personally I prefer no empty lines around blocks, clases, etc... as
recommended by the Ruby style guide [1], and they're the default values
in rubocop, so those are the settings I'm applying.

The exception is the `private` access modifier, since we were leaving
empty lines around it most of the time. That's the default rubocop rule
as well. Personally I don't have a strong preference about this one.


[1] https://rubystyle.guide/#empty-lines-around-bodies
2019-10-24 17:11:47 +02:00
Javi Martín
f9ed186909 Add rubocop spacing rules
We were following these rules in most places; we just didn't define them
anywhere.
2019-09-10 21:04:56 +02:00
Javi Martín
47b2c42a1d Apply IndentationConsistency rubocop rule 2019-09-10 20:02:15 +02:00
Bertocq
57743df197 Enable rubocop Performance/HashEachMethods cop & fix issues 2018-02-10 21:22:47 +01:00
Bertocq
97d7a21791 Fix all Style/RedundantReturn rubocop issues 2017-07-05 11:55:52 +02:00
Bertocq
e6dd33bd66 Fix all Layout/SpaceAfterComma issues and remove from rubocop_todo list 2017-06-26 17:57:53 +02:00
Eloy Gomez
5d91663016 Add a helper for build c3.js chart compatible data 2015-08-17 19:13:56 +02:00