From 24347ace10ad54f7f3bbc687bc7bc37734f558e4 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Mon, 29 Apr 2019 11:58:52 +0800 Subject: [PATCH] FIX: Properly associate user_profiles background urls via upload id. `Upload#url` is more likely and can change from time to time. When it does changes, we don't want to have to look through multiple tables to ensure that the URLs are all up to date. Instead, we simply associate uploads properly to `UserProfile` so that it does not have to replicate the URLs in the table. --- .../components/user-card-contents.js.es6 | 4 +- .../controllers/preferences/profile.js.es6 | 4 +- .../discourse/controllers/user.js.es6 | 4 +- .../javascripts/discourse/models/user.js.es6 | 8 +- .../templates/preferences/profile.hbs | 6 +- .../javascripts/discourse/templates/user.hbs | 4 +- app/controllers/session_controller.rb | 12 ++- app/controllers/users_controller.rb | 4 +- .../recover_user_profile_backgrounds.rb | 25 ------ app/jobs/scheduled/clean_up_uploads.rb | 6 +- app/models/discourse_single_sign_on.rb | 4 +- app/models/user.rb | 2 + app/models/user_profile.rb | 53 ++++++------ app/serializers/user_serializer.rb | 26 +++--- app/services/user_anonymizer.rb | 10 ++- app/services/user_merger.rb | 4 +- app/services/user_updater.rb | 10 ++- ...dd_upload_foreign_keys_to_user_profiles.rb | 34 ++++++++ lib/migration/column_dropper.rb | 18 ++-- lib/upload_recovery.rb | 37 --------- lib/validators/upload_url_validator.rb | 15 ---- script/import_scripts/discuz_x.rb | 2 +- script/import_scripts/lithium.rb | 2 +- script/import_scripts/nodebb/nodebb.rb | 2 +- script/import_scripts/vbulletin.rb | 2 +- script/import_scripts/vbulletin5.rb | 2 +- ...load_foreign_keys_to_user_profiles_spec.rb | 79 ++++++++++++++++++ .../migrate/add_uploads_to_categories_spec.rb | 44 ++++++---- spec/jobs/clean_up_uploads_spec.rb | 4 +- .../recover_user_profile_backgrounds_spec.rb | 41 --------- spec/lib/upload_recovery_spec.rb | 54 ------------ spec/models/discourse_single_sign_on_spec.rb | 83 ++++++++----------- spec/models/user_profile_spec.rb | 21 +---- spec/requests/session_controller_spec.rb | 10 ++- spec/requests/users_controller_spec.rb | 13 ++- spec/serializers/user_serializer_spec.rb | 26 ++---- spec/services/user_anonymizer_spec.rb | 22 +++-- spec/services/user_merger_spec.rb | 17 +++- spec/services/user_updater_spec.rb | 30 ++++--- 39 files changed, 360 insertions(+), 384 deletions(-) delete mode 100644 app/jobs/onceoff/recover_user_profile_backgrounds.rb create mode 100644 db/migrate/20190426011148_add_upload_foreign_keys_to_user_profiles.rb delete mode 100644 lib/validators/upload_url_validator.rb create mode 100644 spec/db/migrate/add_upload_foreign_keys_to_user_profiles_spec.rb delete mode 100644 spec/jobs/recover_user_profile_backgrounds_spec.rb diff --git a/app/assets/javascripts/discourse/components/user-card-contents.js.es6 b/app/assets/javascripts/discourse/components/user-card-contents.js.es6 index 83e83ac9b08..d78d1d6db2f 100644 --- a/app/assets/javascripts/discourse/components/user-card-contents.js.es6 +++ b/app/assets/javascripts/discourse/components/user-card-contents.js.es6 @@ -131,7 +131,7 @@ export default Ember.Component.extend( } }, - @observes("user.card_background") + @observes("user.card_background_upload_url") addBackground() { if (!this.get("allowBackgrounds")) { return; @@ -142,7 +142,7 @@ export default Ember.Component.extend( return; } - const url = this.get("user.card_background"); + const url = this.get("user.card_background_upload_url"); const bg = Ember.isEmpty(url) ? "" : `url(${Discourse.getURLWithCDN(url)})`; diff --git a/app/assets/javascripts/discourse/controllers/preferences/profile.js.es6 b/app/assets/javascripts/discourse/controllers/preferences/profile.js.es6 index 87fe1fa3c42..803eea2d876 100644 --- a/app/assets/javascripts/discourse/controllers/preferences/profile.js.es6 +++ b/app/assets/javascripts/discourse/controllers/preferences/profile.js.es6 @@ -10,8 +10,8 @@ export default Ember.Controller.extend(PreferencesTabController, { "location", "custom_fields", "user_fields", - "profile_background", - "card_background", + "profile_background_upload_url", + "card_background_upload_url", "date_of_birth" ], diff --git a/app/assets/javascripts/discourse/controllers/user.js.es6 b/app/assets/javascripts/discourse/controllers/user.js.es6 index 038dfccfbd0..85e6c3fa673 100644 --- a/app/assets/javascripts/discourse/controllers/user.js.es6 +++ b/app/assets/javascripts/discourse/controllers/user.js.es6 @@ -21,8 +21,8 @@ export default Ember.Controller.extend(CanCheckEmails, { return !profileHidden && viewingSelf; }, - @computed("model.profileBackground") - hasProfileBackground(background) { + @computed("model.profileBackgroundUrl") + hasProfileBackgroundUrl(background) { return !Ember.isEmpty(background.toString()); }, diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index e80e060cef3..4af96febe85 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -77,8 +77,8 @@ const User = RestModel.extend({ return username; }, - @computed("profile_background") - profileBackground(bgUrl) { + @computed("profile_background_upload_url") + profileBackgroundUrl(bgUrl) { if ( Ember.isEmpty(bgUrl) || !Discourse.SiteSettings.allow_profile_backgrounds @@ -250,8 +250,8 @@ const User = RestModel.extend({ "user_fields", "muted_usernames", "ignored_usernames", - "profile_background", - "card_background", + "profile_background_upload_url", + "card_background_upload_url", "muted_tags", "tracked_tags", "watched_tags", diff --git a/app/assets/javascripts/discourse/templates/preferences/profile.hbs b/app/assets/javascripts/discourse/templates/preferences/profile.hbs index b906cfe75d1..3d5a2bcc750 100644 --- a/app/assets/javascripts/discourse/templates/preferences/profile.hbs +++ b/app/assets/javascripts/discourse/templates/preferences/profile.hbs @@ -32,7 +32,8 @@
- {{image-uploader imageUrl=model.profile_background type="profile_background"}} + {{image-uploader imageUrl=model.profile_background_upload_url + type="profile_background"}}
{{i18n 'user.change_profile_background.instructions'}} @@ -42,7 +43,8 @@
- {{image-uploader imageUrl=model.card_background type="card_background"}} + {{image-uploader imageUrl=model.card_background_upload_url + type="card_background"}}
{{i18n 'user.change_card_background.instructions'}} diff --git a/app/assets/javascripts/discourse/templates/user.hbs b/app/assets/javascripts/discourse/templates/user.hbs index afee93b845d..04a4ab8dca8 100644 --- a/app/assets/javascripts/discourse/templates/user.hbs +++ b/app/assets/javascripts/discourse/templates/user.hbs @@ -1,7 +1,7 @@
{{#d-section class="user-main"}} -
+
{{#unless collapsedInfo}} {{#if showStaffCounters}}
@@ -34,7 +34,7 @@ {{/if}}
{{/if}} - + {{/unless}}
diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 47d6c5c93a2..52594e27725 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -69,12 +69,16 @@ class SessionController < ApplicationController sso.avatar_url = UrlHelper.absolute Discourse.store.cdn_url(avatar_url) end - if current_user.user_profile.profile_background.present? - sso.profile_background_url = UrlHelper.absolute upload_cdn_path(current_user.user_profile.profile_background) + if current_user.user_profile.profile_background_upload.present? + sso.profile_background_url = UrlHelper.absolute(upload_cdn_path( + current_user.user_profile.profile_background_upload.url + )) end - if current_user.user_profile.card_background.present? - sso.card_background_url = UrlHelper.absolute upload_cdn_path(current_user.user_profile.card_background) + if current_user.user_profile.card_background_upload.present? + sso.card_background_url = UrlHelper.absolute(upload_cdn_path( + current_user.user_profile.card_background_upload.url + )) end if request.xhr? diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 972c1b6e449..bd04b320f8f 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1241,8 +1241,8 @@ class UsersController < ApplicationController :location, :website, :dismissed_banner_key, - :profile_background, - :card_background + :profile_background_upload_url, + :card_background_upload_url ] permitted << { custom_fields: User.editable_user_custom_fields } unless User.editable_user_custom_fields.blank? diff --git a/app/jobs/onceoff/recover_user_profile_backgrounds.rb b/app/jobs/onceoff/recover_user_profile_backgrounds.rb deleted file mode 100644 index b96523bceec..00000000000 --- a/app/jobs/onceoff/recover_user_profile_backgrounds.rb +++ /dev/null @@ -1,25 +0,0 @@ -module Jobs - class RecoverUserProfileBackgrounds < Jobs::Onceoff - def execute_onceoff(_) - base_url = Discourse.store.absolute_base_url - return if !base_url.match?(/s3\.dualstack/) - - old = base_url.sub('s3.dualstack.', 's3-') - old_like = %"#{old}%" - - DB.exec(<<~SQL, from: old, to: base_url, old_like: old_like) - UPDATE user_profiles - SET profile_background = replace(profile_background, :from, :to) - WHERE profile_background ilike :old_like - SQL - - DB.exec(<<~SQL, from: old, to: base_url, old_like: old_like) - UPDATE user_profiles - SET card_background = replace(card_background, :from, :to) - WHERE card_background ilike :old_like - SQL - - UploadRecovery.new.recover_user_profile_backgrounds - end - end -end diff --git a/app/jobs/scheduled/clean_up_uploads.rb b/app/jobs/scheduled/clean_up_uploads.rb index 031d18d5b73..2367ae5e8a8 100644 --- a/app/jobs/scheduled/clean_up_uploads.rb +++ b/app/jobs/scheduled/clean_up_uploads.rb @@ -56,7 +56,8 @@ module Jobs .joins("LEFT JOIN post_uploads pu ON pu.upload_id = uploads.id") .joins("LEFT JOIN users u ON u.uploaded_avatar_id = uploads.id") .joins("LEFT JOIN user_avatars ua ON ua.gravatar_upload_id = uploads.id OR ua.custom_upload_id = uploads.id") - .joins("LEFT JOIN user_profiles up ON up.profile_background = uploads.url OR up.card_background = uploads.url") + .joins("LEFT JOIN user_profiles up2 ON up2.profile_background = uploads.url OR up2.card_background = uploads.url") + .joins("LEFT JOIN user_profiles up ON up.profile_background_upload_id = uploads.id OR up.card_background_upload_id = uploads.id") .joins("LEFT JOIN categories c ON c.uploaded_logo_id = uploads.id OR c.uploaded_background_id = uploads.id") .joins("LEFT JOIN custom_emojis ce ON ce.upload_id = uploads.id") .joins("LEFT JOIN theme_fields tf ON tf.upload_id = uploads.id") @@ -64,7 +65,8 @@ module Jobs .where("pu.upload_id IS NULL") .where("u.uploaded_avatar_id IS NULL") .where("ua.gravatar_upload_id IS NULL AND ua.custom_upload_id IS NULL") - .where("up.profile_background IS NULL AND up.card_background IS NULL") + .where("up.profile_background_upload_id IS NULL AND up.card_background_upload_id IS NULL") + .where("up2.profile_background IS NULL AND up2.card_background IS NULL") .where("c.uploaded_logo_id IS NULL AND c.uploaded_background_id IS NULL") .where("ce.upload_id IS NULL") .where("tf.upload_id IS NULL") diff --git a/app/models/discourse_single_sign_on.rb b/app/models/discourse_single_sign_on.rb index ef7b925eacd..00d4bad0495 100644 --- a/app/models/discourse_single_sign_on.rb +++ b/app/models/discourse_single_sign_on.rb @@ -266,7 +266,7 @@ class DiscourseSingleSignOn < SingleSignOn end end - profile_background_missing = user.user_profile.profile_background.blank? || Upload.get_from_url(user.user_profile.profile_background).blank? + profile_background_missing = user.user_profile.profile_background_upload.blank? || Upload.get_from_url(user.user_profile.profile_background_upload.url).blank? if (profile_background_missing || SiteSetting.sso_overrides_profile_background) && profile_background_url.present? profile_background_changed = sso_record.external_profile_background_url != profile_background_url if profile_background_changed || profile_background_missing @@ -278,7 +278,7 @@ class DiscourseSingleSignOn < SingleSignOn end end - card_background_missing = user.user_profile.card_background.blank? || Upload.get_from_url(user.user_profile.card_background).blank? + card_background_missing = user.user_profile.card_background_upload.blank? || Upload.get_from_url(user.user_profile.card_background_upload.url).blank? if (card_background_missing || SiteSetting.sso_overrides_profile_background) && card_background_url.present? card_background_changed = sso_record.external_card_background_url != card_background_url if card_background_changed || card_background_missing diff --git a/app/models/user.rb b/app/models/user.rb index 7245e809969..79d55de023e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -79,6 +79,8 @@ class User < ActiveRecord::Base has_one :user_stat, dependent: :destroy has_one :user_profile, dependent: :destroy, inverse_of: :user + has_one :profile_background_upload, through: :user_profile + has_one :card_background_upload, through: :user_profile has_one :single_sign_on_record, dependent: :destroy belongs_to :approved_by, class_name: 'User' belongs_to :primary_group, class_name: 'Group' diff --git a/app/models/user_profile.rb b/app/models/user_profile.rb index 4d36ab55655..a810c308663 100644 --- a/app/models/user_profile.rb +++ b/app/models/user_profile.rb @@ -1,7 +1,13 @@ require_dependency 'upload_creator' class UserProfile < ActiveRecord::Base + self.ignored_columns = %w{ + card_background + profile_background + } belongs_to :user, inverse_of: :user_profile + belongs_to :card_background_upload, class_name: "Upload" + belongs_to :profile_background_upload, class_name: "Upload" validates :bio_raw, length: { maximum: 3000 } validates :website, url: true, allow_blank: true, if: Proc.new { |c| c.new_record? || c.website_changed? } @@ -9,9 +15,6 @@ class UserProfile < ActiveRecord::Base before_save :cook after_save :trigger_badges - validates :profile_background, upload_url: true, if: :profile_background_changed? - validates :card_background, upload_url: true, if: :card_background_changed? - validate :website_domain_validator, if: Proc.new { |c| c.new_record? || c.website_changed? } has_many :user_profile_views, dependent: :destroy @@ -40,23 +43,19 @@ class UserProfile < ActiveRecord::Base end def upload_card_background(upload) - self.card_background = upload.url - self.save! + self.update!(card_background_upload: upload) end def clear_card_background - self.card_background = "" - self.save! + self.update!(card_background_upload: nil) end def upload_profile_background(upload) - self.profile_background = upload.url - self.save! + self.update!(profile_background_upload: upload) end def clear_profile_background - self.profile_background = "" - self.save! + self.update!(profile_background_upload: nil) end def self.rebake_old(limit) @@ -150,21 +149,25 @@ end # # Table name: user_profiles # -# user_id :integer not null, primary key -# location :string -# website :string -# bio_raw :text -# bio_cooked :text -# profile_background :string(255) -# dismissed_banner_key :integer -# bio_cooked_version :integer -# badge_granted_title :boolean default(FALSE) -# card_background :string(255) -# views :integer default(0), not null +# user_id :integer not null, primary key +# location :string +# website :string +# bio_raw :text +# bio_cooked :text +# profile_background :string(255) +# dismissed_banner_key :integer +# bio_cooked_version :integer +# badge_granted_title :boolean default(FALSE) +# card_background :string(255) +# views :integer default(0), not null +# profile_background_upload_id :integer +# card_background_upload_id :integer # # Indexes # -# index_user_profiles_on_bio_cooked_version (bio_cooked_version) -# index_user_profiles_on_card_background (card_background) -# index_user_profiles_on_profile_background (profile_background) +# index_user_profiles_on_bio_cooked_version (bio_cooked_version) +# index_user_profiles_on_card_background (card_background) +# index_user_profiles_on_card_background_upload_id (card_background_upload_id) +# index_user_profiles_on_profile_background (profile_background) +# index_user_profiles_on_profile_background_upload_id (profile_background_upload_id) # diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index 74ac617e861..5a1b0a0bcd3 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -41,8 +41,6 @@ class UserSerializer < BasicUserSerializer :created_at, :website, :website_name, - :profile_background, - :card_background, :location, :can_edit, :can_edit_username, @@ -80,7 +78,9 @@ class UserSerializer < BasicUserSerializer :second_factor_enabled, :second_factor_backup_enabled, :second_factor_remaining_backup_codes, - :associated_accounts + :associated_accounts, + :profile_background_upload_url, + :card_background_upload_url has_one :invited_by, embed: :object, serializer: BasicUserSerializer has_many :groups, embed: :object, serializer: BasicGroupSerializer @@ -127,8 +127,8 @@ class UserSerializer < BasicUserSerializer :location, :website, :website_name, - :profile_background, - :card_background + :profile_background_upload_url, + :card_background_upload_url ### ### ATTRIBUTES @@ -243,14 +243,6 @@ class UserSerializer < BasicUserSerializer website.present? end - def profile_background - object.user_profile.profile_background - end - - def card_background - object.user_profile.card_background - end - def location object.user_profile.location end @@ -491,4 +483,12 @@ class UserSerializer < BasicUserSerializer scope.is_staff? end + def profile_background_upload_url + object.profile_background_upload&.url + end + + def card_background_upload_url + object.card_background_upload&.url + end + end diff --git a/app/services/user_anonymizer.rb b/app/services/user_anonymizer.rb index bde3d3c6090..a5bef8ba05a 100644 --- a/app/services/user_anonymizer.rb +++ b/app/services/user_anonymizer.rb @@ -47,8 +47,14 @@ class UserAnonymizer options.save! if profile = @user.user_profile - profile.update(location: nil, website: nil, bio_raw: nil, bio_cooked: nil, - profile_background: nil, card_background: nil) + profile.update!( + location: nil, + website: nil, + bio_raw: nil, + bio_cooked: nil, + profile_background_upload: nil, + card_background_upload: nil + ) end @user.user_avatar.try(:destroy) diff --git a/app/services/user_merger.rb b/app/services/user_merger.rb index 7635c03b81d..b192aa24aa7 100644 --- a/app/services/user_merger.rb +++ b/app/services/user_merger.rb @@ -221,10 +221,10 @@ class UserMerger bio_raw = COALESCE(t.bio_raw, s.bio_raw), bio_cooked = COALESCE(t.bio_cooked, s.bio_cooked), bio_cooked_version = COALESCE(t.bio_cooked_version, s.bio_cooked_version), - profile_background = COALESCE(t.profile_background, s.profile_background), + profile_background_upload_id = COALESCE(t.profile_background_upload_id, s.profile_background_upload_id), dismissed_banner_key = COALESCE(t.dismissed_banner_key, s.dismissed_banner_key), badge_granted_title = t.badge_granted_title OR s.badge_granted_title, - card_background = COALESCE(t.card_background, s.card_background), + card_background_upload_id = COALESCE(t.card_background_upload_id, s.card_background_upload_id), views = t.views + s.views FROM user_profiles AS s WHERE t.user_id = :target_user_id AND s.user_id = :source_user_id diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index eda38fb0736..1caf4eae5e3 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -54,8 +54,14 @@ class UserUpdater unless SiteSetting.enable_sso && SiteSetting.sso_overrides_bio user_profile.bio_raw = attributes.fetch(:bio_raw) { user_profile.bio_raw } end - user_profile.profile_background = attributes.fetch(:profile_background) { user_profile.profile_background } - user_profile.card_background = attributes.fetch(:card_background) { user_profile.card_background } + + if upload = Upload.get_from_url(attributes[:profile_background_upload_url]) + user_profile.upload_profile_background(upload) + end + + if upload = Upload.get_from_url(attributes[:card_background_upload_url]) + user_profile.upload_card_background(upload) + end old_user_name = user.name.present? ? user.name : "" user.name = attributes.fetch(:name) { user.name } diff --git a/db/migrate/20190426011148_add_upload_foreign_keys_to_user_profiles.rb b/db/migrate/20190426011148_add_upload_foreign_keys_to_user_profiles.rb new file mode 100644 index 00000000000..2c6d78f782b --- /dev/null +++ b/db/migrate/20190426011148_add_upload_foreign_keys_to_user_profiles.rb @@ -0,0 +1,34 @@ +require 'migration/column_dropper' + +class AddUploadForeignKeysToUserProfiles < ActiveRecord::Migration[5.2] + def up + %i{profile_background card_background}.each do |column| + Migration::ColumnDropper.mark_readonly(:user_profiles, column) + end + + add_column :user_profiles, :profile_background_upload_id, :integer, null: true + add_column :user_profiles, :card_background_upload_id, :integer, null: true + add_foreign_key :user_profiles, :uploads, column: :profile_background_upload_id + add_foreign_key :user_profiles, :uploads, column: :card_background_upload_id + + execute <<~SQL + UPDATE user_profiles up1 + SET profile_background_upload_id = u.id + FROM user_profiles up2 + INNER JOIN uploads u ON u.url = up2.profile_background + WHERE up1.user_id = up2.user_id + SQL + + execute <<~SQL + UPDATE user_profiles up1 + SET card_background_upload_id = u.id + FROM user_profiles up2 + INNER JOIN uploads u ON u.url = up2.card_background + WHERE up1.user_id = up2.user_id + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/migration/column_dropper.rb b/lib/migration/column_dropper.rb index a2c421ab52f..ee892af6e09 100644 --- a/lib/migration/column_dropper.rb +++ b/lib/migration/column_dropper.rb @@ -20,17 +20,19 @@ module Migration columns.each do |column| column = column.to_s - - DB.exec <<~SQL - DROP FUNCTION IF EXISTS #{BaseDropper.readonly_function_name(table, column)} CASCADE; - -- Backward compatibility for old functions created in the public - -- schema - DROP FUNCTION IF EXISTS #{BaseDropper.old_readonly_function_name(table, column)} CASCADE; - SQL - + self.drop_readonly(table, column) # safe cause it is protected on method entry, can not be passed in params DB.exec("ALTER TABLE #{table} DROP COLUMN IF EXISTS #{column}") end end + + def self.drop_readonly(table, column) + DB.exec <<~SQL + DROP FUNCTION IF EXISTS #{BaseDropper.readonly_function_name(table, column)} CASCADE; + -- Backward compatibility for old functions created in the public + -- schema + DROP FUNCTION IF EXISTS #{BaseDropper.old_readonly_function_name(table, column)} CASCADE; + SQL + end end end diff --git a/lib/upload_recovery.rb b/lib/upload_recovery.rb index eba98491041..83acb51aced 100644 --- a/lib/upload_recovery.rb +++ b/lib/upload_recovery.rb @@ -44,45 +44,8 @@ class UploadRecovery end end - def recover_user_profile_backgrounds - UserProfile - .where("profile_background IS NOT NULL OR card_background IS NOT NULL") - .find_each do |user_profile| - - %i{card_background profile_background}.each do |column| - background = user_profile.public_send(column) - - if background.present? && !Upload.exists?(url: background) - data = Upload.extract_url(background) - next unless data - sha1 = data[2] - - if @dry_run - puts "#{background}" - else - recover_user_profile_background(sha1, user_profile.user_id) do |upload| - user_profile.update!("#{column}" => upload.url) if upload.persisted? - end - end - end - end - end - end - private - def recover_user_profile_background(sha1, user_id, &block) - return unless valid_sha1?(sha1) - - attributes = { sha1: sha1, user_id: user_id } - - if Discourse.store.external? - recover_from_s3(attributes, &block) - else - recover_from_local(attributes, &block) - end - end - def recover_post_upload(post, sha1) return unless valid_sha1?(sha1) diff --git a/lib/validators/upload_url_validator.rb b/lib/validators/upload_url_validator.rb deleted file mode 100644 index e1f1b790b81..00000000000 --- a/lib/validators/upload_url_validator.rb +++ /dev/null @@ -1,15 +0,0 @@ -class UploadUrlValidator < ActiveModel::EachValidator - def validate_each(record, attribute, value) - if value.present? - uri = - begin - URI.parse(value) - rescue URI::Error - end - - unless uri && Upload.exists?(url: value) - record.errors.add(attribute, options[:message] || I18n.t('errors.messages.invalid')) - end - end - end -end diff --git a/script/import_scripts/discuz_x.rb b/script/import_scripts/discuz_x.rb index 71094ec46af..5530a605e81 100644 --- a/script/import_scripts/discuz_x.rb +++ b/script/import_scripts/discuz_x.rb @@ -199,7 +199,7 @@ class ImportScripts::DiscuzX < ImportScripts::Base end end end - if !user['spacecss'].blank? && newmember.user_profile.profile_background.blank? + if !user['spacecss'].blank? && newmember.user_profile.profile_background_upload.blank? # profile background if matched = user['spacecss'].match(/body\s*{[^}]*url\('?(.+?)'?\)/i) body_background = matched[1].split(ORIGINAL_SITE_PREFIX, 2).last diff --git a/script/import_scripts/lithium.rb b/script/import_scripts/lithium.rb index 9398a6e8c71..ff9654e6576 100644 --- a/script/import_scripts/lithium.rb +++ b/script/import_scripts/lithium.rb @@ -291,7 +291,7 @@ class ImportScripts::Lithium < ImportScripts::Base return if !upload.persisted? - imported_user.user_profile.update(profile_background: upload.url) + imported_user.user_profile.upload_profile_background(upload) ensure file.close rescue nil file.unlink rescue nil diff --git a/script/import_scripts/nodebb/nodebb.rb b/script/import_scripts/nodebb/nodebb.rb index d198d5d8c25..86ba05632e8 100644 --- a/script/import_scripts/nodebb/nodebb.rb +++ b/script/import_scripts/nodebb/nodebb.rb @@ -281,7 +281,7 @@ class ImportScripts::NodeBB < ImportScripts::Base return if !upload.persisted? - imported_user.user_profile.update(profile_background: upload.url) + imported_user.user_profile.upload_profile_background(upload) ensure string_io.close rescue nil file.close rescue nil diff --git a/script/import_scripts/vbulletin.rb b/script/import_scripts/vbulletin.rb index 092d8d536bd..47c87d25844 100644 --- a/script/import_scripts/vbulletin.rb +++ b/script/import_scripts/vbulletin.rb @@ -254,7 +254,7 @@ EOM return if !upload.persisted? - imported_user.user_profile.update(profile_background: upload.url) + imported_user.user_profile.upload_profile_background(upload) ensure file.close rescue nil file.unlink rescue nil diff --git a/script/import_scripts/vbulletin5.rb b/script/import_scripts/vbulletin5.rb index 1bab839561b..424e9210537 100644 --- a/script/import_scripts/vbulletin5.rb +++ b/script/import_scripts/vbulletin5.rb @@ -164,7 +164,7 @@ class ImportScripts::VBulletin < ImportScripts::Base return if !upload.persisted? - imported_user.user_profile.update(profile_background: upload.url) + imported_user.user_profile.upload_profile_background(upload) ensure file.close rescue nil file.unlink rescue nil diff --git a/spec/db/migrate/add_upload_foreign_keys_to_user_profiles_spec.rb b/spec/db/migrate/add_upload_foreign_keys_to_user_profiles_spec.rb new file mode 100644 index 00000000000..78417a7d60f --- /dev/null +++ b/spec/db/migrate/add_upload_foreign_keys_to_user_profiles_spec.rb @@ -0,0 +1,79 @@ +require 'rails_helper' +require 'migration/column_dropper' +require_relative '../../../db/migrate/20190426011148_add_upload_foreign_keys_to_user_profiles' + +RSpec.describe AddUploadForeignKeysToUserProfiles do + before do + %i{card_background profile_background}.each do |column| + # In the future when the column is dropped + # DB.exec("ALTER TABLE user_profiles ADD COLUMN #{column} VARCHAR;") + Migration::ColumnDropper.drop_readonly(:user_profiles, column) + end + + %i{card_background_upload_id profile_background_upload_id}.each do |column| + DB.exec("ALTER TABLE user_profiles DROP COLUMN IF EXISTS #{column}") + end + end + + def select_column_from_user_profiles(column, user_id) + DB.query_single(<<~SQL).first + SELECT #{column} + FROM user_profiles + WHERE user_id = #{user_id} + SQL + end + + it "should migrate the data properly" do + upload = Fabricate(:upload) + upload2 = Fabricate(:upload) + user = Fabricate(:user) + user2 = Fabricate(:user) + user3 = Fabricate(:user) + + DB.exec(<<~SQL) + UPDATE user_profiles + SET card_background = '#{upload.url}', profile_background = '#{upload.url}' + WHERE user_profiles.user_id = #{user.id} + SQL + + DB.exec(<<~SQL) + UPDATE user_profiles + SET card_background = '#{upload.url}', profile_background = '#{upload2.url}' + WHERE user_profiles.user_id = #{user2.id} + SQL + + DB.exec(<<~SQL) + UPDATE user_profiles + SET card_background = '#{upload.url}' + WHERE user_profiles.user_id = #{user3.id} + SQL + + silence_stdout { described_class.new.up } + + %i{card_background profile_background}.each do |column| + expect(select_column_from_user_profiles(column, user.id)) + .to eq(upload.url) + end + + %i{card_background_upload_id profile_background_upload_id}.each do |column| + expect(select_column_from_user_profiles(column, user.id)) + .to eq(upload.id) + end + + expect(select_column_from_user_profiles( + :card_background_upload_id, user2.id + )).to eq(upload.id) + + expect(select_column_from_user_profiles( + :profile_background_upload_id, user2.id + )).to eq(upload2.id) + + expect(select_column_from_user_profiles( + :card_background_upload_id, user3.id + )).to eq(upload.id) + + expect(select_column_from_user_profiles( + :profile_background_upload_id, user3.id + )).to eq(nil) + end +end diff --git a/spec/db/migrate/add_uploads_to_categories_spec.rb b/spec/db/migrate/add_uploads_to_categories_spec.rb index b349a3172fe..b89054348f2 100644 --- a/spec/db/migrate/add_uploads_to_categories_spec.rb +++ b/spec/db/migrate/add_uploads_to_categories_spec.rb @@ -13,32 +13,44 @@ RSpec.describe AddUploadsToCategories do end end + def select_column_from_categories(column, category_id) + DB.query_single(<<~SQL).first + SELECT #{column} + FROM categories + WHERE id = #{category_id} + SQL + end + it "should migrate the data properly" do upload1 = Fabricate(:upload) upload2 = Fabricate(:upload) + category1 = Fabricate(:category) + category2 = Fabricate(:category) - category1 = Fabricate(:category, - logo_url: upload1.url, - background_url: upload2.url - ) + DB.exec(<<~SQL) + UPDATE categories + SET logo_url = '#{upload1.url}', background_url = '#{upload2.url}' + WHERE categories.id = #{category1.id} + SQL - category2 = Fabricate(:category, - logo_url: upload2.url, - background_url: upload1.url - ) + DB.exec(<<~SQL) + UPDATE categories + SET logo_url = '#{upload2.url}', background_url = '#{upload1.url}' + WHERE categories.id = #{category2.id} + SQL silence_stdout { described_class.new.up } - Discourse.reset_active_record_cache + expect(select_column_from_categories(:uploaded_logo_id, category1.id)) + .to eq(upload1.id) - category1.reload + expect(select_column_from_categories(:uploaded_background_id, category1.id)) + .to eq(upload2.id) - expect(category1.uploaded_logo_id).to eq(upload1.id) - expect(category1.uploaded_background_id).to eq(upload2.id) + expect(select_column_from_categories(:uploaded_logo_id, category2.id)) + .to eq(upload2.id) - category2.reload - - expect(category2.uploaded_logo_id).to eq(upload2.id) - expect(category2.uploaded_background_id).to eq(upload1.id) + expect(select_column_from_categories(:uploaded_background_id, category2.id)) + .to eq(upload1.id) end end diff --git a/spec/jobs/clean_up_uploads_spec.rb b/spec/jobs/clean_up_uploads_spec.rb index 0f7e20704bd..934ca6eb216 100644 --- a/spec/jobs/clean_up_uploads_spec.rb +++ b/spec/jobs/clean_up_uploads_spec.rb @@ -156,7 +156,7 @@ describe Jobs::CleanUpUploads do it "does not delete profile background uploads" do profile_background_upload = fabricate_upload - UserProfile.last.update!(profile_background: profile_background_upload.url) + UserProfile.last.upload_profile_background(profile_background_upload) Jobs::CleanUpUploads.new.execute(nil) @@ -166,7 +166,7 @@ describe Jobs::CleanUpUploads do it "does not delete card background uploads" do card_background_upload = fabricate_upload - UserProfile.last.update!(card_background: card_background_upload.url) + UserProfile.last.upload_card_background(card_background_upload) Jobs::CleanUpUploads.new.execute(nil) diff --git a/spec/jobs/recover_user_profile_backgrounds_spec.rb b/spec/jobs/recover_user_profile_backgrounds_spec.rb deleted file mode 100644 index 42a2ec26089..00000000000 --- a/spec/jobs/recover_user_profile_backgrounds_spec.rb +++ /dev/null @@ -1,41 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -require_dependency 'jobs/onceoff/recover_user_profile_backgrounds' - -RSpec.describe Jobs::RecoverUserProfileBackgrounds do - let(:user_profile) { Fabricate(:user).user_profile } - - before do - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "s3-access-key-id" - SiteSetting.s3_secret_access_key = "s3-secret-access-key" - SiteSetting.enable_s3_uploads = true - end - - it "corrects the URL and recovers the uploads" do - current_upload = Upload.create!( - url: '//s3-upload-bucket.s3-us-east-1.amazonaws.com/somewhere/a.png', - original_filename: 'a.png', - filesize: 100, - user_id: -1, - ) - - user_profile.update!( - profile_background: current_upload.url, - card_background: current_upload.url - ) - - Jobs::RecoverUserProfileBackgrounds.new.execute_onceoff({}) - - user_profile.reload - - %i{card_background profile_background}.each do |column| - expect(user_profile.public_send(column)).to eq( - '//s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com/somewhere/a.png' - ) - end - - end -end diff --git a/spec/lib/upload_recovery_spec.rb b/spec/lib/upload_recovery_spec.rb index 0d6892365e4..422baaa6096 100644 --- a/spec/lib/upload_recovery_spec.rb +++ b/spec/lib/upload_recovery_spec.rb @@ -197,58 +197,4 @@ RSpec.describe UploadRecovery do end end end - - describe "#recover_user_profile_backgrounds" do - before do - user.user_profile.update!( - profile_background: upload.url, - card_background: upload.url - ) - end - - it "should recover the background uploads" do - user_profile = user.user_profile - upload.destroy! - - user_profile.update_columns( - profile_background: user_profile.profile_background.sub("default", "X"), - card_background: user_profile.card_background.sub("default", "X") - ) - - expect do - upload_recovery.recover_user_profile_backgrounds - end.to change { Upload.count }.by(1) - - user_profile.reload - - expect(user_profile.profile_background).to eq(upload.url) - expect(user_profile.card_background).to eq(upload.url) - end - - describe 'for a bad upload' do - it 'should not update the urls' do - user_profile = user.user_profile - upload.destroy! - - profile_background = user_profile.profile_background.sub("default", "X") - card_background = user_profile.card_background.sub("default", "X") - - user_profile.update_columns( - profile_background: profile_background, - card_background: card_background - ) - - SiteSetting.authorized_extensions = '' - - expect do - upload_recovery.recover_user_profile_backgrounds - end.to_not change { Upload.count } - - user_profile.reload - - expect(user_profile.profile_background).to eq(profile_background) - expect(user_profile.card_background).to eq(card_background) - end - end - end end diff --git a/spec/models/discourse_single_sign_on_spec.rb b/spec/models/discourse_single_sign_on_spec.rb index 80bb92184a3..6787866f1d0 100644 --- a/spec/models/discourse_single_sign_on_spec.rb +++ b/spec/models/discourse_single_sign_on_spec.rb @@ -668,11 +668,11 @@ describe DiscourseSingleSignOn do user = sso.lookup_or_create_user(ip_address) user.reload user.user_profile.reload - profile_background = user.user_profile.profile_background + profile_background_url = user.profile_background_upload.url # initial creation ... - expect(profile_background).to_not eq(nil) - expect(profile_background).to_not eq('') + expect(profile_background_url).to_not eq(nil) + expect(profile_background_url).to_not eq('') FileHelper.stubs(:download) { raise "should not be called" } sso.profile_background_url = "https://some.new/avatar.png" @@ -681,7 +681,7 @@ describe DiscourseSingleSignOn do user.user_profile.reload # profile_background updated but no override specified ... - expect(user.user_profile.profile_background).to eq(profile_background) + expect(user.profile_background_upload.url).to eq(profile_background_url) end end @@ -704,33 +704,24 @@ describe DiscourseSingleSignOn do end it "deal with no profile_background_url passed for an existing user with a profile_background" do - Sidekiq::Testing.inline! do - # Deliberately not setting profile_background_url so it should not update - sso_record.user.user_profile.update_columns(profile_background: '') - user = sso.lookup_or_create_user(ip_address) - user.reload - user.user_profile.reload + # Deliberately not setting profile_background_url so it should not update + sso_record.user.user_profile.clear_profile_background + user = sso.lookup_or_create_user(ip_address) + user.reload - expect(user).to_not be_nil - expect(user.user_profile.profile_background).to eq('') - end + expect(user.profile_background_upload).to eq(nil) end it "deal with a profile_background_url passed for an existing user with a profile_background" do - Sidekiq::Testing.inline! do - FileHelper.stubs(:download).returns(logo) + url = "http://example.com/a_different_image.png" + stub_request(:get, url).to_return(body: logo) - sso_record.user.user_profile.update_columns(profile_background: '') + sso_record.user.user_profile.clear_profile_background + sso.profile_background_url = "http://example.com/a_different_image.png" + user = sso.lookup_or_create_user(ip_address) + user.reload - sso.profile_background_url = "http://example.com/a_different_image.png" - - user = sso.lookup_or_create_user(ip_address) - user.reload - user.user_profile.reload - - expect(user).to_not be_nil - expect(user.user_profile.profile_background).to_not eq('') - end + expect(user.profile_background_upload).to_not eq(nil) end end @@ -744,16 +735,14 @@ describe DiscourseSingleSignOn do sso.username = "sam" sso.card_background_url = "http://awesome.com/image.png" sso.suppress_welcome_message = true - FileHelper.stubs(:download).returns(file_from_fixtures("logo.png")) user = sso.lookup_or_create_user(ip_address) user.reload user.user_profile.reload - card_background = user.user_profile.card_background + card_background_url = user.user_profile.card_background_upload.url # initial creation ... - expect(card_background).to_not eq(nil) - expect(card_background).to_not eq('') + expect(card_background_url).to be_present FileHelper.stubs(:download) { raise "should not be called" } sso.card_background_url = "https://some.new/avatar.png" @@ -762,7 +751,9 @@ describe DiscourseSingleSignOn do user.user_profile.reload # card_background updated but no override specified ... - expect(user.user_profile.card_background).to eq(card_background) + expect(user.user_profile.card_background_upload.url).to eq( + card_background_url + ) end end @@ -785,33 +776,25 @@ describe DiscourseSingleSignOn do end it "deal with no card_background_url passed for an existing user with a card_background" do - Sidekiq::Testing.inline! do - # Deliberately not setting card_background_url so it should not update - sso_record.user.user_profile.update_columns(card_background: '') - user = sso.lookup_or_create_user(ip_address) - user.reload - user.user_profile.reload + # Deliberately not setting card_background_url so it should not update + sso_record.user.user_profile.clear_card_background + user = sso.lookup_or_create_user(ip_address) + user.reload - expect(user).to_not be_nil - expect(user.user_profile.card_background).to eq('') - end + expect(user.user_profile.card_background_upload).to eq(nil) end it "deal with a card_background_url passed for an existing user with a card_background_url" do - Sidekiq::Testing.inline! do - FileHelper.stubs(:download).returns(logo) + url = "http://example.com/a_different_image.png" + stub_request(:get, url).to_return(body: logo) - sso_record.user.user_profile.update_columns(card_background: '') + sso_record.user.user_profile.clear_card_background + sso.card_background_url = url - sso.card_background_url = "http://example.com/a_different_image.png" + user = sso.lookup_or_create_user(ip_address) + user.reload - user = sso.lookup_or_create_user(ip_address) - user.reload - user.user_profile.reload - - expect(user).to_not be_nil - expect(user.user_profile.card_background).to_not eq('') - end + expect(user.user_profile.card_background_upload.url).to_not eq('') end end diff --git a/spec/models/user_profile_spec.rb b/spec/models/user_profile_spec.rb index 0f661f1fb78..2d19cb13634 100644 --- a/spec/models/user_profile_spec.rb +++ b/spec/models/user_profile_spec.rb @@ -8,23 +8,6 @@ describe UserProfile do expect(user.user_profile).to be_present end - context "url validation" do - let(:user) { Fabricate(:user) } - let(:upload) { Fabricate(:upload) } - - it "ensures profile_background is valid" do - expect(Fabricate.build(:user_profile, user: user, profile_background: "---%")).not_to be_valid - expect(Fabricate.build(:user_profile, user: user, profile_background: "http://example.com/made-up.jpg")).not_to be_valid - expect(Fabricate.build(:user_profile, user: user, profile_background: upload.url)).to be_valid - end - - it "ensures background_url is valid" do - expect(Fabricate.build(:user_profile, user: user, card_background: ";test")).not_to be_valid - expect(Fabricate.build(:user_profile, user: user, card_background: "http://example.com/no.jpg")).not_to be_valid - expect(Fabricate.build(:user_profile, user: user, card_background: upload.url)).to be_valid - end - end - describe 'rebaking' do it 'correctly rebakes bio' do user_profile = Fabricate(:evil_trout).user_profile @@ -228,7 +211,7 @@ describe UserProfile do user.reload - expect(user.user_profile.profile_background).to eq(nil) + expect(user.profile_background_upload).to eq(nil) end end @@ -240,7 +223,7 @@ describe UserProfile do user.reload - expect(user.user_profile.card_background).to eq(nil) + expect(user.card_background_upload).to eq(nil) end end diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb index db93aa94d02..7bee8e84333 100644 --- a/spec/requests/session_controller_spec.rb +++ b/spec/requests/session_controller_spec.rb @@ -804,9 +804,13 @@ RSpec.describe SessionController do ) @user.update_columns(uploaded_avatar_id: upload.id) - @user.user_profile.update_columns( - profile_background: "//test.s3.dualstack.us-east-1.amazonaws.com/something", - card_background: "//test.s3.dualstack.us-east-1.amazonaws.com/something" + + upload1 = Fabricate(:upload_s3) + upload2 = Fabricate(:upload_s3) + + @user.user_profile.update!( + profile_background_upload: upload1, + card_background_upload: upload2 ) @user.reload diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index a3f5ad4ad57..97fed892e26 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -1459,6 +1459,7 @@ describe UsersController do before do sign_in(user) end + let(:user) { Fabricate(:user, username: 'test.test', name: "Test User") } it "should be able to update a user" do @@ -1494,6 +1495,7 @@ describe UsersController do context 'with authenticated user' do context 'with permission to update' do + let(:upload) { Fabricate(:upload) } let!(:user) { sign_in(Fabricate(:user)) } it 'allows the update' do @@ -1504,7 +1506,9 @@ describe UsersController do put "/u/#{user.username}.json", params: { name: 'Jim Tom', muted_usernames: "#{user2.username},#{user3.username}", - watched_tags: "#{tags[0].name},#{tags[1].name}" + watched_tags: "#{tags[0].name},#{tags[1].name}", + card_background_upload_url: upload.url, + profile_background_upload_url: upload.url } expect(response.status).to eq(200) @@ -1512,8 +1516,8 @@ describe UsersController do user.reload expect(user.name).to eq 'Jim Tom' - expect(user.muted_users.pluck(:username).sort).to eq [user2.username, user3.username].sort + expect(TagUser.where( user: user, notification_level: TagUser.notification_levels[:watching] @@ -1532,6 +1536,8 @@ describe UsersController do expect(user.muted_users.pluck(:username).sort).to be_empty expect(user.user_option.theme_ids).to eq([theme.id]) expect(user.user_option.email_level).to eq(UserOption.email_level_types[:always]) + expect(user.profile_background_upload).to eq(upload) + expect(user.card_background_upload).to eq(upload) end context 'a locale is chosen that differs from I18n.locale' do @@ -1986,6 +1992,7 @@ describe UsersController do context 'while logged in' do let(:another_user) { Fabricate(:user) } let(:user) { Fabricate(:user) } + before do sign_in(user) end @@ -2008,7 +2015,7 @@ describe UsersController do it 'can clear the profile background' do delete "/u/#{user.username}/preferences/user_image.json", params: { type: 'profile_background' } - expect(user.reload.user_profile.profile_background).to eq("") + expect(user.reload.profile_background_upload).to eq(nil) expect(response.status).to eq(200) end end diff --git a/spec/serializers/user_serializer_spec.rb b/spec/serializers/user_serializer_spec.rb index 7a7f3472c5b..c409cba1460 100644 --- a/spec/serializers/user_serializer_spec.rb +++ b/spec/serializers/user_serializer_spec.rb @@ -41,13 +41,11 @@ describe UserSerializer do end context "with a user" do - let(:user) { Fabricate.build(:user, user_profile: Fabricate.build(:user_profile)) } + let(:user) { Fabricate(:user) } let(:serializer) { UserSerializer.new(user, scope: Guardian.new, root: false) } let(:json) { serializer.as_json } - - it "produces json" do - expect(json).to be_present - end + let(:upload) { Fabricate(:upload) } + let(:upload2) { Fabricate(:upload) } context "with `enable_names` true" do before do @@ -69,23 +67,15 @@ describe UserSerializer do end end - context "with filled out card background" do + context "with filled out backgrounds" do before do - user.user_profile.card_background = 'http://card.com' + user.user_profile.upload_card_background(upload) + user.user_profile.upload_profile_background(upload2) end it "has a profile background" do - expect(json[:card_background]).to eq 'http://card.com' - end - end - - context "with filled out profile background" do - before do - user.user_profile.profile_background = 'http://background.com' - end - - it "has a profile background" do - expect(json[:profile_background]).to eq 'http://background.com' + expect(json[:card_background_upload_url]).to eq(upload.url) + expect(json[:profile_background_upload_url]).to eq(upload2.url) end end diff --git a/spec/services/user_anonymizer_spec.rb b/spec/services/user_anonymizer_spec.rb index 3855647ffe1..8e57e110d9b 100644 --- a/spec/services/user_anonymizer_spec.rb +++ b/spec/services/user_anonymizer_spec.rb @@ -74,17 +74,23 @@ describe UserAnonymizer do end it "resets profile to default values" do - user.update(name: "Bibi", date_of_birth: 19.years.ago, title: "Super Star") + user.update!( + name: "Bibi", + date_of_birth: 19.years.ago, + title: "Super Star" + ) profile = user.reload.user_profile - profile.update( + upload = Fabricate(:upload) + + profile.update!( location: "Moose Jaw", - website: "www.bim.com", + website: "http://www.bim.com", bio_raw: "I'm Bibi from Moosejaw. I sing and dance.", bio_cooked: "I'm Bibi from Moosejaw. I sing and dance.", - profile_background: "http://example.com/bg.jpg", + profile_background_upload: upload, bio_cooked_version: 2, - card_background: "http://example.com/cb.jpg" + card_background_upload: upload ) prev_username = user.username @@ -104,9 +110,9 @@ describe UserAnonymizer do expect(profile.location).to eq(nil) expect(profile.website).to eq(nil) expect(profile.bio_cooked).to eq(nil) - expect(profile.profile_background).to eq(nil) - expect(profile.bio_cooked_version).to eq(nil) - expect(profile.card_background).to eq(nil) + expect(profile.profile_background_upload).to eq(nil) + expect(profile.bio_cooked_version).to eq(UserProfile::BAKED_VERSION) + expect(profile.card_background_upload).to eq(nil) end end diff --git a/spec/services/user_merger_spec.rb b/spec/services/user_merger_spec.rb index 2e6576f8c5f..f63d9238d30 100644 --- a/spec/services/user_merger_spec.rb +++ b/spec/services/user_merger_spec.rb @@ -951,10 +951,25 @@ describe UserMerger do end it "updates users" do - walter.update_attribute(:approved_by, source_user) + walter.update!(approved_by: source_user) + upload = Fabricate(:upload) + + source_user.update!(admin: true) + + source_user.user_profile.update!( + card_background_upload: upload, + profile_background_upload: upload, + ) + merge_users! expect(walter.reload.approved_by).to eq(target_user) + + target_user.reload + + expect(target_user.admin).to eq(true) + expect(target_user.card_background_upload).to eq(upload) + expect(target_user.profile_background_upload).to eq(upload) end it "deletes the source user even when it's an admin" do diff --git a/spec/services/user_updater_spec.rb b/spec/services/user_updater_spec.rb index a0ca6ab2ad5..e646b0478e6 100644 --- a/spec/services/user_updater_spec.rb +++ b/spec/services/user_updater_spec.rb @@ -145,20 +145,26 @@ describe UserUpdater do date_of_birth = Time.zone.now theme = Fabricate(:theme, user_selectable: true) + upload1 = Fabricate(:upload) + upload2 = Fabricate(:upload) seq = user.user_option.theme_key_seq - val = updater.update(bio_raw: 'my new bio', - email_level: UserOption.email_level_types[:always], - mailing_list_mode: true, - digest_after_minutes: "45", - new_topic_duration_minutes: 100, - auto_track_topics_after_msecs: 101, - notification_level_when_replying: 3, - email_in_reply_to: false, - date_of_birth: date_of_birth, - theme_ids: [theme.id], - allow_private_messages: false) + val = updater.update( + bio_raw: 'my new bio', + email_level: UserOption.email_level_types[:always], + mailing_list_mode: true, + digest_after_minutes: "45", + new_topic_duration_minutes: 100, + auto_track_topics_after_msecs: 101, + notification_level_when_replying: 3, + email_in_reply_to: false, + date_of_birth: date_of_birth, + theme_ids: [theme.id], + allow_private_messages: false, + card_background_upload_url: upload1.url, + profile_background_upload_url: upload2.url + ) expect(val).to be_truthy @@ -176,6 +182,8 @@ describe UserUpdater do expect(user.user_option.theme_key_seq).to eq(seq + 1) expect(user.user_option.allow_private_messages).to eq(false) expect(user.date_of_birth).to eq(date_of_birth.to_date) + expect(user.card_background_upload).to eq(upload1) + expect(user.profile_background_upload).to eq(upload2) end it "disables email_digests when enabling mailing_list_mode" do