Handle GitHub errors

app/models/user.rb

@@ -39,6 +39,10 @@ class User < ApplicationRecord
                  unless: ->(user) { user.sufficient_eligible_prs? }
     end
 
+    event :deactivate do
+      transition all => :inactive
+    end
+
     state all - [:new] do
       validates :terms_acceptance, acceptance: true
       validates :email, presence: true
@@ -52,7 +56,7 @@ class User < ApplicationRecord
       validates :sticker_coupon, absence: true
     end
 
-    state all - %i[new registered waiting] do
+    state all - %i[new registered waiting inactive] do
       validates :receipt, presence: true
     end
 
@@ -90,6 +94,16 @@ def win
         in: [false], message: 'user has too many sufficient eligible prs' }
     end
 
+    state :inactive do
+      validates :last_error, inclusion: {
+        in: ['GithubPullRequestService::UserNotFoundOnGithubError',
+             'Octokit::AccountSuspended'],
+        message: "user's last_error must be
+          UserNotFoundOnGithubError or AccountSuspended" }
+
+      validates :state_before_inactive, presence: true
+    end
+
     before_transition do |user, _transition|
       UserPullRequestSegmentUpdaterService.call(user)
     end
@@ -102,6 +116,10 @@ def win
       user.receipt = user.scoring_pull_requests
     end
 
+    before_transition to: :inactive do |user, transition|
+      user.update_state_before_inactive(transition)
+    end
+
     after_transition to: :waiting, do: :update_waiting_since
 
     after_transition to: :completed do |user, _transition|
@@ -148,6 +166,10 @@ def update_waiting_since
     ))
   end
 
+  def update_state_before_inactive(transition)
+    self.state_before_inactive = transition.from
+  end
+
   def waiting_for_week?
     return false if waiting_since.nil?
 

app/services/github_error_handler.rb

@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+module GithubErrorHandler
+  module_function
+
+  HANDLED_ERRORS = [
+    Octokit::AccountSuspended,
+    GithubPullRequestService::UserNotFoundOnGithubError
+  ].freeze
+
+  def with_error_handling(user)
+    yield
+  rescue *HANDLED_ERRORS => e
+    process_github_error(e, user)
+  end
+
+  def process_github_error(error, user)
+    # rubocop:disable Rails/SkipsModelValidations
+    user.update_attribute(:last_error, error.class)
+    # rubocop:enable Rails/SkipsModelValidations
+    case error
+    when Octokit::AccountSuspended
+      process_user_missing_error(error, user)
+    when GithubPullRequestService::UserNotFoundOnGithubError
+      process_user_missing_error(error, user)
+    else
+      raise error
+    end
+  end
+
+  def process_user_missing_error(_error, user)
+    user.deactivate
+
+    return unless (user_stat = UserStat.where(user_id: user.id).first)
+
+    user_stat.destroy
+  end
+end

app/services/import_pr_metadata_service.rb

@@ -6,17 +6,12 @@ module ImportPrMetadataService
   def call(user)
     pr_service = PullRequestService.new(user, randomize_token: true)
 
-    begin
+    GithubErrorHandler.with_error_handling(user) do
       pr_data = pr_service.all
-    rescue GithubPullRequestService::UserNotFoundOnGithubError
-      user_stat = UserStat.where(user_id: user.id).first
 
-      user_stat.destroy
-      return
-    end
-
-    pr_data.map do |pr|
-      ImportOnePrMetadataJob.perform_async(pr.url)
+      pr_data.map do |pr|
+        ImportOnePrMetadataJob.perform_async(pr.url)
+      end
     end
   end
 end

app/services/import_repos_metadata_service.rb

@@ -6,18 +6,13 @@ module ImportReposMetadataService
   def call(user)
     pr_service = PullRequestService.new(user, randomize_token: true)
 
-    begin
+    GithubErrorHandler.with_error_handling(user) do
       pr_data = pr_service.all
-    rescue GithubPullRequestService::UserNotFoundOnGithubError
-      user_stat = UserStat.where(user_id: user.id).first
 
-      user_stat.destroy
-      return
-    end
-
-    pr_data.map do |pr|
-      repo_id = pr.github_pull_request.repo_id
-      ImportRepoMetadataJob.perform_async(repo_id)
+      pr_data.map do |pr|
+        repo_id = pr.github_pull_request.repo_id
+        ImportRepoMetadataJob.perform_async(repo_id)
+      end
     end
   end
 end

app/services/try_user_transition_service.rb

@@ -2,13 +2,15 @@
 
 module TryUserTransitionService
   def self.call(user)
-    case user.state
-    when 'registered'
-      TryUserTransitionFromRegisteredService.call(user)
-    when 'waiting'
-      TryUserTransitionFromWaitingService.call(user)
-    when 'completed'
-      TryUserTransitionFromCompletedService.call(user)
+    GithubErrorHandler.with_error_handling(user) do
+      case user.state
+      when 'registered'
+        TryUserTransitionFromRegisteredService.call(user)
+      when 'waiting'
+        TryUserTransitionFromWaitingService.call(user)
+      when 'completed'
+        TryUserTransitionFromCompletedService.call(user)
+      end
     end
   end
 end

app/services/user_state_transition_segment_service.rb

@@ -15,6 +15,7 @@ def call(user, transition)
     when :incomplete then incomplete(user)
     when :ineligible then ineligible(user)
     when :won then won(user, transition)
+    when :deactivate then deactivate(user)
     end
   end
 
@@ -61,6 +62,11 @@ def won(user, transition)
     end
   end
 
