From 6aa8d97f32a00941b47b8f8a7c4a81680fb0de0a Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Thu, 4 Jul 2024 10:03:09 +0800 Subject: [PATCH] FIX: Don't error out when loading a badge with a deleted image (#27688) Badges can have their associated image uploads deleted. When this happens, any user who has that badge will have their profile page error out. After this fix, when deleting an upload that's associated with a badge, we nullify the foreign key ID on the badge. This makes the existing safeguard work correctly. --- app/models/upload.rb | 1 + spec/models/badge_spec.rb | 29 +++++++++++++++++++++++++---- spec/models/upload_spec.rb | 2 ++ 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/app/models/upload.rb b/app/models/upload.rb index ec60031cf0d..03152467f93 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -27,6 +27,7 @@ class Upload < ActiveRecord::Base has_many :upload_references, dependent: :destroy has_many :posts, through: :upload_references, source: :target, source_type: "Post" has_many :topic_thumbnails + has_many :badges, foreign_key: :image_upload_id, dependent: :nullify attr_accessor :for_group_message attr_accessor :for_theme diff --git a/spec/models/badge_spec.rb b/spec/models/badge_spec.rb index c7d4db515ed..eb5e6364f9b 100644 --- a/spec/models/badge_spec.rb +++ b/spec/models/badge_spec.rb @@ -98,12 +98,33 @@ RSpec.describe Badge do end describe "#image_url" do - it "has CDN url" do + before do SiteSetting.enable_s3_uploads = true SiteSetting.s3_cdn_url = "https://some-s3-cdn.amzn.com" - upload = Fabricate(:upload_s3) - badge = Fabricate(:badge, image_upload_id: upload.id) - expect(badge.image_url).to start_with("https://some-s3-cdn.amzn.com") + end + + context "when the badge has an existing image" do + it "has a CDN url" do + upload = Fabricate(:upload_s3) + badge = Fabricate(:badge, image_upload_id: upload.id) + + expect(badge.image_url).to start_with("https://some-s3-cdn.amzn.com") + end + end + + context "when the badge does not have a related image" do + it "does not have a CDN url" do + upload = Fabricate(:upload_s3) + badge = Fabricate(:badge, image_upload_id: upload.id) + + store = stub + store.expects(:remove_upload).returns(true) + Discourse.stubs(:store).returns(store) + + upload.destroy! + + expect(badge.reload.image_url).to eq(nil) + end end end diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index af5cd175b7b..a0e50ca655c 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -16,6 +16,8 @@ RSpec.describe Upload do let(:attachment_path) { __FILE__ } let(:attachment) { File.new(attachment_path) } + it { is_expected.to have_many(:badges).dependent(:nullify) } + describe ".with_no_non_post_relations" do it "does not find non-post related uploads" do post_upload = Fabricate(:upload)