Issue search improvements

lib/issue_db/cache.rb

@@ -7,27 +7,33 @@ module Cache
     def update_issue_cache!
       @log.debug("updating issue cache")
 
-      # find all issues in the repo that were created by this library
-      query = "repo:#{@repo.full_name} label:#{@label}"
-
-      search_response = nil
+      issues_response = nil
       begin
-        # issues structure: { "total_count": 0, "incomplete_results": false, "items": [<issues>] }
-        search_response = @client.search_issues(query)
+        # This fetches all issues with the specified label from the repository. This label identifies all issues...
+        # ... that are managed via this library.
+        # The client.auto_paginate setting will handle pagination automatically
+        issues_response = @client.issues(@repo.full_name, labels: @label, state: "all")
+      rescue Octokit::Unauthorized => e
+        # Re-throw authentication errors unchanged for clear user feedback
+        raise e
       rescue StandardError => e
-        retry_err_msg = "error search_issues() call: #{e.message}"
+        # For other errors, wrap with context
+        retry_err_msg = "error issues() call: #{e.message}"
         @log.error(retry_err_msg)
         raise StandardError, retry_err_msg
       end
 
-      # Safety check to ensure search_response and items are not nil
-      if search_response.nil? || search_response.items.nil?
-        @log.error("search_issues returned nil response or nil items")
-        raise StandardError, "search_issues returned invalid response"
+      # Safety check to ensure issues_response is not nil
+      if issues_response.nil?
+        @log.error("issues API returned nil response")
+        raise StandardError, "issues API returned invalid response"
       end
 
-      @log.debug("issue cache updated - cached #{search_response.total_count} issues")
-      @issues = search_response.items
+      # Convert to array if it's a single issue (shouldn't happen with auto_paginate, but safety first)
+      issues_response = [issues_response] unless issues_response.is_a?(Array)
+
+      @log.debug("issue cache updated - cached #{issues_response.length} issues")
+      @issues = issues_response
       @issues_last_updated = Time.now
       return @issues
     end

lib/issue_db/utils/github.rb

@@ -280,6 +280,12 @@ def method_missing(method, *args, **kwargs, &block)
         # Check if retry is explicitly disabled for this call
         disable_retry = kwargs.delete(:disable_retry) || false
 
+        # For add_label method, also check if disable_retry is in the options hash (last positional arg)
+        if method.to_s == "add_label" && args.length >= 4 && args.last.is_a?(Hash)
+          options_hash = args.last
+          disable_retry = options_hash.delete(:disable_retry) || disable_retry
+        end
+
         # Determine the rate limit type based on the method name and arguments
         rate_limit_type = case method.to_s
                           when /search_/

lib/issue_db/utils/init.rb

@@ -10,8 +10,10 @@ def init!
           @repo.full_name,
           @label,
           "000000",
-          { description: "This issue is managed by the issue-db Ruby library. Please do not remove this label." },
-          disable_retry: true
+          {
+            description: "This issue is managed by the issue-db Ruby library. Please do not remove this label.",
+            disable_retry: true
+          }
         )
       rescue StandardError => e
         if e.message.include?("already_exists")

spec/lib/issue_db/cache_spec.rb

@@ -33,8 +33,8 @@ def initialize(client, repo, label, log)
   describe "#update_issue_cache!" do
     context "when cache is updated successfully" do
       it "updates the issue cache and logs the update" do
-        search_response = double("search_response", total_count: 2, items: ["issue1", "issue2"])
-        allow(client).to receive(:search_issues).and_return(search_response)
+        issues_response = ["issue1", "issue2"]
+        allow(client).to receive(:issues).and_return(issues_response)
 
         expect(log).to receive(:debug).with("updating issue cache")
         expect(log).to receive(:debug).with("issue cache updated - cached 2 issues")
@@ -48,47 +48,48 @@ def initialize(client, repo, label, log)
 
     context "when a secondary rate limit error occurs" do
       it "raises the error (handled by GitHub client)" do
-        allow(client).to receive(:search_issues).and_raise(StandardError.new("exceeded a secondary rate limit"))
+        allow(client).to receive(:issues).and_raise(StandardError.new("exceeded a secondary rate limit"))
 
         expect(log).to receive(:debug).with("updating issue cache")
-        expect(log).to receive(:error).with("error search_issues() call: exceeded a secondary rate limit")
+        expect(log).to receive(:error).with("error issues() call: exceeded a secondary rate limit")
 
-        expect { dummy_instance.update_issue_cache! }.to raise_error("error search_issues() call: exceeded a secondary rate limit")
+        expect { dummy_instance.update_issue_cache! }.to raise_error("error issues() call: exceeded a secondary rate limit")
       end
     end
 
     context "when another error occurs" do
       it "logs an error message and raises the error" do
-        allow(client).to receive(:search_issues).and_raise(StandardError.new("some other error"))
+        allow(client).to receive(:issues).and_raise(StandardError.new("some other error"))
 
         expect(log).to receive(:debug).with("updating issue cache")
