diff --git a/app/models/user.rb b/app/models/user.rb index a1cbfa42fc4..93d4b4ba9e4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -40,7 +40,7 @@ class User < ActiveRecord::Base has_one :github_user_info, dependent: :destroy has_one :oauth2_user_info, dependent: :destroy has_one :user_stat, dependent: :destroy - has_one :user_profile, dependent: :destroy + has_one :user_profile, dependent: :destroy, inverse_of: :user has_one :single_sign_on_record, dependent: :destroy belongs_to :approved_by, class_name: 'User' belongs_to :primary_group, class_name: 'Group' @@ -63,7 +63,6 @@ class User < ActiveRecord::Base validate :password_validator validates :ip_address, allowed_ip_address: {on: :create, message: :signup_not_allowed} - before_save :cook before_save :update_username_lower before_save :ensure_password_is_hashed after_initialize :add_trust_level @@ -409,17 +408,6 @@ class User < ActiveRecord::Base (since_reply.count >= SiteSetting.newuser_max_replies_per_topic) end - def bio_excerpt - excerpt = PrettyText.excerpt(bio_cooked, 350) - return excerpt if excerpt.blank? || has_trust_level?(:basic) - PrettyText.strip_links(excerpt) - end - - def bio_processed - return bio_cooked if bio_cooked.blank? || has_trust_level?(:basic) - PrettyText.strip_links(bio_cooked) - end - def delete_all_posts!(guardian) raise Discourse::InvalidAccess unless guardian.can_delete_all_posts? self @@ -455,9 +443,10 @@ class User < ActiveRecord::Base def change_trust_level!(level) raise "Invalid trust level #{level}" unless TrustLevel.valid_level?(level) self.trust_level = TrustLevel.levels[level] - self.bio_raw_will_change! # So it can get re-cooked based on the new trust level transaction do self.save! + self.user_profile.recook_bio + self.user_profile.save! Group.user_trust_level_change!(self.id, self.trust_level) BadgeGranter.update_badges(self, trust_level: trust_level) end @@ -507,11 +496,6 @@ class User < ActiveRecord::Base username end - def bio_summary - return nil unless bio_cooked.present? - Summarize.new(bio_cooked).summary - end - def badge_count user_badges.count end @@ -641,14 +625,6 @@ class User < ActiveRecord::Base protected - def cook - if bio_raw.present? - self.bio_cooked = PrettyText.cook(bio_raw, omit_nofollow: self.has_trust_level?(:leader)) if bio_raw_changed? - else - self.bio_cooked = nil - end - end - def update_tracked_topics return unless auto_track_topics_after_msecs_changed? TrackedTopicsUpdater.new(id, auto_track_topics_after_msecs).call @@ -760,7 +736,6 @@ end # created_at :datetime # updated_at :datetime # name :string(255) -# bio_raw :text # seen_notification_id :integer default(0), not null # last_posted_at :datetime # email :string(256) not null @@ -774,7 +749,6 @@ end # last_emailed_at :datetime # email_digests :boolean not null # trust_level :integer not null -# bio_cooked :text # email_private_messages :boolean default(TRUE) # email_direct :boolean default(TRUE), not null # approved :boolean default(FALSE), not null diff --git a/app/models/user_profile.rb b/app/models/user_profile.rb index 35bd8459211..b4e3732cce2 100644 --- a/app/models/user_profile.rb +++ b/app/models/user_profile.rb @@ -1,4 +1,40 @@ class UserProfile < ActiveRecord::Base + belongs_to :user, inverse_of: :user_profile + + validates :user, presence: true + before_save :cook + + def bio_excerpt + excerpt = PrettyText.excerpt(bio_cooked, 350) + return excerpt if excerpt.blank? || user.has_trust_level?(:basic) + PrettyText.strip_links(excerpt) + end + + def bio_processed + return bio_cooked if bio_cooked.blank? || user.has_trust_level?(:basic) + PrettyText.strip_links(bio_cooked) + end + + def bio_summary + return nil unless bio_cooked.present? + Summarize.new(bio_cooked).summary + end + + def recook_bio + self.bio_raw_will_change! + cook + end + + private + + def cook + if self.bio_raw.present? + self.bio_cooked = PrettyText.cook(self.bio_raw, omit_nofollow: user.has_trust_level?(:leader)) if bio_raw_changed? + else + self.bio_cooked = nil + end + end + end # == Schema Information @@ -6,6 +42,8 @@ end # Table name: user_profiles # # user_id :integer not null, primary key +# bio_cooked :text +# bio_raw :text # location :string(255) # website :string(255) # diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index 3ba2529234d..31f52ff9586 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -41,7 +41,7 @@ class UserSerializer < BasicUserSerializer def bio_excerpt # If they have a bio return it - excerpt = object.bio_excerpt + excerpt = object.user_profile.bio_excerpt return excerpt if excerpt.present? # Without a bio, determine what message to show @@ -124,6 +124,17 @@ class UserSerializer < BasicUserSerializer website.present? end + def include_bio_raw? + bio_raw.present? + end + def bio_raw + object.user_profile.bio_raw + end + + def bio_cooked + object.user_profile.bio_processed + end + def stats UserAction.stats(object.id, scope) end @@ -154,9 +165,4 @@ class UserSerializer < BasicUserSerializer def private_messages_stats UserAction.private_messages_stats(object.id, scope) end - - def bio_cooked - object.bio_processed - end - end diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index 47da2a52c4a..eeaeb1adae8 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -30,8 +30,8 @@ class UserUpdater def update(attributes = {}) user_profile = user.user_profile user_profile.website = format_url(attributes.fetch(:website) { user_profile.website }) + user_profile.bio_raw = attributes.fetch(:bio_raw) { user_profile.bio_raw } - user.bio_raw = attributes.fetch(:bio_raw) { user.bio_raw } user.name = attributes.fetch(:name) { user.name } user.locale = attributes.fetch(:locale) { user.locale } user.digest_after_days = attributes.fetch(:digest_after_days) { user.digest_after_days } diff --git a/db/fixtures/005_users.rb b/db/fixtures/005_users.rb index 5ba7bfe93a1..652245cdb8c 100644 --- a/db/fixtures/005_users.rb +++ b/db/fixtures/005_users.rb @@ -13,8 +13,6 @@ User.seed do |u| u.username_lower = "system" u.email = "no_email" u.password = SecureRandom.hex - # TODO localize this, its going to require a series of hacks - u.bio_raw = "Not a real person. A global user for system notifications and other system tasks." u.active = true u.admin = true u.moderator = true diff --git a/db/migrate/20140610034314_move_bio_to_user_profiles.rb b/db/migrate/20140610034314_move_bio_to_user_profiles.rb new file mode 100644 index 00000000000..d1042c552ab --- /dev/null +++ b/db/migrate/20140610034314_move_bio_to_user_profiles.rb @@ -0,0 +1,25 @@ +class MoveBioToUserProfiles < ActiveRecord::Migration + def up + add_column :user_profiles, :bio_raw, :text + add_column :user_profiles, :bio_cooked, :text + + execute "UPDATE user_profiles SET bio_raw = subquery.bio_raw, bio_cooked = subquery.bio_cooked FROM ( + SELECT bio_raw, bio_cooked, id FROM users + ) as subquery WHERE user_profiles.user_id = subquery.id" + + remove_column :users, :bio_raw + remove_column :users, :bio_cooked + end + + def down + add_column :users, :bio_raw, :text + add_column :users, :bio_cooked, :text + + execute "UPDATE users SET bio_raw = subquery.bio_raw, bio_cooked = subquery.bio_cooked FROM ( + SELECT bio_raw, bio_cooked, user_id FROM user_profiles + ) as subquery WHERE users.id = subquery.user_id" + + remove_column :user_profiles, :bio_raw + remove_column :user_profiles, :bio_cooked + end +end diff --git a/spec/fabricators/user_fabricator.rb b/spec/fabricators/user_fabricator.rb index 6cb60bd42c7..4f02e1ee521 100644 --- a/spec/fabricators/user_fabricator.rb +++ b/spec/fabricators/user_fabricator.rb @@ -7,7 +7,6 @@ Fabricator(:user) do email { sequence(:email) { |i| "bruce#{i}@wayne.com" } } password 'myawesomepassword' trust_level TrustLevel.levels[:basic] - bio_raw "I'm batman!" ip_address { sequence(:ip_address) { |i| "99.232.23.#{i%254}"} } end @@ -60,7 +59,11 @@ Fabricator(:active_user, from: :user) do password 'myawesomepassword' trust_level TrustLevel.levels[:basic] active true - bio_raw "Don't ask me about my dad!" + + after_create do |user| + user.user_profile.bio_raw = "Don't ask me about my dad!" + user.user_profile.save! + end end Fabricator(:leader, from: :user) do diff --git a/spec/fabricators/user_profile_fabricator.rb b/spec/fabricators/user_profile_fabricator.rb index 52bd1977843..14fca5fae17 100644 --- a/spec/fabricators/user_profile_fabricator.rb +++ b/spec/fabricators/user_profile_fabricator.rb @@ -1,2 +1,3 @@ Fabricator(:user_profile) do + bio_raw "I'm batman!" end diff --git a/spec/models/user_profile_spec.rb b/spec/models/user_profile_spec.rb index d91931b6a63..72943292e72 100644 --- a/spec/models/user_profile_spec.rb +++ b/spec/models/user_profile_spec.rb @@ -1,8 +1,109 @@ require 'spec_helper' describe UserProfile do - it "is created automatically when a user is created" do + it 'is created automatically when a user is created' do user = Fabricate(:evil_trout) user.user_profile.should be_present end + + describe 'new' do + let(:user_profile) { Fabricate.build(:user_profile) } + + it 'is not valid without user' do + expect(user_profile.valid?).to be_false + end + + it 'is is valid with user' do + user_profile.user = Fabricate.build(:user) + expect(user_profile.valid?).to be_true + end + + describe 'after save' do + let(:user) { Fabricate(:user) } + + before do + user.user_profile.bio_raw = 'my bio' + user.user_profile.save + end + + it 'has cooked bio' do + expect(user.user_profile.bio_cooked).to be_present + end + + it 'has bio summary' do + expect(user.user_profile.bio_summary).to be_present + end + end + end + + describe 'changing bio' do + let(:user) { Fabricate(:user) } + + before do + user.user_profile.bio_raw = "**turtle power!**" + user.user_profile.save + user.user_profile.reload + end + + it 'should markdown the raw_bio and put it in cooked_bio' do + user.user_profile.bio_cooked.should == "

