maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix Rubocop violations and merge Listing specs (#9963)

Browse source
This commit is contained in:
rhymes 2020-08-24 12:04:29 +02:00 • committed by GitHub
parent 1776377234
commit 5f02c35ece
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 139 additions and 183 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
.rubocop.yml
View file

@ -683,6 +683,11 @@ RSpec/ExampleLength:
Max: 15
StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ExampleLength
RSpec/MultipleMemoizedHelpers:
Description: Checks if example groups contain too many `let` and `subject` calls.
Enabled: false
StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MultipleMemoizedHelpers
RSpec/MultipleExpectations:
Description: Checks if examples contain too many `expect` calls.
Enabled: true

6
.rubocop_todo.yml
View file

@ -6,19 +6,19 @@ require:
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2020-07-29 11:14:03 UTC using RuboCop version 0.88.0.
# on 2020-08-24 06:18:00 UTC using RuboCop version 0.89.1.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.
# Offense count: 6
# Offense count: 3
Performance/OpenStruct:
Exclude:
- 'spec/models/github_repo_spec.rb'
- 'spec/requests/github_repos_spec.rb'
# Offense count: 28
# Offense count: 13
# Configuration parameters: Include.
# Include: app/models/**/*.rb
Rails/HasManyOrHasOneDependent:

2
config/initializers/devise.rb
View file

@ -293,7 +293,7 @@ Devise.setup do |config|
# Add a new OmniAuth provider. Check the wiki for more information on setting
# up on your models and hooks.
# Fun fact, if this is reordered to have Twitter first, it doesn't work for some reason.
# Fun fact, if this is reordered to have Twitter first, it doesn't work for some reason.
config.omniauth :github, setup: GITHUB_OMNIUATH_SETUP
config.omniauth :twitter, setup: TWITTER_OMNIAUTH_SETUP

5
lib/tasks/temporary/env_variables.rake
View file

@ -6,8 +6,7 @@ task create_dot_env_file: :environment do
sample_app_yml = YAML.load_file("config/sample_application.yml")
# read existing lines
application_yml = File.open("config/application.yml", 'r').read
application_yml = File.open("config/application.yml", "r").read
File.open(".env", "w") do |env_file|
# add default values only if they are not already present in the
@ -16,7 +15,7 @@ task create_dot_env_file: :environment do
env_file.write("export #{variable}=#{value}\n") unless application_yml.include?(variable.to_s)
end
File.open("config/application.yml", 'r') do |file|
File.open("config/application.yml", "r") do |file|
file.each_line do |line|
new_line = line.gsub(": ", "=")
unless new_line.blank? || new_line.starts_with?("#")

3
package.json
View file

@ -53,6 +53,9 @@
"Gemfile": [
"bundle exec rubocop --require rubocop-rspec --auto-correct"
],
"*.rake": [
"bundle exec rubocop --require rubocop-rspec --auto-correct"
],
"app/**/*.html.erb": [
"bundle exec erblint --autocorrect"
],

301
spec/requests/listings_spec.rb
View file

@ -1,17 +1,16 @@
require "rails_helper"
RSpec.describe "/listings", type: :request do
let(:edu_category) do
create(:listing_category, cost: 1)
end
let(:user) { create(:user) }
let(:organization) { create(:organization) }
let(:edu_category) { create(:listing_category, cost: 1) }
let(:listing_params) do
{
listing: {
title: "something",
body_markdown: "something else",
listing_category_id: edu_category.id,
tag_list: "",
tag_list: "ruby, rails, go",
contact_via_connect: true
}
}
@ -144,7 +143,7 @@ RSpec.describe "/listings", type: :request do
describe "POST /listings" do
before do
sign_in user
create_list(:credit, 25, user: user)
create_list(:credit, 20, user: user)
end
let(:cfp_category) { create(:listing_category, :cfp) }
@ -199,30 +198,56 @@ RSpec.describe "/listings", type: :request do
expect(response).to redirect_to "/listings"
end
it "creates proper listing if credits are available" do
post "/listings", params: listing_params
expect(Listing.last.processed_html).to include(listing_params[:listing][:body_markdown])
end
it "properly deducts the amount of credits" do
expect do
post "/listings", params: listing_params
end.to change { user.credits.spent.size }.by(edu_category.cost)
end
it "creates a listing draft under the org" do
it "adds tags" do
post "/listings", params: listing_params
tag = listing_params[:listing][:tag_list].split(", ").first
expect(Listing.last.cached_tag_list).to include(tag)
end
it "creates the listing under the user" do
post "/listings", params: listing_params
expect(Listing.last.user_id).to eq(user.id)
end
it "creates the listing for the user if no organization_id is selected" do
create(:organization_membership, user_id: user.id, organization_id: organization.id)
post "/listings", params: listing_params
expect(Listing.last.organization_id).to be(nil)
expect(Listing.last.user_id).to eq(user.id)
end
it "creates a listing draft under the org for an org admin" do
org_admin = create(:user, :org_admin)
org_id = org_admin.organizations.first.id
Credit.create(organization_id: org_id)
draft_params[:listing][:organization_id] = org_id
sign_in org_admin
post "/listings", params: draft_params
expect(Listing.first.organization_id).to eq org_id
expect(Listing.first.organization_id).to eq(org_id)
end
it "creates a listing under the org" do
it "creates a listing under the org for an org admin" do
org_admin = create(:user, :org_admin)
org_id = org_admin.organizations.first.id
Credit.create(organization_id: org_id)
listing_params[:listing][:organization_id] = org_id
sign_in org_admin
post "/listings", params: listing_params
expect(Listing.first.organization_id).to eq org_id
expect(Listing.first.organization_id).to eq(org_id)
end
it "does not create a listing draft for an org not belonging to the user" do
@ -237,6 +262,22 @@ RSpec.describe "/listings", type: :request do
expect { post "/listings", params: listing_params }.to raise_error(Pundit::NotAuthorizedError)
end
it "creates the listing for the organization" do
create(:organization_membership, user_id: user.id, organization_id: organization.id)
listing_params[:listing][:organization_id] = organization.id
post "/listings", params: listing_params
expect(Listing.last.organization_id).to eq(organization.id)
end
it "does not create an org listing if a different org member doesn't belong to the org" do
another_org = create(:organization)
create(:organization_membership, user_id: user.id, organization_id: another_org.id)
listing_params[:listing][:organization_id] = organization.id
expect do
post "/listings", params: listing_params
end.to raise_error Pundit::NotAuthorizedError
end
it "assigns the spent credits to the listing" do
post "/listings", params: listing_params
spent_credit = user.credits.spent.last
@ -269,6 +310,29 @@ RSpec.describe "/listings", type: :request do
end.to raise_error("SUSPENDED")
end
end
context "when rate limiting" do
let(:rate_limiter) { RateLimitChecker.new(user) }
before do
allow(RateLimitChecker).to receive(:new).and_return(rate_limiter)
sign_in user
end
it "increments rate limit for listing_creation" do
allow(rate_limiter).to receive(:track_limit_by_action)
post "/listings", params: listing_params
expect(rate_limiter).to have_received(:track_limit_by_action).with(:listing_creation)
end
it "returns a 429 status when rate limit is reached" do
allow(rate_limiter).to receive(:limit_by_action).and_return(true)
post "/listings", params: listing_params
expect(response.status).to eq(429)
end
end
end
describe "PUT /listings/:id" do
@ -316,7 +380,7 @@ RSpec.describe "/listings", type: :request do
expect do
put "/listings/#{listing.id}", params: params
end.to change(user.credits.spent, :size).by(cost)
expect(listing.reload.bumped_at >= previous_bumped_at).to eq(true)
expect(listing.reload.bumped_at >= previous_bumped_at).to be(true)
end
it "bumps the org listing using org credits before user credits" do
@ -327,7 +391,7 @@ RSpec.describe "/listings", type: :request do
expect do
put "/listings/#{org_listing.id}", params: params
end.to change(organization.credits.spent, :size).by(cost)
expect(org_listing.reload.bumped_at >= previous_bumped_at).to eq(true)
expect(org_listing.reload.bumped_at >= previous_bumped_at).to be(true)
end
it "bumps the org listing using user credits if org credits insufficient and user credits are" do
@ -337,7 +401,7 @@ RSpec.describe "/listings", type: :request do
expect do
put "/listings/#{org_listing.id}", params: params
end.to change(user.credits.spent, :size).by(cost)
expect(org_listing.reload.bumped_at >= previous_bumped_at).to eq(true)
expect(org_listing.reload.bumped_at >= previous_bumped_at).to be(true)
end
end
@ -404,16 +468,64 @@ RSpec.describe "/listings", type: :request do
it "unpublishes a published listing" do
put "/listings/#{listing.id}", params: params
expect(listing.reload.published).to eq(false)
expect(listing.reload.published).to be(false)
end
end
context "when an update is attempted" do
it "updates body_markdown" do
put "/listings/#{listing.id}", params: { listing: { body_markdown: "hello new markdown" } }
expect(listing.reload.body_markdown).to eq("hello new markdown")
end
it "does not update body_markdown if not bumped/created recently" do
listing.update_column(:bumped_at, 50.hours.ago)
put "/listings/#{listing.id}", params: { listing: { body_markdown: "hello new markdown" } }
expect(listing.reload.body_markdown).not_to eq("hello new markdown")
end
it "does not update with an empty body markdown" do
put "/listings/#{listing.id}", params: { listing: { body_markdown: "" } }
expect(response.body).to include(CGI.escapeHTML("can't be blank"))
expect(listing.reload.body_markdown).not_to be_empty
end
it "updates other fields" do
put "/listings/#{listing.id}", params: {
listing: { body_markdown: "hello new markdown", title: "New title!", tag_list: "new, tags, hey" }
}
listing.reload
expect(listing.title).to eq("New title!")
expect(listing.cached_tag_list).to include("hey")
end
it "does not update other fields if not bumped/created recently" do
listing.update_column(:bumped_at, 50.hours.ago)
put "/listings/#{listing.id}", params: {
listing: { body_markdown: "hello new markdown", title: "New title!", tag_list: "new, tags, hey" }
}
listing.reload
expect(listing.title).not_to eq("New title!")
expect(listing.cached_tag_list).not_to include("hey")
end
end
end
describe "GET /listings/edit" do
let(:listing) { create(:listing, user: user) }
before do
create_list(:credit, 20, user: user)
sign_in user
end
it "has page content" do
get "/listings/#{listing.id}/edit"
expect(response.body).to include("You can bump your listing")
end
end
@ -517,166 +629,3 @@ RSpec.describe "/listings", type: :request do
end
end
end
# TODO: [@forem/oss] We used to have 2 request spec files, listing_spec.rb
# and classified_listing_spec.rb. This context contains the specs of the former,
# but we should eventually unify them into one set to remove some redundancy.
context "when running the specs that were previously in another file" do
let(:user) { create(:user) }
let(:organization) { create(:organization) }
let(:cl_category) { create(:listing_category, cost: 1) }
let(:params) do
{
listing: {
title: "Hey",
listing_category_id: cl_category.id,
body_markdown: "hey hey my my",
tag_list: "ruby, rails, go"
}
}
end
describe "POST /listings" do
before do
create_list(:credit, 20, user_id: user.id)
sign_in user
end
it "creates proper listing if credits are available" do
post "/listings", params: params
expect(Listing.last.processed_html).to include("hey my")
end
it "spends credits" do
num_credits = Credit.where(spent: true).size
post "/listings", params: params
expect(Credit.where(spent: true).size).to be > num_credits
end
it "adds tags" do
post "/listings", params: params
expect(Listing.last.cached_tag_list).to include("rails")
end
it "creates the listing under the user" do
post "/listings", params: params
expect(Listing.last.user_id).to eq user.id
end
it "creates the listing for the user if no organization_id is selected" do
create(:organization_membership, user_id: user.id, organization_id: organization.id)
post "/listings", params: params
expect(Listing.last.organization_id).to eq nil
expect(Listing.last.user_id).to eq user.id
end
it "creates the listing for the organization" do
create(:organization_membership, user_id: user.id, organization_id: organization.id)
params[:listing][:organization_id] = organization.id
post "/listings", params: params
expect(Listing.last.organization_id).to eq(organization.id)
end
it "does not create an org listing if a different org member doesn't belong to the org" do
another_org = create(:organization)
create(:organization_membership, user_id: user.id, organization_id: another_org.id)
params[:listing][:organization_id] = organization.id
expect do
post "/listings", params: params
end.to raise_error Pundit::NotAuthorizedError
end
context "when rate limiting" do
let(:rate_limiter) { RateLimitChecker.new(user) }
before do
allow(RateLimitChecker).to receive(:new).and_return(rate_limiter)
sign_in user
end
it "increments rate limit for listing_creation" do
allow(rate_limiter).to receive(:track_limit_by_action)
post "/listings", params: params
expect(rate_limiter).to have_received(:track_limit_by_action).with(:listing_creation)
end
it "returns a 429 status when rate limit is reached" do
allow(rate_limiter).to receive(:limit_by_action).and_return(true)
post "/listings", params: params
expect(response.status).to eq(429)
end
end
end
describe "GET /listings/edit" do
let(:listing) { create(:listing, user_id: user.id) }
before do
create_list(:credit, 20, user_id: user.id)
sign_in user
end
it "has page content" do
get "/listings/#{listing.id}/edit"
expect(response.body).to include("You can bump your listing")
end
end
describe "PUT /listings/:id" do
let(:listing) { create(:listing, user_id: user.id) }
before do
create_list(:credit, 20, user_id: user.id)
sign_in user
end
it "updates bumped_at if action is bump" do
put "/listings/#{listing.id}", params: {
listing: { action: "bump" }
}
expect(Listing.last.bumped_at).to be > 10.seconds.ago
end
it "updates publish if action is unpublish" do
put "/listings/#{listing.id}", params: {
listing: { action: "unpublish" }
}
expect(Listing.last.published).to eq(false)
end
it "updates body_markdown" do
put "/listings/#{listing.id}", params: {
listing: { body_markdown: "hello new markdown" }
}
expect(Listing.last.body_markdown).to eq("hello new markdown")
end
it "does not update body_markdown if not bumped/created recently" do
listing.update_column(:bumped_at, 50.hours.ago)
put "/listings/#{listing.id}", params: {
listing: { body_markdown: "hello new markdown" }
}
expect(Listing.last.body_markdown).not_to eq("hello new markdown")
end
it "updates other fields" do
put "/listings/#{listing.id}", params: {
listing: { body_markdown: "hello new markdown", title: "New title!", tag_list: "new, tags, hey" }
}
expect(Listing.last.title).to eq("New title!")
expect(Listing.last.cached_tag_list).to include("hey")
end
it "does not update other fields" do
listing.update_column(:bumped_at, 50.hours.ago)
put "/listings/#{listing.id}", params: {
listing: { body_markdown: "hello new markdown", title: "New title!", tag_list: "new, tags, hey" }
}
expect(Listing.last.title).not_to eq("New title!")
expect(Listing.last.cached_tag_list).not_to include("hey")
end
end
end