fix(hub-common): rename "request" method used by Discussions API fns … (

#1742)

packages/common/src/discussions/api/channels/channels.ts

@@ -1,4 +1,4 @@
-import { request } from "../request";
+import { discussionsApiRequest } from "../discussions-api-request";
 import { IChannel, IPagedResponse, ISearchChannelsParams } from "../types";
 
 /**
@@ -13,5 +13,5 @@ export function searchChannels(
   options: ISearchChannelsParams
 ): Promise<IPagedResponse<IChannel>> {
   options.httpMethod = "GET";
-  return request(`/channels`, options);
+  return discussionsApiRequest(`/channels`, options);
 }

packages/common/src/discussions/api/request.ts → packages/common/src/discussions/api/discussions-api-request.ts

@@ -14,7 +14,7 @@ import { apiRequest, authenticateRequest } from "./utils/request";
 // if that method didn't prepend `/api/v3` to the supplied path. Additionally,
 // there is the difference that hubApiRequest sets Authorization header without `Bearer`
 // https://github.com/Esri/hub.js/blob/f35b1a0a868916bd07e1dfd84cb084bc2c876267/packages/common/src/request.ts#L62
-export function request<T>(
+export function discussionsApiRequest<T>(
   url: string,
   options: IDiscussionsRequestOptions
 ): Promise<T> {

packages/common/src/discussions/api/index.ts

@@ -1,3 +1,4 @@
 export * from "./channels";
 export * from "./settings";
 export * from "./types";
+export * from "./discussions-api-request";

packages/common/src/discussions/api/settings/settings.ts

@@ -1,4 +1,4 @@
-import { request } from "../request";
+import { discussionsApiRequest } from "../discussions-api-request";
 import {
   ICreateSettingParams,
   IEntitySetting,
@@ -19,7 +19,7 @@ export function createSetting(
   options: ICreateSettingParams
 ): Promise<IEntitySetting> {
   options.httpMethod = "POST";
-  return request(`/settings`, options);
+  return discussionsApiRequest(`/settings`, options);
 }
 
 /**
@@ -33,7 +33,7 @@ export function fetchSetting(
   options: IFetchSettingParams
 ): Promise<IEntitySetting> {
   options.httpMethod = "GET";
-  return request(`/settings/${options.id}`, options);
+  return discussionsApiRequest(`/settings/${options.id}`, options);
 }
 
 /**
@@ -47,7 +47,7 @@ export function updateSetting(
   options: IUpdateSettingParams
 ): Promise<IEntitySetting> {
   options.httpMethod = "PATCH";
-  return request(`/settings/${options.id}`, options);
+  return discussionsApiRequest(`/settings/${options.id}`, options);
 }
 
 /**
@@ -61,5 +61,5 @@ export function removeSetting(
   options: IRemoveSettingParams
 ): Promise<IRemoveSettingResponse> {
   options.httpMethod = "DELETE";
-  return request(`/settings/${options.id}`, options);
+  return discussionsApiRequest(`/settings/${options.id}`, options);
 }

packages/common/src/discussions/api/utils/request.ts

@@ -1,6 +1,6 @@
 import { buildUrl } from "../../../urls";
 import { RemoteServerError as _RemoteServerError } from "../../../request";
-import { IDiscussionsRequestOptions } from "../types";
+import { IDiscussionsRequestOptions, SearchPostsFormat } from "../types";
 
 export class RemoteServerError extends _RemoteServerError {
   error: string;
@@ -52,6 +52,7 @@ export function apiRequest<T>(
   options: IDiscussionsRequestOptions,
   token?: string
 ): Promise<T> {
+  let routeWithParams = route;
   const headers = new Headers(options.headers);
   headers.append("Content-Type", "application/json");
   if (token) {
@@ -76,16 +77,26 @@ export function apiRequest<T>(
   if (options.data) {
     if (options.httpMethod === "GET") {
       const queryParams = new URLSearchParams(options.data).toString();
-      route += `?${queryParams}`;
+      routeWithParams += `?${queryParams}`;
     } else {
       opts.body = JSON.stringify(options.data);
     }
   }
 
-  const url = [apiBase.replace(/\/$/, ""), route.replace(/^\//, "")].join("/");
+  // this currently only applies to the search post route. we should rework things in the future such that we don't need
+  // to do this sort of evaluation in common logic.
+  const isCSV =
+    route === "/posts" &&
+    (options.data?.f === SearchPostsFormat.CSV ||
+      headers.get("Accept") === "text/csv");
+
+  const url = [
+    apiBase.replace(/\/$/, ""),
+    routeWithParams.replace(/^\//, ""),
+  ].join("/");
   return fetch(url, opts).then((res) => {
     if (res.ok) {
-      return res.json();
+      return isCSV ? res.text() : res.json();
     } else {
       const { statusText, status } = res;
       return res.json().then((err) => {

packages/common/test/discussions/api/channels.test.ts

@@ -3,7 +3,7 @@ import {
   SharingAccess,
 } from "../../../src/discussions/api/types";
 import { searchChannels } from "../../../src/discussions/api/channels";
-import * as req from "../../../src/discussions/api/request";
+import * as req from "../../../src/discussions/api/discussions-api-request";
 
 describe("channels", () => {
   let requestSpy: any;
@@ -14,7 +14,7 @@ describe("channels", () => {
   } as unknown as IDiscussionsRequestOptions;
 
   beforeEach(() => {
-    requestSpy = spyOn(req, "request").and.returnValue(
+    requestSpy = spyOn(req, "discussionsApiRequest").and.returnValue(
       Promise.resolve(response)
     );
   });

packages/common/test/discussions/api/request.test.ts → packages/common/test/discussions/api/discussions-api-request.test.ts

@@ -1,10 +1,13 @@
-import { request } from "../../../src/discussions/api/request";
+import { discussionsApiRequest } from "../../../src/discussions/api/discussions-api-request";
 import * as utils from "../../../src/discussions/api/utils/request";
 import * as fetchMock from "fetch-mock";
 import { IAuthenticationManager } from "@esri/arcgis-rest-request";
-import { IDiscussionsRequestOptions } from "../../../src/discussions/api/types";
+import {
+  IDiscussionsRequestOptions,
+  SearchPostsFormat,
+} from "../../../src/discussions/api/types";
 
-describe("request", () => {
+describe("discussionsApiRequest", () => {
   const url = "foo";
   const options = { params: { foo: "bar" } };
   it("resolves token before making api request", (done) => {
@@ -15,7 +18,7 @@ describe("request", () => {
     ).and.callFake(async () => token);
     const apiRequestSpy = spyOn(utils, "apiRequest");
 
-    request(url, options as unknown as IDiscussionsRequestOptions)
+    discussionsApiRequest(url, options as unknown as IDiscussionsRequestOptions)
       .then(() => {
         expect(authenticateRequestSpy).toHaveBeenCalledWith(options);
         expect(apiRequestSpy).toHaveBeenCalledWith(url, options, token);
@@ -67,12 +70,13 @@ describe("apiRequest", () => {
   const response = { ok: true };
 
   const hubApiUrl = "https://hub.arcgis.com/api/discussions/v1";
-  const url = "foo";
+  let url: string;
 
   let expectedOpts: RequestInit;
   let opts: IDiscussionsRequestOptions;
 
   beforeEach(() => {
+    url = "foo";
     fetchMock.mock("*", { status: 200, body: response });
 
     const headers = new Headers();
@@ -219,4 +223,48 @@ describe("apiRequest", () => {
     expect(calledUrl).toEqual([hubApiUrl, url].join("/"));
     expect(calledOpts).toEqual(expectedOpts);
   });
+
+  it("resolves plain-text when f=csv for search posts route", async () => {
+    url = "/posts";
+    const options = {
+      data: {
+        f: SearchPostsFormat.CSV,
+      },
+      httpMethod: "GET",
+    } as IDiscussionsRequestOptions;
+    const result = await utils.apiRequest(url, options);
+
+    expect(result).toEqual(JSON.stringify(response));
+
+    const [calledUrl, calledOpts] = fetchMock.calls()[0];
+    expect(calledUrl).toEqual(
+      "https://hub.arcgis.com/api/discussions/v1/posts?f=csv"
+    );
+    expect(calledOpts).toEqual(expectedOpts);
+  });
+
+  it('resolves plain-text when HTTP "Accept" header has a value of "text/csv" for search posts route', async () => {
+    url = "/posts";
+    const options = {
+      headers: {
+        Accept: "text/csv",
+      },
+      httpMethod: "GET",
+    } as IDiscussionsRequestOptions;
+    const result = await utils.apiRequest(url, options);
+    const expectedHeaders = new Headers(expectedOpts.headers);
+    expectedHeaders.set("accept", "text/csv");
+    expectedOpts = {
+      ...expectedOpts,
+      headers: expectedHeaders,
+    };
+
+    expect(result).toEqual(JSON.stringify(response));
+
+    const [calledUrl, calledOpts] = fetchMock.calls()[0];
+    expect(calledUrl).toEqual(
+      "https://hub.arcgis.com/api/discussions/v1/posts"
+    );
+    expect(calledOpts).toEqual(expectedOpts);
+  });
 });

packages/common/test/discussions/api/settings.test.ts

@@ -1,4 +1,4 @@
-import * as req from "../../../src/discussions/api/request";
+import * as req from "../../../src/discussions/api/discussions-api-request";
 import { Geometry } from "geojson";
 import {
   createSetting,
@@ -40,7 +40,7 @@ describe("settings", () => {
   };
 
   beforeEach(() => {
-    requestSpy = spyOn(req, "request").and.returnValue(
+    requestSpy = spyOn(req, "discussionsApiRequest").and.returnValue(
       Promise.resolve(response)
     );
   });