Add better return types for the API functions

.env

@@ -1,8 +1,12 @@
 REACT_APP_API_HOST=https://addons.mozilla.org
+
 # This is the placeholder used in the `index.html` file to inject the
 # authentication token on runtime.
 REACT_APP_AUTH_TOKEN_PLACEHOLDER=__AUTH_TOKEN__
 REACT_APP_AUTHENTICATION_COOKIE=frontend_auth_token
+
 # This is the language that will be sent with all API requests.
 REACT_APP_DEFAULT_API_LANG=en-US
+REACT_APP_DEFAULT_API_VERSION=v4dev
+
 REACT_APP_FXA_CONFIG=amo

src/api/index.spec.tsx

@@ -5,7 +5,14 @@ import {
   initialState as defaultApiState,
 } from '../reducers/api';
 
-import { HttpMethod, callApi, getVersion, logOutFromServer } from '.';
+import {
+  HttpMethod,
+  callApi,
+  getCurrentUserProfile,
+  getVersion,
+  isErrorResponse,
+  logOutFromServer,
+} from '.';
 
 describe(__filename, () => {
   const defaultLang = process.env.REACT_APP_DEFAULT_API_LANG;
@@ -179,6 +186,20 @@ describe(__filename, () => {
     });
   });
 
+  describe('isErrorResponse', () => {
+    it('returns true if a response object is an error', () => {
+      const response = { error: 'this is an error' };
+
+      expect(isErrorResponse(response)).toEqual(true);
+    });
+
+    it('returns false if a response object is not an error', () => {
+      const response = { id: 123 };
+
+      expect(isErrorResponse(response)).toEqual(false);
+    });
+  });
+
   describe('getVersion', () => {
     it('calls the API to retrieve version information', async () => {
       const addonId = 999;
@@ -232,4 +253,18 @@ describe(__filename, () => {
       );
     });
   });
+
+  describe('getCurrentUserProfile', () => {
+    it('calls the API to retrieve the current logged-in user profile', async () => {
+      await getCurrentUserProfile(defaultApiState);
+
+      expect(fetch).toHaveBeenCalledWith(
+        expect.stringMatching(`/api/${defaultVersion}/accounts/profile/`),
+        {
+          headers: {},
+          method: HttpMethod.GET,
+        },
+      );
+    });
+  });
 });

src/api/index.tsx

@@ -1,6 +1,8 @@
 import log from 'loglevel';
 
 import { ApiState } from '../reducers/api';
