Merge pull request codetriage#1743 from hbontempo-br/feature/ignore-deleted-archived-repos

Changes default behaviour to ignore archived and deleted repos

Author: schneems

app/controllers/repo_based_controller.rb

@@ -7,7 +7,9 @@ def name_from_params(options)
     [options[:name], options[:format]].compact.join('.')
   end
 
-  def find_repo(options)
-    Repo.find_by_full_name(options[:full_name].downcase)
+  def find_repo(options, only_active: true)
+    repo = Repo
+    repo = repo.active if only_active
+    repo.find_by_full_name(options[:full_name].downcase)
   end
 end

app/controllers/repos_controller.rb

@@ -53,12 +53,12 @@ def create
   end
 
   def edit
-    @repo = find_repo(params)
+    @repo = find_repo(params, only_active: false)
     redirect_to root_path, notice: "You cannot edit this repo" unless current_user.able_to_edit_repo?(@repo)
   end
 
   def update
-    @repo = find_repo(params)
+    @repo = find_repo(params, only_active: false)
     if @repo.update(repo_params)
       redirect_to @repo, notice: "Repo updated"
     else

app/mailers/user_mailer.rb

@@ -79,7 +79,7 @@ def poke_inactive(user:, min_issue_count:, min_subscriber_count:)
     return unless set_and_check_user(user)
     languages = @user.favorite_languages&.sort || []
 
-    query = Repo
+    query = Repo.active
     query = repo.where(language: languages) if !languages.empty?
     query = query
             .where("issues_count >= ?", min_issue_count)

app/models/repo.rb

@@ -24,6 +24,13 @@ class Repo < ActiveRecord::Base
 
   CLASS_FOR_DOC_LANGUAGE = { "ruby" => DocsDoctor::Parsers::Ruby::Yard }
 
+  scope :archived, -> { where(archived: true) }
+  scope :not_archived, -> { where(archived: false) }
+  scope :removed_from_github, -> { where(removed_from_github: true) }
+  scope :not_removed_from_github, -> { where(removed_from_github: false) }
+
+  scope :active, -> { not_archived.not_removed_from_github }
+
   def class_for_doc_language
     language && CLASS_FOR_DOC_LANGUAGE[language.downcase]
   end
@@ -208,6 +215,10 @@ def repo_path
     File.join 'repos', path
   end
 
+  def active?
+    removed_from_github == false && archived == false
+  end
+
   def self.find_by_full_name(full_name)
     Repo.find_by!(full_name: full_name)
   end

app/models/user.rb

@@ -203,6 +203,7 @@ def issue_assignments_to_deliver(assign: true)
   private
 
   def issue_assigner
-    @issue_assigner ||= IssueAssigner.new(self, repo_subscriptions)
+    subs = repo_subscriptions.select { |s| s.repo.active? }
+    @issue_assigner ||= IssueAssigner.new(self, subs)
   end
 end

lib/tasks/schedule.rake

@@ -13,18 +13,14 @@ namespace :schedule do
 
   desc "pulls in files from repos and adds them to the database"
   task process_repos: :environment do
-    Repo.where("docs_subscriber_count > 0").select(:id, :removed_from_github).find_each(batch_size: 1000) do |repo|
-      next if repo.removed_from_github?
-
+    Repo.active.where("docs_subscriber_count > 0").select(:id).find_each(batch_size: 1000) do |repo|
       PopulateDocsJob.perform_later(repo.id)
     end
   end
 
   desc 'Populates github issues'
   task populate_issues: :environment do
-    Repo.select(:id, :removed_from_github).find_each(batch_size: 100) do |repo|
-      next if repo.removed_from_github?
-
+    Repo.active.select(:id, :removed_from_github).find_each(batch_size: 100) do |repo|
       PopulateIssuesJob.perform_later(repo.id)
     end
   end
@@ -50,9 +46,7 @@ namespace :schedule do
   desc 'Marks issues as closed'
   task mark_closed: :environment do
     Issue.queue_mark_old_as_closed!
-    Repo.find_each(batch_size: 100) do |repo|
-      next if repo.removed_from_github?
-
+    Repo.active.find_each(batch_size: 100) do |repo|
       repo.force_issues_count_sync!
     end
   end
@@ -61,6 +55,7 @@ namespace :schedule do
   task poke_inactive: :environment do
     next unless Date.today.tuesday?
 
