Skip to content

Commit

Permalink
Merge pull request diaspora#4542 from Raven24/empty_publishing
Browse files Browse the repository at this point in the history
avoid publishing empty posts
  • Loading branch information
Dennis Schubert committed Sep 25, 2013
2 parents 5aa13ad + e614562 commit 6e62a99
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 14 deletions.
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
* Fix date color and alignment in the notifications dropdown [#4502](https://github.com/diaspora/diaspora/issues/4502)
* Add a white background to images shown in the lightbox [#4475](https://github.com/diaspora/diaspora/issues/4475)
* Refactor getting_started page, test if facebook is available, fix [#4520](https://github.com/diaspora/diaspora/issues/4520)
* Avoid publishing empty posts [#4542](https://github.com/diaspora/diaspora/pull/4542)

## Features
* Add oEmbed content to the mobile view [#4343](https://github.com/diaspora/diaspora/pull/4353)
Expand Down
3 changes: 3 additions & 0 deletions app/assets/javascripts/app/views/publisher/uploader_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ app.views.PublisherUploader = Backbone.View.extend({
url = response.data.photo.unprocessed_image.url;

this._addFinishedPhoto(id, url);
this.trigger('change');
},

// replace the first photo placeholder with the finished uploaded image and
Expand Down Expand Up @@ -114,6 +115,8 @@ app.views.PublisherUploader = Backbone.View.extend({
// no more photos left...
self.options.publisher.el_wrapper.removeClass('with_attachments');
}

self.trigger('change');
});
}
});
Expand Down
1 change: 1 addition & 0 deletions app/assets/javascripts/app/views/publisher_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ app.views.Publisher = Backbone.View.extend({
el: this.$('#file-upload'),
publisher: this
});
this.view_uploader.on('change', this.checkSubmitAvailability, this);

},

Expand Down
12 changes: 11 additions & 1 deletion app/models/status_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ class StatusMessage < Post
extract_tags_from :raw_message

validates_length_of :text, :maximum => 65535, :message => I18n.t('status_messages.too_long', :count => 65535)

# don't allow creation of empty status messages
validate :presence_of_content, on: :create, if: proc { |sm| sm.author.local? }

xml_name :status_message
xml_attr :raw_message
xml_attr :photos, :as => [Photo]
Expand All @@ -23,7 +27,7 @@ class StatusMessage < Post

# 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 :presence_of_content
before_destroy :absence_of_content

attr_accessor :oembed_url
attr_accessor :open_graph_url
Expand Down Expand Up @@ -165,6 +169,12 @@ def address

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
Expand Down
1 change: 1 addition & 0 deletions features/desktop/posts_from_main_page.feature
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ Feature: posting from the main page
When I attach the file "spec/fixtures/button.png" to hidden "file" within "#file-upload"
And I click to delete the first uploaded photo
Then I should not see an uploaded image within the photo drop zone
And I should not be able to submit the publisher

Scenario: back out of uploading a picture to a post with text
Given I expand the publisher
Expand Down
4 changes: 4 additions & 0 deletions features/step_definitions/posts_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
all(".stream_element").should be_empty
end

Then /^I should not be able to submit the publisher$/ do
expect(publisher_submittable?).to be_false
end

Given /^"([^"]*)" has a public post with text "([^"]*)"$/ do |email, text|
user = User.find_by_email(email)
user.post(:status_message, :text => text, :public => true, :to => user.aspects)
Expand Down
5 changes: 5 additions & 0 deletions features/support/publishing_cuke_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ def click_publisher
')
end

def publisher_submittable?
submit_btn = find("#publisher input[type=submit]")
!submit_btn[:disabled]
end

def expand_first_post
within(".stream_element", match: :first) do
find(".expander").click
Expand Down
33 changes: 20 additions & 13 deletions spec/models/status_message_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,23 +101,30 @@
end
end

it "should have either a message or at least one photo" do
n = FactoryGirl.build(:status_message, :text => nil)
# n.valid?.should be_false
context "emptyness" do
it "needs either a message or at least one photo" do
n = @user.build_post(:status_message, :text => nil)
n.should_not be_valid

# n.text = ""
# n.valid?.should be_false
n.text = ""
n.should_not be_valid

n.text = "wales"
n.valid?.should be_true
n.text = nil
n.text = "wales"
n.should be_valid
n.text = nil

photo = @user.build_post(:photo, :user_file => uploaded_photo, :to => @aspect.id)
photo.save!
photo = @user.build_post(:photo, :user_file => uploaded_photo, :to => @aspect.id)
photo.save!

n.photos << photo
n.valid?.should be_true
n.errors.full_messages.should == []
n.photos << photo
n.should be_valid
n.errors.full_messages.should == []
end

it "doesn't check for content when author is remote (federation...)" do
p = FactoryGirl.build(:status_message, text: nil)
p.should be_valid
end
end

it 'should be postable through the user' do
Expand Down

0 comments on commit 6e62a99

Please sign in to comment.