New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a link to a reported comment in the admin panel #5337

Merged
merged 1 commit into from Oct 14, 2014

Conversation

Projects
None yet
3 participants
@Flaburgan
Copy link
Member

Flaburgan commented Oct 12, 2014

Fix "There is no link to the reported comment in the admin interface" from #4711

@Flaburgan Flaburgan force-pushed the Flaburgan:add-link-to-comment-in-report branch 2 times, most recently from f4d2450 to 1ccbaa1 Oct 12, 2014

describe ReportHelper, type: helper do

describe "#report_content"

This comment has been minimized.

@Flaburgan

Flaburgan Oct 12, 2014

Member

Hey, any idea what I should test?

This comment has been minimized.

@svbergerem

svbergerem Oct 12, 2014

Member

I'd say create a status message and a comment. report_content(id,'comment') should include a link to the comment. (should include href=...)

This comment has been minimized.

@svbergerem

svbergerem Oct 12, 2014

Member

Oh, and you could test the same for reported posts. Once you wrote the test for comments that shouldn't be much additional work but a test wouldn't hurt.

This comment has been minimized.

@Flaburgan

Flaburgan Oct 12, 2014

Member

Yeah I was thinking about writing it for both. I try to discover the syntax :P I guess the creation is something like that:

  before do
    @post = FactoryGirl.create(:post)
    @comment = FactoryGirl.create(:comment)
    post.comment = comment
  end

This comment has been minimized.

@Flaburgan

Flaburgan Oct 12, 2014

Member

Should I include the new file somewhere? It seems it is not run

This comment has been minimized.

@jhass

jhass Oct 12, 2014

Member
comment = FactoryGirl.create(:comment)
post = comment.post

See https://github.com/diaspora/diaspora/blob/develop/spec/factories.rb#L189

This comment has been minimized.

@jhass

jhass Oct 12, 2014

Member

No, it should be included automatically. You can run just it with rspec spec/helpers/report_helper_spec.rb

This comment has been minimized.

@Flaburgan

Flaburgan Oct 13, 2014

Member

Okay so here is the test I wrote:

describe ReportHelper, :type => :helper do
  before do
    @comment = FactoryGirl.create(:comment)
    @post = @comment.post
  end

  describe "#report_content" do
    it "contains the link to the post" do
      expect(report_content(@post, 'post')).to include('href="/posts/' + post.id + '"')
    end
    it "contains an anchor to the comment" do 
      expect(report_content(@comment, 'comment')).to include('href="/posts/' + post.id + '#' + comment.guid + '"')
    end
  end
end

But I get the errors
undefined method post_page_title' for #RSpec::ExampleGroups::ReportHelper::ReportContent:0x000000095a3e40undefined method comment_message' for #<RSpec::ExampleGroups::ReportHelper::ReportContent:0x0000000994d410>

Those are the methods called by report_helper, how can I give access to them to rspec?

This comment has been minimized.

@jhass

jhass Oct 13, 2014

Member