+    # TODO Ensure not archived, ensure not removed from github
     perc_90_issues_count = ActiveRecord::Base.connection.select_one("SELECT PERCENTILE_CONT(0.90) WITHIN GROUP(ORDER BY issues_count) FROM repos;")["percentile_cont"]
     perc_90_subscriber_count = ActiveRecord::Base.connection.select_one("SELECT PERCENTILE_CONT(0.90) WITHIN GROUP(ORDER BY subscribers_count) FROM repos;")["percentile_cont"]
 
@@ -115,7 +110,7 @@ namespace :schedule do
 
   desc 'fetch and assign labels for repos'
   task fetch_labels_and_assign: :environment do
-    Repo.find_each(batch_size: 100) do |repo|
+    Repo.active.find_each(batch_size: 100) do |repo|
       RepoLabelAssigner.new(repo: repo).create_and_associate_labels!
     end
   end

test/fixtures/repo_subscriptions.yml

@@ -26,6 +26,14 @@ jroes_to_rails:
   last_sent_at:
   email_limit: 5
 
+empty_to_archived:
+  repo: archived_repo
+  user: empty
+  created_at: 2013-10-29 21:09:48.351554000 Z
+  updated_at: 2013-10-29 21:09:48.351554000 Z
+  last_sent_at:
+  email_limit: 5
+
 read_doc_only:
   repo: issue_triage_sandbox
   user: foo_user

test/fixtures/repos.yml

@@ -72,4 +72,25 @@ scene_hub_v2:
   updated_at: 2015-01-21T22:08:43Z
   issues_count: 0
 
+deleted_repo:
+  user_name: empty
+  name: deleted
+  full_name: empty/deleted
+  language: Ruby
+  created_at: 2014-12-19T02:33:42Z
+  updated_at: 2015-01-21T22:08:43Z
+  issues_count: 0
+  removed_from_github: true
+
+
+archived_repo:
+  user_name: empty
+  name: archived
+  full_name: empty/archived
+  language: Ruby
+  created_at: 2014-12-19T02:33:42Z
+  updated_at: 2015-01-21T22:08:43Z
+  issues_count: 0
+  removed_from_github: true
+
 

test/integration/repos_test.rb

@@ -2,7 +2,7 @@
 
 require 'test_helper'
 
-class ReposTest < ActionController::TestCase
+class ReposTest < ActionDispatch::IntegrationTest
   test "regular repo routes" do
     assert_routing(
       'rails/rails',
@@ -27,4 +27,49 @@ class ReposTest < ActionController::TestCase
       { controller: "repos", action: "show", full_name: "angular/angular.js" },
     )
   end
+
+  test "access valid repo" do
+    get repo_url 'rails/rails'
+    assert_response :success
+  end
+
+  test "access deleted_from_github repo" do
+    get repo_url 'empty/deleted'
+    assert_redirected_to new_repo_url(user_name: 'empty', name: 'deleted')
+  end
+
+  test "access archived repo" do
+    get repo_url 'empty/archived'
+    assert_redirected_to new_repo_url(user_name: 'empty', name: 'archived')
+  end
+
+  test "edit repo where user is not the owner" do
+    login_as(users(:empty))
+    get edit_repo_url 'rails/rails'
+    assert_redirected_to root_path
+  end
+
+  test "edit deleted_from_github repo" do
+    login_as(users(:empty))
+    get edit_repo_url 'empty/deleted'
+    assert_response :success
+  end
+
+  test "edit archived repo" do
+    login_as(users(:empty))
+    get edit_repo_url 'empty/archived'
+    assert_response :success
+  end
+
+  test "update repo" do
+    login_as(users(:empty))
+    repo = repos(:archived_repo)
+    assert_changes -> {
+      repo.notes
+    } do
+      patch repo_url 'empty/archived', params: { repo: { notes: 'Updated notes' } }
+      repo.reload
+    end
+    assert_redirected_to repo_url 'empty/archived'
+  end
 end

test/unit/user_test.rb

@@ -160,4 +160,8 @@ class UserTest < ActiveSupport::TestCase
   test "#repos_fetcher" do
     assert users(:mockstar).repos_fetcher(GithubFetcher::Repos::OWNED).is_a? GithubFetcher::Repos
   end
+
+  test "#issue_assignments_to_deliver should ignore repos not active" do
+    assert_empty users(:empty).issue_assignments_to_deliver
+  end
 end