fix: handle 'is:pr' syntax (#108)

Closes #107

packages/github/src/client.ts

@@ -13,7 +13,7 @@ import {
   type Review,
   Discussion,
 } from "@repo/model";
-import { SearchQuery } from "./search.js";
+import { prepareQuery } from "./search.js";
 
 const MAX_PULLS_TO_FETCH = 50;
 
@@ -135,32 +135,11 @@ export class DefaultGitHubClient implements GitHubClient {
     connection: Connection,
     search: string,
   ): Promise<PullResult[]> {
-    // Enforce searching for PRs, and filter by org as required by the connection.
-    const q = new SearchQuery(search);
-    q.set("type", "pr");
-    if (connection.orgs.length > 0) {
-      q.setAll("org", connection.orgs);
-    }
-    if (!q.has("archived")) {
-      // Unless the query explicitely allows PRs from archived repositories, exclude
-      // them by default as we cannot act on them anymore.
-      q.set("archived", "false");
-    }
-    if (q.has("org") && q.has("repo")) {
-      // GitHub API does not seem to support having both terms with an "org"
-      // and "repo" qualifier in a given query. In this situation, the term
-      // with the "repo" qualifier is apparently ignored. We remediate to this
-      // situation by keeping droping terms with the "org" qualifier, and
-      // keeping the more precise "repo" qualifier.
-      //
-      // Note: We ignore the situation where the targeted repo(s) are not within
-      // the originally targeted org(s).
-      q.delete("org");
-    }
+    const q = prepareQuery(search, connection);
 
     const octokit = this.getOctokit(connection);
     const response = await octokit.rest.search.issuesAndPullRequests({
-      q: q.toString(),
+      q,
       sort: "updated",
       per_page: MAX_PULLS_TO_FETCH,
     });

packages/github/src/search.ts

@@ -1,3 +1,5 @@
+import { Connection } from "@repo/model";
+
 /**
  * Split a search string into multiple GitHub search queries.
  *
@@ -26,6 +28,39 @@ export function splitQueries(search: string): string[] {
   return queries.map((s) => s.trim()).filter((s) => s.length > 0);
 }
 
+export function prepareQuery(search: string, connection: Connection): string {
+  const q = new SearchQuery(search);
+  // Enforce searching for PRs. There are two syntaxes to search for PRs: "type:pr"
+  // and "is:pr". We make sure there are no duplicate filters, which would return
+  // no results.
+  q.set("type", "pr");
+  q.delete("is", "pr");
+
+  if (!q.has("archived")) {
+    // Unless the query explicitely allows PRs from archived repositories, exclude
+    // them by default as we cannot act on them anymore.
+    q.set("archived", "false");
+  }
+
+  // Filter by org as required by the connection.
+  if (connection.orgs.length > 0) {
+    q.setAll("org", connection.orgs);
+  }
+  if (q.has("org") && q.has("repo")) {
+    // GitHub API does not seem to support having both terms with an "org"
+    // and "repo" qualifier in a given query. In this situation, the term
+    // with the "repo" qualifier is apparently ignored. We remediate to this
+    // situation by keeping droping terms with the "org" qualifier, and
+    // keeping the more precise "repo" qualifier.
+    //
+    // Note: We ignore the situation where the targeted repo(s) are not within
+    // the originally targeted org(s).
+    q.delete("org");
+  }
+
+  return q.toString();
+}
+
 /**
  * Represents a GitHub search query for issues and pull requests.
  *
@@ -66,8 +101,11 @@ export class SearchQuery {
     );
   }
 
-  delete(qualifier: string): undefined {
-    this.terms = this.terms.filter((t) => t.qualifier !== qualifier);
+  delete(qualifier: string, value?: string): undefined {
+    const matches = (t: SearchTerm) =>
+      t.qualifier === qualifier &&
+      (value === undefined || (t.values.length === 1 && t.values[0] === value));
+    this.terms = this.terms.filter((t) => !matches(t));
   }
 
   toString(): string {

packages/github/tests/search.test.ts

@@ -4,7 +4,9 @@ import {
   SearchOp,
   SearchTerm,
   splitQueries,
+  prepareQuery,
 } from "../src/search.js";
+import { mockConnection } from "@repo/testing";
 
 const expectTerms = (str: string, terms: SearchTerm[]) => {
   const q = new SearchQuery(str);
@@ -99,6 +101,16 @@ test("delete term(s) with qualifier", () => {
   expect(q.toString()).toEqual("stars:>10");
 });
 
+test("delete term(s) with qualifier and value", () => {
+  const q = new SearchQuery("org:foo org:bar user:pvcnt");
+
+  q.delete("org", "foobar");
+  expect(q.toString()).toEqual("org:foo org:bar user:pvcnt");
+
+  q.delete("org", "foo");
+  expect(q.toString()).toEqual("org:bar user:pvcnt");
+});
+
 test("set term(s) with qualifier", () => {
   const q = new SearchQuery("user:pvcnt org:foo org:bar");
 
@@ -119,3 +131,37 @@ test("split a search string into queries", () => {
   expect(splitQueries('"a;b";c')).toEqual(["a;b", "c"]);
   expect(splitQueries("a; ;c")).toEqual(["a", "c"]);
 });
+
+test("should search for non-archived pulls only", () => {
+  expect(prepareQuery("is:open", mockConnection())).toEqual(
+    "is:open type:pr archived:false",
+  );
+
+  expect(prepareQuery("is:open type:pr", mockConnection())).toEqual(
+    "is:open type:pr archived:false",
+  );
+
+  expect(prepareQuery("is:open is:pr", mockConnection())).toEqual(
+    "is:open type:pr archived:false",
+  );
+
+  expect(prepareQuery("archived:true", mockConnection())).toEqual(
+    "archived:true type:pr",
+  );
+});
+
+test("should filter by orgs", () => {
+  const connection = mockConnection({ orgs: ["apache", "kubernetes"] });
+
+  expect(prepareQuery("is:open", connection)).toEqual(
+    "is:open type:pr archived:false org:apache org:kubernetes",
+  );
+
+  expect(prepareQuery("is:open org:pvcnt", connection)).toEqual(
+    "is:open type:pr archived:false org:apache org:kubernetes",
+  );
+
+  expect(prepareQuery("is:open repo:apache/solr", connection)).toEqual(
+    "is:open repo:apache/solr type:pr archived:false",
+  );
+});