Remove controller specs (forem#2224)

* Create more request specs

* Update wording

* Update guard to work with requests

* Fix broken spec

* Remove rails-controller-testing gem

* Move article_specs

* Move last controller spec out

* Remove controller spec config

* Fix broken spec

* Remove all use of render_template

* Refactor spec

* Tweak User#remember_me

* Create ArticleAnalyticFetcher spec

* Undo User#remember_me

* Use srand in rspec

* Create FeedbackMessagesHelper spec

* Add DigestMailer spec

* Fix broken test

Gemfile

@@ -145,7 +145,6 @@ group :test do
   gem "fake_stripe", "~> 0.2"
   gem "launchy", "~> 2.4"
   gem "pundit-matchers", "~> 1.6"
-  gem "rails-controller-testing", "~> 1.0"
   gem "ruby-prof", "~> 0.17", require: false
   gem "selenium-webdriver", "~> 3.141"
   gem "shoulda-matchers", "4.0.0.rc1", require: false

Gemfile.lock

@@ -719,10 +719,6 @@ GEM
       railties (= 5.1.6.2)
       sprockets-rails (>= 2.0.0)
     rails-assets-airbrake-js-client (1.5.0)
-    rails-controller-testing (1.0.4)
-      actionpack (>= 5.0.1.x)
-      actionview (>= 5.0.1.x)
-      activesupport (>= 5.0.1.x)
     rails-dom-testing (2.0.3)
       activesupport (>= 4.2.0)
       nokogiri (>= 1.6)
@@ -1057,7 +1053,6 @@ DEPENDENCIES
   rack-timeout (~> 0.5)
   rails (~> 5.1.6)
   rails-assets-airbrake-js-client (~> 1.5)!
-  rails-controller-testing (~> 1.0)
   rails-observers (~> 0.1)
   rb-fsevent (~> 0.10)
   recaptcha (~> 4.13)

Guardfile

@@ -64,7 +64,7 @@ guard :rspec, rspec_options do
   watch(rails.controllers) do |m|
     [
       rspec.spec.call("routing/#{m[1]}_routing"),
-      rspec.spec.call("controllers/#{m[1]}_controller"),
+      rspec.spec.call("requests/#{m[1]}"),
       rspec.spec.call("acceptance/#{m[1]}"),
     ]
   end

app/helpers/feedback_messages_helper.rb

@@ -10,10 +10,8 @@ def offender_email_details
       Thanks,
       DEV team
     HEREDOC
-    {
-      subject: "DEV Code of Conduct Violation",
-      body: body
-    }.freeze
+
+    { subject: "DEV Code of Conduct Violation", body: body }.freeze
   end
 
   def reporter_email_details
@@ -26,10 +24,8 @@ def reporter_email_details
 
       DEV Team
     HEREDOC
-    {
-      subject: "DEV Report",
-      body: body
-    }.freeze
+
+    { subject: "DEV Report", body: body }.freeze
   end
 
   def affected_email_details
@@ -42,9 +38,7 @@ def affected_email_details
 
       DEV Team
     HEREDOC
-    {
-      subject: "Courtesy Notice from DEV",
-      body: body
-    }.freeze
+
+    { subject: "Courtesy Notice from DEV", body: body }.freeze
   end
 end

app/labor/article_analytics_fetcher.rb

@@ -43,6 +43,6 @@ def should_fetch(article)
   end
 
   def occasionally_force_fetch?
-    rand(25) == 1 if Rails.env.production?
+    rand(25) == 1
   end
 end

spec/helpers/feedback_messages_helper_spec.rb

@@ -0,0 +1,21 @@
+require "rails_helper"
+
+RSpec.describe FeedbackMessagesHelper, type: :helper do
+  describe "#offender_email_details" do
+    it "have proper subject and body" do
+      expect(helper.offender_email_details).to include(subject: "DEV Code of Conduct Violation", body: a_string_starting_with("Hi"))
+    end
+  end
+
+  describe "#reporter_email_details" do
+    it "have proper subject and body" do
+      expect(helper.reporter_email_details).to include(subject: "DEV Report", body: a_string_starting_with("Hi"))
+    end
+  end
+
+  describe "#affected_email_details" do
+    it "have proper subject and body" do
+      expect(helper.affected_email_details).to include(subject: "Courtesy Notice from DEV", body: a_string_starting_with("Hi"))
+    end
+  end
+end

spec/labor/article_analytics_fetcher_spec.rb

@@ -0,0 +1,47 @@
+require "rails_helper"
+
+RSpec.describe ArticleAnalyticsFetcher do
+  let(:stubbed_ga) { double }
+
+  before do
+    srand(2) # disabling #occasionally_force_fetch
+    allow(Notification).to receive(:send_milestone_notification)
+    allow(GoogleAnalytics).to receive(:new).and_return(stubbed_ga)
+  end
+
+  describe "#update_analytics" do
+    context "when positive_reactions_count is LOWER than previous_positive_reactions_count" do
+      it "does nothing " do
+        article = build_stubbed(:article, positive_reactions_count: 2, previous_positive_reactions_count: 3)
+        allow(Article).to receive(:where).and_return([article])
+        described_class.new.update_analytics(1)
+        expect(Notification).not_to have_received(:send_milestone_notification)
+      end
+    end
+
+    context "when positive_reactions_count is HIGHER than previous_positive_reactions_count" do
+      let(:article) { build_stubbed(:article, positive_reactions_count: 5, previous_positive_reactions_count: 3) }
+      let(:pageview) { {} }
+      let(:counts) { 1000 }
+
+      before do
+        pageview[article.id] = counts
+        allow(stubbed_ga).to receive(:get_pageviews).and_return(pageview)
+        allow(article).to receive(:update_columns)
+        allow(Article).to receive(:where).and_return([article])
+        described_class.new.update_analytics(1)
+      end
+
+      it "sends send_milestone_notification for Reaction and View" do
+        %w[Reaction View].each do |type|
+          expect(Notification).to have_received(:send_milestone_notification).with(type: type, article: article)
+        end
+      end
+
+      it "updates appropriate column" do
+        expect(article).to have_received(:update_columns).with(previous_positive_reactions_count: article.positive_reactions_count)
+        expect(article).to have_received(:update_columns).with(page_views_count: counts)
+      end
+    end
+  end
+end

spec/mailers/digest_mailer_spec.rb

@@ -0,0 +1,16 @@
+require "rails_helper"
+
+RSpec.describe DigestMailer, type: :mailer do
+  describe "#digest_email" do
+    it "works correctly" do
+      user = build_stubbed(:user)
+      article = build_stubbed(:article)
+      allow(article).to receive(:title).and_return("test title")
+      email = described_class.digest_email(user, [article])
+
+      expect(email.subject).not_to be_nil
+      expect(email.to).to eq([user.email])
+      expect(email.from).to eq(["[email protected]"])
+    end
+  end
+end

spec/rails_helper.rb

@@ -42,7 +42,6 @@
   config.fixture_path = "#{::Rails.root}/spec/fixtures"
 
   config.include ApplicationHelper
-  config.include Devise::Test::ControllerHelpers, type: :controller
   config.include Devise::Test::ControllerHelpers, type: :view
   config.include Devise::Test::IntegrationHelpers, type: :feature
   config.include Devise::Test::IntegrationHelpers, type: :request

spec/requests/articles_api_spec.rb → spec/requests/api/v0/articles_spec.rb

@@ -1,6 +1,6 @@
 require "rails_helper"
 
-RSpec.describe "ArticlesApi", type: :request do
+RSpec.describe "Api::V0::Articles", type: :request do
   let(:user1) { create(:user) }
   let(:user2) { create(:user) }
 

spec/controllers/articles_controller_spec.rb → spec/requests/articles_spec.rb

@@ -1,23 +1,20 @@
-# controller specs are now discouraged in favor of request specs.
-# This file should eventually be removed
 require "rails_helper"
 
-RSpec.describe ArticlesController, type: :controller do
+RSpec.describe "Articles", type: :request do
   let(:user) { create(:user) }
 
-  describe "GET #feed" do
-    render_views
-
-    it "works" do
-      get :feed, format: :rss
+  describe "GET /feed" do
+    it "returns rss+xml content" do
+      get "/feed"
       expect(response.status).to eq(200)
+      expect(response.content_type).to eq("application/rss+xml")
     end
 
     context "when :username param is not given" do
       let!(:featured_article) { create(:article, featured: true) }
       let!(:not_featured_article) { create(:article, featured: false) }
 
-      before { get :feed, format: :rss }
+      before { get "/feed" }
 
       it "returns only featured articles" do
         expect(response.body).to include(featured_article.title)
@@ -33,7 +30,7 @@
 
     context "when :username param is given and belongs to a user" do
       include_context "when user/organization articles exist"
-      before { get :feed, params: { username: user.username }, format: :rss }
+      before { get "/feed", params: { username: user.username } }
 
       it "returns only articles for that user" do
         expect(response.body).to include(user_article.title)
@@ -43,7 +40,7 @@
 
     context "when :username param is given and belongs to an organization" do
       include_context "when user/organization articles exist"
-      before { get :feed, params: { username: organization.slug }, format: :rss }
+      before { get "/feed", params: { username: organization.slug } }
 
       it "returns only articles for that organization" do
         expect(response.body).not_to include(user_article.title)
@@ -53,26 +50,26 @@
 
     context "when :username param is given but it belongs to nither user nor organization" do
       include_context "when user/organization articles exist"
-      before { get :feed, params: { username: "unknown" }, format: :rss }
+      before { get "/feed", params: { username: "unknown" } }
 
       it("renders empty body") { expect(response.body).to be_empty }
     end
   end
 
-  describe "GET #new" do
+  describe "GET /new" do
     before { sign_in user }
 
     context "with authorized user" do
       it "returns a new article" do
-        get :new
-        expect(response).to render_template(:new)
+        get "/new"
+        expect(response).to have_http_status(:ok)
       end
     end
 
     context "with authorized user with tag param" do
       it "returns a new article" do
-        get :new, params: { slug: "shecoded" }
-        expect(response).to render_template(:new)
+        get "/new", params: { slug: "shecoded" }
+        expect(response).to have_http_status(:ok)
       end
     end
   end

spec/requests/async_info_spec.rb

@@ -1,13 +1,9 @@
 require "rails_helper"
 
 RSpec.describe "AsyncInfo", type: :request do
-  let(:user) { build(:user) }
-
   describe "GET /async_info/base_data" do
-    describe "anonymous user" do
-      before do
-        get "/async_info/base_data"
-      end
+    context "when not logged-in" do
+      before { get "/async_info/base_data" }
 
       it "returns token" do
         expect(response.body).to include("token")
@@ -18,18 +14,12 @@
       end
     end
 
-    describe "logged in user" do
-      before do
-        sign_in user
+    context "when logged int" do
+      it "returns token and user" do
+        allow(cookies).to receive(:[]).with(:remember_user_token).and_return(nil)
+        sign_in create(:user)
         get "/async_info/base_data"
-      end
-
-      it "returns token" do
-        expect(response.body).to include("token")
-      end
-
-      it "does return user" do
-        expect(response.body).to include("user")
+        expect(response.body).to include("token", "user")
       end
     end
   end

spec/requests/chat_channels_spec.rb

@@ -79,11 +79,12 @@
       end
 
       it "returns 200" do
-        expect(response.status).to eq(200)
+        expect(response).to have_http_status(:ok)
       end
 
       it "returns the channel" do
-        expect(response).to render_template(:show)
+        result = { messages: [], chatChannelId: chat_channel.id }.to_json
+        expect(response.body).to eq(result)
       end
     end
 

spec/requests/editor_spec.rb

@@ -3,14 +3,12 @@
 RSpec.describe "Editor", type: :request do
   describe "GET /new" do
     context "when not logged-in" do
-      it "does not render markdown form" do
-        get "/new"
-        expect(response).not_to render_template(:markdown_form)
-      end
-
       it "asks the stray-user to 'Sign In or Create Your Account'" do
         get "/new"
+        expect(response).to have_http_status(:ok)
         expect(response.body).to include("Great to have you")
+        # should actually be looking for textarea tag
+        expect(response.body).not_to include("textarea")
       end
     end
   end
@@ -30,7 +28,7 @@
       it "render markdown form" do
         sign_in user
         get "/#{user.username}/#{article.slug}/edit"
-        expect(response).to render_template("articles/_markdown_form")
+        expect(response).to have_http_status(:ok)
       end
     end
   end

spec/requests/email_subscriptions_spec.rb

@@ -36,7 +36,7 @@ def generate_token(user_id)
       token = generate_token(user.id)
       Timecop.freeze(32.days.from_now) do
         get email_subscriptions_unsubscribe_url(ut: token)
-        expect(response).to render_template("invalid_token")
+        expect(response.body).to include("Token expired or invalid")
       end
     end
   end

spec/controllers/internal_users_controller_spec.rb → spec/requests/internal/users_spec.rb

@@ -1,6 +1,6 @@
 require "rails_helper"
 
-RSpec.describe "internal/users", type: :request do
+RSpec.describe "Internal::Users", type: :request do
   let(:user) { create(:user, twitter_username: nil) }
   let(:user2) { create(:user, twitter_username: "Twitter") }
   let(:user3) { create(:user) }

spec/requests/notifications/reads_spec.rb

@@ -0,0 +1,27 @@
+require "rails_helper"
+
+RSpec.describe "Notifications::Reads", type: :request do
+  describe "POST /notifications/reads" do
+    let(:stubbed_service_object) { double }
+    let(:user) { create(:user) }
+
+    before do
+      sign_in user
+      allow(ReadNotificationsService).to receive(:new).and_return(stubbed_service_object)
+      allow(stubbed_service_object).to receive(:mark_as_read)
+    end
+
+    it "marks notifications as read" do
+      post "/notifications/reads/"
+      expect(response).to have_http_status(:ok)
+      expect(stubbed_service_object).to have_received(:mark_as_read).once
+    end
+
+    it "marks personal and org Notifications as read" do
+      allow(user).to receive(:organization_id).and_return(1)
+      post "/notifications/reads/", params: { org_id: 1 }
+      expect(response).to have_http_status(:ok)
+      expect(stubbed_service_object).to have_received(:mark_as_read).twice
+    end
+  end
+end