fix: Adding Accept-Language header for Xhr.js (#1368)

Conrad Chan · mergify[bot] · commit 361d73f4fefa · 2019-07-08T21:47:43.000Z
* fix: Adding Accept-Language header for Xhr.js

* chore: fix unit tests and pr feedback

* chore: add unit tests for Xhr.js
diff --git a/flow-typed/box-ui-elements.js b/flow-typed/box-ui-elements.js
@@ -538,6 +538,7 @@ type Options = {
     consoleError?: boolean,
     consoleLog?: boolean,
     id?: string,
+    language?: string,
     requestInterceptor?: Function,
     responseInterceptor?: Function,
     retryableStatusCodes?: Array<number>,
diff --git a/src/api/APIFactory.js b/src/api/APIFactory.js
@@ -162,6 +162,7 @@ class APIFactory {
             apiHost: options.apiHost || DEFAULT_HOSTNAME_API,
             uploadHost: options.uploadHost || DEFAULT_HOSTNAME_UPLOAD,
             cache: options.cache || new Cache(),
+            language: options.language,
         });
     }
 
diff --git a/src/api/OpenWith.js b/src/api/OpenWith.js
@@ -5,13 +5,7 @@
  */
 
 import Base from './Base';
-import {
-    HEADER_ACCEPT_LANGUAGE,
-    DEFAULT_LOCALE,
-    ERROR_CODE_FETCH_INTEGRATIONS,
-    BOX_EDIT_INTEGRATION_ID,
-    BOX_EDIT_SFC_INTEGRATION_ID,
-} from '../constants';
+import { ERROR_CODE_FETCH_INTEGRATIONS, BOX_EDIT_INTEGRATION_ID, BOX_EDIT_SFC_INTEGRATION_ID } from '../constants';
 
 class OpenWith extends Base {
     /**
@@ -34,25 +28,13 @@ class OpenWith extends Base {
      * @param {string} fileId - Box file ID
      * @param {Function} successCallback - Success callback
      * @param {Function} errorCallback - Error callback
-     * @param {string} locale - locale to receive translated strings from the API
      * @return {void}
      */
-    getOpenWithIntegrations(
-        fileId: string,
-        successCallback: Function,
-        errorCallback: ElementsErrorCallback,
-        locale: ?string = DEFAULT_LOCALE,
-    ) {
+    getOpenWithIntegrations(fileId: string, successCallback: Function, errorCallback: ElementsErrorCallback) {
         this.errorCode = ERROR_CODE_FETCH_INTEGRATIONS;
-        const params = {
-            headers: {
-                [HEADER_ACCEPT_LANGUAGE]: locale,
-            },
-        };
 
         this.get({
             id: fileId,
-            requestData: params,
             successCallback: openWithIntegrations => {
                 const formattedOpenWithData = this.formatOpenWithData(openWithIntegrations);
                 const consolidatedOpenWithIntegrations = this.consolidateBoxEditIntegrations(formattedOpenWithData);
diff --git a/src/api/__tests__/OpenWith-test.js b/src/api/__tests__/OpenWith-test.js
@@ -24,17 +24,6 @@ describe('api/ContentOpenWith', () => {
             expect(openWith.formatOpenWithData).toBeCalledWith(data);
             expect(openWith.consolidateBoxEditIntegrations).toBeCalled();
         });
-
-        test('should correctly pass language header', () => {
-            const successFn = jest.fn();
-            const errorFn = jest.fn();
-            openWith.get = ({ requestData }) => {
-                const { headers } = requestData;
-                expect(headers['Accept-Language']).toEqual('es-ES');
-            };
-
-            openWith.getOpenWithIntegrations('123', successFn, errorFn, 'es-ES');
-        });
     });
 
     describe('consolidateBoxEditIntegrations()', () => {
diff --git a/src/elements/content-explorer/ContentExplorer.js b/src/elements/content-explorer/ContentExplorer.js
@@ -192,30 +192,32 @@ class ContentExplorer extends Component<Props, State> {
         super(props);
 
         const {
-            token,
-            sharedLink,
-            sharedLinkPassword,
             apiHost,
-            uploadHost,
             initialPage,
             initialPageSize,
-            sortBy,
-            sortDirection,
+            language,
             requestInterceptor,
             responseInterceptor,
             rootFolderId,
+            sharedLink,
+            sharedLinkPassword,
+            sortBy,
+            sortDirection,
+            token,
+            uploadHost,
         }: Props = props;
 
         this.api = new API({
-            token,
-            sharedLink,
-            sharedLinkPassword,
             apiHost,
-            uploadHost,
-            requestInterceptor,
-            responseInterceptor,
             clientName: CLIENT_NAME_CONTENT_EXPLORER,
             id: `${TYPED_ID_FOLDER_PREFIX}${rootFolderId}`,
+            language,
+            requestInterceptor,
+            responseInterceptor,
+            sharedLink,
+            sharedLinkPassword,
+            token,
+            uploadHost,
         });
 
         this.id = uniqueid('bce_');
diff --git a/src/elements/content-open-with/ContentOpenWith.js b/src/elements/content-open-with/ContentOpenWith.js
@@ -126,14 +126,15 @@ class ContentOpenWith extends PureComponent<Props, State> {
     constructor(props: Props) {
         super(props);
 
-        const { token, apiHost, clientName, requestInterceptor, responseInterceptor } = props;
+        const { token, apiHost, clientName, language, requestInterceptor, responseInterceptor } = props;
         this.id = uniqueid('bcow_');
         this.api = new API({
-            token,
             apiHost,
             clientName,
+            language,
             requestInterceptor,
             responseInterceptor,
+            token,
         });
 
         // Clone initial state to allow for state reset on new files
@@ -224,10 +225,10 @@ class ContentOpenWith extends PureComponent<Props, State> {
      * @return {void}
      */
     fetchOpenWithData(): void {
-        const { fileId, language }: Props = this.props;
+        const { fileId }: Props = this.props;
         this.api
             .getOpenWithAPI(false)
-            .getOpenWithIntegrations(fileId, this.fetchOpenWithSuccessHandler, this.fetchErrorHandler, language);
+            .getOpenWithIntegrations(fileId, this.fetchOpenWithSuccessHandler, this.fetchErrorHandler);
     }
 
     /**
diff --git a/src/elements/content-open-with/__tests__/ContentOpenWith-test.js b/src/elements/content-open-with/__tests__/ContentOpenWith-test.js
@@ -86,7 +86,7 @@ describe('elements/content-open-with/ContentOpenWith', () => {
             instance.api = api;
 
             instance.fetchOpenWithData();
-            expect(openWithStub).toHaveBeenCalledWith('1234', expect.any(Function), expect.any(Function), 'en-US');
+            expect(openWithStub).toHaveBeenCalledWith('1234', expect.any(Function), expect.any(Function));
         });
     });
 
diff --git a/src/elements/content-picker/ContentPicker.js b/src/elements/content-picker/ContentPicker.js
@@ -161,31 +161,33 @@ class ContentPicker extends Component<Props, State> {
         super(props);
 
         const {
-            token,
-            sharedLink,
-            sharedLinkPassword,
             apiHost,
-            uploadHost,
+            clientName,
             initialPage,
             initialPageSize,
-            sortBy,
-            sortDirection,
-            clientName,
+            language,
             requestInterceptor,
             responseInterceptor,
             rootFolderId,
+            sharedLink,
+            sharedLinkPassword,
+            sortBy,
+            sortDirection,
+            token,
+            uploadHost,
         } = props;
 
         this.api = new API({
-            token,
-            sharedLink,
-            sharedLinkPassword,
             apiHost,
-            uploadHost,
             clientName,
+            id: `${TYPED_ID_FOLDER_PREFIX}${rootFolderId}`,
+            language,
             requestInterceptor,
             responseInterceptor,
-            id: `${TYPED_ID_FOLDER_PREFIX}${rootFolderId}`,
+            sharedLink,
+            sharedLinkPassword,
+            token,
+            uploadHost,
         });
 
         this.id = uniqueid('bcp_');
diff --git a/src/elements/content-preview/ContentPreview.js b/src/elements/content-preview/ContentPreview.js
@@ -213,26 +213,28 @@ class ContentPreview extends PureComponent<Props, State> {
     constructor(props: Props) {
         super(props);
         const {
-            cache,
-            token,
-            sharedLink,
-            sharedLinkPassword,
             apiHost,
+            cache,
+            fileId,
+            language,
             requestInterceptor,
             responseInterceptor,
-            fileId,
+            sharedLink,
+            sharedLinkPassword,
+            token,
         } = props;
 
         this.id = uniqueid('bcpr_');
         this.api = new API({
-            cache,
-            token,
-            sharedLink,
-            sharedLinkPassword,
             apiHost,
+            cache,
             clientName: CLIENT_NAME_CONTENT_PREVIEW,
+            language,
             requestInterceptor,
             responseInterceptor,
+            sharedLink,
+            sharedLinkPassword,
+            token,
         });
         this.state = {
             ...this.initialState,
@@ -1183,6 +1185,7 @@ class ContentPreview extends PureComponent<Props, State> {
                                 getPreview={this.getPreview}
                                 getViewer={this.getViewer}
                                 history={history}
+                                language={language}
                                 sharedLink={sharedLink}
                                 sharedLinkPassword={sharedLinkPassword}
                                 requestInterceptor={requestInterceptor}
diff --git a/src/elements/content-sidebar/ContentSidebar.js b/src/elements/content-sidebar/ContentSidebar.js
@@ -110,6 +110,7 @@ class ContentSidebar extends React.Component<Props, State> {
             apiHost,
             cache,
             clientName,
+            language,
             requestInterceptor,
             responseInterceptor,
             sharedLink,
@@ -121,6 +122,7 @@ class ContentSidebar extends React.Component<Props, State> {
             apiHost,
             cache,
             clientName,
+            language,
             requestInterceptor,
             responseInterceptor,
             sharedLink,
diff --git a/src/utils/Xhr.js b/src/utils/Xhr.js
@@ -12,6 +12,7 @@ import lowerCase from 'lodash/lowerCase';
 import TokenService from './TokenService';
 import {
     HEADER_ACCEPT,
+    HEADER_ACCEPT_LANGUAGE,
     HEADER_CLIENT_NAME,
     HEADER_CLIENT_VERSION,
     HEADER_CONTENT_TYPE,
@@ -39,6 +40,8 @@ class Xhr {
 
     clientName: ?string;
 
+    language: ?string;
+
     token: Token;
 
     version: ?string;
@@ -70,6 +73,7 @@ class Xhr {
      * @param {string} options.id - item id
      * @param {string} options.clientName - Client Name
      * @param {string|function} options.token - Auth token
+     * @param {string} [options.language] - Accept-Language header value
      * @param {string} [options.sharedLink] - Shared link
      * @param {string} [options.sharedLinkPassword] - Shared link password
      * @param {string} [options.requestInterceptor] - Request interceptor
@@ -81,6 +85,7 @@ class Xhr {
     constructor({
         id,
         clientName,
+        language,
         token,
         version,
         sharedLink,
@@ -90,17 +95,19 @@ class Xhr {
         retryableStatusCodes = [HTTP_STATUS_CODE_RATE_LIMIT],
         shouldRetry = true,
     }: Options = {}) {
-        this.id = id;
-        this.token = token;
         this.clientName = clientName;
-        this.version = version;
+        this.id = id;
+        this.language = language;
+        this.responseInterceptor = responseInterceptor || this.defaultResponseInterceptor;
+        this.retryableStatusCodes = retryableStatusCodes;
         this.sharedLink = sharedLink;
         this.sharedLinkPassword = sharedLinkPassword;
-        this.responseInterceptor = responseInterceptor || this.defaultResponseInterceptor;
+        this.shouldRetry = shouldRetry;
+        this.token = token;
+        this.version = version;
+
         this.axios = axios.create();
         this.axiosSource = axios.CancelToken.source();
-        this.shouldRetry = shouldRetry;
-        this.retryableStatusCodes = retryableStatusCodes;
         this.axios.interceptors.response.use(this.responseInterceptor, this.errorInterceptor);
 
         if (typeof requestInterceptor === 'function') {
@@ -213,6 +220,10 @@ class Xhr {
             args,
         );
 
+        if (this.language && !headers[HEADER_ACCEPT_LANGUAGE]) {
+            headers[HEADER_ACCEPT_LANGUAGE] = this.language;
+        }
+
         if (this.sharedLink) {
             headers.BoxApi = `shared_link=${this.sharedLink}`;
 
diff --git a/src/utils/__tests__/Xhr-test.js b/src/utils/__tests__/Xhr-test.js
@@ -1,6 +1,10 @@
 import noop from 'lodash/noop';
+import TokenService from '../TokenService';
 import Xhr from '../Xhr';
 
+jest.mock('../TokenService');
+TokenService.getReadToken.mockImplementation(() => Promise.resolve(`${Math.random()}`));
+
 describe('util/Xhr', () => {
     let xhrInstance;
 
@@ -411,4 +415,20 @@ describe('util/Xhr', () => {
             expect(xhrInstance.responseInterceptor).toHaveBeenCalledWith(response.data);
         });
     });
+
+    describe('getHeaders()', () => {
+        it('should not override any existing Accept-Language header', async () => {
+            xhrInstance.language = 'bar';
+            const actHeaders = await xhrInstance.getHeaders('123', { 'Accept-Language': 'foo' });
+
+            expect(actHeaders['Accept-Language']).toBe('foo');
+        });
+
+        it('should apply Accept-Language header if language exists', async () => {
+            xhrInstance.language = 'bar';
+            const actHeaders = await xhrInstance.getHeaders('123');
+
+            expect(actHeaders['Accept-Language']).toBe('bar');
+        });
+    });
 });

Original file line number	Diff line number	Diff line change
`@@ -162,6 +162,7 @@ class APIFactory {`
`162`	`162`	`apiHost: options.apiHost \|\| DEFAULT_HOSTNAME_API,`
`163`	`163`	`uploadHost: options.uploadHost \|\| DEFAULT_HOSTNAME_UPLOAD,`
`164`	`164`	`cache: options.cache \|\| new Cache(),`
	`165`	`+ language: options.language,`
`165`	`166`	`});`
`166`	`167`	`}`
`167`	`168`