Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

strip comment text upon comment creation #3363

Merged
merged 3 commits into from

3 participants

@fermionic

This address issue #3350

@maxwell
Owner

I think this might be slightly more rails-y if it was something like this

(in the comment model)

def text=(text)
   self[:text] = text.to_s.strip #to_s if for nil, for whatever reason
end

Also, some rspec testing this behavior would be appreciated.

@fermionic

Ok, I've incorporated your suggested change and an RSpec test

@travisbot

This pull request passes (merged 44e4315 into 260a116).

@travisbot

This pull request passes (merged 9f6d841 into 260a116).

@maxwell maxwell commented on the diff
app/models/comment.rb
@@ -75,6 +75,10 @@ def parent= parent
self.post = parent
end
+ def text= text
+ self[:text] = text.to_s.strip #to_s if for nil, for whatever reason
@maxwell Owner
maxwell added a note

to verify, if you comment this line out, the test you wrote fails, right?

Yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@maxwell maxwell merged commit 6e39ffa into diaspora:master
@hfase01 hfase01 referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@hfase01 hfase01 referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@wenzowski wenzowski referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 20 additions and 0 deletions.
  1. +4 −0 app/models/comment.rb
  2. +16 −0 spec/integration/receiving_spec.rb
View
4 app/models/comment.rb
@@ -75,6 +75,10 @@ def parent= parent
self.post = parent
end
+ def text= text
+ self[:text] = text.to_s.strip #to_s if for nil, for whatever reason
@maxwell Owner
maxwell added a note

to verify, if you comment this line out, the test you wrote fails, right?

Yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ end
+
class Generator < Federated::Generator
def self.federated_class
Comment
View
16 spec/integration/receiving_spec.rb
@@ -183,12 +183,28 @@ def receive_with_zord(user, person, xml)
receive_with_zord(eve, alice.person, xml)
comment = eve.comment!(@post, 'tada')
+ # After Eve creates her comment, it gets sent to Alice, who signs it with her private key
+ # before relaying it out to the contacts on the top-level post
comment.parent_author_signature = comment.sign_with_key(alice.encryption_key)
@xml = comment.to_diaspora_xml
comment.delete
+
+ comment_with_whitespace = alice.comment!(@post, ' I cannot lift my thumb from the spacebar ')
+ @xml_with_whitespace = comment_with_whitespace.to_diaspora_xml
+ @guid_with_whitespace = comment_with_whitespace.guid
+ comment_with_whitespace.delete
end
end
+ it 'should receive a relayed comment with leading whitespace' do
+ eve.reload.visible_shareables(Post).size.should == 1
+ post_in_db = StatusMessage.find(@post.id)
+ post_in_db.comments.should == []
+ receive_with_zord(eve, alice.person, @xml_with_whitespace)
+
+ post_in_db.comments(true).first.guid.should == @guid_with_whitespace
+ end
+
it 'should correctly attach the user already on the pod' do
bob.reload.visible_shareables(Post).size.should == 1
post_in_db = StatusMessage.find(@post.id)
Something went wrong with that request. Please try again.