From fef915e7605e2120032fe8dfded280c8572a31f4 Mon Sep 17 00:00:00 2001 From: "Aleksandar N. Kostadinov" Date: Tue, 17 Jan 2023 18:34:43 +0200 Subject: [PATCH 1/6] replace gruff with svg-graph2 --- Gemfile | 4 +- Gemfile.lock | 14 +-- app/lib/pdf/data.rb | 71 +-------------- app/lib/pdf/report.rb | 167 ++++++++++++++++++++++++++++++++++- config/initializers/prawn.rb | 3 - 5 files changed, 177 insertions(+), 82 deletions(-) diff --git a/Gemfile b/Gemfile index 0a89905ce1..978afd09ba 100644 --- a/Gemfile +++ b/Gemfile @@ -74,9 +74,7 @@ gem 'braintree', '~> 2.93' gem 'bugsnag', '~> 6.11' gem 'cancancan', '~> 2.3.0' gem 'formtastic', '~> 2.3.1' -gem 'gruff', '~>0.3', require: false gem 'htmlentities', '~>4.3', '>= 4.3.4' -gem 'rmagick', '~> 2.15.3', require: false # TODO: Not actively maintained https://github.com/activeadmin/inherited_resources#notice replace with respond_with and fix things the rails way gem 'inherited_resources', '~> 1.12.0' gem 'has_scope', '~> 0.7.2' # remove line after we stop supporting Ruby 2.4 @@ -106,6 +104,7 @@ gem 'local-fastimage_resize', '~> 3.4.0', require: 'fastimage/resize' gem 'paperclip', '~> 6.0' gem 'prawn' gem 'prawn-table', git: "https://github.com/prawnpdf/prawn-table.git", branch: "38b5bdb5dd95237646675c968091706f57a7a641" +gem 'prawn-svg' gem 'rails_event_store', '~> 0.9.0', require: false gem 'ratelimit' gem 'recaptcha', '4.13.1', require: 'recaptcha/rails' @@ -115,6 +114,7 @@ gem 'redis', '~> 4.1.3', require: ['redis', 'redis/connection/hiredis'] gem 'redis-namespace', '~> 1.7.0' gem 'rest-client', '~> 2.0.2' gem 'rubyzip', '~>1.3.0', require: false +gem 'svg-graph', require: false gem 'swagger-ui_rails', git: 'https://github.com/3scale/swagger-ui_rails.git', branch: 'dev' gem 'swagger-ui_rails2', git: 'https://github.com/3scale/swagger-ui_rails.git', branch: 'dev-2.1.3' gem 'thinking-sphinx', '~> 5.4.0' diff --git a/Gemfile.lock b/Gemfile.lock index 41ddd198f5..296f0e0a8a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -265,6 +265,8 @@ GEM crack (0.4.5) rexml crass (1.0.6) + css_parser (1.12.0) + addressable cucumber (7.0.0) builder (~> 3.2, >= 3.2.4) cucumber-core (~> 10.0, >= 10.0.1) @@ -364,8 +366,6 @@ GEM github-markdown (0.6.9) globalid (1.0.1) activesupport (>= 5.0) - gruff (0.7.0) - rmagick (~> 2.13, >= 2.13.4) has_scope (0.7.2) actionpack (>= 4.1) activesupport (>= 4.1) @@ -531,6 +531,10 @@ GEM prawn (2.4.0) pdf-core (~> 0.9.0) ttfunk (~> 1.7) + prawn-svg (0.32.0) + css_parser (~> 1.6) + prawn (>= 0.11.1, < 3) + rexml (~> 3.2) prometheus-client-mmap (0.11.0) protected_attributes_continued (1.8.2) activemodel (>= 5.0) @@ -657,7 +661,6 @@ GEM netrc (~> 0.8) rexml (3.2.5) riddle (2.4.3) - rmagick (2.15.4) roar (1.0.4) representable (>= 2.0.1, < 2.4.0) roar-rails (1.0.2) @@ -795,6 +798,7 @@ GEM stripe (5.28.0) strong_migrations (0.6.8) activerecord (>= 5) + svg-graph (2.2.1) sys-uname (1.2.2) ffi (~> 1.1) temple (0.8.1) @@ -953,7 +957,6 @@ DEPENDENCIES formtastic (~> 2.3.1) github-markdown globalid (~> 1.0.1) - gruff (~> 0.3) has_scope (~> 0.7.2) hashie hiredis (~> 0.6.3) @@ -993,6 +996,7 @@ DEPENDENCIES pg (~> 0.21.0) pisoni (~> 1.29) prawn + prawn-svg prawn-table! protected_attributes_continued (~> 1.8.2) pry-byebug (>= 3.7.0) @@ -1019,7 +1023,6 @@ DEPENDENCIES reek (= 6.01) reform (~> 2.0.3) rest-client (~> 2.0.2) - rmagick (~> 2.15.3) roar-rails rspec-html-matchers! rspec-rails (~> 4.1) @@ -1051,6 +1054,7 @@ DEPENDENCIES statsd-ruby stripe (~> 5.28.0) strong_migrations (~> 0.6.8) + svg-graph swagger-ui_rails! swagger-ui_rails2! thin diff --git a/app/lib/pdf/data.rb b/app/lib/pdf/data.rb index 3721bc3a83..6229512dce 100644 --- a/app/lib/pdf/data.rb +++ b/app/lib/pdf/data.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require 'gruff' - module Pdf class Data @@ -74,74 +72,11 @@ def metrics end end - def traffic_graph - return if @hit_metric.nil? - - data = @source.usage(@options) - - g = Gruff::Line.new('1200x300') - - g.theme = { - :colors => ['#9172EC', '#306EFF', '#000066', '#B4B4B4'], - :font_color => '#555', - :marker_color => '#eeeeee', - :background_colors => ['#ffffff', '#ffffff'] - } - - g.legend_box_size = 10 - g.hide_title = true - g.legend_font_size = 13 - g.hide_line_markers = false - g.marker_font_size = 13 - g.hide_dots = false - g.dot_radius = 3 - g.line_width = 2 - g.marker_count = 5 - g.margins = 2 - - g.sort = false - g.x_axis_label = @period == :day ? "Hour" : "Week Days" - g.y_axis_label = @hit_metric.friendly_name - - data = data[:values] - g.data(@hit_metric.friendly_name, data) - - max = data.max - g.maximum_value = max + (max / 5) - - g.labels = send("#{@period}_labels") - StringIO.new(g.to_blob("JPG")) + def usage + @source.usage(@options) unless @hit_metric.nil? end - def week_labels - # Week report, there are 28 data points, at 6 hour intervals - # What was the date 1 week ago - date = 1.week.ago.beginning_of_day - - #Gruff expects labels to be presented as a hash - labels= {} - (0..27).each do |point| - # Insert blanks except for every 4th data point (covering a 24hr period) - if (point % 4).zero? - labels[point] = date.strftime("%d %b") - date += 1.day - end - end - - labels - end - - def day_labels - # See week_labels - date = 1.day.ago.beginning_of_day - - labels = {} - (0..23).each do |point| - labels[point] = date.strftime("%k:00") if (point % 4).zero? - date += 1.hour - end - labels - end + private def sanitize_text(text) EscapeUtils.escape_javascript(text.to_s) diff --git a/app/lib/pdf/report.rb b/app/lib/pdf/report.rb index e282d7c179..94dec08e1e 100644 --- a/app/lib/pdf/report.rb +++ b/app/lib/pdf/report.rb @@ -1,5 +1,8 @@ # frozen_string_literal: true +require 'SVG/Graph/DataPoint' +require 'SVG/Graph/Line' + # REFACTOR: extract abstract Report class, and DRY functionality with InvoiceReporter module Pdf class Report @@ -35,6 +38,7 @@ def generate header + move_down 3 traffic_graph move_down 3 @@ -128,11 +132,158 @@ def traffic_and_users end end + def graph_key_formatter(usage) + if @period == :day + ->(point) { (point % 4).zero? ? sprintf("%02d:00", point.succ) : "" } + else + since = Time.parse(usage.dig(:period, :since)) + granularity = usage.dig(:period, :granularity) + ->(point) { (point % 4).zero? ? (since + point * granularity).strftime("%d %b") : "" } + end + end + def traffic_graph - graph = @data.traffic_graph - return unless graph - subtitle "Traffic" - @pdf.image graph, position: :left, width: 520 + usage = @data.usage + + return unless usage + + options = { + graph_title: "Traffic", + show_graph_title: true, + key: false, + area_fill: false, + show_data_values: false, + add_popups: false, + width: TABLE_FULL_WIDTH, + height: TABLE_FULL_WIDTH / 4, + step_x_labels: 4, + step_include_first_x_label: true, + fields: usage[:values].each_index.map(&graph_key_formatter(usage)), + show_x_title: false, + x_title: @period == :day ? "Hour" : "Week Days", + show_y_guidelines: true, + scale_integers: true, + scale_divisions: [2, (usage[:values].max - usage[:values].min) / 5].max, + number_format: IntegerWithDelimiterFormatter.new, + show_y_title: false, + y_title: usage.dig(:metric, :name), + y_title_location: :middle, + no_css: false, + } + + graph = SVG::Graph::Line.new(options) + + graph.add_data(data: usage[:values], title: options[:y_title]) + + @pdf.svg traffic_graph_style(graph.burn_svg_only) + end + + def traffic_graph_style(svg) + xml = Nokogiri::XML(svg) + style = xml.at_css("style") + css = CssParser::Parser.new + css.load_string!(style.text.gsub(/ff0000/i, "9273ED")) + traffic_graph_first_data_point(xml) + traffic_graph_y_align(xml) + traffic_graph_style_clean_up(css) + traffic_graph_style_guide_lines(css) + traffic_graph_style_axes(css) + traffic_graph_style_line_width(css) + traffic_graph_style_background(css) + traffic_graph_style_text(css) + + style.content = css.to_s + xml.to_s + end + + # TODO: remove this hack ofter fix is acepted upstream + # https://github.com/lumean/svg-graph2/pull/43 + def traffic_graph_first_data_point(xml) + line = xml.at_css(".line1") + line[:d] = line[:d].sub(/^M.+L\s*(\S+\s+\S+)(.*)$/, 'M\1 L\2') + end + + # TODO: remove this hack after fix is accepted upstream + # https://github.com/lumean/svg-graph2/pull/44 + def traffic_graph_y_align(xml) + xml.css(".yAxisLabels").each do |label| + label[:x] = "-8" + label.delete("style") + label.delete("transform") + end + end + + def traffic_graph_style_background(css) + desired = <<-EOT + .graphBackground { + fill:#ffffff; + } + EOT + + css.remove_rule_set!(css.find_rule_sets([".graphBackground"]).first) + css.add_block!(desired) + end + + def traffic_graph_style_text(css) + desired = <<-EOT + .xAxisLabels,.yAxisLabels { + fill:#909090; + font-size: 10px; + font-family: "#{@style[:font]}", sans-serif; font-weight: normal; + } + .mainTitle { + fill:#505050; + font-family: "#{@style[:font]}", sans-serif; font-weight: normal; + } + EOT + + css.add_block!(desired) + end + + def traffic_graph_style_axes(css) + desired = <<-EOT + .axis{ + stroke: #ffffff; + stroke-width: 0px; + } + EOT + + css.remove_rule_set!(css.find_rule_sets([".axis"]).first) + css.add_block!(desired) + end + + def traffic_graph_style_line_width(css) + desired = <<-EOT + .line1 { + stroke-width: 2px; + } + EOT + + css.add_block!(desired) + end + + def traffic_graph_style_guide_lines(css) + desired = <<-EOT + .guideLines,#yAxis { + stroke: #eeeeee; + stroke-width: 0.3px; + stroke-dasharray: 0.01 1; + stroke-linejoin: round; + stroke-linecap: round; + } + EOT + + css.remove_rule_set!(css.find_rule_sets([".guideLines"]).first) + css.add_block!(desired) + end + + def traffic_graph_style_clean_up(css) + (2..12).each do |num| + %w[line fill key dataPoint].each do |type| + rule = css.find_rule_sets([".#{type}#{num}"]).first + css.remove_rule_set!(rule) if rule.present? + end + end end def metrics @@ -189,5 +340,13 @@ def colorize_num(numstr) { content: numstr, **@style[:green] } end end + + class IntegerWithDelimiterFormatter + include ActionView::Helpers::NumberHelper + + def %(num) + number_with_delimiter(num.to_i) + end + end end end diff --git a/config/initializers/prawn.rb b/config/initializers/prawn.rb index 70f4280822..cb1bf7c55c 100644 --- a/config/initializers/prawn.rb +++ b/config/initializers/prawn.rb @@ -1,7 +1,4 @@ -# require 'prawn' -# require 'prawn/format' require "prawn/measurement_extensions" -require 'gruff' require "open-uri" module Prawn From 64578f449e6311ff671a079b4087c92c83c1a380 Mon Sep 17 00:00:00 2001 From: "Aleksandar N. Kostadinov" Date: Wed, 18 Jan 2023 21:59:46 +0200 Subject: [PATCH 2/6] ImageMagick not needed anymore --- openshift/system/Dockerfile | 2 -- 1 file changed, 2 deletions(-) diff --git a/openshift/system/Dockerfile b/openshift/system/Dockerfile index 8b285bf4ab..2f35044ce9 100644 --- a/openshift/system/Dockerfile +++ b/openshift/system/Dockerfile @@ -23,8 +23,6 @@ RUN yum-config-manager --save --setopt=cbs.centos.org_repos_sclo7-$RUBY_SCL-rh-c && yum remove -y postgresql \ && yum install -y epel-release \ && yum -y install centos-release-scl-rh \ - ImageMagick \ - ImageMagick-devel \ gd-devel \ unixODBC-devel \ mysql \ From b2830f022678ccffd4850d04111773e138ebaef5 Mon Sep 17 00:00:00 2001 From: "Aleksandar N. Kostadinov" Date: Wed, 18 Jan 2023 22:05:37 +0200 Subject: [PATCH 3/6] remove ImageMagick references --- fedora-manual-setup.md | 2 +- osx-manual-setup.md | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/fedora-manual-setup.md b/fedora-manual-setup.md index fd27cf4627..d854d8fc9c 100644 --- a/fedora-manual-setup.md +++ b/fedora-manual-setup.md @@ -30,7 +30,7 @@ asdf install ### Dependencies ``` -sudo dnf install sphinx chromedriver postgresql-devel gd-devel mysql-devel ImageMagick ImageMagick-devel openssl-devel zlib-devel sqlite-devel readline-devel libyaml-devel libtool libffi-devel bison automake autoconf patch +sudo dnf install sphinx chromedriver postgresql-devel gd-devel mysql-devel openssl-devel zlib-devel sqlite-devel readline-devel libyaml-devel libtool libffi-devel bison automake autoconf patch ``` ### Database diff --git a/osx-manual-setup.md b/osx-manual-setup.md index c6988a231c..7e0aa67c60 100644 --- a/osx-manual-setup.md +++ b/osx-manual-setup.md @@ -58,8 +58,7 @@ xcode-select -—install ### Dependencies ``` -brew install chromedriver imagemagick@6 gs pkg-config openssl geckodriver sphinx gd mysql@5.7 postgresql@14 -brew link imagemagick@6 +brew install chromedriver gs pkg-config openssl geckodriver sphinx gd mysql@5.7 postgresql@14 ``` * **Macs with M1** also require the following: From 5fb1e039b26eec1e7acd9ef25006a3d3828730d0 Mon Sep 17 00:00:00 2001 From: "Aleksandar N. Kostadinov" Date: Tue, 24 Jan 2023 19:55:05 +0200 Subject: [PATCH 4/6] metric name and day start from 0 --- app/lib/pdf/report.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/lib/pdf/report.rb b/app/lib/pdf/report.rb index 94dec08e1e..813b972704 100644 --- a/app/lib/pdf/report.rb +++ b/app/lib/pdf/report.rb @@ -134,7 +134,7 @@ def traffic_and_users def graph_key_formatter(usage) if @period == :day - ->(point) { (point % 4).zero? ? sprintf("%02d:00", point.succ) : "" } + ->(point) { (point % 4).zero? ? sprintf("%02d:00", point) : "" } else since = Time.parse(usage.dig(:period, :since)) granularity = usage.dig(:period, :granularity) @@ -148,7 +148,7 @@ def traffic_graph return unless usage options = { - graph_title: "Traffic", + graph_title: "Traffic - #{usage.dig(:metric, :name)}", show_graph_title: true, key: false, area_fill: false, From 536c5d0b41198b03891c7c4cc3a193561b902842 Mon Sep 17 00:00:00 2001 From: "Aleksandar N. Kostadinov" Date: Tue, 25 Apr 2023 14:34:34 +0300 Subject: [PATCH 5/6] usage->period->since is already TimeWithZone --- app/lib/pdf/report.rb | 2 +- test/fixtures/report_mocks.yml | 10 +++++----- test/unit/pdf/report_test.rb | 14 +++++++++++++- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/app/lib/pdf/report.rb b/app/lib/pdf/report.rb index 813b972704..750e7b5d54 100644 --- a/app/lib/pdf/report.rb +++ b/app/lib/pdf/report.rb @@ -136,7 +136,7 @@ def graph_key_formatter(usage) if @period == :day ->(point) { (point % 4).zero? ? sprintf("%02d:00", point) : "" } else - since = Time.parse(usage.dig(:period, :since)) + since = usage.dig(:period, :since) granularity = usage.dig(:period, :granularity) ->(point) { (point % 4).zero? ? (since + point * granularity).strftime("%d %b") : "" } end diff --git a/test/fixtures/report_mocks.yml b/test/fixtures/report_mocks.yml index e20b9e67cc..cd1b3775ce 100644 --- a/test/fixtures/report_mocks.yml +++ b/test/fixtures/report_mocks.yml @@ -66,8 +66,8 @@ usage: unit: hit period: name: week - since: '2022-03-13T00:00:00+01:00' - until: '2022-03-19T23:59:59+01:00' + since: '2022-03-13T00:00:00+02:00' + until: '2022-03-19T23:59:59+02:00' timezone: Europe/Sofia granularity: 21600 total: 327406 @@ -103,9 +103,9 @@ usage: usage_progress_for_all_metrics: period: name: week - since: '2022-03-13T00:00:00+01:00' - until: '2022-03-19T23:59:59+01:00' - timezone: Europe/Vienna + since: '2022-03-13T00:00:00+02:00' + until: '2022-03-19T23:59:59+02:00' + timezone: Europe/Sofia granularity: 21600 metrics: - data: diff --git a/test/unit/pdf/report_test.rb b/test/unit/pdf/report_test.rb index 5ad6d48af8..99980eaf54 100644 --- a/test/unit/pdf/report_test.rb +++ b/test/unit/pdf/report_test.rb @@ -53,10 +53,13 @@ class MultiPageTest < ActiveSupport::TestCase test 'generates a multi-page report' do mock_values = YAML.load_file(file_fixture("report_mocks.yml").to_s).deep_symbolize_keys mock_values[:usage_progress_for_all_metrics][:metrics].each(&:deep_symbolize_keys!) + mock_values.dig(:usage, :period).tap do |period| + period[:since] = Time.parse(period[:since]).in_time_zone(period[:timezone]) + period[:until] = Time.parse(period[:until]).in_time_zone(period[:timezone]) + end Stats::Service.any_instance.stubs(mock_values.slice(*%i[usage_progress_for_all_metrics usage])) Pdf::Data.any_instance.stubs(mock_values.slice(*%i[latest_users top_users users])) - @report.generate text = PDF::Inspector::Text.analyze_file(@report.pdf_file_path) @@ -86,6 +89,15 @@ class MultiPageTest < ActiveSupport::TestCase assert report.generate end + test 'simple week report' do + account = FactoryBot.create(:simple_provider) + service = FactoryBot.create(:simple_service, account: account) + + report = Pdf::Report.new(account, service, period: :week) + + assert report.generate + end + test 'supports service names with special characters and tags' do name_with_special_chars = "Name's with `, & and tag" account = FactoryBot.build_stubbed(:simple_provider) From 9e6a0d155668747fd281906cb738879f8eeca18b Mon Sep 17 00:00:00 2001 From: "Aleksandar N. Kostadinov" Date: Tue, 25 Apr 2023 14:52:34 +0300 Subject: [PATCH 6/6] traffic_graph method is moved --- test/unit/pdf/data_test.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/unit/pdf/data_test.rb b/test/unit/pdf/data_test.rb index 27a5d946c2..dcc99d6ee0 100644 --- a/test/unit/pdf/data_test.rb +++ b/test/unit/pdf/data_test.rb @@ -19,14 +19,14 @@ class Pdf::DataTest < ActiveSupport::TestCase assert_raises(ArgumentError) { Pdf::Data.new(@provider_account) } end - test "generates traffic graph for week" do + test "generates usage data for week" do @data = Pdf::Data.new(@provider_account, @service, :period => :week) - assert_not_nil @data.traffic_graph + assert_not_nil @data.usage end - test "generates traffic graph for day" do + test "generates usage date for day" do @data = Pdf::Data.new(@provider_account, @service, :period => :day) - assert_not_nil @data.traffic_graph + assert_not_nil @data.usage end test 'scope data by service' do