+import { ExternalUser } from '../reducers/users';
+import { ExternalVersion } from '../reducers/versions';
 
 export enum HttpMethod {
   DELETE = 'DELETE',
@@ -19,20 +21,37 @@ type CallApiParams = {
   lang?: string;
 };
 
-type CallApiResponse = object | { error: Error };
+type ErrorResponseType = { error: Error };
+
+type CallApiResponse<SuccessResponseType> =
+  | SuccessResponseType
+  | ErrorResponseType;
+
+// This function below is a user-defined type guard that we use to check
+// whether a callApi response object is successful or unsuccessful. We use
+// `any` on purpose because we don't know the exact type for
+// `SuccessResponseType`.
+export const isErrorResponse = (
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  arg: CallApiResponse<any>,
+): arg is ErrorResponseType => {
+  return arg.error !== undefined;
+};
 
 type Headers = {
   [name: string]: string;
 };
 
-export const callApi = async ({
+// For the `extends {}` part, please see:
+// https://github.com/Microsoft/TypeScript/issues/4922
+export const callApi = async <SuccessResponseType extends {}>({
   apiState,
   endpoint,
   method = HttpMethod.GET,
   version = process.env.REACT_APP_DEFAULT_API_VERSION,
   query = {},
-  lang = process.env.REACT_APP_DEFAULT_API_LANG as string,
-}: CallApiParams): Promise<CallApiResponse> => {
+  lang = process.env.REACT_APP_DEFAULT_API_LANG,
+}: CallApiParams): Promise<CallApiResponse<SuccessResponseType>> => {
   let adjustedEndpoint = endpoint;
   if (!adjustedEndpoint.startsWith('/')) {
     adjustedEndpoint = `/${adjustedEndpoint}`;
@@ -50,8 +69,8 @@ export const callApi = async ({
     [key: string]: string;
   };
 
-  // Add the lang parameter to the querystring
-  const queryWithLang: QueryWithLang = { ...query, lang };
+  // Add the lang parameter to the query string.
+  const queryWithLang: QueryWithLang = lang ? { ...query, lang } : query;
 
   const queryString = Object.keys(queryWithLang)
     .map((k) => `${k}=${queryWithLang[k]}`)
@@ -96,17 +115,24 @@ export const getVersion = async ({
   addonId,
   versionId,
 }: GetVersionParams) => {
-  return callApi({
+  return callApi<ExternalVersion>({
     apiState,
     endpoint: `reviewers/addon/${addonId}/versions/${versionId}`,
     query: path ? { file: path } : undefined,
   });
 };
 
 export const logOutFromServer = async (apiState: ApiState) => {
-  return callApi({
+  return callApi<{}>({
     apiState,
     endpoint: 'accounts/session',
     method: HttpMethod.DELETE,
   });
 };
+
+export const getCurrentUserProfile = async (apiState: ApiState) => {
+  return callApi<ExternalUser>({
+    apiState,
+    endpoint: '/accounts/profile/',
+  });
+};

src/components/App/index.spec.tsx

@@ -9,22 +9,26 @@ import { actions as apiActions } from '../../reducers/api';
 import { actions as userActions } from '../../reducers/users';
 import {
   createContextWithFakeRouter,
+  createFakeThunk,
   fakeUser,
   shallowUntilTarget,
+  spyOn,
 } from '../../test-helpers';
 import Navbar from '../Navbar';
 
-import App, { AppBase } from '.';
+import App, { AppBase, PublicProps } from '.';
 
 describe(__filename, () => {
   type RenderParams = {
+    _fetchCurrentUserProfile?: PublicProps['_fetchCurrentUserProfile'];
+    authToken?: PublicProps['authToken'];
     store?: Store;
-    authToken?: string | null;
   };
 
   const render = ({
-    store = configureStore(),
+    _fetchCurrentUserProfile = createFakeThunk().createThunk,
     authToken = 'some-token',
+    store = configureStore(),
   }: RenderParams = {}) => {
     const contextWithRouter = createContextWithFakeRouter();
     const context = {
@@ -35,7 +39,12 @@ describe(__filename, () => {
       },
     };
 
-    const root = shallowUntilTarget(<App authToken={authToken} />, AppBase, {
+    const props = {
+      _fetchCurrentUserProfile,
+      authToken,
+    };
+
+    const root = shallowUntilTarget(<App {...props} />, AppBase, {
       shallowOptions: { ...context },
     });
 
@@ -68,7 +77,7 @@ describe(__filename, () => {
   it('dispatches setAuthToken on mount when authToken is valid', () => {
     const authToken = 'my-token';
     const store = configureStore();
-    const dispatch = jest.spyOn(store, 'dispatch');
+    const dispatch = spyOn(store, 'dispatch');
 
     render({ authToken, store });
 
@@ -79,7 +88,7 @@ describe(__filename, () => {
 
   it('does not dispatch setAuthToken on mount when authToken is null', () => {
     const store = configureStore();
-    const dispatch = jest.spyOn(store, 'dispatch');
+    const dispatch = spyOn(store, 'dispatch');
 
     render({ authToken: null, store });
 
@@ -102,4 +111,52 @@ describe(__filename, () => {
     expect(root.find(Route)).toHaveLength(3);
     expect(root.find(`.${styles.loginMessage}`)).toHaveLength(0);
   });
+
+  it('fetches the current user profile on update when there is no loaded profile and authToken changes', () => {
+    const store = configureStore();
+    const fakeThunk = createFakeThunk();
+
+    const root = render({
+      _fetchCurrentUserProfile: fakeThunk.createThunk,
+      authToken: null,
+      store,
+    });
+
+    store.dispatch(apiActions.setAuthToken({ authToken: 'some-token' }));
+    const { api: apiState } = store.getState();
+    const dispatch = spyOn(store, 'dispatch');
+
+    root.setProps({ apiState, dispatch });
+
+    expect(dispatch).toHaveBeenCalledWith(fakeThunk.thunk);
+  });
+
+  it('does not fetch the current user profile on update when there is no loaded profile and authToken is the same', () => {
+    const store = configureStore();
+    store.dispatch(apiActions.setAuthToken({ authToken: 'some-token' }));
+
+    const root = render({ store });
+
+    const { api: apiState } = store.getState();
+    const dispatch = spyOn(store, 'dispatch');
+
+    root.setProps({ apiState, dispatch });
+
+    expect(dispatch).not.toHaveBeenCalled();
+  });
+
+  it('does not fetch the current user profile on update when the profile has been already loaded', () => {
+    const store = configureStore();
+    store.dispatch(userActions.loadCurrentUser({ user: fakeUser }));
+
+    const root = render({ authToken: null, store });
+
+    store.dispatch(apiActions.setAuthToken({ authToken: 'some-token' }));
+    const { api: apiState } = store.getState();
+    const dispatch = spyOn(store, 'dispatch');
+
+    root.setProps({ apiState, dispatch });
+
+    expect(dispatch).not.toHaveBeenCalled();
+  });
 });

src/components/App/index.tsx

@@ -8,24 +8,25 @@ import {
   withRouter,
 } from 'react-router-dom';
 import makeClassName from 'classnames';
+import log from 'loglevel';
 
 import { ApplicationState, ConnectedReduxProps } from '../../configureStore';
 import styles from './styles.module.scss';
 import { ApiState, actions as apiActions } from '../../reducers/api';
 import {
-  ExternalUser,
   User,
-  actions as userActions,
+  fetchCurrentUserProfile,
   getCurrentUser,
 } from '../../reducers/users';
-import * as api from '../../api';
 import Navbar from '../Navbar';
 import Browse from '../../pages/Browse';
 import Index from '../../pages/Index';
 import NotFound from '../../pages/NotFound';
 import { gettext } from '../../utils';
 
-type PublicProps = {
+export type PublicProps = {
+  _fetchCurrentUserProfile: typeof fetchCurrentUserProfile;
+  _log: typeof log;
   authToken: string | null;
 };
 
@@ -43,6 +44,11 @@ type Props = PublicProps &
 /* eslint-enable @typescript-eslint/indent */
 
 export class AppBase extends React.Component<Props> {
+  static defaultProps = {
+    _fetchCurrentUserProfile: fetchCurrentUserProfile,
+    _log: log,
+  };
+
   componentDidMount() {
     const { authToken, dispatch } = this.props;
 
@@ -51,18 +57,13 @@ export class AppBase extends React.Component<Props> {
     }
   }
 
-  async componentDidUpdate(prevProps: Props) {
-    const { apiState, dispatch, profile } = this.props;
+  componentDidUpdate(prevProps: Props) {
+    const { apiState, profile } = this.props;
 
     if (!profile && prevProps.apiState.authToken !== apiState.authToken) {
-      const response = (await api.callApi({
-        apiState,
-        endpoint: '/accounts/profile/',
-      })) as ExternalUser;
-
-      if (response && response.name) {
-        dispatch(userActions.loadCurrentUser({ user: response }));
-      }
+      const { _fetchCurrentUserProfile, dispatch } = this.props;
+
+      dispatch(_fetchCurrentUserProfile());
     }
   }
 

src/components/Navbar/index.spec.tsx

@@ -5,7 +5,7 @@ import { Store } from 'redux';
 import configureStore from '../../configureStore';
 import LoginButton from '../LoginButton';
 import styles from './styles.module.scss';
-import { createFakeThunk, fakeUser } from '../../test-helpers';
+import { createFakeThunk, fakeUser, spyOn } from '../../test-helpers';
 import { actions as userActions, requestLogOut } from '../../reducers/users';
 
 import Navbar from '.';
@@ -81,9 +81,7 @@ describe(__filename, () => {
   describe('Log out button', () => {
     it('dispatches requestLogOut when clicked', () => {
       const store = storeWithUser();
-      const dispatch = jest
-        .spyOn(store, 'dispatch')
-        .mockImplementation(jest.fn());
+      const dispatch = spyOn(store, 'dispatch');
 
       const fakeThunk = createFakeThunk();
       const root = render({