-        expect(log).to receive(:error).with("error search_issues() call: some other error")
+        expect(log).to receive(:error).with("error issues() call: some other error")
 
-        expect { dummy_instance.update_issue_cache! }.to raise_error("error search_issues() call: some other error")
+        expect { dummy_instance.update_issue_cache! }.to raise_error("error issues() call: some other error")
       end
     end
 
-    context "when search_issues returns nil response" do
+    context "when issues API returns nil response" do
       it "logs an error and raises a StandardError" do
-        allow(client).to receive(:search_issues).and_return(nil)
+        allow(client).to receive(:issues).and_return(nil)
 
         expect(log).to receive(:debug).with("updating issue cache")
-        expect(log).to receive(:error).with("search_issues returned nil response or nil items")
+        expect(log).to receive(:error).with("issues API returned nil response")
 
-        expect { dummy_instance.update_issue_cache! }.to raise_error(StandardError, "search_issues returned invalid response")
+        expect { dummy_instance.update_issue_cache! }.to raise_error(StandardError, "issues API returned invalid response")
       end
     end
 
-    context "when search_issues returns response with nil items" do
-      it "logs an error and raises a StandardError" do
-        nil_items_response = double("response")
-        allow(nil_items_response).to receive(:items).and_return(nil)
-        allow(client).to receive(:search_issues).and_return(nil_items_response)
+    context "when issues API returns single issue instead of array" do
+      it "converts to array and works correctly" do
+        single_issue = { title: "single_issue", number: 1 }
+        allow(client).to receive(:issues).and_return(single_issue)
 
         expect(log).to receive(:debug).with("updating issue cache")
-        expect(log).to receive(:error).with("search_issues returned nil response or nil items")
+        expect(log).to receive(:debug).with("issue cache updated - cached 1 issues")
 
-        expect { dummy_instance.update_issue_cache! }.to raise_error(StandardError, "search_issues returned invalid response")
+        result = dummy_instance.update_issue_cache!
+        expect(result).to eq([single_issue])
+        expect(dummy_instance.issues).to eq([single_issue])
       end
     end
   end

spec/lib/issue_db/database_spec.rb

@@ -148,7 +148,7 @@
   context "update" do
     it "updates an issue successfully" do
       issue = subject.update("event999", { cool: false })
-      expect(issue.source_data.number).to eq(12)
+      expect(issue.source_data.number).to eq(11)
       expect(issue.source_data.state).to eq("open")
     end
 
@@ -281,6 +281,7 @@
       # Verify the closed record has the correct state
       closed_record = all_records.find { |r| r.key == "event999" }
       expect(closed_record.source_data.state).to eq("closed")
+      expect(closed_record.source_data.number).to eq(11)
     end
 
     it "deletes an issue with additional labels before closing" do
@@ -630,18 +631,18 @@
   context "list" do
     it "lists all records successfully" do
       records = subject.list
-      expect(records.first.data).to eq({ "age" => 333, "apple" => "red", "cool" => true, "user" => "mona" })
       expect(records.first.source_data.number).to eq(8)
       expect(records.last.source_data.number).to eq(6)
       expect(records.size).to eq(3)
+      expect(records.first.data).to eq({ "age" => 333, "apple" => "red", "cool" => true, "user" => "mona" })
     end
   end
 
   context "refresh!" do
     it "refreshes the cache successfully" do
       results = subject.refresh!
-      expect(results.length).to eq(5)
-      expect(results.first.number).to eq(11)
+      expect(results.length).to eq(8)
+      expect(results.first.number).to eq(19)
     end
   end
 

spec/lib/issue_db/utils/github_spec.rb

@@ -408,6 +408,134 @@ def stub_dependencies
       allow(mock_client).to receive(:respond_to?).with(:private_method, true).and_return(true)
       expect(github.respond_to?(:private_method, true)).to be true
     end
