diff --git a/Changelog.md b/Changelog.md index dc5a3f5e150d36b34cca9b8203ecfa4f405fae5c..9a2b77fb2da1b87e546eb7f7c8b0d3f04a36ab3c 100644 --- a/Changelog.md +++ b/Changelog.md @@ -112,6 +112,7 @@ before. * Use Poltergeist instead of Selenium [#6768](https://github.com/diaspora/diaspora/pull/6768) * Redesigned the landing page and added dedicated notes for podmins [#6268](https://github.com/diaspora/diaspora/pull/6268) * Moved the entire federation implementation into its own gem. 🎉 [#6873](https://github.com/diaspora/diaspora/pull/6873) +* Remove `StatusMessage#raw_message` [#6921](https://github.com/diaspora/diaspora/pull/6921) ## Bug fixes * Destroy Participation when removing interactions with a post [#5852](https://github.com/diaspora/diaspora/pull/5852) diff --git a/app/models/post.rb b/app/models/post.rb index 071d27b64168d4b8db44fd82ca61c4c6cfc8cc35..cb94c408ad9f932dc7e9d65a3d9a3afa59bb7dcd 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -59,7 +59,6 @@ class Post < ActiveRecord::Base end def root; end - def raw_message; ""; end def mentioned_people; []; end def photos; []; end diff --git a/app/models/reshare.rb b/app/models/reshare.rb index e6d925c57e0128c78d66fa02d9ab9ad1629fe833..d69c9d278143fbf95d8b2f08c922a39da968bb3e 100644 --- a/app/models/reshare.rb +++ b/app/models/reshare.rb @@ -29,8 +29,8 @@ class Reshare < Post :message, :nsfw, to: :absolute_root, allow_nil: true - def raw_message - absolute_root.try(:raw_message) || super + def text + absolute_root.try(:text) || "" end def mentioned_people diff --git a/app/models/status_message.rb b/app/models/status_message.rb index b2be566e18e2cce0cdaa26a22e5284f841762927..56279d8f8b954221d5cbec0b35221fca5a07a49a 100644 --- a/app/models/status_message.rb +++ b/app/models/status_message.rb @@ -8,23 +8,18 @@ class StatusMessage < Post include PeopleHelper acts_as_taggable_on :tags - extract_tags_from :raw_message + extract_tags_from :text validates_length_of :text, :maximum => 65535, :message => proc {|p, v| I18n.t('status_messages.too_long', :count => 65535, :current_length => v[:value].length)} # don't allow creation of empty status messages - validate :presence_of_content, on: :create, if: proc {|sm| sm.author && sm.author.local? } + validate :presence_of_content, on: :create has_many :photos, :dependent => :destroy, :foreign_key => :status_message_guid, :primary_key => :guid has_one :location has_one :poll, autosave: true - - # a StatusMessage is federated before its photos are so presence_of_content() fails erroneously if no text is present - # therefore, we put the validation in a before_destory callback instead of a validation - before_destroy :absence_of_content - attr_accessor :oembed_url attr_accessor :open_graph_url @@ -42,29 +37,23 @@ class StatusMessage < Post end def self.user_tag_stream(user, tag_ids) - owned_or_visible_by_user(user). - tag_stream(tag_ids) + owned_or_visible_by_user(user).tag_stream(tag_ids) end def self.public_tag_stream(tag_ids) - all_public. - tag_stream(tag_ids) - end - - def raw_message - read_attribute(:text) || "" + all_public.tag_stream(tag_ids) end - def raw_message=(text) - write_attribute(:text, text) + def self.tag_stream(tag_ids) + joins(:taggings).where("taggings.tag_id IN (?)", tag_ids) end def nsfw - self.raw_message.match(/#nsfw/i) || super + text.try(:match, /#nsfw/i) || super end def message - @message ||= Diaspora::MessageRenderer.new raw_message, mentioned_people: mentioned_people + @message ||= Diaspora::MessageRenderer.new(text, mentioned_people: mentioned_people) end def mentioned_people @@ -72,7 +61,7 @@ class StatusMessage < Post create_mentions if self.mentions.empty? self.mentions.includes(:person => :profile).map{ |mention| mention.person } else - Diaspora::Mentionable.people_from_string(self.raw_message) + Diaspora::Mentionable.people_from_string(text) end end @@ -84,7 +73,7 @@ class StatusMessage < Post ## ---- ---- def create_mentions - ppl = Diaspora::Mentionable.people_from_string(self.raw_message) + ppl = Diaspora::Mentionable.people_from_string(text) ppl.each do |person| self.mentions.find_or_create_by(person_id: person.id) end @@ -103,7 +92,7 @@ class StatusMessage < Post end def text_and_photos_blank? - self.raw_message.blank? && self.photos.blank? + text.blank? && photos.blank? end def queue_gather_oembed_data @@ -132,22 +121,10 @@ class StatusMessage < Post } end - protected - def presence_of_content - if text_and_photos_blank? - errors[:base] << "Cannot create a StatusMessage without content" - end - end - - def absence_of_content - unless text_and_photos_blank? - errors[:base] << "Cannot destory a StatusMessage with text and/or photos present" - end - end - private - def self.tag_stream(tag_ids) - joins(:taggings).where('taggings.tag_id IN (?)', tag_ids) + + def presence_of_content + errors[:base] << "Cannot create a StatusMessage without content" if text_and_photos_blank? end end diff --git a/app/presenters/post_presenter.rb b/app/presenters/post_presenter.rb index 3aef5fd5d61cd892a75e501c9836d19680a65fd5..6cee90e2fb433c8d157c609f93be105a04a61d3b 100644 --- a/app/presenters/post_presenter.rb +++ b/app/presenters/post_presenter.rb @@ -42,7 +42,7 @@ class PostPresenter < BasePresenter if @post.message @post.message.plain_text_for_json else - @post.raw_message + @post.text end end diff --git a/lib/diaspora/federation/entities.rb b/lib/diaspora/federation/entities.rb index f0c373dee3f4a5124a8ff8a2dafbdd95235f78d7..f18f74c2b41c31ed15e41a7db1bb902d369d221a 100644 --- a/lib/diaspora/federation/entities.rb +++ b/lib/diaspora/federation/entities.rb @@ -222,7 +222,7 @@ module Diaspora DiasporaFederation::Entities::StatusMessage.new( author: status_message.diaspora_handle, guid: status_message.guid, - text: status_message.raw_message, + text: status_message.text, photos: status_message.photos.map {|photo| photo(photo) }, location: status_message.location ? location(status_message.location) : nil, poll: status_message.poll ? poll(status_message.poll) : nil, diff --git a/lib/diaspora/federation/receive.rb b/lib/diaspora/federation/receive.rb index d8b75c2e6b1dcf8be481f3416d3f2a9ba9f6b678..9b0ceea714280db17888140fd3ccf31ca77b3fc6 100644 --- a/lib/diaspora/federation/receive.rb +++ b/lib/diaspora/federation/receive.rb @@ -161,9 +161,20 @@ module Diaspora end def self.status_message(entity) - save_status_message(entity).tap do - entity.photos.map do |photo| - ignore_existing_guid(Photo, photo.guid, author_of(photo)) { save_photo(photo) } + try_load_existing_guid(StatusMessage, entity.guid, author_of(entity)) do + StatusMessage.new( + author: author_of(entity), + guid: entity.guid, + text: entity.text, + public: entity.public, + created_at: entity.created_at, + provider_display_name: entity.provider_display_name + ).tap do |status_message| + status_message.location = build_location(entity.location) if entity.location + status_message.poll = build_poll(entity.poll) if entity.poll + status_message.photos = save_or_load_photos(entity.photos) + + status_message.save! end end end @@ -228,6 +239,12 @@ module Diaspora ) end + private_class_method def self.save_or_load_photos(photos) + photos.map do |photo| + try_load_existing_guid(Photo, photo.guid, author_of(photo)) { save_photo(photo) } + end + end + private_class_method def self.receive_relayable(klass, entity) save_relayable(klass, entity) { yield }.tap {|relayable| relay_relayable(relayable) if relayable } end @@ -243,24 +260,6 @@ module Diaspora end end - private_class_method def self.save_status_message(entity) - try_load_existing_guid(StatusMessage, entity.guid, author_of(entity)) do - StatusMessage.new( - author: author_of(entity), - guid: entity.guid, - raw_message: entity.text, - public: entity.public, - created_at: entity.created_at, - provider_display_name: entity.provider_display_name - ).tap do |status_message| - status_message.location = build_location(entity.location) if entity.location - status_message.poll = build_poll(entity.poll) if entity.poll - - status_message.save! - end - end - end - private_class_method def self.retract_if_author_ignored(relayable) parent_author = relayable.parent.author.owner return unless parent_author && parent_author.ignored_people.include?(relayable.author) diff --git a/lib/diaspora/message_renderer.rb b/lib/diaspora/message_renderer.rb index 7edfc711d94e1f3dcbc44e1b9f645d21f04fdc17..7c39470fb09dd998e712f24d05c81d997bfc0fd4 100644 --- a/lib/diaspora/message_renderer.rb +++ b/lib/diaspora/message_renderer.rb @@ -120,7 +120,7 @@ module Diaspora delegate :empty?, :blank?, :present?, to: :raw - # @param [String] raw_message Raw input text + # @param [String] text Raw input text # @param [Hash] opts Global options affecting output # @option opts [Array<Person>] :mentioned_people ([]) List of people # allowed to mention @@ -147,8 +147,8 @@ module Diaspora # to Redcarpet # @option opts [Hash] :markdown_render_options Override default options # passed to the Redcarpet renderer - def initialize raw_message, opts={} - @raw_message = raw_message + def initialize(text, opts={}) + @text = text @options = DEFAULTS.deep_merge opts end @@ -211,12 +211,12 @@ module Diaspora # this length. If not given defaults to 70. def title opts={} # Setext-style header - heading = if /\A(?<setext_content>.{1,200})\n(?:={1,200}|-{1,200})(?:\r?\n|$)/ =~ @raw_message.lstrip - setext_content - # Atx-style header - elsif /\A\#{1,6}\s+(?<atx_content>.{1,200}?)(?:\s+#+)?(?:\r?\n|$)/ =~ @raw_message.lstrip - atx_content - end + heading = if /\A(?<setext_content>.{1,200})\n(?:={1,200}|-{1,200})(?:\r?\n|$)/ =~ @text.lstrip + setext_content + # Atx-style header + elsif /\A\#{1,6}\s+(?<atx_content>.{1,200}?)(?:\s+#+)?(?:\r?\n|$)/ =~ @text.lstrip + atx_content + end heading &&= self.class.new(heading).plain_text_without_markdown @@ -236,7 +236,7 @@ module Diaspora end def raw - @raw_message + @text end def to_s @@ -245,8 +245,8 @@ module Diaspora private - def process opts, &block - Processor.process(@raw_message, @options.deep_merge(opts), &block) + def process(opts, &block) + Processor.process(@text, @options.deep_merge(opts), &block) end end end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index bde9b7d1d81539d9b4bb1e6db4318e43bab126bc..00bcc6ea88e616752d6bf6496852a5109c887d6e 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -61,7 +61,7 @@ describe UsersController, :type => :controller do it 'renders xml if atom is requested' do sm = FactoryGirl.create(:status_message, :public => true, :author => @user.person) get :public, :username => @user.username, :format => :atom - expect(response.body).to include(sm.raw_message) + expect(response.body).to include(sm.text) end it 'renders xml if atom is requested with clickalbe urls' do @@ -77,7 +77,7 @@ describe UsersController, :type => :controller do it 'includes reshares in the atom feed' do reshare = FactoryGirl.create(:reshare, :author => @user.person) get :public, :username => @user.username, :format => :atom - expect(response.body).to include reshare.root.raw_message + expect(response.body).to include reshare.root.text end it 'do not show reshares in atom feed if origin post is deleted' do diff --git a/spec/lib/diaspora/federation/receive_spec.rb b/spec/lib/diaspora/federation/receive_spec.rb index c3ffd040865232f01d98ff5e50a305370cb0978d..6f720e0a2d1cac6c4c0af8a09c5ad7da59ffb367 100644 --- a/spec/lib/diaspora/federation/receive_spec.rb +++ b/spec/lib/diaspora/federation/receive_spec.rb @@ -591,6 +591,19 @@ describe Diaspora::Federation::Receive do expect(status_message.photos.map(&:guid)).to include(photo1.guid, photo2.guid) end + it "receives a status message only with photos and without text" do + entity = DiasporaFederation::Entities::StatusMessage.new(status_message_entity.to_h.merge(text: nil)) + received = Diaspora::Federation::Receive.perform(entity) + + status_message = StatusMessage.find_by!(guid: status_message_entity.guid) + + expect(received).to eq(status_message) + expect(status_message.author).to eq(sender) + + expect(status_message.text).to be_nil + expect(status_message.photos.map(&:guid)).to include(photo1.guid, photo2.guid) + end + it "does not overwrite the photos if they already exist" do received_photo = Diaspora::Federation::Receive.photo(photo1) received_photo.text = "foobar" diff --git a/spec/lib/diaspora/fetcher/public_spec.rb b/spec/lib/diaspora/fetcher/public_spec.rb index 82eb8f2e7e4f87ab287adf55f5789f3bd994b016..6ad2fbebe7dec059b1c3bb1ef0b5ecdbc6ae186c 100644 --- a/spec/lib/diaspora/fetcher/public_spec.rb +++ b/spec/lib/diaspora/fetcher/public_spec.rb @@ -122,10 +122,10 @@ describe Diaspora::Fetcher::Public do end end - it 'copied the text correctly' do + it "copied the text correctly" do @data.each do |post| - entry = StatusMessage.find_by_guid(post['guid']) - expect(entry.raw_message).to eql(post['text']) + entry = StatusMessage.find_by_guid(post["guid"]) + expect(entry.text).to eql(post["text"]) end end diff --git a/spec/lib/diaspora/mentionable_spec.rb b/spec/lib/diaspora/mentionable_spec.rb index a328fe7cc7e295591fc317b1d849f2c6a29d32b5..efc0860b02350d44be900b8844bec816c8ed6464 100644 --- a/spec/lib/diaspora/mentionable_spec.rb +++ b/spec/lib/diaspora/mentionable_spec.rb @@ -24,7 +24,7 @@ STR describe "#format" do context "html output" do it "adds the links to the formatted message" do - fmt_msg = Diaspora::Mentionable.format(@status_msg.raw_message, @people) + fmt_msg = Diaspora::Mentionable.format(@status_msg.text, @people) @people.each do |person| expect(fmt_msg).to include person_link(person, class: "mention hovercardable") @@ -32,7 +32,7 @@ STR end it "should work correct when message is escaped html" do - raw_msg = @status_msg.raw_message + raw_msg = @status_msg.text fmt_msg = Diaspora::Mentionable.format(CGI.escapeHTML(raw_msg), @people) @people.each do |person| @@ -45,7 +45,7 @@ STR p.first_name = "</a><script>alert('h')</script>" p.save! - fmt_msg = Diaspora::Mentionable.format(@status_msg.raw_message, @people) + fmt_msg = Diaspora::Mentionable.format(@status_msg.text, @people) expect(fmt_msg).not_to include(p.first_name) expect(fmt_msg).to include(">", "<", "'") # ">", "<", "'" @@ -54,7 +54,7 @@ STR context "plain text output" do it "removes mention markup and displays unformatted name" do - fmt_msg = Diaspora::Mentionable.format(@status_msg.raw_message, @people, plain_text: true) + fmt_msg = Diaspora::Mentionable.format(@status_msg.text, @people, plain_text: true) @people.each do |person| expect(fmt_msg).to include person.first_name @@ -64,7 +64,7 @@ STR end it "leaves the name of people that cannot be found" do - fmt_msg = Diaspora::Mentionable.format(@status_msg.raw_message, []) + fmt_msg = Diaspora::Mentionable.format(@status_msg.text, []) expect(fmt_msg).to eql @test_txt_plain end end diff --git a/spec/models/status_message_spec.rb b/spec/models/status_message_spec.rb index 88d15b923c7074585fda16437b3adad54e02751e..f60f2f332c8bf3f89d6827aba1a8363d5fc4181a 100644 --- a/spec/models/status_message_spec.rb +++ b/spec/models/status_message_spec.rb @@ -91,7 +91,7 @@ describe StatusMessage, type: :model do end end - context "emptyness" do + context "emptiness" do it "needs either a message or at least one photo" do post = user.build_post(:status_message, text: nil) expect(post).not_to be_valid @@ -109,14 +109,14 @@ describe StatusMessage, type: :model do post.photos << photo expect(post).to be_valid expect(post.message.to_s).to be_empty - expect(post.raw_message).to eq "" - expect(post.nsfw).to be_falsy + expect(post.text).to be_nil + expect(post.nsfw).to be_falsey expect(post.errors.full_messages).to eq([]) end - it "doesn't check for content when author is remote (federation...)" do + it "also checks for content when author is remote" do post = FactoryGirl.build(:status_message, text: nil) - expect(post).to be_valid + expect(post).not_to be_valid end end