FIX: only create new category when repo_id is present (#53)

FIX: only create new category when repo_id is present (#53)

diff --git a/lib/discourse_code_review/state/github_repo_categories.rb b/lib/discourse_code_review/state/github_repo_categories.rb
index 7acf14d..4feff49 100644
--- a/lib/discourse_code_review/state/github_repo_categories.rb
+++ b/lib/discourse_code_review/state/github_repo_categories.rb
@@ -20,6 +20,7 @@ module DiscourseCodeReview::State::GithubRepoCategories
           category.save_custom_fields
         end
 
+        # search for category via repo_id
         if category.blank? && repo_id.present?
           category =
             Category.where(
@@ -33,34 +34,32 @@ module DiscourseCodeReview::State::GithubRepoCategories
             # update repository name in category custom field
             category.custom_fields[GITHUB_REPO_NAME] = repo_name
             category.save_custom_fields
+          else
+            # create new category
+            short_name = find_category_name(repo_name.split("/", 2).last)
+            category = Category.new(
+              name: short_name,
+              user: Discourse.system_user,
+              description: I18n.t('discourse_code_review.category_description', repo_name: repo_name)
+            )
+
+            if SiteSetting.code_review_default_parent_category.present?
+              category.parent_category_id = SiteSetting.code_review_default_parent_category.to_i
+            end
+
+            category.save!
+
+            if SiteSetting.code_review_default_mute_new_categories
+              existing_category_ids = Category.where(id: SiteSetting.default_categories_muted.split("|")).pluck(:id)
+              SiteSetting.default_categories_muted = (existing_category_ids << category.id).join("|")
+            end
+
+            category.custom_fields[GITHUB_REPO_ID] = repo_id
+            category.custom_fields[GITHUB_REPO_NAME] = repo_name
+            category.save_custom_fields
           end
         end
 
-        if !category
-          short_name = find_category_name(repo_name.split("/", 2).last)
-
-          category = Category.new(
-            name: short_name,
-            user: Discourse.system_user,
-            description: I18n.t('discourse_code_review.category_description', repo_name: repo_name)
-          )
-
-          if SiteSetting.code_review_default_parent_category.present?
-            category.parent_category_id = SiteSetting.code_review_default_parent_category.to_i
-          end
-
-          category.save!
-
-          if SiteSetting.code_review_default_mute_new_categories
-            existing_category_ids = Category.where(id: SiteSetting.default_categories_muted.split("|")).pluck(:id)
-            SiteSetting.default_categories_muted = (existing_category_ids << category.id).join("|")
-          end
-
-          category.custom_fields[GITHUB_REPO_ID] = repo_id if repo_id.present?
-          category.custom_fields[GITHUB_REPO_NAME] = repo_name
-          category.save_custom_fields
-        end
-
         category
       end
     end
diff --git a/spec/discourse_code_review/lib/github_pr_syncer_spec.rb b/spec/discourse_code_review/lib/github_pr_syncer_spec.rb
index e49eed5..37a7d69 100644
--- a/spec/discourse_code_review/lib/github_pr_syncer_spec.rb
+++ b/spec/discourse_code_review/lib/github_pr_syncer_spec.rb
@@ -100,7 +100,8 @@ describe DiscourseCodeReview::GithubPRSyncer do
 
   fab!(:category) do
     DiscourseCodeReview::State::GithubRepoCategories.ensure_category(
-      repo_name: 'owner/name'
+      repo_name: 'owner/name',
+      repo_id: '24'
     )
   end
 
diff --git a/spec/discourse_code_review/lib/github_repo_spec.rb b/spec/discourse_code_review/lib/github_repo_spec.rb
index 4bdfc9d..6ca1333 100644
--- a/spec/discourse_code_review/lib/github_repo_spec.rb
+++ b/spec/discourse_code_review/lib/github_repo_spec.rb
@@ -53,7 +53,7 @@ module DiscourseCodeReview
       end
 
       it "does not explode" do
-        repo = GithubRepo.new('fake_repo/fake_repo', nil, nil)
+        repo = GithubRepo.new('fake_repo/fake_repo', nil, nil, repo_id: 24)
         repo.stubs(:default_branch).returns("origin/master")
         repo.path = checkout_path
         repo.last_commit = nil
@@ -76,7 +76,7 @@ module DiscourseCodeReview
       end
 
       it "truncates the diff" do
-        repo = GithubRepo.new('fake_repo/fake_repo', nil, nil)
+        repo = GithubRepo.new('fake_repo/fake_repo', nil, nil, repo_id: 24)
         repo.stubs(:default_branch).returns("origin/master")
         repo.path = checkout_path
         repo.last_commit = nil
@@ -108,7 +108,7 @@ module DiscourseCodeReview
         sha = `git rev-parse HEAD`.strip
       end
 
-      repo = GithubRepo.new('fake_repo/fake_repo', nil, nil)
+      repo = GithubRepo.new('fake_repo/fake_repo', nil, nil, repo_id: 24)
       repo.stubs(:default_branch).returns("origin/master")
       repo.path = checkout_path
 
@@ -130,7 +130,7 @@ module DiscourseCodeReview
         sha = `git rev-parse HEAD`.strip
       end
 
-      repo = GithubRepo.new('fake_repo/fake_repo', nil, nil)
+      repo = GithubRepo.new('fake_repo/fake_repo', nil, nil, repo_id: 24)
       repo.stubs(:default_branch).returns("origin/master")
       repo.path = checkout_path
 
diff --git a/spec/discourse_code_review/lib/importer_spec.rb b/spec/discourse_code_review/lib/importer_spec.rb
index 2fdd907..88a645c 100644
--- a/spec/discourse_code_review/lib/importer_spec.rb
+++ b/spec/discourse_code_review/lib/importer_spec.rb
@@ -9,7 +9,7 @@ module DiscourseCodeReview
     end
 
     let(:parent_category) { Fabricate(:category) }
-    let(:repo) { GithubRepo.new("discourse/discourse", Octokit::Client.new, nil) }
+    let(:repo) { GithubRepo.new("discourse/discourse", Octokit::Client.new, nil, repo_id: 24) }
 
     it "creates categories with a description" do
       category = Category.find_by(id: Importer.new(repo).category_id)
@@ -41,7 +41,7 @@ module DiscourseCodeReview
       # lets muck stuff up first ... and create a dupe category
       Category.create!(name: 'discourse', user: Discourse.system_user)
 
-      repo = GithubRepo.new("discourse/discourse", Octokit::Client.new, nil)
+      repo = GithubRepo.new("discourse/discourse", Octokit::Client.new, nil, repo_id: 24)
       id = Importer.new(repo).category_id
 
       expect(id).to be > 0
@@ -49,7 +49,7 @@ module DiscourseCodeReview
     end
 
     it "can cleanly associate old commits" do
-      repo = GithubRepo.new("discourse/discourse", Octokit::Client.new, nil)
+      repo = GithubRepo.new("discourse/discourse", Octokit::Client.new, nil, repo_id: 24)
 
       diff = "`‍``\nwith a diff"
 
@@ -84,7 +84,7 @@ module DiscourseCodeReview
 
     it "can handle complex imports" do
 
-      repo = GithubRepo.new("discourse/discourse", Octokit::Client.new, nil)
+      repo = GithubRepo.new("discourse/discourse", Octokit::Client.new, nil, repo_id: 24)
 
       diff = "`‍``\nwith a diff"
 
@@ -113,7 +113,7 @@ module DiscourseCodeReview
     end
 
     it "approves followed-up topics" do
-      repo = GithubRepo.new("discourse/discourse", Octokit::Client.new, nil)
+      repo = GithubRepo.new("discourse/discourse", Octokit::Client.new, nil, repo_id: 24)
       repo.expects(:default_branch_contains?).with('a91843f0dc7b97e700dc85505404eafd62b7f8c5').returns(true)
       repo.expects(:followees).with('a91843f0dc7b97e700dc85505404eafd62b7f8c5').returns([])
       repo.expects(:default_branch_contains?).with('ca1208a63669d4d4ad7452367008d40fa090f645').returns(true)
@@ -143,7 +143,7 @@ module DiscourseCodeReview
     end
 
     it "approves followed-up topics with partial hashes" do
-      repo = GithubRepo.new("discourse/discourse", Octokit::Client.new, nil)
+      repo = GithubRepo.new("discourse/discourse", Octokit::Client.new, nil, repo_id: 24)
       repo.expects(:default_branch_contains?).with('5ff6c10320cab7ef82ecda40c57cfb9e539b7e72').returns(true)
       repo.expects(:followees).with('5ff6c10320cab7ef82ecda40c57cfb9e539b7e72').returns([])

[... diff too long, it was truncated ...]

GitHub sha: 2fd8f0d6

1 Like

This commit appears in #53 which was approved by CvX. It was merged by techAPJ.