diff --git a/app/controllers/repo_based_controller.rb b/app/controllers/repo_based_controller.rb index 66980aebf..315f553d0 100644 --- a/app/controllers/repo_based_controller.rb +++ b/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 diff --git a/app/controllers/repos_controller.rb b/app/controllers/repos_controller.rb index 6e408614d..822892d99 100644 --- a/app/controllers/repos_controller.rb +++ b/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 diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 3d234e5c4..c10f257d5 100644 --- a/app/mailers/user_mailer.rb +++ b/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) diff --git a/app/models/repo.rb b/app/models/repo.rb index eaf6ba5ae..034359c03 100644 --- a/app/models/repo.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index 65955de9e..c77d87cf3 100644 --- a/app/models/user.rb +++ b/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 diff --git a/lib/tasks/schedule.rake b/lib/tasks/schedule.rake index 83a918e40..4e80a08d2 100644 --- a/lib/tasks/schedule.rake +++ b/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 diff --git a/test/fixtures/repo_subscriptions.yml b/test/fixtures/repo_subscriptions.yml index 986a3b04b..ffff15655 100644 --- a/test/fixtures/repo_subscriptions.yml +++ b/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 diff --git a/test/fixtures/repos.yml b/test/fixtures/repos.yml index 51db50db1..914694165 100644 --- a/test/fixtures/repos.yml +++ b/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 + diff --git a/test/integration/repos_test.rb b/test/integration/repos_test.rb index 1f5889d8b..dcd864a7b 100644 --- a/test/integration/repos_test.rb +++ b/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 diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index eceb4a5b7..7b8fa06a7 100644 --- a/test/unit/user_test.rb +++ b/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