From da675bb47e37da4778a94741427230ffeb1ede67 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 2 Jan 2019 16:16:15 +0100 Subject: [PATCH 1/2] api: fix formatting error when a request fails --- app/lib/helpscout/api.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/lib/helpscout/api.rb b/app/lib/helpscout/api.rb index 1b321b166..c01fb29b6 100644 --- a/app/lib/helpscout/api.rb +++ b/app/lib/helpscout/api.rb @@ -69,7 +69,7 @@ class Helpscout::API response = call_api(:get, 'reports/conversations?' + params.to_query) if !response.success? - raise StandardError, "Error while fetching conversation report: #{response.status} '#{response.body}'" + raise StandardError, "Error while fetching conversation report: #{response.response_code} '#{response.body}'" end parse_response_body(response) From 86b9e2d0922e4cd6c39bfacd3b307ab652250b4f Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 3 Jan 2019 16:24:24 +0000 Subject: [PATCH 2/2] stats: fix exception when HelpScout env vars are missing This fixes the stats page, which used to raise an exception when HelpScout env vars are not present. --- app/controllers/stats_controller.rb | 7 ++++- app/lib/helpscout/api.rb | 9 +++++++ .../helpscout/user_conversations_adapter.rb | 12 ++++++--- .../user_conversations_adapter_spec.rb | 26 ++++++++++++++++--- 4 files changed, 47 insertions(+), 7 deletions(-) diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index 40729b7d4..52cb324d2 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -168,7 +168,12 @@ class StatsController < ApplicationController from = Date.new(2018, 1) to = Date.today.prev_month - Helpscout::UserConversationsAdapter.new(from, to) + adapter = Helpscout::UserConversationsAdapter.new(from, to) + if !adapter.can_fetch_reports? + return nil + end + + adapter .reports .map do |monthly_report| start_date = monthly_report[:start_date].to_time.localtime diff --git a/app/lib/helpscout/api.rb b/app/lib/helpscout/api.rb index c01fb29b6..e7cdbc2db 100644 --- a/app/lib/helpscout/api.rb +++ b/app/lib/helpscout/api.rb @@ -7,6 +7,15 @@ class Helpscout::API PHONES = 'phones' OAUTH2_TOKEN = 'oauth2/token' + def ready? + required_secrets = [ + Rails.application.secrets.helpscout[:mailbox_id], + Rails.application.secrets.helpscout[:client_id], + Rails.application.secrets.helpscout[:client_secret] + ] + required_secrets.all?(&:present?) + end + def add_tags(conversation_id, tags) call_api(:put, "#{CONVERSATIONS}/#{conversation_id}/#{TAGS}", { tags: tags diff --git a/app/lib/helpscout/user_conversations_adapter.rb b/app/lib/helpscout/user_conversations_adapter.rb index 6b265e13e..f62662a0d 100644 --- a/app/lib/helpscout/user_conversations_adapter.rb +++ b/app/lib/helpscout/user_conversations_adapter.rb @@ -7,6 +7,10 @@ class Helpscout::UserConversationsAdapter @to = to end + def can_fetch_reports? + api_client.ready? + end + # Return an array of monthly reports def reports @reports ||= (@from..@to) @@ -35,15 +39,17 @@ class Helpscout::UserConversationsAdapter } end + def api_client + @api_client ||= Helpscout::API.new + end + def fetch_conversations_report(year, month) if year == Date.today.year && month == Date.today.month raise ArgumentError, 'The report for the current month will change in the future, and cannot be cached.' end - @helpscout_api ||= Helpscout::API.new - Rails.cache.fetch("helpscout-conversation-report-#{year}-#{month}") do - @helpscout_api.conversations_report(year, month) + api_client.conversations_report(year, month) end end end diff --git a/spec/lib/helpscout/user_conversations_adapter_spec.rb b/spec/lib/helpscout/user_conversations_adapter_spec.rb index 126b92590..7978bbb54 100644 --- a/spec/lib/helpscout/user_conversations_adapter_spec.rb +++ b/spec/lib/helpscout/user_conversations_adapter_spec.rb @@ -1,10 +1,30 @@ require 'spec_helper' describe Helpscout::UserConversationsAdapter do - describe '#reports', vcr: { cassette_name: 'helpscout_conversations_reports' } do - let(:from) { Date.new(2017, 11) } - let(:to) { Date.new(2017, 12) } + let(:from) { Date.new(2017, 11) } + let(:to) { Date.new(2017, 12) } + describe '#can_fetch_reports?' do + context 'when a required secret is missing' do + before do + Rails.application.secrets.helpscout[:mailbox_id] = nil + end + + it { expect(described_class.new(from, to).can_fetch_reports?).to be false } + end + + context 'when all required secrets are present' do + before do + Rails.application.secrets.helpscout[:mailbox_id] = '9999' + Rails.application.secrets.helpscout[:client_id] = '1234' + Rails.application.secrets.helpscout[:client_secret] = '5678' + end + + it { expect(described_class.new(from, to).can_fetch_reports?).to be true } + end + end + + describe '#reports', vcr: { cassette_name: 'helpscout_conversations_reports' } do before { Rails.cache.clear } subject { described_class.new(from, to) }