Does calling expect(helper.report_content(.... work?

@Flaburgan Flaburgan force-pushed the Flaburgan:add-link-to-comment-in-report branch from 1ccbaa1 to e7bd2f9 Oct 13, 2014


describe "#report_content" do
it "contains the link to the post" do
expect(report_content(@post, 'post')).to include('href="/posts/' + post.id + '"')

This comment has been minimized.

@Flaburgan

Flaburgan Oct 13, 2014

Member

The test fails because it is not able to find the method called by report_content, any idea how to tell rspec where to find them?

@Flaburgan

This comment has been minimized.

Copy link
Member

Flaburgan commented Oct 13, 2014

Hm, with helper.report_content I get

ReportHelper#report_content contains the link to the post
     Failure/Error: expect(helper.report_content(@post, 'post')).to include('href="/posts/' + post.id + '"')
     ArgumentError:
       wrong number of arguments (0 for 1..2)

with report_content alone I have

Failure/Error: expect(report_content(@post, 'post')).to include('href="/posts/' + post.id + '"')
     NoMethodError:
       undefined method `post_page_title' for #<RSpec::ExampleGroups::ReportHelper::ReportContent:0x0000000acd7288>
@jhass

This comment has been minimized.

Copy link
Member

jhass commented Oct 13, 2014

Do you have a trace for the ArgumentError? Also I'd write the matcher as include %Q(href="#{post_path(@post}")

@Flaburgan

This comment has been minimized.

Copy link
Member

Flaburgan commented Oct 13, 2014

Here is the full trace:

DB=postgres rspec spec/helpers/report_helper_spec.rb 
No DRb server is running. Running in local process instead ...
Faraday::Builder is now Faraday::RackBuilder.
[deprecated] I18n.enforce_available_locales will default to true in the future. If you really want to skip validation of your locale you can set I18n.enforce_available_locales = false to avoid this message.
=> Building fixtures
=> Built people.yml, aspect_memberships.yml, profiles.yml, aspects.yml, users.yml, and contacts.yml
Run options: exclude {:performance=>true}                                                                                                                                                                                                    

  1) ReportHelper#report_content contains the link to the post
     Failure/Error: expect(helper.report_content(@post, 'post')).to include('href="/posts/' + post.id + '"')
     ArgumentError:
       wrong number of arguments (0 for 1..2)
     # /home/laroueverte/.rvm/gems/ruby-2.1.2@diaspora/gems/sinatra-1.3.3/lib/sinatra/base.rb:1228:in `post'
     # /home/laroueverte/.rvm/gems/ruby-2.1.2@diaspora/gems/sinatra-1.3.3/lib/sinatra/base.rb:1660:in `block (2 levels) in delegate'
     # ./spec/helpers/report_helper_spec.rb:11:in `block (3 levels) in <top (required)>'


  2) ReportHelper#report_content contains an anchor to the comment
     Failure/Error: expect(helper.report_content(@comment, 'comment')).to include('href="/posts/' + post.id + '#' + comment.guid + '"')
     ArgumentError:
       wrong number of arguments (0 for 1..2)
     # /home/laroueverte/.rvm/gems/ruby-2.1.2@diaspora/gems/sinatra-1.3.3/lib/sinatra/base.rb:1228:in `post'
     # /home/laroueverte/.rvm/gems/ruby-2.1.2@diaspora/gems/sinatra-1.3.3/lib/sinatra/base.rb:1660:in `block (2 levels) in delegate'
     # ./spec/helpers/report_helper_spec.rb:14:in `block (3 levels) in <top (required)>'

 2/2 |======================================================================================================== 100 ========================================================================================================>| Time: 00:00:00 

Finished in 0.30541 seconds (files took 5.42 seconds to load)
2 examples, 2 failures

Failed examples:

rspec ./spec/helpers/report_helper_spec.rb:10 # ReportHelper#report_content contains the link to the post
rspec ./spec/helpers/report_helper_spec.rb:13 # ReportHelper#report_content contains an anchor to the comment

Top 2 slowest examples (0.27418 seconds, 89.8% of total time):
  ReportHelper#report_content contains the link to the post
    0.17729 seconds ./spec/helpers/report_helper_spec.rb:10
  ReportHelper#report_content contains an anchor to the comment
    0.09689 seconds ./spec/helpers/report_helper_spec.rb:13

Randomized with seed 46644
@jhass

This comment has been minimized.

Copy link
Member

jhass commented Oct 13, 2014

Ah, it's because you still use post instead of @post, look at my last comment.

@Flaburgan

This comment has been minimized.

Copy link
Member

Flaburgan commented Oct 13, 2014

The test now passes, but are you sure using post_path in the test is a good idea? Isn't the purpose of the test to check if the path is good?

@Flaburgan Flaburgan force-pushed the Flaburgan:add-link-to-comment-in-report branch from e7bd2f9 to 20d2b8f Oct 13, 2014

@jhass

This comment has been minimized.

Copy link
Member

jhass commented Oct 13, 2014

The purpose of the test to check it generates a link to the post. post_path is an external dependency (Rails) and as such we just assume it works right. That way we don't have to update our tests when post_path changes behavior.

@Flaburgan

This comment has been minimized.

Copy link
Member

Flaburgan commented Oct 13, 2014

Okay, please check my last commit, it should be ready to go.

@Flaburgan

This comment has been minimized.

Copy link
Member

Flaburgan commented Oct 13, 2014

Aaaaand... Green travis \o/

@jhass jhass added this to the next-major milestone Oct 14, 2014

jhass added a commit that referenced this pull request Oct 14, 2014

Merge pull request #5337 from Flaburgan/add-link-to-comment-in-report
Add a link to a reported comment in the admin panel

@jhass jhass merged commit dcc629b into diaspora:develop Oct 14, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@jhass

This comment has been minimized.

Copy link
Member

jhass commented Oct 14, 2014

Thank you!

@Flaburgan Flaburgan deleted the Flaburgan:add-link-to-comment-in-report branch Oct 14, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment