Remove the parsePage helper function (#4503)

We do not need the functionality of parsePage on the frontend because it is fine to pass `undefined` for `page` into API calls, and in fact we want to do that so the API will produce a 404 for us.

Author: bobsilverberg

src/amo/components/AddonReviewList/index.js

@@ -16,7 +16,7 @@ import { fetchAddon, getAddonBySlug } from 'core/reducers/addons';
 import Paginate from 'core/components/Paginate';
 import { withFixedErrorHandler } from 'core/errorHandler';
 import translate from 'core/i18n/translate';
-import { parsePage, sanitizeHTML } from 'core/utils';
+import { sanitizeHTML } from 'core/utils';
 import { getAddonIconUrl } from 'core/imageUtils';
 import log from 'core/logger';
 import Link from 'amo/components/Link';
@@ -117,8 +117,7 @@ export class AddonReviewListBase extends React.Component<Props> {
   }
 
   getCurrentPage() {
-    const { location } = this.props;
-    return parsePage(location.query.page);
+    return this.props.location.query.page || 1;
   }
 
   onReviewSubmitted = () => {
@@ -320,7 +319,7 @@ export function mapStateToProps(state: AppState, ownProps: Props) {
 export const extractId = (ownProps: Props) => {
   const { location, params } = ownProps.router;
 
-  return `${params.addonSlug}-${parsePage(location.query.page)}`;
+  return `${params.addonSlug}-${location.query.page || ''}`;
 };
 
 export default compose(

src/amo/components/Collection/index.js

@@ -25,7 +25,7 @@ import { openFormOverlay } from 'core/reducers/formOverlay';
 import log from 'core/logger';
 import { hasPermission } from 'amo/reducers/users';
 import translate from 'core/i18n/translate';
-import { parsePage, sanitizeHTML } from 'core/utils';
+import { sanitizeHTML } from 'core/utils';
 import Card from 'ui/components/Card';
 import LoadingText from 'ui/components/LoadingText';
 import MetadataCard from 'ui/components/MetadataCard';
@@ -112,7 +112,7 @@ export class CollectionBase extends React.Component<Props> {
     if (!collection || collectionChanged) {
       this.props.dispatch(fetchCurrentCollection({
         errorHandlerId: errorHandler.id,
-        page: parsePage(location.query.page),
+        page: location.query.page,
         slug: params.slug,
         user: params.user,
       }));
@@ -123,7 +123,7 @@ export class CollectionBase extends React.Component<Props> {
     if (collection && addonsPageChanged) {
       this.props.dispatch(fetchCurrentCollectionPage({
         errorHandlerId: errorHandler.id,
-        page: parsePage(location.query.page),
+        page: location.query.page,
         slug: params.slug,
         user: params.user,
       }));
@@ -218,7 +218,7 @@ export class CollectionBase extends React.Component<Props> {
             <Paginate
               LinkComponent={Link}
               count={collection.numberOfAddons}
-              currentPage={parsePage(location.query.page)}
+              currentPage={location.query.page}
               pathname={this.url()}
             />
           )}
@@ -284,7 +284,7 @@ export const extractId = (ownProps: Props) => {
   return [
     ownProps.params.user,
     ownProps.params.slug,
-    parsePage(ownProps.location.query.page),
+    ownProps.location.query.page,
   ].join('/');
 };
 

src/amo/components/Search/index.js

@@ -29,7 +29,6 @@ import {
   convertFiltersToQueryParams,
   hasSearchFilters,
 } from 'core/searchUtils';
-import { parsePage } from 'core/utils';
 
 import './styles.scss';
 
@@ -191,8 +190,6 @@ export class SearchBase extends React.Component {
       }
     }
 
-    const page = parsePage(filters.page);
-
     // We allow specific paginationQueryParams instead of always using
     // convertFiltersToQueryParams(filters) so certain search filters
     // aren't repeated if they are elsewhere in the URL. This is useful
@@ -209,7 +206,7 @@ export class SearchBase extends React.Component {
       <Paginate
         LinkComponent={LinkComponent}
         count={count}
-        currentPage={page}
+        currentPage={filters.page}
         pathname={pathname}
         queryParams={queryParams}
       />
@@ -254,7 +251,7 @@ export function mapStateToProps(state) {
 // This ID does not need to differentiate between component instances because
 // the error handler gets cleared every time the search filters change.
 export const extractId = (ownProps) => {
-  return parsePage(ownProps.filters.page);
+  return ownProps.filters.page;
 };
 
 export default compose(

src/core/utils/index.js

@@ -288,11 +288,6 @@ export function getCategoryColor(category) {
   return category.id;
 }
 
-export function parsePage(page) {
-  const parsed = parseInt(page, 10);
-  return Number.isNaN(parsed) || parsed < 1 ? 1 : parsed;
-}
-
 export function addonHasVersionHistory(addon) {
   if (!addon) {
     throw new Error('addon is required');

tests/unit/amo/api/test_collections.js

@@ -8,7 +8,6 @@ import {
   listCollections,
   updateCollection,
 } from 'amo/api/collections';
-import { parsePage } from 'core/utils';
 import { apiResponsePage, createApiResponse } from 'tests/unit/helpers';
 import {
   createFakeCollectionAddons,
@@ -94,7 +93,7 @@ describe(__filename, () => {
     });
 
     it('calls the collection add-ons list API', async () => {
-      const queryParams = { page: parsePage(1) };
+      const queryParams = { page: 1 };
       const params = getParams({ ...queryParams });
 
       mockApi

tests/unit/amo/components/TestAddonReviewList.js

@@ -668,7 +668,7 @@ describe(__filename, () => {
         location: fakeRouterLocation(),
       });
 
-      expect(extractId(ownProps)).toEqual(`foobar-1`);
+      expect(extractId(ownProps)).toEqual(`foobar-`);
     });
   });
 });

tests/unit/amo/components/TestCollection.js

@@ -121,7 +121,7 @@ describe(__filename, () => {
     sinon.assert.callCount(fakeDispatch, 1);
     sinon.assert.calledWith(fakeDispatch, fetchCurrentCollection({
       errorHandlerId: errorHandler.id,
-      page: 1,
+      page: undefined,
       slug,
       user,
     }));
@@ -351,7 +351,7 @@ describe(__filename, () => {
     sinon.assert.callCount(fakeDispatch, 1);
     sinon.assert.calledWith(fakeDispatch, fetchCurrentCollection({
       errorHandlerId: errorHandler.id,
-      page: 1,
+      page: undefined,
       ...newParams,
     }));
   });
@@ -402,7 +402,7 @@ describe(__filename, () => {
     sinon.assert.callCount(fakeDispatch, 1);
     sinon.assert.calledWith(fakeDispatch, fetchCurrentCollection({
       errorHandlerId: errorHandler.id,
-      page: 1,
+      page: undefined,
       ...newParams,
     }));
   });
@@ -619,7 +619,7 @@ describe(__filename, () => {
         location: fakeRouterLocation(),
       });
 
-      expect(extractId(props)).toEqual('foo/collection-bar/1');
+      expect(extractId(props)).toEqual('foo/collection-bar/');
     });
 
     it('adds the page as part of unique ID', () => {

tests/unit/amo/components/TestSearch.js

@@ -360,7 +360,7 @@ describe(__filename, () => {
         },
       };
 
-      expect(extractId(ownProps)).toEqual(1);
+      expect(extractId(ownProps)).toEqual(undefined);
     });
   });
 });

tests/unit/amo/reducers/test_collections.js

@@ -20,7 +20,6 @@ import reducer, {
   loadUserCollections,
   updateCollection,
 } from 'amo/reducers/collections';
-import { parsePage } from 'core/utils';
 import { createStubErrorHandler } from 'tests/unit/helpers';
 import {
   createFakeCollectionAddons,
@@ -31,6 +30,8 @@ import {
 
 describe(__filename, () => {
   describe('reducer', () => {
+    const pageToFetch = 2;
+
     it('initializes properly', () => {
       const state = reducer(undefined, {});
       expect(state).toEqual(initialState);
@@ -55,7 +56,7 @@ describe(__filename, () => {
     it('sets a loading flag when fetching a collection page', () => {
       const state = reducer(undefined, fetchCurrentCollectionPage({
         errorHandlerId: createStubErrorHandler().id,
-        page: parsePage(2),
+        page: pageToFetch,
         slug: 'some-collection-slug',
         user: 'some-user-id-or-name',
       }));
@@ -74,7 +75,7 @@ describe(__filename, () => {
 
       state = reducer(state, fetchCurrentCollectionPage({
         errorHandlerId: createStubErrorHandler().id,
-        page: parsePage(2),
+        page: pageToFetch,
         slug: collectionDetail.slug,
         user: 'some-user-id-or-name',
       }));
@@ -135,7 +136,7 @@ describe(__filename, () => {
       // 2. User clicks the "next" pagination link.
       state = reducer(state, fetchCurrentCollectionPage({
         errorHandlerId: createStubErrorHandler().id,
-        page: parsePage(2),
+        page: pageToFetch,
         slug: 'some-collection-slug',
         user: 'some-user-id-or-name',
       }));

tests/unit/amo/sagas/test_collections.js

@@ -22,7 +22,6 @@ import apiReducer from 'core/reducers/api';
 import {
   beginFormOverlaySubmit, closeFormOverlay, finishFormOverlaySubmit,
 } from 'core/reducers/formOverlay';
-import { parsePage } from 'core/utils';
 import { createStubErrorHandler } from 'tests/unit/helpers';
 import {
   createFakeCollectionAddons,
@@ -34,6 +33,7 @@ import {
 describe(__filename, () => {
   const user = 'user-id-or-name';
   const slug = 'collection-slug';
+  const page = 1;
 
   let clientData;
   let errorHandler;
@@ -82,14 +82,14 @@ describe(__filename, () => {
         .expects('getCollectionAddons')
         .withArgs({
           api: state.api,
-          page: parsePage(1),
+          page,
           slug,
           user,
         })
         .once()
         .returns(Promise.resolve(collectionAddons));
 
-      _fetchCurrentCollection({ page: parsePage(1), slug, user });
+      _fetchCurrentCollection({ page, slug, user });
 
       const expectedLoadAction = loadCurrentCollection({
         addons: collectionAddons,
@@ -143,14 +143,14 @@ describe(__filename, () => {
         .expects('getCollectionAddons')
         .withArgs({
           api: state.api,
-          page: parsePage(1),
+          page,
           slug,
           user,
         })
         .once()
         .returns(Promise.resolve(collectionAddons));
 
-      _fetchCurrentCollectionPage({ page: parsePage(1), slug, user });
+      _fetchCurrentCollectionPage({ page, slug, user });
 
       const expectedLoadAction = loadCurrentCollectionPage({
         addons: collectionAddons,
@@ -162,7 +162,7 @@ describe(__filename, () => {
     });
 
     it('clears the error handler', async () => {
-      _fetchCurrentCollectionPage({ page: parsePage(1), slug, user });
+      _fetchCurrentCollectionPage({ page, slug, user });
 
       const expectedAction = errorHandler.createClearingAction();
 
@@ -178,7 +178,7 @@ describe(__filename, () => {
         .once()
         .returns(Promise.reject(error));
 
-      _fetchCurrentCollectionPage({ page: parsePage(1), slug, user });
+      _fetchCurrentCollectionPage({ page, slug, user });
 
       const expectedAction = errorHandler.createErrorAction(error);
       const action = await sagaTester.waitFor(expectedAction.type);