fix: ensure no space is inserted between search fields and terms (#552)

fix: ensure no space is inserted between search fields and terms

packages/arcgis-rest-portal/src/util/SearchQueryBuilder.ts

@@ -27,7 +27,7 @@ import { IParamBuilder, warn } from "@esri/arcgis-rest-request";
  *
  * Will search for items matching
  * ```
- * "owner: Patrick AND (type: "Web Mapping Application" OR type: "Mobile Application" OR type: Application) AND Demo App"
+ * "owner: Patrick AND (type:"Web Mapping Application" OR type:"Mobile Application" OR type:Application) AND Demo App"
  * ```
  */
 export class SearchQueryBuilder implements IParamBuilder {
@@ -82,7 +82,7 @@ export class SearchQueryBuilder implements IParamBuilder {
     }
 
     if (field && field !== "*") {
-      this.q += `${field}: `;
+      this.q += `${field}:`;
     }
 
     return this.commit();
@@ -137,9 +137,10 @@ export class SearchQueryBuilder implements IParamBuilder {
       );
       return this;
     }
+    this.commit();
     this.openGroups--;
     this.q += ")";
-    return this.commit();
+    return this;
   }
 
   /**
@@ -175,15 +176,19 @@ export class SearchQueryBuilder implements IParamBuilder {
   }
 
   /**
-   * Joins two sets of queries with a `NOT` clause.
+   * Joins two sets of queries with a `NOT` clause. Another option for filtering results is the [prohibit operator '-'](https://developers.arcgis.com/rest/users-groups-and-items/search-reference.htm#ESRI_SECTION1_5C6C35DB9E4A4F4492C5B937BDA2BF67).
    *
    * ```js
+   * // omit results with "Rivers" in their title
    * const query = new SearchQueryBuilder()
-   *   .match("Water")
-   *   .in("title")
    *   .not()
    *   .match("Rivers")
    *   .in("title")
+   *
+   * // equivalent
+   * const query = new SearchQueryBuilder()
+   *   .match("Rivers")
+   *   .in("-title")
    * ```
    */
   public not(this: SearchQueryBuilder) {
@@ -282,15 +287,16 @@ export class SearchQueryBuilder implements IParamBuilder {
 
     this.commit();
 
-    if (this.q === "") {
+    if (this.q === "" && modifier !== "not") {
       warn(
         `You have called \`${modifier}()\` without calling another method to modify your query first. Try calling \`match()\` first.`
       );
       return this;
     }
 
     this.currentModifer = modifier;
-    this.q += ` ${modifier.toUpperCase()} `;
+    this.q += this.q === "" ? "" : " ";
+    this.q += `${modifier.toUpperCase()} `;
     return this;
   }
 

packages/arcgis-rest-portal/test/groups/search.test.ts

@@ -55,7 +55,7 @@ describe("groups", () => {
 
   it("should make a simple, single search request with a builder", done => {
     fetchMock.once("*", GroupSearchResponse);
-    const expectedParam = "Trees AND owner: USFS";
+    const expectedParam = "Trees AND owner:USFS";
     const q = new SearchQueryBuilder()
       .match("Trees")
       .and()

packages/arcgis-rest-portal/test/items/search.test.ts

@@ -33,7 +33,7 @@ describe("search", () => {
 
   it("should make a simple, single search request with a builder", done => {
     fetchMock.once("*", SearchResponse);
-    const expectedParam = "DC AND typekeywords: hubSiteApplication";
+    const expectedParam = "DC AND typekeywords:hubSiteApplication";
     const q = new SearchQueryBuilder()
       .match("DC")
       .and()
@@ -108,7 +108,7 @@ describe("search", () => {
 
   it("should pass through other requestOptions at the same time with a builder", done => {
     fetchMock.once("*", SearchResponse);
-    const expectedParam = "DC AND typekeywords: hubSiteApplication";
+    const expectedParam = "DC AND typekeywords:hubSiteApplication";
     const q = new SearchQueryBuilder()
       .match("DC")
       .and()

packages/arcgis-rest-portal/test/util/SearchQueryBuilder.test.ts

@@ -16,7 +16,7 @@ describe("SearchQueryBuilder", () => {
     console.warn = originalWarn;
   });
 
-  it("should return and empty string when called with no other functions", () => {
+  it("should return an empty string when called with no other functions", () => {
     const query = new SearchQueryBuilder().toParam();
     expect(query).toEqual("");
   });
@@ -31,7 +31,7 @@ describe("SearchQueryBuilder", () => {
       .match("test")
       .in("tags")
       .toParam();
-    expect(query).toEqual("tags: test");
+    expect(query).toEqual("tags:test");
   });
 
   it("should warp multi word search terms in quotes", () => {
@@ -63,7 +63,7 @@ describe("SearchQueryBuilder", () => {
       .in("tags")
       .toParam();
 
-    expect(query).toEqual("bar AND tags: foo");
+    expect(query).toEqual("bar AND tags:foo");
   });
 
   it("should format a simple range", () => {
@@ -72,7 +72,7 @@ describe("SearchQueryBuilder", () => {
       .to("z")
       .in("title")
       .toParam();
-    expect(query).toEqual("title: [a TO z]");
+    expect(query).toEqual("title:[a TO z]");
   });
 
   it("should format a simple group", () => {
@@ -83,7 +83,20 @@ describe("SearchQueryBuilder", () => {
       .in("title")
       .endGroup()
       .toParam();
-    expect(query).toEqual("(title: [a TO z])");
+    expect(query).toEqual("(title:[a TO z])");
+  });
+
+  it("should format a more complex group", () => {
+    const query = new SearchQueryBuilder()
+      .startGroup()
+      .match("California")
+      .or()
+      .match("recent")
+      .endGroup()
+      .and()
+      .match("fires")
+      .toParam();
+    expect(query).toEqual("(California OR recent) AND fires");
   });
 
   it("should boost the previous search", () => {
@@ -110,7 +123,7 @@ describe("SearchQueryBuilder", () => {
       .toParam();
 
     expect(query).toEqual(
-      `created: [${expectedDate1} TO ${expectedDate2}] AND tags: test`
+      `created:[${expectedDate1} TO ${expectedDate2}] AND tags:test`
     );
   });
 
@@ -135,10 +148,23 @@ describe("SearchQueryBuilder", () => {
       .toParam();
 
     expect(query).toEqual(
-      `owner: fred AND (type: "Web Mapping Application" OR type: "Mobile Application" OR type: Application) AND test`
+      `owner:fred AND (type:"Web Mapping Application" OR type:"Mobile Application" OR type:Application) AND test`
     );
   });
 
+  it("should allow .not to be called without a preceding search value", () => {
+    const query = new SearchQueryBuilder()
+      .not()
+      .match("public")
+      .in("access")
+      .not()
+      .match("code attachment")
+      .in("type")
+      .toParam();
+
+    expect(query).toEqual(`NOT access:public NOT type:"code attachment"`);
+  });
+
   it("should clone searches for modification", () => {
     const myAppsQuery = new SearchQueryBuilder()
       .match("fred")
@@ -162,11 +188,11 @@ describe("SearchQueryBuilder", () => {
       .in("*");
 
     expect(myAppsQuery.toParam()).toEqual(
-      `owner: fred AND (type: "Web Mapping Application" OR type: "Mobile Application" OR type: Application)`
+      `owner:fred AND (type:"Web Mapping Application" OR type:"Mobile Application" OR type:Application)`
     );
 
     expect(myTestAppsQuery.toParam()).toEqual(
-      `owner: fred AND (type: "Web Mapping Application" OR type: "Mobile Application" OR type: Application) AND test`
+      `owner:fred AND (type:"Web Mapping Application" OR type:"Mobile Application" OR type:Application) AND test`
     );
   });
 
@@ -182,11 +208,8 @@ describe("SearchQueryBuilder", () => {
 
   it("should not allow chains of logic modifiers, and warn user", () => {
     const query = new SearchQueryBuilder()
-      .not()
       .and()
       .or()
-      .not()
-      .or()
       .or()
       .toParam();
 
@@ -296,7 +319,7 @@ describe("SearchQueryBuilder", () => {
       .toParam();
 
     expect(console.warn).toHaveBeenCalled();
-    expect(query).toEqual("title: a");
+    expect(query).toEqual("title:a");
   });
 
   it("should not allow .match().from().in(), and warn user", () => {
@@ -307,10 +330,10 @@ describe("SearchQueryBuilder", () => {
       .toParam();
 
     expect(console.warn).toHaveBeenCalled();
-    expect(query).toEqual("title: test");
+    expect(query).toEqual("title:test");
   });
 
-  it("should", () => {
+  it("should produce an empty string when no methods are called", () => {
     const query = new SearchQueryBuilder().toParam();
     expect(query).toEqual("");
   });