From 72166d7d056b273aa831fc3d022f82cba0f9ff16 Mon Sep 17 00:00:00 2001 From: Minhazur Rahaman Date: Tue, 26 Mar 2024 13:37:00 -0400 Subject: [PATCH 01/16] APPEALS-42711 adjusted query for webex recordings list --- .../hearings/fetch_webex_recordings_list_job.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/app/jobs/hearings/fetch_webex_recordings_list_job.rb b/app/jobs/hearings/fetch_webex_recordings_list_job.rb index 8e3f3271c3a..4605f3369e7 100644 --- a/app/jobs/hearings/fetch_webex_recordings_list_job.rb +++ b/app/jobs/hearings/fetch_webex_recordings_list_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # This job will retrieve a list of webex hearing recordings and details -# in a 24 hours period from the previous day +# every hour class Hearings::FetchWebexRecordingsListJob < CaseflowJob include Hearings::EnsureCurrentUserIsSet @@ -10,9 +10,10 @@ class Hearings::FetchWebexRecordingsListJob < CaseflowJob application_attr :hearing_schedule retry_on(Caseflow::Error::WebexApiError, wait: :exponentially_longer) do |job, exception| - from = 2.days.ago.in_time_zone("America/New_York").end_of_day - to = 1.day.ago.in_time_zone("America/New_York").end_of_day - query = "?from=#{CGI.escape(from.iso8601)}?to=#{CGI.escape(to.iso8601)}" + from = 2.hours.ago.in_time_zone("America/New_York") + to = 1.hour.ago.in_time_zone("America/New_York") + max = 100 + query = "?max=#{max}?from=#{CGI.escape(from.iso8601)}?to=#{CGI.escape(to.iso8601)}" details = { action: "retrieve", filetype: "vtt", @@ -51,9 +52,10 @@ def log_error(error) private def fetch_recordings_list - from = CGI.escape(2.days.ago.in_time_zone("America/New_York").end_of_day.iso8601) - to = CGI.escape(1.day.ago.in_time_zone("America/New_York").end_of_day.iso8601) - query = { "from": from, "to": to } + from = CGI.escape(2.hours.ago.in_time_zone("America/New_York").iso8601) + to = CGI.escape(1.hour.ago.in_time_zone("America/New_York").iso8601) + max = 100 + query = { "from": from, "to": to, "max": max } WebexService.new( host: ENV["WEBEX_HOST_MAIN"], From f0e205c4efd8e065bc301b63433c7cc0ea33e3a9 Mon Sep 17 00:00:00 2001 From: Minhazur Rahaman Date: Wed, 27 Mar 2024 11:17:50 -0400 Subject: [PATCH 02/16] APPEALS-42711 added rspec tests for webex_conference_link --- .../hearings/webex_conference_link_spec.rb | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 spec/models/hearings/webex_conference_link_spec.rb diff --git a/spec/models/hearings/webex_conference_link_spec.rb b/spec/models/hearings/webex_conference_link_spec.rb new file mode 100644 index 00000000000..3b290275254 --- /dev/null +++ b/spec/models/hearings/webex_conference_link_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +describe WebexConferenceLink do + let(:hearing_day) { create(:hearing_day, id: 1) } + let(:link) { create(:webex_conference_link, hearing_day_id: hearing_day.id) } + + describe "method calls" do + it "soft removal of link" do + link + expect(ConferenceLink.all.size).to eq(1) + link.soft_removal_of_link + expect(link.conference_deleted).to eq(true) + expect(ConferenceLink.all.size).to eq(0) + end + + context "#create" do + it "Conference link was created and links generated" do + expect(link.id).not_to eq(nil) + expect(link.host_link).not_to eq(nil) + expect(link.created_by_id).not_to eq(nil) + expect(link.host_link).to eq(link.host_link) + expect(link.guest_hearing_link).to eq(link.guest_hearing_link) + expect(link.guest_pin_long).to eq(link.guest_pin_long) + expect(link.updated_by_id).not_to eq(nil) + end + end + + context "update conference day" do + let(:hash) do + { + host_pin: 12_345_678, + host_pin_long: "12345678", + guest_pin_long: "123456789", + created_at: DateTime.new(2022, 3, 15, 10, 15, 30), + updated_at: DateTime.new(2022, 4, 27, 11, 20, 35) + } + end + + subject { link.update!(hash) } + + it "Conference link was updated" do + subject + + updated_link = ConferenceLink.find(link.id).reload + expect(updated_link.host_pin).to eq(123_456_78) + expect(updated_link.host_pin_long).to eq("12345678") + expect(updated_link.guest_pin_long).to eq("123456789") + expect(updated_link.created_at).to eq(DateTime.new(2022, 3, 15, 10, 15, 30)) + expect(updated_link.updated_at).to eq(DateTime.new(2022, 4, 27, 11, 20, 35)) + expect(updated_link.updated_by_id).not_to eq(nil) + expect(updated_link.created_by_id).not_to eq(nil) + end + end + end +end From b14a68f0c84898c442f633176dd2852a23f76a21 Mon Sep 17 00:00:00 2001 From: Minhazur Rahaman Date: Wed, 27 Mar 2024 12:27:50 -0400 Subject: [PATCH 03/16] APPEALS-42711 some code climate refactors --- .../download_transcription_file_job.rb | 35 ++++++++++++------- .../fetch_webex_recordings_details_job.rb | 12 ++----- .../fetch_webex_recordings_list_job.rb | 12 ++----- client/test/data/hearings.js | 2 +- .../transcript_file_issues_mailer_spec.rb | 18 +++++----- 5 files changed, 36 insertions(+), 43 deletions(-) diff --git a/app/jobs/hearings/download_transcription_file_job.rb b/app/jobs/hearings/download_transcription_file_job.rb index 594af5130db..5050d7e0dbb 100644 --- a/app/jobs/hearings/download_transcription_file_job.rb +++ b/app/jobs/hearings/download_transcription_file_job.rb @@ -19,7 +19,12 @@ class FileDownloadError < StandardError; end class HearingAssociationError < StandardError; end retry_on(FileDownloadError, wait: 5.minutes) do |job, exception| - details = build_error_details("retrieve", "vtt", "from", "Webex", "download_file_to_tmp!", exception) + action_hash = { + action: "retrieve", + direction: "from", + call: "download_file_to_tmp!" + } + details = build_error_details(action_hash, "vtt", "Webex", exception) TranscriptFileIssuesMailer.send_issue_details(details, appeal_id) job.log_error(exception) end @@ -31,13 +36,23 @@ class HearingAssociationError < StandardError; end retry_on(TranscriptionTransformer::FileConversionError, wait: 10.seconds) do |job, exception| job.transcription_file.clean_up_tmp_location - details = build_error_details("convert", "vtt", "to", "rtf", "convert_to_rtf_and_upload_to_s3!", exception) + action_hash = { + action: "convert", + direction: "to", + call: "convert_to_rtf_and_upload_to_s3!" + } + details = build_error_details(action_hash, "vtt", "rtf", exception) TranscriptFileIssuesMailer.send_issue_details(details, appeal_id) job.log_error(exception) end discard_on(FileNameError) do |job, error| - details = build_error_details("retrieve", "vtt", "from", "Webex", "parse_hearing", error) + action_hash = { + action: "retrieve", + direction: "from", + call: "parse_hearing" + } + details = build_error_details(action_hash, "vtt", "Webex", error) TranscriptFileIssuesMailer.send_issue_details(details, appeal_id) Rails.logger.error("#{job.class.name} (#{job.job_id}) discarded with error: #{error}") end @@ -181,26 +196,23 @@ def log_info(message) Rails.logger.info(message) end - # rubocop:disable Metrics/ParameterLists # Purpose: Builds object detailing error for mail template # Params: - # action - string, the action that the job was doing + # action_hash - object, hash object for describing the action that was attempted # filetype - string, the filetype that was getting worked on - # direction - string, either to/from in relative to provider # provider - string, either the destination or starting point - # call - string, the method call where the error occured # error - Exception - the error that was raised # # Returns: The hash for details on the error - def build_error_details(action, filetype, direction, provider, call, error) + def build_error_details(action_hash, filetype, provider, error) { - action: action, + action: action_hash[:action], filetype: filetype, - direction: direction, + direction: action_hash[:direction], provider: provider, error: error, docket_number: hearing.docket_number, - api_call: call + api_call: action_hash[:call] } end @@ -209,5 +221,4 @@ def build_error_details(action, filetype, direction, provider, call, error) def appeal_id hearing.appeal.external_id end - # rubocop:enable Metrics/ParameterLists end diff --git a/app/jobs/hearings/fetch_webex_recordings_details_job.rb b/app/jobs/hearings/fetch_webex_recordings_details_job.rb index e5d5b5d2cad..2eee5f13072 100644 --- a/app/jobs/hearings/fetch_webex_recordings_details_job.rb +++ b/app/jobs/hearings/fetch_webex_recordings_details_job.rb @@ -30,7 +30,8 @@ class Hearings::FetchWebexRecordingsDetailsJob < CaseflowJob docket_number: docket_number } TranscriptFileIssuesMailer.send_issue_details(details, hearing.appeal.external_id) - job.log_error(exception) + Rails.logger.error("Retrying #{self.class.name} because failed with error: #{error}") + log_error(exception, extra: { application: self.class.name, job_id: job.id }) end # rubocop:enable Layout/LineLength @@ -51,15 +52,6 @@ def perform(id:, file_name:) end # rubocop:enable Lint/UnusedMethodArgument - def log_error(error) - Rails.logger.error("Retrying #{self.class.name} because failed with error: #{error}") - extra = { - application: self.class.name, - job_id: job_id - } - Raven.capture_exception(error, extra: extra) - end - private def fetch_recording_details(id) diff --git a/app/jobs/hearings/fetch_webex_recordings_list_job.rb b/app/jobs/hearings/fetch_webex_recordings_list_job.rb index 4605f3369e7..461e8c7ba37 100644 --- a/app/jobs/hearings/fetch_webex_recordings_list_job.rb +++ b/app/jobs/hearings/fetch_webex_recordings_list_job.rb @@ -26,7 +26,8 @@ class Hearings::FetchWebexRecordingsListJob < CaseflowJob times: "From: #{from}, To: #{to}" } TranscriptFileIssuesMailer.webex_recording_list_issues(details) - job.log_error(exception) + Rails.logger.error("Retrying #{self.class.name} because failed with error: #{error}") + log_error(exception, extra: { application: self.class.name, job_id: job.id }) end def perform @@ -40,15 +41,6 @@ def perform end end - def log_error(error) - Rails.logger.error("Retrying #{self.class.name} because failed with error: #{error}") - extra = { - application: self.class.name, - job_id: job_id - } - Raven.capture_exception(error, extra: extra) - end - private def fetch_recordings_list diff --git a/client/test/data/hearings.js b/client/test/data/hearings.js index db1ac41bac1..be5b55cd76d 100644 --- a/client/test/data/hearings.js +++ b/client/test/data/hearings.js @@ -504,7 +504,7 @@ export const amaWebexHearing = { judgeId: 3, judge: { id: 3, - createdAt: '2020-06-25T11:00:43.257-04:00', + createdAt: '2020-06-26T11:00:43.257-04:00', cssId: 'BVAAABSHIRE', efolderDocumentsFetchedAt: null, email: null, diff --git a/spec/mailers/test/transcript_file_issues_mailer_spec.rb b/spec/mailers/test/transcript_file_issues_mailer_spec.rb index 53c3b0ca144..694ce26c8d5 100644 --- a/spec/mailers/test/transcript_file_issues_mailer_spec.rb +++ b/spec/mailers/test/transcript_file_issues_mailer_spec.rb @@ -3,6 +3,12 @@ require "rails_helper" RSpec.describe TranscriptFileIssuesMailer, type: :mailer do + shared_examples "assigns @details" do + expect(mail.body.encoded).to match(details[:action]) + expect(mail.body.encoded).to match(details[:provider]) + expect(mail.body.encoded).to match(details[:docket_number]) + end + describe "#send_issue_details" do let(:details) do { @@ -26,11 +32,7 @@ expect(mail.from).to eq(["BoardofVeteransAppealsHearings@messages.va.gov"]) end - it "assigns @details" do - expect(mail.body.encoded).to match(details[:action]) - expect(mail.body.encoded).to match(details[:provider]) - expect(mail.body.encoded).to match(details[:docket_number]) - end + include_examples "assigns @details" it "assigns @case_link" do expect(mail.body.encoded).to match("localhost:3000/queue/appeals/#{appeal_id}") @@ -92,11 +94,7 @@ expect(mail.from).to eq(["BoardofVeteransAppealsHearings@messages.va.gov"]) end - it "assigns @details" do - expect(mail.body.encoded).to match(details[:action]) - expect(mail.body.encoded).to match(details[:provider]) - expect(mail.body.encoded).to match(details[:docket_number]) - end + include_examples "assigns @details" it "does not assign @case_link if times is present" do details[:times] = "10:00 AM" From 979a056faeca056d9ad052448f40276a84315e19 Mon Sep 17 00:00:00 2001 From: Minhazur Rahaman Date: Wed, 27 Mar 2024 14:13:02 -0400 Subject: [PATCH 04/16] APPEALS-42711 fixed code climate issues and webex service argument refactor --- .../transcription_files_controller.rb | 2 +- .../download_transcription_file_job.rb | 15 +++++++----- .../fetch_webex_recordings_details_job.rb | 17 +++++++------ .../fetch_webex_recordings_list_job.rb | 14 +++++------ .../virtual_hearings/conference_client.rb | 5 ++-- app/models/concerns/hearing_concern.rb | 2 +- app/models/hearings/webex_conference_link.rb | 6 +++-- app/services/external_api/webex_service.rb | 24 +++++++------------ .../external_api/webex_service_spec.rb | 11 +++++---- 9 files changed, 48 insertions(+), 48 deletions(-) diff --git a/app/controllers/hearings/transcription_files_controller.rb b/app/controllers/hearings/transcription_files_controller.rb index ceb678aea9b..119a4e564c1 100644 --- a/app/controllers/hearings/transcription_files_controller.rb +++ b/app/controllers/hearings/transcription_files_controller.rb @@ -9,7 +9,7 @@ class Hearings::TranscriptionFilesController < ApplicationController # Downloads file and sends to user's local computer def download_transcription_file tmp_location = file.download_from_s3 - File.open(tmp_location, "r") { |f| send_data f.read, filename: file.file_name } + File.open(tmp_location, "r") { |stream| send_data stream.read, filename: file.file_name } file.clean_up_tmp_location end diff --git a/app/jobs/hearings/download_transcription_file_job.rb b/app/jobs/hearings/download_transcription_file_job.rb index 5050d7e0dbb..b45d85cb598 100644 --- a/app/jobs/hearings/download_transcription_file_job.rb +++ b/app/jobs/hearings/download_transcription_file_job.rb @@ -22,9 +22,10 @@ class HearingAssociationError < StandardError; end action_hash = { action: "retrieve", direction: "from", + filetype: "vtt", call: "download_file_to_tmp!" } - details = build_error_details(action_hash, "vtt", "Webex", exception) + details = build_error_details(action_hash, "Webex", exception) TranscriptFileIssuesMailer.send_issue_details(details, appeal_id) job.log_error(exception) end @@ -39,9 +40,10 @@ class HearingAssociationError < StandardError; end action_hash = { action: "convert", direction: "to", + filetype: "vtt", call: "convert_to_rtf_and_upload_to_s3!" } - details = build_error_details(action_hash, "vtt", "rtf", exception) + details = build_error_details(action_hash, "rtf", exception) TranscriptFileIssuesMailer.send_issue_details(details, appeal_id) job.log_error(exception) end @@ -50,9 +52,10 @@ class HearingAssociationError < StandardError; end action_hash = { action: "retrieve", direction: "from", + filetype: "vtt", call: "parse_hearing" } - details = build_error_details(action_hash, "vtt", "Webex", error) + details = build_error_details(action_hash, "Webex", error) TranscriptFileIssuesMailer.send_issue_details(details, appeal_id) Rails.logger.error("#{job.class.name} (#{job.job_id}) discarded with error: #{error}") end @@ -63,7 +66,7 @@ class HearingAssociationError < StandardError; end def perform(download_link:, file_name:) ensure_current_user_is_set @file_name = file_name - @transcription_file = find_or_create_transcription_file + @transcription_file ||= find_or_create_transcription_file ensure_hearing_held download_file_to_tmp!(download_link) @@ -204,10 +207,10 @@ def log_info(message) # error - Exception - the error that was raised # # Returns: The hash for details on the error - def build_error_details(action_hash, filetype, provider, error) + def build_error_details(action_hash, provider, error) { action: action_hash[:action], - filetype: filetype, + filetype: action[:filetype], direction: action_hash[:direction], provider: provider, error: error, diff --git a/app/jobs/hearings/fetch_webex_recordings_details_job.rb b/app/jobs/hearings/fetch_webex_recordings_details_job.rb index 2eee5f13072..5eca9e3dc73 100644 --- a/app/jobs/hearings/fetch_webex_recordings_details_job.rb +++ b/app/jobs/hearings/fetch_webex_recordings_details_job.rb @@ -12,7 +12,7 @@ class Hearings::FetchWebexRecordingsDetailsJob < CaseflowJob # rubocop:disable Layout/LineLength retry_on(Caseflow::Error::WebexApiError, wait: :exponentially_longer) do |job, exception| - file_name = job.arguments&.first&.[](:file_name) + file_name = @file_name docket_number, hearing_id, class_name = file_name.split("_") hearing = if class_name == "Hearing" Hearing.find_by(id: hearing_id) @@ -35,9 +35,9 @@ class Hearings::FetchWebexRecordingsDetailsJob < CaseflowJob end # rubocop:enable Layout/LineLength - # rubocop:disable Lint/UnusedMethodArgument - def perform(id:, file_name:) + def perform(id:, file_name: nil) ensure_current_user_is_set + @file_name ||= file_name data = fetch_recording_details(id) topic = data.topic @@ -50,22 +50,21 @@ def perform(id:, file_name:) mp3_link = data.mp3_link send_file(topic, "mp3", mp3_link) end - # rubocop:enable Lint/UnusedMethodArgument private def fetch_recording_details(id) - query = { "id": id } - - WebexService.new( + config = { host: ENV["WEBEX_HOST_MAIN"], port: ENV["WEBEX_PORT"], aud: ENV["WEBEX_ORGANIZATION"], apikey: ENV["WEBEX_BOTTOKEN"], domain: ENV["WEBEX_DOMAIN_MAIN"], api_endpoint: ENV["WEBEX_API_MAIN"], - query: query - ).fetch_recording_details + query: { "id": id } + } + + WebexService.new(config).fetch_recording_details end def create_file_name(topic, extension) diff --git a/app/jobs/hearings/fetch_webex_recordings_list_job.rb b/app/jobs/hearings/fetch_webex_recordings_list_job.rb index 461e8c7ba37..200ec068997 100644 --- a/app/jobs/hearings/fetch_webex_recordings_list_job.rb +++ b/app/jobs/hearings/fetch_webex_recordings_list_job.rb @@ -35,8 +35,8 @@ def perform response = fetch_recordings_list topics = response.topics topic_num = 0 - response.ids.each do |n| - Hearings::FetchWebexRecordingsDetailsJob.perform_later(id: n, topic: topics[topic_num]) + response.ids.each do |id| + Hearings::FetchWebexRecordingsDetailsJob.perform_later(id: id, topic: topics[topic_num]) topic_num += 1 end end @@ -47,16 +47,16 @@ def fetch_recordings_list from = CGI.escape(2.hours.ago.in_time_zone("America/New_York").iso8601) to = CGI.escape(1.hour.ago.in_time_zone("America/New_York").iso8601) max = 100 - query = { "from": from, "to": to, "max": max } - - WebexService.new( + config = { host: ENV["WEBEX_HOST_MAIN"], port: ENV["WEBEX_PORT"], aud: ENV["WEBEX_ORGANIZATION"], apikey: ENV["WEBEX_BOTTOKEN"], domain: ENV["WEBEX_DOMAIN_MAIN"], api_endpoint: ENV["WEBEX_API_MAIN"], - query: query - ).fetch_recordings_list + query: { "from": from, "to": to, "max": max } + } + + WebexService.new(config).fetch_recordings_list end end diff --git a/app/jobs/virtual_hearings/conference_client.rb b/app/jobs/virtual_hearings/conference_client.rb index 602684cd313..426abf7864a 100644 --- a/app/jobs/virtual_hearings/conference_client.rb +++ b/app/jobs/virtual_hearings/conference_client.rb @@ -13,7 +13,7 @@ def client(virtual_hearing) client_host: ENV["PEXIP_CLIENT_HOST"] ) when "webex" - @client ||= WebexService.new( + config = { host: ENV["WEBEX_HOST_IC"], port: ENV["WEBEX_PORT"], aud: ENV["WEBEX_ORGANIZATION"], @@ -21,7 +21,8 @@ def client(virtual_hearing) domain: ENV["WEBEX_DOMAIN_IC"], api_endpoint: ENV["WEBEX_API_IC"], query: nil - ) + } + @client ||= WebexService.new(config) else msg = "Conference Provider for the Virtual Hearing Not Found" fail Caseflow::Error::MeetingTypeNotFoundError, message: msg diff --git a/app/models/concerns/hearing_concern.rb b/app/models/concerns/hearing_concern.rb index 31cbd122d8c..1f5996b55ec 100644 --- a/app/models/concerns/hearing_concern.rb +++ b/app/models/concerns/hearing_concern.rb @@ -106,7 +106,7 @@ def calculate_submission_window # Associate hearing with transcription files across multiple dockets and order accordingly def transcription_files_by_docket_number # Remove hyphen in case of counter at end of file name to allow for alphabetical sort - transcription_files.sort_by { |f| f.file_name.split("-").join }.group_by(&:docket_number).values + transcription_files.sort_by { |file| file.file_name.split("-").join }.group_by(&:docket_number).values end # Group transcription files by docket number before mapping through nested array and serializing diff --git a/app/models/hearings/webex_conference_link.rb b/app/models/hearings/webex_conference_link.rb index 0ece8dee5d1..4c0bce9c8d5 100644 --- a/app/models/hearings/webex_conference_link.rb +++ b/app/models/hearings/webex_conference_link.rb @@ -14,14 +14,16 @@ def guest_link def generate_conference_information meeting_type.update!(service_name: "webex") - conference_response = WebexService.new( + config = { host: ENV["WEBEX_HOST_IC"], port: ENV["WEBEX_PORT"], aud: ENV["WEBEX_ORGANIZATION"], apikey: ENV["WEBEX_BOTTOKEN"], domain: ENV["WEBEX_DOMAIN_IC"], api_endpoint: ENV["WEBEX_API_IC"] - ).create_conference(hearing) + } + + conference_response = WebexService.new(config).create_conference(hearing) update!( host_link: conference_response.host_link, diff --git a/app/services/external_api/webex_service.rb b/app/services/external_api/webex_service.rb index ed2e7ef9dd2..03a81e97162 100644 --- a/app/services/external_api/webex_service.rb +++ b/app/services/external_api/webex_service.rb @@ -3,17 +3,9 @@ require "json" class ExternalApi::WebexService - # rubocop:disable Metrics/ParameterLists - def initialize(host:, port:, aud:, apikey:, domain:, api_endpoint:, query:) - @host = host - @port = port - @aud = aud - @apikey = apikey - @domain = domain - @api_endpoint = api_endpoint - @query = query + def initialize(config:) + @config = config end - # rubocop:enable Metrics/ParameterLists def create_conference(conferenced_item) body = { @@ -22,7 +14,7 @@ def create_conference(conferenced_item) "nbf": conferenced_item.nbf, "exp": conferenced_item.exp }, - "aud": @aud, + "aud": @config[:aud], "numHost": 2, "provideShortUrls": true, "verticalType": "gen" @@ -43,7 +35,7 @@ def delete_conference(conferenced_item) "nbf": 0, "exp": 0 }, - "aud": @aud, + "aud": @config[:aud], "numHost": 2, "provideShortUrls": true, "verticalType": "gen" @@ -79,16 +71,16 @@ def fetch_recording_details # :nocov: def send_webex_request(body, method) - url = "https://#{@host}#{@domain}#{@api_endpoint}" + url = "https://#{@config[:host]}#{@config[:domain]}#{@config[:api_endpoint]}" request = HTTPI::Request.new(url) request.open_timeout = 300 request.read_timeout = 300 request.body = body.to_json unless body.nil? - request.query = @query - request.headers = { "Authorization": "Bearer #{@apikey}", "Content-Type": "application/json" } + request.query = @config[:query] + request.headers = { "Authorization": "Bearer #{@config[:apikey]}", "Content-Type": "application/json" } MetricsService.record( - "#{@host} #{method} request to #{url}", + "#{@config[:host]} #{method} request to #{url}", service: :webex, name: @api_endpoint ) do diff --git a/spec/services/external_api/webex_service_spec.rb b/spec/services/external_api/webex_service_spec.rb index e2f05d2b695..4796fc74db4 100644 --- a/spec/services/external_api/webex_service_spec.rb +++ b/spec/services/external_api/webex_service_spec.rb @@ -8,9 +8,8 @@ let(:domain) { "gov.fake.com" } let(:api_endpoint) { "/api/v2/fake" } let(:query) { nil } - - let(:webex_service) do - ExternalApi::WebexService.new( + let(:config) do + { host: host, domain: domain, api_endpoint: api_endpoint, @@ -18,7 +17,11 @@ apikey: apikey, port: port, query: query - ) + } + end + + let(:webex_service) do + ExternalApi::WebexService.new(config) end describe "webex requests" do From 926c8e0f47ca373063b70984594f22a16af06321 Mon Sep 17 00:00:00 2001 From: Minhazur Rahaman Date: Wed, 27 Mar 2024 15:18:41 -0400 Subject: [PATCH 05/16] APPEALS-42711 fixed som broken rspec tests --- app/jobs/hearings/fetch_webex_recordings_details_job.rb | 4 ++-- app/jobs/hearings/fetch_webex_recordings_list_job.rb | 2 +- app/jobs/virtual_hearings/conference_client.rb | 2 +- app/models/hearings/webex_conference_link.rb | 2 +- spec/services/external_api/webex_service_spec.rb | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/jobs/hearings/fetch_webex_recordings_details_job.rb b/app/jobs/hearings/fetch_webex_recordings_details_job.rb index 5eca9e3dc73..fb6f2ab5a46 100644 --- a/app/jobs/hearings/fetch_webex_recordings_details_job.rb +++ b/app/jobs/hearings/fetch_webex_recordings_details_job.rb @@ -35,7 +35,7 @@ class Hearings::FetchWebexRecordingsDetailsJob < CaseflowJob end # rubocop:enable Layout/LineLength - def perform(id:, file_name: nil) + def perform(id:, file_name:) ensure_current_user_is_set @file_name ||= file_name data = fetch_recording_details(id) @@ -64,7 +64,7 @@ def fetch_recording_details(id) query: { "id": id } } - WebexService.new(config).fetch_recording_details + WebexService.new(config:config).fetch_recording_details end def create_file_name(topic, extension) diff --git a/app/jobs/hearings/fetch_webex_recordings_list_job.rb b/app/jobs/hearings/fetch_webex_recordings_list_job.rb index 200ec068997..f438b3a18e8 100644 --- a/app/jobs/hearings/fetch_webex_recordings_list_job.rb +++ b/app/jobs/hearings/fetch_webex_recordings_list_job.rb @@ -57,6 +57,6 @@ def fetch_recordings_list query: { "from": from, "to": to, "max": max } } - WebexService.new(config).fetch_recordings_list + WebexService.new(config:config).fetch_recordings_list end end diff --git a/app/jobs/virtual_hearings/conference_client.rb b/app/jobs/virtual_hearings/conference_client.rb index 426abf7864a..cc7aec4577f 100644 --- a/app/jobs/virtual_hearings/conference_client.rb +++ b/app/jobs/virtual_hearings/conference_client.rb @@ -22,7 +22,7 @@ def client(virtual_hearing) api_endpoint: ENV["WEBEX_API_IC"], query: nil } - @client ||= WebexService.new(config) + @client ||= WebexService.new(config: config) else msg = "Conference Provider for the Virtual Hearing Not Found" fail Caseflow::Error::MeetingTypeNotFoundError, message: msg diff --git a/app/models/hearings/webex_conference_link.rb b/app/models/hearings/webex_conference_link.rb index 4c0bce9c8d5..f9ad73a037b 100644 --- a/app/models/hearings/webex_conference_link.rb +++ b/app/models/hearings/webex_conference_link.rb @@ -23,7 +23,7 @@ def generate_conference_information api_endpoint: ENV["WEBEX_API_IC"] } - conference_response = WebexService.new(config).create_conference(hearing) + conference_response = WebexService.new(config: config).create_conference(hearing) update!( host_link: conference_response.host_link, diff --git a/spec/services/external_api/webex_service_spec.rb b/spec/services/external_api/webex_service_spec.rb index 4796fc74db4..7737a5d2cfb 100644 --- a/spec/services/external_api/webex_service_spec.rb +++ b/spec/services/external_api/webex_service_spec.rb @@ -21,7 +21,7 @@ end let(:webex_service) do - ExternalApi::WebexService.new(config) + ExternalApi::WebexService.new(config: config) end describe "webex requests" do From 3709abf238124876ee17801430a3d84d789516a6 Mon Sep 17 00:00:00 2001 From: Minhazur Rahaman Date: Thu, 28 Mar 2024 12:20:45 -0400 Subject: [PATCH 06/16] APPEALS-42711 fixed a few code climate issues with mailer --- .../fetch_webex_recordings_list_job.rb | 2 +- app/mailers/transcript_file_issues_mailer.rb | 51 ++++++++++--------- app/services/external_api/webex_service.rb | 11 ++-- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/app/jobs/hearings/fetch_webex_recordings_list_job.rb b/app/jobs/hearings/fetch_webex_recordings_list_job.rb index f438b3a18e8..fcc1fa12b20 100644 --- a/app/jobs/hearings/fetch_webex_recordings_list_job.rb +++ b/app/jobs/hearings/fetch_webex_recordings_list_job.rb @@ -26,7 +26,7 @@ class Hearings::FetchWebexRecordingsListJob < CaseflowJob times: "From: #{from}, To: #{to}" } TranscriptFileIssuesMailer.webex_recording_list_issues(details) - Rails.logger.error("Retrying #{self.class.name} because failed with error: #{error}") + Rails.logger.error("Retrying #{self.class.name} because failed with error: #{exception}") log_error(exception, extra: { application: self.class.name, job_id: job.id }) end diff --git a/app/mailers/transcript_file_issues_mailer.rb b/app/mailers/transcript_file_issues_mailer.rb index da46ca210b1..cb1dcf6ba31 100644 --- a/app/mailers/transcript_file_issues_mailer.rb +++ b/app/mailers/transcript_file_issues_mailer.rb @@ -9,22 +9,11 @@ class TranscriptFileIssuesMailer < ActionMailer::Base # off the template to the above recipients def send_issue_details(details, appeal_id) @details = details - @case_link = case Rails.deploy_env - when :demo - "https://demo.appeals.va.gov/appeals/#{appeal_id}" - when :uat - "https://appeals.cf.uat.ds.va.gov/queue/appeals/#{appeal_id}" - when :prod - "https://appeals.cf.ds.va.gov/queue/appeals/#{appeal_id}" - when :prodtest - "https://appeals.cf.prodtest.ds.va.gov/queue/appeals/#{appeal_id}" - when :preprod - "https://appeals.cf.preprod.ds.va.gov/queue/appeals/#{appeal_id}" - else - "localhost:3000/queue/appeals/#{appeal_id}" - end + @deploy_env = Rails.deploy_env + @config = mailer_config(appeal_id) + @case_link = @config[:link] @subject = "File #{details[:action]} Error - #{details[:provider]} #{details[:docket_number]}" - mail(subject: @subject, to: to_email_address, cc: cc_email_address) do |format| + mail(subject: @subject, to: @mailer_config[:to_email_address], cc: @mailer_config[:cc_email_address]) do |format| format.html { render "layouts/transcript_file_issues" } end end @@ -38,21 +27,33 @@ def webex_recording_list_issues(details) end end - # The email address to send mail to - def to_email_address - case Rails.deploy_env - when :demo, :development, :test - "Caseflow@test.com" + def mailer_config(appeal_id) + case @deploy_env + when :development, :demo, :test + { link: non_external_link(appeal_id), to_email_address: "Caseflow@test.com" } when :uat - "BID_Appeals_UAT@bah.com" + { + link: "https://appeals.cf.uat.ds.va.gov/queue/appeals/#{appeal_id}", + to_email_address: "BID_Appeals_UAT@bah.com" + } + when :prodtest + { link: "https://appeals.cf.prodtest.ds.va.gov/queue/appeals/#{appeal_id}" } + when :preprod + { link: "https://appeals.cf.preprod.ds.va.gov/queue/appeals/#{appeal_id}" } when :prod - "BVAHearingTeam@VA.gov" + { + link: "https://appeals.cf.ds.va.gov/queue/appeals/#{appeal_id}", + to_email_address: "BVAHearingTeam@VA.gov", + cc_email_address: "OITAppealsHelpDesk@va.gov" + } end end - # The email address to cc - def cc_email_address - "OITAppealsHelpDesk@va.gov" if Rails.deploy_env == :prod + # The link for the case details page when not in prod or uat + def non_external_link(appeal_id) + return "https://demo.appeals.va.gov/appeals/#{appeal_id}" if @deploy_env == "demo" + + "localhost:3000/queue/appeals/#{appeal_id}" end end diff --git a/app/services/external_api/webex_service.rb b/app/services/external_api/webex_service.rb index 03a81e97162..6d4376b0db3 100644 --- a/app/services/external_api/webex_service.rb +++ b/app/services/external_api/webex_service.rb @@ -23,9 +23,8 @@ def create_conference(conferenced_item) method = "POST" resp = send_webex_request(body, method) - return if resp.nil? - ExternalApi::WebexService::CreateResponse.new(resp) + ExternalApi::WebexService::CreateResponse.new(resp) if !resp.nil? end def delete_conference(conferenced_item) @@ -46,7 +45,7 @@ def delete_conference(conferenced_item) resp = send_webex_request(body, method) return if resp.nil? - ExternalApi::WebexService::DeleteResponse.new(resp) + ExternalApi::WebexService::DeleteResponse.new(resp) if !resp.nil? end def fetch_recordings_list @@ -55,7 +54,7 @@ def fetch_recordings_list resp = send_webex_request(body, method) return if resp.nil? - ExternalApi::WebexService::RecordingsListResponse.new(resp) + ExternalApi::WebexService::RecordingsListResponse.new(resp) if !resp.nil? end def fetch_recording_details @@ -64,7 +63,7 @@ def fetch_recording_details resp = send_webex_request(body, method) return if resp.nil? - ExternalApi::WebexService::RecordingDetailsResponse.new(resp) + ExternalApi::WebexService::RecordingDetailsResponse.new(resp) if !resp.nil? end private @@ -82,7 +81,7 @@ def send_webex_request(body, method) MetricsService.record( "#{@config[:host]} #{method} request to #{url}", service: :webex, - name: @api_endpoint + name: @config[:api_endpoint] ) do case method when "POST" From ce970b994a1caa2c1a9943ccea560ca2a7a4cd5d Mon Sep 17 00:00:00 2001 From: Minhazur Rahaman Date: Thu, 28 Mar 2024 13:19:06 -0400 Subject: [PATCH 07/16] APPEALS-42711 refactored more code climate issues with mailer and webex implementation --- .../fetch_webex_recordings_details_job.rb | 2 +- .../fetch_webex_recordings_list_job.rb | 2 +- app/mailers/transcript_file_issues_mailer.rb | 5 ++- app/workflows/transcription_transformer.rb | 32 +++++++++++-------- lib/fakes/webex_service.rb | 4 +-- 5 files changed, 25 insertions(+), 20 deletions(-) diff --git a/app/jobs/hearings/fetch_webex_recordings_details_job.rb b/app/jobs/hearings/fetch_webex_recordings_details_job.rb index fb6f2ab5a46..46fd5a3563b 100644 --- a/app/jobs/hearings/fetch_webex_recordings_details_job.rb +++ b/app/jobs/hearings/fetch_webex_recordings_details_job.rb @@ -64,7 +64,7 @@ def fetch_recording_details(id) query: { "id": id } } - WebexService.new(config:config).fetch_recording_details + WebexService.new(config: config).fetch_recording_details end def create_file_name(topic, extension) diff --git a/app/jobs/hearings/fetch_webex_recordings_list_job.rb b/app/jobs/hearings/fetch_webex_recordings_list_job.rb index fcc1fa12b20..bb7d1102d4c 100644 --- a/app/jobs/hearings/fetch_webex_recordings_list_job.rb +++ b/app/jobs/hearings/fetch_webex_recordings_list_job.rb @@ -57,6 +57,6 @@ def fetch_recordings_list query: { "from": from, "to": to, "max": max } } - WebexService.new(config:config).fetch_recordings_list + WebexService.new(config: config).fetch_recordings_list end end diff --git a/app/mailers/transcript_file_issues_mailer.rb b/app/mailers/transcript_file_issues_mailer.rb index cb1dcf6ba31..5ba30757995 100644 --- a/app/mailers/transcript_file_issues_mailer.rb +++ b/app/mailers/transcript_file_issues_mailer.rb @@ -9,7 +9,6 @@ class TranscriptFileIssuesMailer < ActionMailer::Base # off the template to the above recipients def send_issue_details(details, appeal_id) @details = details - @deploy_env = Rails.deploy_env @config = mailer_config(appeal_id) @case_link = @config[:link] @subject = "File #{details[:action]} Error - #{details[:provider]} #{details[:docket_number]}" @@ -28,7 +27,7 @@ def webex_recording_list_issues(details) end def mailer_config(appeal_id) - case @deploy_env + case Rails.deploy_env when :development, :demo, :test { link: non_external_link(appeal_id), to_email_address: "Caseflow@test.com" } when :uat @@ -51,7 +50,7 @@ def mailer_config(appeal_id) # The link for the case details page when not in prod or uat def non_external_link(appeal_id) - return "https://demo.appeals.va.gov/appeals/#{appeal_id}" if @deploy_env == "demo" + return "https://demo.appeals.va.gov/appeals/#{appeal_id}" if Rails.deploy_env == "demo" "localhost:3000/queue/appeals/#{appeal_id}" end diff --git a/app/workflows/transcription_transformer.rb b/app/workflows/transcription_transformer.rb index 202b57fd666..14bb1f18a33 100644 --- a/app/workflows/transcription_transformer.rb +++ b/app/workflows/transcription_transformer.rb @@ -21,7 +21,12 @@ def call if File.exist?(csv_path) paths.push(csv_path) elsif @error_count > 0 - paths.push(build_csv(csv_path, @error_count, @length, @hearing_info)) + error_hash = { + error_count: @error_count, + length: @length, + hearing_info: @hearing_info + } + paths.push(build_csv(csv_path, error_hash)) end paths end @@ -83,12 +88,12 @@ def create_transcription_pages(transcript, document) styles["PS_CODE"].line_spacing = -1 styles["CS_CODE"].underline = true format_transcript(transcript).each do |cue| - document.paragraph(styles["PS_CODE"]) do |n1| - n1.apply(styles["CS_CODE"]) do |n2| - n2 << cue[:identifier].upcase + document.paragraph(styles["PS_CODE"]) do |paragraph_style| + paragraph_style.apply(styles["CS_CODE"]) do |char_style| + char_style << cue[:identifier].upcase end - n1.paragraph << ": #{cue[:text]}" - n1.paragraph + paragraph_style.paragraph << ": #{cue[:text]}" + paragraph_style.paragraph end end end @@ -130,21 +135,22 @@ def create_footer_and_spacing(document) # row - the current row in the document # count - amount of line breaks to add def insert_line_breaks(row, count) - i = 0 + breaks = 0 while i < count row.line_break - i += 1 + breaks += 1 end end - # Builds csv which captures error and details of vtt file - # Params: count - count of how many inaudibles were found - # length - total length of meeting - # hearing_info - object containing info about the hearing + # Params: path - the path to save the csv + # details - hash that has details pertaining to the error # Returns the created csv - def build_csv(path, count, length, hearing_info) + def build_csv(path, details) filename = path.split("/").last.sub(".csv", "") header = %w[length appeal_id hearing_date judge issues filename] + length = details[:length] + count = details[:count] + hearing_info = details[:hearing_info] length_string = "#{(length / 3600).floor}:#{(length / 60 % 60).floor}:#{(length % 60).floor}" CSV.open(path, "w") do |writer| writer << header diff --git a/lib/fakes/webex_service.rb b/lib/fakes/webex_service.rb index a7a88e1c69e..1811d4e922a 100644 --- a/lib/fakes/webex_service.rb +++ b/lib/fakes/webex_service.rb @@ -31,7 +31,7 @@ def create_conference(virtual_hearing) ExternalApi::WebexService::CreateResponse.new( HTTPI::Response.new( 200, - {}, + { virtual_hearing: virtual_hearing }, build_meeting_response ) ) @@ -47,7 +47,7 @@ def delete_conference(virtual_hearing) ExternalApi::WebexService::DeleteResponse.new( HTTPI::Response.new( 200, - {}, + { virtual_hearing: virtual_hearing }, build_meeting_response ) ) From c31d36a3068d9bac0c16bf33a1846e8f68559ad7 Mon Sep 17 00:00:00 2001 From: Minhazur Rahaman Date: Thu, 28 Mar 2024 16:03:10 -0400 Subject: [PATCH 08/16] APPEALS-42711 fixed broken rspec tests and finishing up with clearing up viable code climate issues --- app/mailers/transcript_file_issues_mailer.rb | 7 ++++--- spec/feature/hearings/hearing_details_spec.rb | 3 ++- .../test/transcript_file_issues_mailer_spec.rb | 15 +++++++++------ 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/app/mailers/transcript_file_issues_mailer.rb b/app/mailers/transcript_file_issues_mailer.rb index 5ba30757995..d29dc00126d 100644 --- a/app/mailers/transcript_file_issues_mailer.rb +++ b/app/mailers/transcript_file_issues_mailer.rb @@ -12,7 +12,7 @@ def send_issue_details(details, appeal_id) @config = mailer_config(appeal_id) @case_link = @config[:link] @subject = "File #{details[:action]} Error - #{details[:provider]} #{details[:docket_number]}" - mail(subject: @subject, to: @mailer_config[:to_email_address], cc: @mailer_config[:cc_email_address]) do |format| + mail(subject: @subject, to: @config[:to_email_address], cc: @config[:cc_email_address]) do |format| format.html { render "layouts/transcript_file_issues" } end end @@ -20,8 +20,9 @@ def send_issue_details(details, appeal_id) # Handles specifically the transcript recording list issues def webex_recording_list_issues(details) @details = details + @config = mailer_config(nil) @subject = "File #{details[:action]} Error - #{details[:provider]}" - mail(subject: @subject, to: to_email_address, cc: cc_email_address) do |format| + mail(subject: @subject, to: @config[:to_email_address], cc: @config[:cc_email_address]) do |format| format.html { render "layouts/transcript_file_issues" } end end @@ -50,7 +51,7 @@ def mailer_config(appeal_id) # The link for the case details page when not in prod or uat def non_external_link(appeal_id) - return "https://demo.appeals.va.gov/appeals/#{appeal_id}" if Rails.deploy_env == "demo" + return "https://demo.appeals.va.gov/appeals/#{appeal_id}" if Rails.deploy_env == :demo "localhost:3000/queue/appeals/#{appeal_id}" end diff --git a/spec/feature/hearings/hearing_details_spec.rb b/spec/feature/hearings/hearing_details_spec.rb index f3e555af7b6..dc9947c1f5e 100644 --- a/spec/feature/hearings/hearing_details_spec.rb +++ b/spec/feature/hearings/hearing_details_spec.rb @@ -160,8 +160,9 @@ def check_transcription_file_download(file) end def wait_for_download(file_location) + count = 0 Timeout.timeout(60) do - sleep 1 until !DownloadHelpers.downloading? && File.exist?(file_location) + count += 1 until !DownloadHelpers.downloading? && File.exist?(file_location) end end diff --git a/spec/mailers/test/transcript_file_issues_mailer_spec.rb b/spec/mailers/test/transcript_file_issues_mailer_spec.rb index 694ce26c8d5..c481993421f 100644 --- a/spec/mailers/test/transcript_file_issues_mailer_spec.rb +++ b/spec/mailers/test/transcript_file_issues_mailer_spec.rb @@ -3,12 +3,15 @@ require "rails_helper" RSpec.describe TranscriptFileIssuesMailer, type: :mailer do - shared_examples "assigns @details" do - expect(mail.body.encoded).to match(details[:action]) - expect(mail.body.encoded).to match(details[:provider]) - expect(mail.body.encoded).to match(details[:docket_number]) + shared_examples "mail assignment" do + it "assigns @details" do + expect(mail.body.encoded).to match(details[:action]) + expect(mail.body.encoded).to match(details[:provider]) + expect(mail.body.encoded).to match(details[:docket_number]) + end end + describe "#send_issue_details" do let(:details) do { @@ -32,7 +35,7 @@ expect(mail.from).to eq(["BoardofVeteransAppealsHearings@messages.va.gov"]) end - include_examples "assigns @details" + include_examples "mail assignment" it "assigns @case_link" do expect(mail.body.encoded).to match("localhost:3000/queue/appeals/#{appeal_id}") @@ -94,7 +97,7 @@ expect(mail.from).to eq(["BoardofVeteransAppealsHearings@messages.va.gov"]) end - include_examples "assigns @details" + include_examples "mail assignment" it "does not assign @case_link if times is present" do details[:times] = "10:00 AM" From 2ebd4cd0b8db635d5cdce3b13d3f919410e9ee26 Mon Sep 17 00:00:00 2001 From: Minhazur Rahaman Date: Fri, 29 Mar 2024 12:51:52 -0400 Subject: [PATCH 09/16] APPEALS-42711 fixed some broken rspec tests and more code climate issues --- .../fetch_webex_recordings_details_job.rb | 14 ++++++-- .../fetch_webex_recordings_list_job.rb | 36 ++++++++++--------- app/services/external_api/webex_service.rb | 11 ------ .../transcript_file_issues_mailer_spec.rb | 1 - 4 files changed, 30 insertions(+), 32 deletions(-) diff --git a/app/jobs/hearings/fetch_webex_recordings_details_job.rb b/app/jobs/hearings/fetch_webex_recordings_details_job.rb index 46fd5a3563b..9c73b5b4d34 100644 --- a/app/jobs/hearings/fetch_webex_recordings_details_job.rb +++ b/app/jobs/hearings/fetch_webex_recordings_details_job.rb @@ -12,7 +12,7 @@ class Hearings::FetchWebexRecordingsDetailsJob < CaseflowJob # rubocop:disable Layout/LineLength retry_on(Caseflow::Error::WebexApiError, wait: :exponentially_longer) do |job, exception| - file_name = @file_name + file_name = @file_name || job.arguments&.first&.[](:file_name) docket_number, hearing_id, class_name = file_name.split("_") hearing = if class_name == "Hearing" Hearing.find_by(id: hearing_id) @@ -30,8 +30,7 @@ class Hearings::FetchWebexRecordingsDetailsJob < CaseflowJob docket_number: docket_number } TranscriptFileIssuesMailer.send_issue_details(details, hearing.appeal.external_id) - Rails.logger.error("Retrying #{self.class.name} because failed with error: #{error}") - log_error(exception, extra: { application: self.class.name, job_id: job.id }) + job.log_error(exception) end # rubocop:enable Layout/LineLength @@ -51,6 +50,15 @@ def perform(id:, file_name:) send_file(topic, "mp3", mp3_link) end + def log_error(error) + Rails.logger.error("Retrying #{self.class.name} because failed with error: #{error}") + extra = { + application: self.class.name, + job_id: job_id + } + Raven.capture_exception(error, extra: extra) + end + private def fetch_recording_details(id) diff --git a/app/jobs/hearings/fetch_webex_recordings_list_job.rb b/app/jobs/hearings/fetch_webex_recordings_list_job.rb index bb7d1102d4c..c24c292fb37 100644 --- a/app/jobs/hearings/fetch_webex_recordings_list_job.rb +++ b/app/jobs/hearings/fetch_webex_recordings_list_job.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # This job will retrieve a list of webex hearing recordings and details -# every hour +# in a 24 hours period from the previous day class Hearings::FetchWebexRecordingsListJob < CaseflowJob include Hearings::EnsureCurrentUserIsSet @@ -10,10 +10,9 @@ class Hearings::FetchWebexRecordingsListJob < CaseflowJob application_attr :hearing_schedule retry_on(Caseflow::Error::WebexApiError, wait: :exponentially_longer) do |job, exception| - from = 2.hours.ago.in_time_zone("America/New_York") - to = 1.hour.ago.in_time_zone("America/New_York") - max = 100 - query = "?max=#{max}?from=#{CGI.escape(from.iso8601)}?to=#{CGI.escape(to.iso8601)}" + from = 2.days.ago.in_time_zone("America/New_York").end_of_day + to = 1.day.ago.in_time_zone("America/New_York").end_of_day + query = "?from=#{CGI.escape(from.iso8601)}?to=#{CGI.escape(to.iso8601)}" details = { action: "retrieve", filetype: "vtt", @@ -26,8 +25,7 @@ class Hearings::FetchWebexRecordingsListJob < CaseflowJob times: "From: #{from}, To: #{to}" } TranscriptFileIssuesMailer.webex_recording_list_issues(details) - Rails.logger.error("Retrying #{self.class.name} because failed with error: #{exception}") - log_error(exception, extra: { application: self.class.name, job_id: job.id }) + job.log_error(exception) end def perform @@ -35,28 +33,32 @@ def perform response = fetch_recordings_list topics = response.topics topic_num = 0 - response.ids.each do |id| - Hearings::FetchWebexRecordingsDetailsJob.perform_later(id: id, topic: topics[topic_num]) + response.ids.each do |n| + Hearings::FetchWebexRecordingsDetailsJob.perform_later(id: n, topic: topics[topic_num]) topic_num += 1 end end + def log_error(error) + Rails.logger.error("Retrying #{self.class.name} because failed with error: #{error}") + Raven.capture_exception(error, extra: { application: self.class.name, job_id: job_id }) + end + private def fetch_recordings_list - from = CGI.escape(2.hours.ago.in_time_zone("America/New_York").iso8601) - to = CGI.escape(1.hour.ago.in_time_zone("America/New_York").iso8601) - max = 100 - config = { + from = CGI.escape(2.days.ago.in_time_zone("America/New_York").end_of_day.iso8601) + to = CGI.escape(1.day.ago.in_time_zone("America/New_York").end_of_day.iso8601) + query = { "from": from, "to": to } + + WebexService.new( host: ENV["WEBEX_HOST_MAIN"], port: ENV["WEBEX_PORT"], aud: ENV["WEBEX_ORGANIZATION"], apikey: ENV["WEBEX_BOTTOKEN"], domain: ENV["WEBEX_DOMAIN_MAIN"], api_endpoint: ENV["WEBEX_API_MAIN"], - query: { "from": from, "to": to, "max": max } - } - - WebexService.new(config: config).fetch_recordings_list + query: query + ).fetch_recordings_list end end diff --git a/app/services/external_api/webex_service.rb b/app/services/external_api/webex_service.rb index 6d4376b0db3..4b7643d1df2 100644 --- a/app/services/external_api/webex_service.rb +++ b/app/services/external_api/webex_service.rb @@ -19,11 +19,8 @@ def create_conference(conferenced_item) "provideShortUrls": true, "verticalType": "gen" } - method = "POST" - resp = send_webex_request(body, method) - ExternalApi::WebexService::CreateResponse.new(resp) if !resp.nil? end @@ -39,12 +36,8 @@ def delete_conference(conferenced_item) "provideShortUrls": true, "verticalType": "gen" } - method = "POST" - resp = send_webex_request(body, method) - return if resp.nil? - ExternalApi::WebexService::DeleteResponse.new(resp) if !resp.nil? end @@ -52,8 +45,6 @@ def fetch_recordings_list body = nil method = "GET" resp = send_webex_request(body, method) - return if resp.nil? - ExternalApi::WebexService::RecordingsListResponse.new(resp) if !resp.nil? end @@ -61,8 +52,6 @@ def fetch_recording_details body = nil method = "GET" resp = send_webex_request(body, method) - return if resp.nil? - ExternalApi::WebexService::RecordingDetailsResponse.new(resp) if !resp.nil? end diff --git a/spec/mailers/test/transcript_file_issues_mailer_spec.rb b/spec/mailers/test/transcript_file_issues_mailer_spec.rb index c481993421f..8eb8bdf9358 100644 --- a/spec/mailers/test/transcript_file_issues_mailer_spec.rb +++ b/spec/mailers/test/transcript_file_issues_mailer_spec.rb @@ -11,7 +11,6 @@ end end - describe "#send_issue_details" do let(:details) do { From 7d86c07327776c1c40c41fc10097d5669f93bc86 Mon Sep 17 00:00:00 2001 From: Minhazur Rahaman Date: Fri, 29 Mar 2024 13:30:50 -0400 Subject: [PATCH 10/16] APPEALS-42711 changed names of classes and methods to fix code climate issue --- .../hearings/transcription_files_controller.rb | 2 +- .../hearings/download_transcription_file_job.rb | 16 +++++++++------- app/models/hearings/transcription_file.rb | 4 ++-- ...ile_to_s3.rb => transcription_file_upload.rb} | 2 +- spec/models/hearings/transcription_file_spec.rb | 2 +- 5 files changed, 14 insertions(+), 12 deletions(-) rename app/workflows/{upload_transcription_file_to_s3.rb => transcription_file_upload.rb} (97%) diff --git a/app/controllers/hearings/transcription_files_controller.rb b/app/controllers/hearings/transcription_files_controller.rb index 119a4e564c1..a3c4f389529 100644 --- a/app/controllers/hearings/transcription_files_controller.rb +++ b/app/controllers/hearings/transcription_files_controller.rb @@ -8,7 +8,7 @@ class Hearings::TranscriptionFilesController < ApplicationController # Downloads file and sends to user's local computer def download_transcription_file - tmp_location = file.download_from_s3 + tmp_location = file.fetch_from_s3 File.open(tmp_location, "r") { |stream| send_data stream.read, filename: file.file_name } file.clean_up_tmp_location end diff --git a/app/jobs/hearings/download_transcription_file_job.rb b/app/jobs/hearings/download_transcription_file_job.rb index b45d85cb598..900f2d55c52 100644 --- a/app/jobs/hearings/download_transcription_file_job.rb +++ b/app/jobs/hearings/download_transcription_file_job.rb @@ -30,7 +30,7 @@ class HearingAssociationError < StandardError; end job.log_error(exception) end - retry_on(UploadTranscriptionFileToS3::FileUploadError, wait: :exponentially_longer) do |job, exception| + retry_on(TranscriptionFileUpload::FileUploadError, wait: :exponentially_longer) do |job, exception| job.transcription_file.clean_up_tmp_location job.log_error(exception) end @@ -102,16 +102,17 @@ def log_error(error) # # Returns: Updated @transcription_file def download_file_to_tmp!(link) - return if File.exist?(@transcription_file.tmp_location) + transcription_file = @transcription_file + return if File.exist?(transcription_file.tmp_location) URI(link).open do |download| - IO.copy_stream(download, @transcription_file.tmp_location) + IO.copy_stream(download, transcription_file.tmp_location) end - @transcription_file.update_status!(process: :retrieval, status: :success) + transcription_file.update_status!(process: :retrieval, status: :success) log_info("File #{file_name} successfully downloaded from Webex. Uploading to S3...") rescue OpenURI::HTTPError => error - @transcription_file.update_status!(process: :retrieval, status: :failure) - @transcription_file.clean_up_tmp_location + transcription_file.update_status!(process: :retrieval, status: :failure) + transcription_file.clean_up_tmp_location raise FileDownloadError, "Webex temporary download link responded with error: #{error}" end @@ -171,7 +172,8 @@ def find_or_create_transcription_file(file_name_arg = file_name) # Returns: integer value of 1 if tmp file deleted after successful upload def convert_to_rtf_and_upload_to_s3! log_info("Converting file #{file_name} to rtf...") - file_paths = @transcription_file.convert_to_rtf! + transcription_file = @transcription_file + file_paths = transcription_file.convert_to_rtf! file_paths.each do |file_path| output_file_name = file_path.split("/").last output_file = find_or_create_transcription_file(output_file_name) diff --git a/app/models/hearings/transcription_file.rb b/app/models/hearings/transcription_file.rb index 12f05033502..eb7aedb33fa 100644 --- a/app/models/hearings/transcription_file.rb +++ b/app/models/hearings/transcription_file.rb @@ -12,14 +12,14 @@ class TranscriptionFile < CaseflowRecord # Purpose: Fetches file from S3 # Return: The temporary save location of the file - def download_from_s3 + def fetch_from_s3 S3Service.fetch_file(aws_link, tmp_location) tmp_location end # Purpose: Uploads transcription file to its corresponding location in S3 def upload_to_s3! - UploadTranscriptionFileToS3.new(self).call + TranscriptionFileUpload.new(self).call end # Purpose: Converts transcription file from vtt to rtf diff --git a/app/workflows/upload_transcription_file_to_s3.rb b/app/workflows/transcription_file_upload.rb similarity index 97% rename from app/workflows/upload_transcription_file_to_s3.rb rename to app/workflows/transcription_file_upload.rb index d9030cc631e..0ac2d2273d8 100644 --- a/app/workflows/upload_transcription_file_to_s3.rb +++ b/app/workflows/transcription_file_upload.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class UploadTranscriptionFileToS3 +class TranscriptionFileUpload attr_reader :file_name, :file_type S3_SUB_BUCKET = "vaec-appeals-caseflow" diff --git a/spec/models/hearings/transcription_file_spec.rb b/spec/models/hearings/transcription_file_spec.rb index 8cdf8d46b39..18526ece900 100644 --- a/spec/models/hearings/transcription_file_spec.rb +++ b/spec/models/hearings/transcription_file_spec.rb @@ -12,7 +12,7 @@ describe "model functions" do it "downloading a file" do - expect(uploaded_file.download_from_s3).to eq("#{Rails.root}/tmp/transcription_files/vtt/transcript.vtt") + expect(uploaded_file.fetch_from_s3).to eq("#{Rails.root}/tmp/transcription_files/vtt/transcript.vtt") end it "uploading a file" do From 3d4325f1a8637a258721612129b33a845d9408fa Mon Sep 17 00:00:00 2001 From: Minhazur Rahaman Date: Fri, 29 Mar 2024 13:53:53 -0400 Subject: [PATCH 11/16] APPEALS-42711 changed names of classes and methods to fix code climate issue --- app/controllers/hearings/transcription_files_controller.rb | 2 +- app/jobs/hearings/fetch_webex_recordings_list_job.rb | 4 ++-- app/models/hearings/transcription_file.rb | 2 +- spec/models/hearings/transcription_file_spec.rb | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/controllers/hearings/transcription_files_controller.rb b/app/controllers/hearings/transcription_files_controller.rb index a3c4f389529..aecdf590b02 100644 --- a/app/controllers/hearings/transcription_files_controller.rb +++ b/app/controllers/hearings/transcription_files_controller.rb @@ -8,7 +8,7 @@ class Hearings::TranscriptionFilesController < ApplicationController # Downloads file and sends to user's local computer def download_transcription_file - tmp_location = file.fetch_from_s3 + tmp_location = file.download_file_from_s3 File.open(tmp_location, "r") { |stream| send_data stream.read, filename: file.file_name } file.clean_up_tmp_location end diff --git a/app/jobs/hearings/fetch_webex_recordings_list_job.rb b/app/jobs/hearings/fetch_webex_recordings_list_job.rb index c24c292fb37..a67a40074fb 100644 --- a/app/jobs/hearings/fetch_webex_recordings_list_job.rb +++ b/app/jobs/hearings/fetch_webex_recordings_list_job.rb @@ -33,8 +33,8 @@ def perform response = fetch_recordings_list topics = response.topics topic_num = 0 - response.ids.each do |n| - Hearings::FetchWebexRecordingsDetailsJob.perform_later(id: n, topic: topics[topic_num]) + response.ids.each do |id| + Hearings::FetchWebexRecordingsDetailsJob.perform_later(id: id, topic: topics[topic_num]) topic_num += 1 end end diff --git a/app/models/hearings/transcription_file.rb b/app/models/hearings/transcription_file.rb index eb7aedb33fa..efb2c747ffd 100644 --- a/app/models/hearings/transcription_file.rb +++ b/app/models/hearings/transcription_file.rb @@ -12,7 +12,7 @@ class TranscriptionFile < CaseflowRecord # Purpose: Fetches file from S3 # Return: The temporary save location of the file - def fetch_from_s3 + def download_file_from_s3 S3Service.fetch_file(aws_link, tmp_location) tmp_location end diff --git a/spec/models/hearings/transcription_file_spec.rb b/spec/models/hearings/transcription_file_spec.rb index 18526ece900..14e8602b05f 100644 --- a/spec/models/hearings/transcription_file_spec.rb +++ b/spec/models/hearings/transcription_file_spec.rb @@ -12,7 +12,7 @@ describe "model functions" do it "downloading a file" do - expect(uploaded_file.fetch_from_s3).to eq("#{Rails.root}/tmp/transcription_files/vtt/transcript.vtt") + expect(uploaded_file.download_file_from_s3).to eq("#{Rails.root}/tmp/transcription_files/vtt/transcript.vtt") end it "uploading a file" do From 9c296c0c15d8d967c14951995a015a0f64f4de03 Mon Sep 17 00:00:00 2001 From: Minhazur Rahaman Date: Fri, 29 Mar 2024 14:40:32 -0400 Subject: [PATCH 12/16] APPEALS-42711 changed names of classes and methods to fix code climate issue --- app/controllers/hearings/transcription_files_controller.rb | 2 +- app/models/hearings/transcription_file.rb | 2 +- spec/models/hearings/transcription_file_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/hearings/transcription_files_controller.rb b/app/controllers/hearings/transcription_files_controller.rb index aecdf590b02..9701580f50e 100644 --- a/app/controllers/hearings/transcription_files_controller.rb +++ b/app/controllers/hearings/transcription_files_controller.rb @@ -8,7 +8,7 @@ class Hearings::TranscriptionFilesController < ApplicationController # Downloads file and sends to user's local computer def download_transcription_file - tmp_location = file.download_file_from_s3 + tmp_location = file.fetch_file_from_s3! File.open(tmp_location, "r") { |stream| send_data stream.read, filename: file.file_name } file.clean_up_tmp_location end diff --git a/app/models/hearings/transcription_file.rb b/app/models/hearings/transcription_file.rb index efb2c747ffd..6342e0f8fc6 100644 --- a/app/models/hearings/transcription_file.rb +++ b/app/models/hearings/transcription_file.rb @@ -12,7 +12,7 @@ class TranscriptionFile < CaseflowRecord # Purpose: Fetches file from S3 # Return: The temporary save location of the file - def download_file_from_s3 + def fetch_file_from_s3! S3Service.fetch_file(aws_link, tmp_location) tmp_location end diff --git a/spec/models/hearings/transcription_file_spec.rb b/spec/models/hearings/transcription_file_spec.rb index 14e8602b05f..7939dfb3822 100644 --- a/spec/models/hearings/transcription_file_spec.rb +++ b/spec/models/hearings/transcription_file_spec.rb @@ -12,7 +12,7 @@ describe "model functions" do it "downloading a file" do - expect(uploaded_file.download_file_from_s3).to eq("#{Rails.root}/tmp/transcription_files/vtt/transcript.vtt") + expect(uploaded_file.fetch_file_from_s3!).to eq("#{Rails.root}/tmp/transcription_files/vtt/transcript.vtt") end it "uploading a file" do From 8b23f807d42a0fcd18334e792c4a250bf67b6882 Mon Sep 17 00:00:00 2001 From: Minhazur Rahaman Date: Mon, 1 Apr 2024 12:09:37 -0400 Subject: [PATCH 13/16] APPEALS-42711 added max value back in to webex recordings --- app/jobs/hearings/fetch_webex_recordings_list_job.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/jobs/hearings/fetch_webex_recordings_list_job.rb b/app/jobs/hearings/fetch_webex_recordings_list_job.rb index a67a40074fb..98b67d05f0a 100644 --- a/app/jobs/hearings/fetch_webex_recordings_list_job.rb +++ b/app/jobs/hearings/fetch_webex_recordings_list_job.rb @@ -12,7 +12,8 @@ class Hearings::FetchWebexRecordingsListJob < CaseflowJob retry_on(Caseflow::Error::WebexApiError, wait: :exponentially_longer) do |job, exception| from = 2.days.ago.in_time_zone("America/New_York").end_of_day to = 1.day.ago.in_time_zone("America/New_York").end_of_day - query = "?from=#{CGI.escape(from.iso8601)}?to=#{CGI.escape(to.iso8601)}" + max = 100 + query = "?max=#{max}from=#{CGI.escape(from.iso8601)}?to=#{CGI.escape(to.iso8601)}" details = { action: "retrieve", filetype: "vtt", @@ -49,7 +50,8 @@ def log_error(error) def fetch_recordings_list from = CGI.escape(2.days.ago.in_time_zone("America/New_York").end_of_day.iso8601) to = CGI.escape(1.day.ago.in_time_zone("America/New_York").end_of_day.iso8601) - query = { "from": from, "to": to } + max = 100 + query = { "from": from, "to": to, "max": max } WebexService.new( host: ENV["WEBEX_HOST_MAIN"], From 1e3d2bfffdd5de87cc19ea14b2be64c97d570f71 Mon Sep 17 00:00:00 2001 From: Minhazur Rahaman Date: Mon, 1 Apr 2024 12:12:08 -0400 Subject: [PATCH 14/16] APPEALS-42711 added max value back in to webex recordings --- app/jobs/hearings/fetch_webex_recordings_list_job.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/jobs/hearings/fetch_webex_recordings_list_job.rb b/app/jobs/hearings/fetch_webex_recordings_list_job.rb index 98b67d05f0a..b72f3bb3fef 100644 --- a/app/jobs/hearings/fetch_webex_recordings_list_job.rb +++ b/app/jobs/hearings/fetch_webex_recordings_list_job.rb @@ -13,7 +13,7 @@ class Hearings::FetchWebexRecordingsListJob < CaseflowJob from = 2.days.ago.in_time_zone("America/New_York").end_of_day to = 1.day.ago.in_time_zone("America/New_York").end_of_day max = 100 - query = "?max=#{max}from=#{CGI.escape(from.iso8601)}?to=#{CGI.escape(to.iso8601)}" + query = "?max=#{max}?from=#{CGI.escape(from.iso8601)}?to=#{CGI.escape(to.iso8601)}" details = { action: "retrieve", filetype: "vtt", From d853be7660f2fa6560b69e4f998640aa65ec625c Mon Sep 17 00:00:00 2001 From: Minhazur Rahaman Date: Mon, 1 Apr 2024 12:27:10 -0400 Subject: [PATCH 15/16] min/APPEALS-42711 adjusted query --- app/jobs/hearings/fetch_webex_recordings_list_job.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/jobs/hearings/fetch_webex_recordings_list_job.rb b/app/jobs/hearings/fetch_webex_recordings_list_job.rb index b72f3bb3fef..f8db0c14cf3 100644 --- a/app/jobs/hearings/fetch_webex_recordings_list_job.rb +++ b/app/jobs/hearings/fetch_webex_recordings_list_job.rb @@ -10,8 +10,8 @@ class Hearings::FetchWebexRecordingsListJob < CaseflowJob application_attr :hearing_schedule retry_on(Caseflow::Error::WebexApiError, wait: :exponentially_longer) do |job, exception| - from = 2.days.ago.in_time_zone("America/New_York").end_of_day - to = 1.day.ago.in_time_zone("America/New_York").end_of_day + from = 2.hours.ago.in_time_zone("America/New_York") + to = 1.hour.ago.in_time_zone("America/New_York") max = 100 query = "?max=#{max}?from=#{CGI.escape(from.iso8601)}?to=#{CGI.escape(to.iso8601)}" details = { @@ -48,8 +48,8 @@ def log_error(error) private def fetch_recordings_list - from = CGI.escape(2.days.ago.in_time_zone("America/New_York").end_of_day.iso8601) - to = CGI.escape(1.day.ago.in_time_zone("America/New_York").end_of_day.iso8601) + from = CGI.escape(2.hours.ago.in_time_zone("America/New_York").iso8601) + to = CGI.escape(1.hour.ago.in_time_zone("America/New_York").iso8601) max = 100 query = { "from": from, "to": to, "max": max } From 04008ad17955f071b621ecb09fdb49fea2075796 Mon Sep 17 00:00:00 2001 From: Minhazur Rahaman Date: Tue, 2 Apr 2024 10:21:09 -0400 Subject: [PATCH 16/16] APPEALS-42711 refactored some code --- app/jobs/hearings/fetch_webex_recordings_details_job.rb | 3 +-- app/jobs/hearings/fetch_webex_recordings_list_job.rb | 6 ++---- .../hearings/fetch_webex_recordings_details_job_spec.rb | 2 +- spec/jobs/hearings/fetch_webex_recordings_list_job_spec.rb | 2 +- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/app/jobs/hearings/fetch_webex_recordings_details_job.rb b/app/jobs/hearings/fetch_webex_recordings_details_job.rb index 9c73b5b4d34..54515ecd4dd 100644 --- a/app/jobs/hearings/fetch_webex_recordings_details_job.rb +++ b/app/jobs/hearings/fetch_webex_recordings_details_job.rb @@ -51,12 +51,11 @@ def perform(id:, file_name:) end def log_error(error) - Rails.logger.error("Retrying #{self.class.name} because failed with error: #{error}") extra = { application: self.class.name, job_id: job_id } - Raven.capture_exception(error, extra: extra) + super(error, extra: extra) end private diff --git a/app/jobs/hearings/fetch_webex_recordings_list_job.rb b/app/jobs/hearings/fetch_webex_recordings_list_job.rb index f8db0c14cf3..1e41861d9c5 100644 --- a/app/jobs/hearings/fetch_webex_recordings_list_job.rb +++ b/app/jobs/hearings/fetch_webex_recordings_list_job.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true -# This job will retrieve a list of webex hearing recordings and details -# in a 24 hours period from the previous day +# This job will retrieve a list of webex hearing recordings and details every hour class Hearings::FetchWebexRecordingsListJob < CaseflowJob include Hearings::EnsureCurrentUserIsSet @@ -41,8 +40,7 @@ def perform end def log_error(error) - Rails.logger.error("Retrying #{self.class.name} because failed with error: #{error}") - Raven.capture_exception(error, extra: { application: self.class.name, job_id: job_id }) + super(error, extra: { application: self.class.name, job_id: job_id }) end private diff --git a/spec/jobs/hearings/fetch_webex_recordings_details_job_spec.rb b/spec/jobs/hearings/fetch_webex_recordings_details_job_spec.rb index d92ac545c57..1826e6d5743 100644 --- a/spec/jobs/hearings/fetch_webex_recordings_details_job_spec.rb +++ b/spec/jobs/hearings/fetch_webex_recordings_details_job_spec.rb @@ -58,7 +58,7 @@ it "retries and logs errors" do subject - expect(Rails.logger).to receive(:error).with(/Retrying/) + expect(Rails.logger).to receive(:error).at_least(:once) perform_enqueued_jobs { described_class.perform_later(id: id, file_name: file_name) } end end diff --git a/spec/jobs/hearings/fetch_webex_recordings_list_job_spec.rb b/spec/jobs/hearings/fetch_webex_recordings_list_job_spec.rb index a20c96e96ed..e8970091297 100644 --- a/spec/jobs/hearings/fetch_webex_recordings_list_job_spec.rb +++ b/spec/jobs/hearings/fetch_webex_recordings_list_job_spec.rb @@ -30,7 +30,7 @@ it "retries and logs errors" do subject - expect(Rails.logger).to receive(:error).with(/Retrying/) + expect(Rails.logger).to receive(:error).at_least(:once) perform_enqueued_jobs { described_class.perform_later } end end