Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes default behaviour to ignore archived and deleted repos #1743

Merged
merged 7 commits into from
Oct 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions app/controllers/repo_based_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions app/controllers/repos_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/mailers/user_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 11 additions & 0 deletions app/models/repo.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
15 changes: 5 additions & 10 deletions lib/tasks/schedule.rake
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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"]

Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions test/fixtures/repo_subscriptions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions test/fixtures/repos.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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


47 changes: 46 additions & 1 deletion test/integration/repos_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

require 'test_helper'

class ReposTest < ActionController::TestCase
class ReposTest < ActionDispatch::IntegrationTest
test "regular repo routes" do
assert_routing(
'rails/rails',
Expand All @@ -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
4 changes: 4 additions & 0 deletions test/unit/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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