From bd77795d7ae385f922042ddd90519d40413440d7 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Tue, 24 Apr 2018 09:46:57 -0400 Subject: [PATCH] REFACTOR: move support for user card badge images to a plugin discourse-user-card-badges --- .../components/user-card-contents.js.es6 | 5 +-- .../controllers/preferences/card-badge.js.es6 | 30 ----------------- .../discourse/routes/app-route-map.js.es6 | 1 - .../routes/preferences-card-badge.js.es6 | 32 ------------------- .../components/user-card-contents.hbs | 9 ++---- .../templates/preferences/card-badge.hbs | 25 --------------- .../templates/preferences/profile.hbs | 10 ------ app/assets/stylesheets/desktop/user-card.scss | 15 --------- app/controllers/users_controller.rb | 20 +----------- app/models/user_profile.rb | 6 ++-- app/serializers/user_serializer.rb | 23 ------------- app/services/user_merger.rb | 1 - config/locales/client.en.yml | 2 -- config/routes.rb | 2 -- db/fixtures/999_delayed.rb | 10 ++++++ ...0425152503_drop_user_card_badge_columns.rb | 21 ++++++++++++ spec/controllers/users_controller_spec.rb | 29 ----------------- 17 files changed, 39 insertions(+), 202 deletions(-) delete mode 100644 app/assets/javascripts/discourse/controllers/preferences/card-badge.js.es6 delete mode 100644 app/assets/javascripts/discourse/routes/preferences-card-badge.js.es6 delete mode 100644 app/assets/javascripts/discourse/templates/preferences/card-badge.hbs create mode 100644 db/migrate/20180425152503_drop_user_card_badge_columns.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 0d80e962e14..140b69a1b57 100644 --- a/app/assets/javascripts/discourse/components/user-card-contents.js.es6 +++ b/app/assets/javascripts/discourse/components/user-card-contents.js.es6 @@ -9,7 +9,7 @@ import CleansUp from 'discourse/mixins/cleans-up'; export default Ember.Component.extend(CardContentsBase, CanCheckEmails, CleansUp, { elementId: 'user-card', triggeringLinkClass: 'mention', - classNameBindings: ['visible:show', 'showBadges', 'hasCardBadgeImage', 'user.card_background::no-bg', 'isFixed:fixed'], + classNameBindings: ['visible:show', 'showBadges', 'user.card_background::no-bg', 'isFixed:fixed'], allowBackgrounds: setting('allow_profile_backgrounds'), showBadges: setting('enable_badges'), @@ -92,9 +92,6 @@ export default Ember.Component.extend(CardContentsBase, CanCheckEmails, CleansUp $this.css('background-image', bg); }, - @computed('user.card_badge.image') - hasCardBadgeImage: image => image && image.indexOf('fa-') !== 0, - _showCallback(username, $target) { const args = { stats: false }; args.include_post_count_for = this.get('topic.id'); diff --git a/app/assets/javascripts/discourse/controllers/preferences/card-badge.js.es6 b/app/assets/javascripts/discourse/controllers/preferences/card-badge.js.es6 deleted file mode 100644 index 9f820f9647f..00000000000 --- a/app/assets/javascripts/discourse/controllers/preferences/card-badge.js.es6 +++ /dev/null @@ -1,30 +0,0 @@ -import { ajax } from 'discourse/lib/ajax'; -import BadgeSelectController from "discourse/mixins/badge-select-controller"; - -export default Ember.Controller.extend(BadgeSelectController, { - filteredList: function() { - return this.get('model').filter(function(b) { - return !Ember.isEmpty(b.get('badge.image')); - }); - }.property('model'), - - actions: { - save: function() { - this.setProperties({ saved: false, saving: true }); - - ajax(this.get('user.path') + "/preferences/card-badge", { - type: "PUT", - data: { user_badge_id: this.get('selectedUserBadgeId') } - }).then(() => { - this.setProperties({ - saved: true, - saving: false, - "user.card_image_badge": this.get('selectedUserBadge.badge.image') - }); - }).catch(() => { - this.set('saving', false); - bootbox.alert(I18n.t('generic_error')); - }); - } - } -}); diff --git a/app/assets/javascripts/discourse/routes/app-route-map.js.es6 b/app/assets/javascripts/discourse/routes/app-route-map.js.es6 index da873ecf9dd..161a22220e1 100644 --- a/app/assets/javascripts/discourse/routes/app-route-map.js.es6 +++ b/app/assets/javascripts/discourse/routes/app-route-map.js.es6 @@ -127,7 +127,6 @@ export default function() { this.route('second-factor'); this.route('about', { path: '/about-me' }); this.route('badgeTitle', { path: '/badge_title' }); - this.route('card-badge', { path: '/card-badge' }); }); this.route('userInvited', { path: '/invited', resetNamespace: true }, function() { diff --git a/app/assets/javascripts/discourse/routes/preferences-card-badge.js.es6 b/app/assets/javascripts/discourse/routes/preferences-card-badge.js.es6 deleted file mode 100644 index b8acd6d729a..00000000000 --- a/app/assets/javascripts/discourse/routes/preferences-card-badge.js.es6 +++ /dev/null @@ -1,32 +0,0 @@ -import UserBadge from 'discourse/models/user-badge'; -import RestrictedUserRoute from "discourse/routes/restricted-user"; - -export default RestrictedUserRoute.extend({ - showFooter: true, - - model: function() { - return UserBadge.findByUsername(this.modelFor('user').get('username')); - }, - - renderTemplate: function() { - return this.render({ into: 'user' }); - }, - - // A bit odd, but if we leave to /preferences we need to re-render that outlet - deactivate: function() { - this._super(); - this.render('preferences', { into: 'user', controller: 'preferences' }); - }, - - setupController: function(controller, model) { - controller.set('model', model); - controller.set('user', this.modelFor('user')); - - model.forEach(function(userBadge) { - if (userBadge.get('badge.image') === controller.get('user.card_image_badge')) { - controller.set('selectedUserBadgeId', userBadge.get('id')); - } - }); - } -}); - diff --git a/app/assets/javascripts/discourse/templates/components/user-card-contents.hbs b/app/assets/javascripts/discourse/templates/components/user-card-contents.hbs index d38a81bcc6f..92f8e832a47 100644 --- a/app/assets/javascripts/discourse/templates/components/user-card-contents.hbs +++ b/app/assets/javascripts/discourse/templates/components/user-card-contents.hbs @@ -23,7 +23,7 @@ {{#if user.name}}

{{user.name}}

{{/if}} -{{else}} + {{else}}

{{username}}

{{/unless}} @@ -78,6 +78,7 @@ {{/if}} + {{plugin-outlet name="user-card-additional-controls" args=(hash user=user close=(action "close")) @@ -93,12 +94,6 @@ {{#if user.bio_cooked}}
{{text-overflow class="overflow" text=user.bio_excerpt}}
{{/if}} {{/if}} - {{#if user.card_badge}} - {{#link-to 'badges.show' user.card_badge class="card-badge" title=user.card_badge.name}} - {{icon-or-image user.card_badge.image title=user.card_badge.name}} - {{/link-to}} - {{/if}} - {{#if hasLocationOrWebsite}}
{{#if user.location}} diff --git a/app/assets/javascripts/discourse/templates/preferences/card-badge.hbs b/app/assets/javascripts/discourse/templates/preferences/card-badge.hbs deleted file mode 100644 index 12f06094ea7..00000000000 --- a/app/assets/javascripts/discourse/templates/preferences/card-badge.hbs +++ /dev/null @@ -1,25 +0,0 @@ -
-
- -
-
-

{{i18n 'user.card_badge.title'}}

-
-
- -
- -
- {{combo-box value=selectedUserBadgeId nameProperty="badge.name" content=selectableUserBadges}} -
-
- -
-
- - {{#if saved}}{{i18n 'saved'}}{{/if}} -
-
- -
-
diff --git a/app/assets/javascripts/discourse/templates/preferences/profile.hbs b/app/assets/javascripts/discourse/templates/preferences/profile.hbs index 39686f2b128..31bb62c512e 100644 --- a/app/assets/javascripts/discourse/templates/preferences/profile.hbs +++ b/app/assets/javascripts/discourse/templates/preferences/profile.hbs @@ -50,16 +50,6 @@
{{/if}} -
- -
- {{#if model.card_image_badge}} - {{icon-or-image model.card_image_badge}} - {{/if}} - {{#link-to "preferences.card-badge" class="btn btn-small pad-left no-text"}}{{d-icon "pencil"}}{{/link-to}} -
-
- {{plugin-outlet name="user-preferences-profile"}} {{plugin-outlet name="user-custom-preferences" args=(hash model=model)}} diff --git a/app/assets/stylesheets/desktop/user-card.scss b/app/assets/stylesheets/desktop/user-card.scss index a23e089ae04..df18f58a828 100644 --- a/app/assets/stylesheets/desktop/user-card.scss +++ b/app/assets/stylesheets/desktop/user-card.scss @@ -140,10 +140,6 @@ $user_card_background: $secondary; padding-top: 10px; } - &.has-card-badge-image .bio { - width: 75%; - } - .bio { padding: 10px 0 0 0; clear: left; @@ -296,17 +292,6 @@ $user_card_background: $secondary; padding-top: 15px; } - .card-badge { - img { - max-width: 120px; - } - position: absolute; - right: 12px; - bottom: 12px; - font-size: $font-up-5; - i {color: $user_card_primary;} - } - .metadata .email .btn { padding: 2px 12px; } diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 61507d2c0d6..dfae9f3bdd9 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -48,8 +48,7 @@ class UsersController < ApplicationController return redirect_to path('/login') if SiteSetting.hide_user_profiles_from_public && !current_user @user = fetch_user_from_params( - { include_inactive: current_user.try(:staff?) || (current_user && SiteSetting.show_inactive_accounts) }, - [{ user_profile: :card_image_badge }] + include_inactive: current_user.try(:staff?) || (current_user && SiteSetting.show_inactive_accounts) ) user_serializer = UserSerializer.new(@user, scope: guardian, root: 'user') @@ -90,23 +89,6 @@ class UsersController < ApplicationController show end - def card_badge - end - - def update_card_badge - user = fetch_user_from_params - guardian.ensure_can_edit!(user) - - user_badge = UserBadge.find_by(id: params[:user_badge_id].to_i) - if user_badge && user_badge.user == user && user_badge.badge.image.present? - user.user_profile.update_column(:card_image_badge_id, user_badge.badge.id) - else - user.user_profile.update_column(:card_image_badge_id, nil) - end - - render body: nil - end - def user_preferences_redirect redirect_to email_preferences_path(current_user.username_lower) end diff --git a/app/models/user_profile.rb b/app/models/user_profile.rb index c320be92183..825b9451074 100644 --- a/app/models/user_profile.rb +++ b/app/models/user_profile.rb @@ -1,4 +1,8 @@ class UserProfile < ActiveRecord::Base + + # TODO: remove this after Nov 1, 2018 + self.ignored_columns = %w{card_image_badge_id} + belongs_to :user, inverse_of: :user_profile validates :bio_raw, length: { maximum: 3000 } @@ -12,7 +16,6 @@ class UserProfile < ActiveRecord::Base validate :website_domain_validator, if: Proc.new { |c| c.new_record? || c.website_changed? } - belongs_to :card_image_badge, class_name: 'Badge' has_many :user_profile_views, dependent: :destroy BAKED_VERSION = 1 @@ -126,7 +129,6 @@ end # bio_cooked_version :integer # badge_granted_title :boolean default(FALSE) # card_background :string(255) -# card_image_badge_id :integer # views :integer default(0), not null # # Indexes diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index 06e620e25bf..b93aa2ee78d 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -79,7 +79,6 @@ class UserSerializer < BasicUserSerializer has_many :groups, embed: :object, serializer: BasicGroupSerializer has_many :group_users, embed: :object, serializer: BasicGroupUserSerializer has_many :featured_user_badges, embed: :ids, serializer: UserBadgeSerializer, root: :user_badges - has_one :card_badge, embed: :object, serializer: BadgeSerializer has_one :user_option, embed: :object, serializer: UserOptionSerializer def include_user_option? @@ -107,8 +106,6 @@ class UserSerializer < BasicUserSerializer :custom_avatar_upload_id, :custom_avatar_template, :has_title_badges, - :card_image_badge, - :card_image_badge_id, :muted_usernames, :mailing_list_posts_per_day, :can_change_bio, @@ -171,10 +168,6 @@ class UserSerializer < BasicUserSerializer keys.length > 0 ? keys : nil end - def card_badge - object.user_profile.card_image_badge - end - def bio_raw object.user_profile.bio_raw end @@ -201,22 +194,6 @@ class UserSerializer < BasicUserSerializer website.present? end - def card_image_badge_id - object.user_profile.card_image_badge.try(:id) - end - - def include_card_image_badge_id? - card_image_badge_id.present? - end - - def card_image_badge - object.user_profile.card_image_badge.try(:image) - end - - def include_card_image_badge? - card_image_badge.present? - end - def profile_background object.user_profile.profile_background end diff --git a/app/services/user_merger.rb b/app/services/user_merger.rb index b9fe2bb7732..bc982523a01 100644 --- a/app/services/user_merger.rb +++ b/app/services/user_merger.rb @@ -261,7 +261,6 @@ class UserMerger 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_image_badge_id = COALESCE(t.card_image_badge_id, s.card_image_badge_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/config/locales/client.en.yml b/config/locales/client.en.yml index 45575a270f0..21c50a9abaf 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -834,8 +834,6 @@ en: created: "Joined" log_out: "Log Out" location: "Location" - card_badge: - title: "User Card Badge" website: "Web Site" email_settings: "Email" diff --git a/config/routes.rb b/config/routes.rb index f0bb580453a..76a3a69063d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -397,8 +397,6 @@ Discourse::Application.routes.draw do get "#{root_path}/:username/preferences/second-factor" => "users#preferences", constraints: { username: RouteFormat.username } delete "#{root_path}/:username/preferences/user_image" => "users#destroy_user_image", constraints: { username: RouteFormat.username } put "#{root_path}/:username/preferences/avatar/pick" => "users#pick_avatar", constraints: { username: RouteFormat.username } - get "#{root_path}/:username/preferences/card-badge" => "users#card_badge", constraints: { username: RouteFormat.username } - put "#{root_path}/:username/preferences/card-badge" => "users#update_card_badge", constraints: { username: RouteFormat.username } get "#{root_path}/:username/staff-info" => "users#staff_info", constraints: { username: RouteFormat.username } get "#{root_path}/:username/summary" => "users#summary", constraints: { username: RouteFormat.username } get "#{root_path}/:username/invited" => "users#invited", constraints: { username: RouteFormat.username } diff --git a/db/fixtures/999_delayed.rb b/db/fixtures/999_delayed.rb index 7b6d280db97..00f440f3798 100644 --- a/db/fixtures/999_delayed.rb +++ b/db/fixtures/999_delayed.rb @@ -26,3 +26,13 @@ Migration::TableDropper.delayed_drop( STDERR.puts "Dropping versions. It isn't used anymore." } ) + +Migration::ColumnDropper.drop( + table: 'user_profiles', + after_migration: 'DropUserCardBadgeColumns', + columns: ['card_image_badge_id'], + on_drop: ->() { + STDERR.puts "Removing user_profiles column card_image_badge_id" + }, + delay: 3600 +) diff --git a/db/migrate/20180425152503_drop_user_card_badge_columns.rb b/db/migrate/20180425152503_drop_user_card_badge_columns.rb new file mode 100644 index 00000000000..753aaaf7a1c --- /dev/null +++ b/db/migrate/20180425152503_drop_user_card_badge_columns.rb @@ -0,0 +1,21 @@ +class DropUserCardBadgeColumns < ActiveRecord::Migration[5.1] + def up + # User card images have been moved to a plugin which uses the + # user's custom fields instead of the card_image_badge_id column. + execute "INSERT INTO user_custom_fields (user_id, name, value, created_at, updated_at) + SELECT user_id, 'card_image_badge_id', card_image_badge_id, now(), now() + FROM user_profiles + WHERE card_image_badge_id IS NOT NULL + AND user_id NOT IN ( + SELECT user_id + FROM user_custom_fields + WHERE name = 'card_image_badge_id' + )" + + # delayed drop of the user_profiles.card_image_badge_id column + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 8b3cfc4743c..3b5e60c131d 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -1687,35 +1687,6 @@ describe UsersController do end end - describe "badge_card" do - let(:user) { Fabricate(:user) } - let(:badge) { Fabricate(:badge) } - let(:user_badge) { BadgeGranter.grant(badge, user) } - - it "sets the user's card image to the badge" do - log_in_user user - put :update_card_badge, params: { - user_badge_id: user_badge.id, username: user.username - }, format: :json - - expect(user.user_profile.reload.card_image_badge_id).to be_blank - badge.update_attributes image: "wat.com/wat.jpg" - - put :update_card_badge, params: { - user_badge_id: user_badge.id, username: user.username - }, format: :json - - expect(user.user_profile.reload.card_image_badge_id).to eq(badge.id) - - # Can set to nothing - put :update_card_badge, params: { - username: user.username - }, format: :json - - expect(user.user_profile.reload.card_image_badge_id).to be_blank - end - end - describe "badge_title" do let(:user) { Fabricate(:user) } let(:badge) { Fabricate(:badge) }