+
+    context "add_label method with disable_retry in options hash" do
+      before do
+        # Mock the rate limit call that happens in wait_for_rate_limit!
+        allow(mock_client).to receive(:get).with("rate_limit").and_return(default_rate_limit_response)
+      end
+
+      it "extracts disable_retry from options hash and passes clean options to Octokit" do
+        options_with_disable_retry = {
+          description: "Test label",
+          disable_retry: true
+        }
+
+        expected_clean_options = {
+          description: "Test label"
+        }
+
+        allow(mock_client).to receive(:add_label).with(
+          "owner/repo",
+          "test-label",
+          "ff0000",
+          expected_clean_options
+        ).and_return("label_created")
+
+        result = github.add_label("owner/repo", "test-label", "ff0000", options_with_disable_retry)
+        expect(result).to eq("label_created")
+        expect(mock_client).to have_received(:add_label).with(
+          "owner/repo",
+          "test-label",
+          "ff0000",
+          expected_clean_options
+        )
+      end
+
+      it "handles disable_retry in options hash and skips retry on failure" do
+        options_with_disable_retry = {
+          description: "Test label",
+          disable_retry: true
+        }
+
+        error = StandardError.new("API error")
+        allow(mock_client).to receive(:add_label).and_raise(error)
+
+        expect { github.add_label("owner/repo", "test-label", "ff0000", options_with_disable_retry) }.to raise_error(error)
+        expect(mock_client).to have_received(:add_label).once # Should not retry
+      end
+
+      it "handles options hash without disable_retry normally" do
+        options_without_disable_retry = {
+          description: "Test label",
+          color: "blue"
+        }
+
+        allow(mock_client).to receive(:add_label).with(
+          "owner/repo",
+          "test-label",
+          "ff0000",
+          options_without_disable_retry
+        ).and_return("label_created")
+
+        result = github.add_label("owner/repo", "test-label", "ff0000", options_without_disable_retry)
+        expect(result).to eq("label_created")
+      end
+
+      it "prioritizes keyword argument disable_retry over options hash disable_retry" do
+        options_with_disable_retry = {
+          description: "Test label",
+          disable_retry: false  # This should be overridden
+        }
+
+        expected_clean_options = {
+          description: "Test label"
+        }
+
+        allow(mock_client).to receive(:add_label).with(
+          "owner/repo",
+          "test-label",
+          "ff0000",
+          expected_clean_options
+        ).and_return("label_created")
+
+        # Keyword argument should take precedence
+        result = github.add_label("owner/repo", "test-label", "ff0000", options_with_disable_retry, disable_retry: true)
+        expect(result).to eq("label_created")
+      end
+
+      it "works with minimal arguments (no options hash)" do
+        allow(mock_client).to receive(:add_label).with(
+          "owner/repo",
+          "test-label",
+          "ff0000"
+        ).and_return("label_created")
+
+        result = github.add_label("owner/repo", "test-label", "ff0000")
+        expect(result).to eq("label_created")
+      end
+
+      it "handles non-hash fourth argument gracefully" do
+        # If someone passes a non-hash as the fourth argument, it should not crash
+        allow(mock_client).to receive(:add_label).with(
+          "owner/repo",
+          "test-label",
+          "ff0000",
+          "not_a_hash"
+        ).and_return("label_created")
+
+        result = github.add_label("owner/repo", "test-label", "ff0000", "not_a_hash")
+        expect(result).to eq("label_created")
+      end
+
+      it "only processes disable_retry for add_label method, not other methods" do
+        options_with_disable_retry = {
+          description: "Test description",
+          disable_retry: true
+        }
+
+        # For non-add_label methods, disable_retry should remain in the options
+        allow(mock_client).to receive(:create_issue).with(
+          "owner/repo",
+          "Test Issue",
+          "Body",
+          options_with_disable_retry  # disable_retry should NOT be filtered out
+        ).and_return("issue_created")
+
+        result = github.create_issue("owner/repo", "Test Issue", "Body", options_with_disable_retry)
+        expect(result).to eq("issue_created")
+      end
+    end
   end
 
   describe "integration scenarios" do

spec/lib/issue_db/utils/init_spec.rb

@@ -30,8 +30,10 @@ def initialize(client, repo, label, log)
           "user/repo",
           "issue-db",
           "000000",
-          { description: "This issue is managed by the issue-db Ruby library. Please do not remove this label." },
-          disable_retry: true
+          {
+            description: "This issue is managed by the issue-db Ruby library. Please do not remove this label.",
+            disable_retry: true
+          }
         )
 
         dummy_instance.init!
@@ -44,8 +46,10 @@ def initialize(client, repo, label, log)
           "user/repo",
           "issue-db",
           "000000",
-          { description: "This issue is managed by the issue-db Ruby library. Please do not remove this label." },
-          disable_retry: true
+          {
+            description: "This issue is managed by the issue-db Ruby library. Please do not remove this label.",
+            disable_retry: true
+          }
         ).and_raise(StandardError.new("already_exists"))
 
         expect(log).to receive(:debug).with("label issue-db already exists")
@@ -60,8 +64,10 @@ def initialize(client, repo, label, log)
           "user/repo",
           "issue-db",
           "000000",
-          { description: "This issue is managed by the issue-db Ruby library. Please do not remove this label." },
-          disable_retry: true
+          {
+            description: "This issue is managed by the issue-db Ruby library. Please do not remove this label.",
+            disable_retry: true
+          }
         ).and_raise(StandardError.new("some other error"))
 
         expect(log).to receive(:error).with("error creating label: some other error")
@@ -74,8 +80,10 @@ def initialize(client, repo, label, log)
           "user/repo",
           "issue-db",
           "000000",
-          { description: "This issue is managed by the issue-db Ruby library. Please do not remove this label." },
-          disable_retry: true
+          {
+            description: "This issue is managed by the issue-db Ruby library. Please do not remove this label.",
+            disable_retry: true
+          }
         ).and_raise(StandardError.new("some other error"))
         allow(ENV).to receive(:fetch).with("ENV", nil).and_return("acceptance")