From 01e012728722ea75faae9be9307d74fcce8ee770 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonne=20Ha=C3=9F?= Date: Sat, 6 Sep 2014 04:52:18 +0200 Subject: [PATCH] Ignore embedded photos if invalid For example if they're already present Also refactor StatusMessage XML specs --- app/models/status_message.rb | 5 ++ lib/diaspora/guid.rb | 2 +- spec/factories.rb | 5 ++ spec/models/status_message_spec.rb | 82 +++++++++++++----------------- 4 files changed, 46 insertions(+), 48 deletions(-) diff --git a/app/models/status_message.rb b/app/models/status_message.rb index e817b8507..fa04daf4d 100644 --- a/app/models/status_message.rb +++ b/app/models/status_message.rb @@ -187,5 +187,10 @@ class StatusMessage < Post def self.tag_stream(tag_ids) joins(:taggings).where('taggings.tag_id IN (?)', tag_ids) end + + def after_parse + # Make sure already received photos don't invalidate the model + self.photos = photos.select(&:valid?) + end end diff --git a/lib/diaspora/guid.rb b/lib/diaspora/guid.rb index 6f509a543..baf840a1f 100644 --- a/lib/diaspora/guid.rb +++ b/lib/diaspora/guid.rb @@ -4,7 +4,7 @@ module Diaspora::Guid # Creates a before_create callback which calls #set_guid and makes the guid serialize in to_xml def self.included(model) model.class_eval do - before_create :set_guid + after_initialize :set_guid xml_attr :guid validates :guid, :uniqueness => true diff --git a/spec/factories.rb b/spec/factories.rb index ac040e185..c7d5b18c0 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -113,6 +113,11 @@ FactoryGirl.define do end end + factory(:location) do + lat 1 + lng 2 + end + factory(:poll) do sequence(:question) { |n| "What do you think about #{n} ninjas?" } after(:build) do |p| diff --git a/spec/models/status_message_spec.rb b/spec/models/status_message_spec.rb index fc0ac505b..95c7531dc 100644 --- a/spec/models/status_message_spec.rb +++ b/spec/models/status_message_spec.rb @@ -258,116 +258,104 @@ STR end describe "XML" do - before do - @message = FactoryGirl.build(:status_message, :text => "I hate WALRUSES!", :author => @user.person) - @xml = @message.to_xml.to_s - end + let(:message) { FactoryGirl.build(:status_message, text: "I hate WALRUSES!", author: @user.person) } + let(:xml) { message.to_xml.to_s } + let(:marshalled) { StatusMessage.from_xml(xml) } + it 'serializes the escaped, unprocessed message' do text = "[url](http://example.org)" - @message.text = text - expect(@message.to_xml.to_s).to include Builder::XChar.encode(text) + message.text = text + expect(xml).to include Builder::XChar.encode(text) end it 'serializes the message' do - expect(@xml).to include "I hate WALRUSES!" + expect(xml).to include "I hate WALRUSES!" end it 'serializes the author address' do - expect(@xml).to include(@user.person.diaspora_handle) + expect(xml).to include(@user.person.diaspora_handle) end describe '.from_xml' do - before do - @marshalled = StatusMessage.from_xml(@xml) - end it 'marshals the message' do - expect(@marshalled.text).to eq("I hate WALRUSES!") + expect(marshalled.text).to eq("I hate WALRUSES!") end + it 'marshals the guid' do - expect(@marshalled.guid).to eq(@message.guid) + expect(marshalled.guid).to eq(message.guid) end + it 'marshals the author' do - expect(@marshalled.author).to eq(@message.author) + expect(marshalled.author).to eq(message.author) end + it 'marshals the diaspora_handle' do - expect(@marshalled.diaspora_handle).to eq(@message.diaspora_handle) + expect(marshalled.diaspora_handle).to eq(message.diaspora_handle) end end context 'with some photos' do before do - @message.photos << FactoryGirl.build(:photo) - @message.photos << FactoryGirl.build(:photo) - @xml = @message.to_xml.to_s + message.photos << FactoryGirl.build(:photo) + message.photos << FactoryGirl.build(:photo) end it 'serializes the photos' do - expect(@xml).to include "photo" - expect(@xml).to include @message.photos.first.remote_photo_path + expect(xml).to include "photo" + expect(xml).to include message.photos.first.remote_photo_path end describe '.from_xml' do - before do - @marshalled = StatusMessage.from_xml(@xml) + it 'marshals the photos' do + expect(marshalled.photos.size).to eq(2) end - it 'marshals the photos' do - expect(@marshalled.photos.size).to eq(2) + it 'handles existing photos' do + message.photos.each(&:save!) + expect(marshalled).to be_valid end end end context 'with a location' do before do - @message.location = Location.new(coordinates: "1, 2").tap(&:save) - @xml = @message.to_xml.to_s + message.location = FactoryGirl.build(:location) end it 'serializes the location' do - expect(@xml).to include "location" - expect(@xml).to include "lat" - expect(@xml).to include "lng" + expect(xml).to include "location" + expect(xml).to include "lat" + expect(xml).to include "lng" end describe ".from_xml" do - before do - @marshalled = StatusMessage.from_xml(@xml) - end - it 'marshals the location' do - expect(@marshalled.location).to be_present + expect(marshalled.location).to be_present end end end context 'with a poll' do before do - @message.poll = FactoryGirl.create(:poll, :status_message => @message) - @xml = @message.to_xml.to_s + message.poll = FactoryGirl.build(:poll) end it 'serializes the poll' do - expect(@xml).to include "poll" - expect(@xml).to include "question" - expect(@xml).to include "poll_answer" + expect(xml).to include "poll" + expect(xml).to include "question" + expect(xml).to include "poll_answer" end describe ".from_xml" do - before do - @marshalled = StatusMessage.from_xml(@xml) - end - it 'marshals the poll' do - expect(@marshalled.poll).to be_present + expect(marshalled.poll).to be_present end it 'marshals the poll answers' do - expect(@marshalled.poll.poll_answers.size).to eq(2) + expect(marshalled.poll.poll_answers.size).to eq(2) end end end - - end describe '#after_dispatch' do