-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
View contributions publicly when peer-to-peer is on #810
Changes from all commits
af1616a
e301ccd
c6e0d71
150bd48
001326a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,16 +13,68 @@ | |
location_attributes: { zip: '12e45' }, | ||
}} | ||
|
||
before { sign_in create(:user) } | ||
let(:user) { create(:user) } | ||
|
||
describe 'GET /index' do | ||
it 'renders a successful response' do | ||
create(:listing) | ||
get contributions_url | ||
expect(response).to be_successful | ||
describe 'access' do | ||
describe 'when logged in' do | ||
before do | ||
sign_in user | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In general, i don't love repeating buckets of examples across multiple contexts. There are of course circumstances when that's appropriate, eg for cross-cutting concerns. But in many cases, it can quickly become difficult to know which examples really care about being tested in all contexts and which just happened to be lumped in out of convenience. This then can hinder future refactoring. In this case, iirc the key thing to test between the three contexts is whether All that said, this is just my gut feeling -- i'm certainly fine with leaving it as is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've updated the tests as you've suggested |
||
|
||
it 'renders the index of contributions' do | ||
create(:listing) | ||
get contributions_url | ||
expect(response).to be_successful | ||
end | ||
|
||
it 'is shows a contribution response page' do | ||
contribution = create(:listing) | ||
get contribution_url(contribution) | ||
expect(response).to be_successful | ||
end | ||
end | ||
|
||
describe 'when logged out but p2p is enabled' do | ||
before do | ||
allow_any_instance_of(SystemSetting).to receive(:peer_to_peer?).and_return(true) | ||
end | ||
|
||
it 'renders the index of contributions' do | ||
create(:listing) | ||
get contributions_url | ||
expect(response).to be_successful | ||
end | ||
|
||
# # This test is commented out because it probably should redirect to | ||
# # a page that's different than would dispatchers etc. see | ||
# | ||
# it 'is shows a contribution response page' do | ||
# contribution = create(:listing) | ||
# get contribution_url(contribution) | ||
# expect(response).to be_successful | ||
# end | ||
end | ||
|
||
describe 'when logged out and p2p is disabled' do | ||
before do | ||
allow_any_instance_of(SystemSetting).to receive(:peer_to_peer?).and_return(false) | ||
end | ||
|
||
it 'redirects to the user login page for the index of contributions' do | ||
create(:listing) | ||
get contributions_url | ||
assert_redirected_to new_user_session_url | ||
end | ||
|
||
it 'redirects to the user login page for a link to an individual contribution' do | ||
contribution = create(:listing) | ||
get contribution_url(contribution) | ||
assert_redirected_to new_user_session_url | ||
end | ||
end | ||
|
||
it 'allows asking for a specific subtype of listing' do | ||
sign_in user | ||
ask = create(:ask, title: 'this is the ask title') | ||
offer = create(:offer, title: 'this is the offer title') | ||
get contributions_url, params: { ContributionType: { 'Ask' => 1 } } | ||
|
@@ -37,6 +89,7 @@ | |
end | ||
|
||
it 'parses requests for a filtered list' do | ||
sign_in user | ||
categories = [ | ||
create(:category, name: Faker::Lorem.word), | ||
create(:category, name: Faker::Lorem.word) | ||
|
@@ -64,16 +117,4 @@ | |
expect(response_ids).not_to include(no_tags_correct_area_listing.id) | ||
end | ||
end | ||
|
||
describe 'GET /contributions/:id' do | ||
it 'is successful' do | ||
contribution = create(:listing) | ||
|
||
get( | ||
contribution_url(contribution), | ||
) | ||
|
||
expect(response).to be_successful | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 62 above references
@system_setting
. Can we do that here as well?