turtle power!

" + end + end + + describe 'bio link stripping' do + + it 'returns an empty string with no bio' do + expect(Fabricate.build(:user_profile).bio_excerpt).to be_blank + end + + context 'with a user that has a link in their bio' do + let(:user_profile) { Fabricate.build(:user_profile, bio_raw: "im sissy and i love http://ponycorns.com") } + let(:user) do + user = Fabricate.build(:user, user_profile: user_profile) + user_profile.user = user + user + end + + let(:created_user) do + user = Fabricate(:user) + user.user_profile.bio_raw = 'im sissy and i love http://ponycorns.com' + user.user_profile.save! + user + end + + it 'includes the link as nofollow if the user is not new' do + user.user_profile.send(:cook) + expect(user_profile.bio_excerpt).to eq("im sissy and i love http://ponycorns.com") + expect(user_profile.bio_processed).to eq("

im sissy and i love http://ponycorns.com

") + end + + it 'removes the link if the user is new' do + user.trust_level = TrustLevel.levels[:newuser] + user_profile.send(:cook) + expect(user_profile.bio_excerpt).to eq("im sissy and i love http://ponycorns.com") + expect(user_profile.bio_processed).to eq("

im sissy and i love http://ponycorns.com

") + end + + it 'includes the link without nofollow if the user is trust level 3 or higher' do + user.trust_level = TrustLevel.levels[:leader] + user_profile.send(:cook) + expect(user_profile.bio_excerpt).to eq("im sissy and i love http://ponycorns.com") + expect(user_profile.bio_processed).to eq("

im sissy and i love http://ponycorns.com

") + end + + it 'removes nofollow from links in bio when trust level is increased' do + created_user.change_trust_level!(:leader) + expect(created_user.user_profile.bio_excerpt).to eq("im sissy and i love http://ponycorns.com") + expect(created_user.user_profile.bio_processed).to eq("

im sissy and i love http://ponycorns.com

") + end + + it 'adds nofollow to links in bio when trust level is decreased' do + created_user.trust_level = TrustLevel.levels[:leader] + created_user.save + created_user.change_trust_level!(:regular) + expect(created_user.user_profile.bio_excerpt).to eq("im sissy and i love http://ponycorns.com") + expect(created_user.user_profile.bio_processed).to eq("

im sissy and i love http://ponycorns.com

") + end + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 2e1dce54513..e7208f515db 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -241,8 +241,6 @@ describe User do end its(:email_tokens) { should be_present } - its(:bio_cooked) { should be_present } - its(:bio_summary) { should be_present } end end @@ -565,21 +563,6 @@ describe User do end end - describe 'changing bio' do - let(:user) { Fabricate(:user) } - - before do - user.bio_raw = "**turtle power!**" - user.save - user.reload - end - - it "should markdown the raw_bio and put it in cooked_bio" do - user.bio_cooked.should == "

