diff --git a/app/jobs/onceoff/migrate_group_flair_images.rb b/app/jobs/onceoff/migrate_group_flair_images.rb index d400cfa17d2..75616fd7454 100644 --- a/app/jobs/onceoff/migrate_group_flair_images.rb +++ b/app/jobs/onceoff/migrate_group_flair_images.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true +require 'uri' module Jobs class MigrateGroupFlairImages < ::Jobs::Onceoff @@ -12,6 +13,8 @@ module Jobs end old_url = group[:flair_url] + next if old_url.blank? || old_url !~ URI::regexp + group_name = group.name count = 0 @@ -42,7 +45,7 @@ module Jobs Discourse::InvalidParameters => e logger.warn( - "Error encountered when trying to download file " + + "Error encountered when trying to download from URL '#{old_url}' " + "for group '#{group_name}'.\n#{e.class}: #{e.message}\n#{e.backtrace.join("\n")}" ) end diff --git a/spec/jobs/migrate_group_flair_images_spec.rb b/spec/jobs/migrate_group_flair_images_spec.rb index a06b025d264..a1c0c512523 100644 --- a/spec/jobs/migrate_group_flair_images_spec.rb +++ b/spec/jobs/migrate_group_flair_images_spec.rb @@ -4,16 +4,21 @@ require 'rails_helper' RSpec.describe Jobs::MigrateGroupFlairImages do let(:image_url) { "https://omg.aws.somestack/test.png" } - let(:group) { Fabricate(:group, flair_url: image_url) } before do stub_request(:get, image_url).to_return( status: 200, body: file_from_fixtures("smallest.png").read ) + @orig_logger = Rails.logger + Rails.logger = @fake_logger = FakeLogger.new + end + + after do + Rails.logger = @orig_logger end it 'should migrate to the new group `flair_upload_id` column correctly' do - group + group = Fabricate(:group, flair_url: image_url) expect do described_class.new.execute_onceoff({}) @@ -23,4 +28,10 @@ RSpec.describe Jobs::MigrateGroupFlairImages do expect(group.flair_upload).to eq(Upload.last) expect(group[:flair_url]).to eq(nil) end + + it 'should skip groups with invalid flair URLs' do + group = Fabricate(:group, flair_url: "abc") + described_class.new.execute_onceoff({}) + expect(Rails.logger.warnings.count).to eq(0) + end end