+  def deactivate(user)
+    segment(user).track('user_inactive')
+    segment(user).identify(state: 'inactive')
+  end
+
   def segment(user)
     SegmentService.new(user)
   end

db/migrate/20191107151321_add_last_error_to_user.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddLastErrorToUser < ActiveRecord::Migration[5.2]
+  def change
+    add_column :users, :last_error, :string
+  end
+end

db/migrate/20191111195936_add_state_before_inactive_to_user.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddStateBeforeInactiveToUser < ActiveRecord::Migration[5.2]
+  def change
+    add_column :users, :state_before_inactive, :string
+  end
+end

db/schema.rb

@@ -10,11 +10,19 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_10_28_180239) do
+ActiveRecord::Schema.define(version: 2019_11_11_195936) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
+  create_table "daily_pr_counts", force: :cascade do |t|
+    t.date "date", null: false
+    t.integer "count", default: 0
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["date"], name: "index_daily_pr_counts_on_date", unique: true
+  end
+
   create_table "issues", force: :cascade do |t|
     t.string "title", limit: 191, null: false
     t.bigint "number", null: false
@@ -104,6 +112,7 @@
     t.integer "user_id"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.boolean "deleted"
   end
 
   create_table "users", force: :cascade do |t|
@@ -119,6 +128,8 @@
     t.string "state"
     t.datetime "waiting_since"
     t.jsonb "receipt"
+    t.string "last_error"
+    t.string "state_before_inactive"
   end
 
 end

spec/models/user_spec.rb

@@ -559,5 +559,49 @@
         end
       end
     end
+
+    describe '#deactivate' do
+      before do
+        allow(UserStateTransitionSegmentService)
+          .to receive(:call).and_return(true)
+
+        allow_any_instance_of(User)
+          .to receive(:last_error).and_return('Octokit::AccountSuspended')
+      end
+
+      context 'the user is in any state' do
+        let(:user) { FactoryBot.create(:user) }
+        context 'the state_before_inactive gets set successfully' do
+          before { user.deactivate }
+
+          it 'transitions the user to the inactive state' do
+            expect(user.state).to eq('inactive')
+          end
+
+          it 'saves the previous state as state_before_inactive' do
+            expect(user.state_before_inactive).to eq('registered')
+          end
+        end
+
+        context 'the state_before_inactive does not get set' do
+          before do
+            allow(user).to receive(:state_before_inactive).and_return(nil)
+          end
+
+          it 'does not transition the user to the inactive state' do
+            user.deactivate
+
+            expect(user.state).to_not eq('inactive')
+          end
+
+          it 'applies the correct errors to the user object' do
+            user.deactivate
+
+            expect(user.errors.messages[:state_before_inactive].first)
+              .to eq("can't be blank")
+          end
+        end
+      end
+    end
   end
 end

spec/services/github_error_handler_spec.rb

@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe GithubErrorHandler do
+  let(:user) { FactoryBot.create(:user) }
+
+  # stub API call that will happen due to User#deactivate
+  before do
+    allow(UserStateTransitionSegmentService)
+      .to receive(:call).and_return(true)
+  end
+
+  describe '.process_github_error' do
+    context 'The error is UserNotFoundOnGithubError' do
+      let(:error) { GithubPullRequestService::UserNotFoundOnGithubError.new }
+
+      it 'calls process_user_missing_error' do
+        expect(GithubErrorHandler)
+          .to receive(:process_user_missing_error).with(error, user)
+
+        GithubErrorHandler.process_github_error(error, user)
+      end
+
+      it 'correctly updates the last_error column on the user' do
+        GithubErrorHandler.process_github_error(error, user)
+
+        expect(user.last_error).to eq(error.class.to_s)
+      end
+    end
+
+    context 'The error is Octokit::AccountSuspended' do
+      let(:error) { Octokit::AccountSuspended.new }
+
+      it 'calls process_user_missing_error' do
+        expect(GithubErrorHandler)
+          .to receive(:process_user_missing_error).with(error, user)
+
+        GithubErrorHandler.process_github_error(error, user)
+      end
+
+      it 'correctly updates the last_error column on the user' do
+        GithubErrorHandler.process_github_error(error, user)
+
+        expect(user.last_error).to eq(error.class.to_s)
+      end
+    end
+
+    context 'The error is an unhandled error' do
+      let(:error) { StandardError.new }
+
+      it 'raises the error' do
+        expect { GithubErrorHandler.process_github_error(error, user) }
+          .to raise_error(StandardError)
+      end
+    end
+  end
+
+  describe '.process_user_missing_error' do
+    let(:error) { GithubPullRequestService::UserNotFoundOnGithubError.new }
+    it 'destroys the UserStat' do
+      UserStat.create(user_id: user.id, data: user)
+
+      GithubErrorHandler.process_user_missing_error(error, user)
+
+      expect(UserStat.count).to eq(0)
+    end
+
+    it 'deactivates the user' do
+      # to pass user#deactivate validations
+      allow(user)
+        .to receive(:last_error).and_return('Octokit::AccountSuspended')
+
+      GithubErrorHandler.process_user_missing_error(error, user)
+
+      expect(user.state).to eq('inactive')
+    end
+  end
+end

spec/services/import_repos_metadata_service_spec.rb

@@ -5,6 +5,13 @@
 RSpec.describe ImportReposMetadataService do
   describe '.call' do
     let(:user) { FactoryBot.create(:user) }
+
+    # stub API call that will happen due to User#deactivate
+    before do
+      allow(UserStateTransitionSegmentService)
+        .to receive(:call).and_return(true)
+    end
+
     context 'The user has no PRs' do
       before do
         allow_any_instance_of(PullRequestService)