turtle power!

" - end - - end - describe "previous_visit_at" do let(:user) { Fabricate(:user) } @@ -744,53 +727,6 @@ describe User do end - describe "bio link stripping" do - - it "returns an empty string with no bio" do - expect(Fabricate.build(:user).bio_excerpt).to be_blank - end - - context "with a user that has a link in their bio" do - let(:user) { Fabricate.build(:user, bio_raw: "im sissy and i love http://ponycorns.com") } - - it "includes the link as nofollow if the user is not new" do - user.send(:cook) - expect(user.bio_excerpt).to eq("im sissy and i love http://ponycorns.com") - expect(user.bio_processed).to eq("

im sissy and i love http://ponycorns.com

") - end - - it "removes the link if the user is new" do - user.trust_level = TrustLevel.levels[:newuser] - user.send(:cook) - expect(user.bio_excerpt).to eq("im sissy and i love http://ponycorns.com") - expect(user.bio_processed).to eq("

im sissy and i love http://ponycorns.com

") - end - - it "includes the link without nofollow if the user is trust level 3 or higher" do - user.trust_level = TrustLevel.levels[:leader] - user.send(:cook) - expect(user.bio_excerpt).to eq("im sissy and i love http://ponycorns.com") - expect(user.bio_processed).to eq("

im sissy and i love http://ponycorns.com

") - end - - it "removes nofollow from links in bio when trust level is increased" do - user.save - user.change_trust_level!(:leader) - expect(user.bio_excerpt).to eq("im sissy and i love http://ponycorns.com") - expect(user.bio_processed).to eq("

im sissy and i love http://ponycorns.com

") - end - - it "adds nofollow to links in bio when trust level is decreased" do - user.trust_level = TrustLevel.levels[:leader] - user.save - user.change_trust_level!(:regular) - expect(user.bio_excerpt).to eq("im sissy and i love http://ponycorns.com") - expect(user.bio_processed).to eq("

im sissy and i love http://ponycorns.com

") - end - end - - end - describe '#readable_name' do context 'when name is missing' do it 'returns just the username' do diff --git a/spec/serializers/user_serializer_spec.rb b/spec/serializers/user_serializer_spec.rb index 527ae3a9eab..f6e55bc417e 100644 --- a/spec/serializers/user_serializer_spec.rb +++ b/spec/serializers/user_serializer_spec.rb @@ -41,5 +41,20 @@ describe UserSerializer do expect(json[:website]).to eq 'http://example.com' end end + + context "with filled out bio" do + before do + user.user_profile.bio_raw = 'my raw bio' + user.user_profile.bio_cooked = 'my cooked bio' + end + + it "has a bio" do + expect(json[:bio_raw]).to eq 'my raw bio' + end + + it "has a cooked bio" do + expect(json[:bio_cooked]).to eq 'my cooked bio' + end + end end end diff --git a/spec/services/user_updater_spec.rb b/spec/services/user_updater_spec.rb index f43ef1f4453..02ad0fec06e 100644 --- a/spec/services/user_updater_spec.rb +++ b/spec/services/user_updater_spec.rb @@ -14,6 +14,15 @@ describe UserUpdater do expect(user.reload.name).to eq 'Jim Tom' end + it 'updates bio' do + user = Fabricate(:user) + updater = described_class.new(acting_user, user) + + updater.update(bio_raw: 'my new bio') + + expect(user.reload.user_profile.bio_raw).to eq 'my new bio' + end + context 'when update succeeds' do it 'returns true' do user = Fabricate(:user)