From bee55de717eac6577acc15487de8cd15f43fd936 Mon Sep 17 00:00:00 2001 From: Kelly Innes Date: Mon, 26 Aug 2019 18:16:19 -0400 Subject: [PATCH] Fetch next page of facilities via infinite scroll Adjust facilities tab so that it will fetch the next page of facilities on reaching the bottom of the infinite scroll list. Also: - remove some now unused utility code & tests - update facilities list endpoint to return results in alphabetical order - update url to retrieve facilities to request pages of 25 results at a time - add a loading indicator contextually if there's a next page of facilities to fetch --- CHANGELOG.md | 1 + src/app/src/__tests__/utils.tests.js | 91 +++---------------- src/app/src/actions/facilities.js | 31 +++++++ src/app/src/actions/ui.js | 5 - .../components/FilterSidebarFacilitiesTab.jsx | 72 +++++---------- src/app/src/reducers/FacilitiesReducer.js | 29 ++++++ src/app/src/reducers/UIReducer.js | 17 ---- src/app/src/util/constants.js | 2 + src/app/src/util/util.js | 45 +++------ src/django/api/views.py | 3 +- 10 files changed, 116 insertions(+), 180 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12e005ab8..f11f4b067 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] ### Added - Adjust marker icon on selecting a new facility on the vector tiles layer [#749](https://github.com/open-apparel-registry/open-apparel-registry/pull/749) +- Fetch next page of facilities while scrolling through sidebar list [#750](https://github.com/open-apparel-registry/open-apparel-registry/pull/750) ### Changed diff --git a/src/app/src/__tests__/utils.tests.js b/src/app/src/__tests__/utils.tests.js index d2c5aa961..2b3fc2fc8 100644 --- a/src/app/src/__tests__/utils.tests.js +++ b/src/app/src/__tests__/utils.tests.js @@ -61,8 +61,6 @@ const { makeUserProfileURL, makeProfileRouteLink, joinDataIntoCSVString, - caseInsensitiveIncludes, - sortFacilitiesAlphabeticallyByName, updateListWithLabels, makeSubmitFormOnEnterKeyPressFunction, makeFacilityListItemsRetrieveCSVItemsURL, @@ -75,6 +73,7 @@ const { claimFacilityContactInfoStepIsValid, claimFacilityFacilityInfoStepIsValid, anyListItemMatchesAreInactive, + pluralizeResultsCount, } = require('../util/util'); const { @@ -88,6 +87,7 @@ const { ENTER_KEY, facilityListItemStatusChoicesEnum, facilityListSummaryStatusMessages, + FACILITIES_REQUEST_PAGE_SIZE, } = require('../util/constants'); it('creates a route for checking facility list items', () => { @@ -140,7 +140,7 @@ it('creates an API URL for getting a single facility by OAR ID', () => { it('creates an API URL for getting facilities with a query string', () => { const qs = 'hello=world'; - const expectedMatch = '/api/facilities/?hello=world'; + const expectedMatch = `/api/facilities/?hello=world&pageSize=${FACILITIES_REQUEST_PAGE_SIZE}`; expect(makeGetFacilitiesURLWithQueryString(qs)).toEqual(expectedMatch); }); @@ -877,83 +877,6 @@ it('joins a 2-d array into a correctly escaped CSV string', () => { expect(joinDataIntoCSVString(escapedArray)).toBe(expectedEscapedArrayMatch); }); -it('checks whether one string includes another regardless of char case', () => { - const uppercaseTarget = 'HELLOWORLD'; - const lowercaseTest = 'world'; - const lowercaseTarget = 'helloworld'; - const uppercaseTest = 'WORLD'; - const uppercaseNonMatchTest = 'FOO'; - const lowercaseNonMatchTest = 'foo'; - - expect(caseInsensitiveIncludes(uppercaseTarget, lowercaseTest)).toBe(true); - expect(caseInsensitiveIncludes(lowercaseTarget, uppercaseTest)).toBe(true); - expect(caseInsensitiveIncludes(lowercaseTarget, lowercaseTest)).toBe(true); - expect(caseInsensitiveIncludes(uppercaseTarget, uppercaseTest)).toBe(true); - - expect(caseInsensitiveIncludes(uppercaseTarget, lowercaseNonMatchTest)).toBe(false); - expect(caseInsensitiveIncludes(lowercaseTarget, uppercaseNonMatchTest)).toBe(false); - expect(caseInsensitiveIncludes(lowercaseTarget, lowercaseNonMatchTest)).toBe(false); - expect(caseInsensitiveIncludes(uppercaseTarget, uppercaseNonMatchTest)).toBe(false); -}); - -it('sorts an array of facilities alphabetically by name without mutating the input', () => { - const inputData = [ - { - properties: { - name: 'hello World', - }, - }, - { - properties: { - name: 'FOO', - }, - }, - { - properties: { - name: 'Bar', - }, - }, - { - properties: { - name: 'baz', - }, - }, - ]; - - const expectedSortedData = [ - { - properties: { - name: 'Bar', - }, - }, - { - properties: { - name: 'baz', - }, - }, - { - properties: { - name: 'FOO', - }, - }, - { - properties: { - name: 'hello World', - }, - }, - ]; - - expect(isEqual( - sortFacilitiesAlphabeticallyByName(inputData), - expectedSortedData, - )).toBe(true); - - expect(isEqual( - inputData, - expectedSortedData, - )).toBe(false); -}); - it('updates a list of unlabeled values with the correct labels from a given source', () => { const source = [ { @@ -1348,3 +1271,11 @@ it('checks a facility list item to see whether any matches have been set to inac expect(anyListItemMatchesAreInactive(listItemWithInactiveMatches)).toBe(true); }); + +it('pluralizes a results count correclty, returning null if count is undefined or null', () => { + expect(pluralizeResultsCount(undefined)).toBeNull(); + expect(pluralizeResultsCount(null)).toBeNull(); + expect(pluralizeResultsCount(1)).toBe('1 result'); + expect(pluralizeResultsCount(0)).toBe('0 results'); + expect(pluralizeResultsCount(200)).toBe('200 results'); +}); diff --git a/src/app/src/actions/facilities.js b/src/app/src/actions/facilities.js index 1d7cd3c24..910975b3f 100644 --- a/src/app/src/actions/facilities.js +++ b/src/app/src/actions/facilities.js @@ -65,6 +65,37 @@ export function fetchFacilities(pushNewRoute = noop) { }; } +export const startFetchNextPageOfFacilities = createAction('START_FETCH_NEXT_PAGE_OF_FACILITIES'); +export const failFetchNextPageOfFacilities = createAction('FAIL_FETCH_NEXT_PAGE_OF_FACILITIES'); +export const completeFetchNextPageOfFacilities = createAction('COMPLETE_FETCH_NEXT_PAGE_OF_FACILITIES'); + +export function fetchNextPageOfFacilities() { + return (dispatch, getState) => { + const { + facilities: { + facilities: { + nextPageURL, + }, + }, + } = getState(); + + if (!nextPageURL) { + return noop(); + } + + dispatch(startFetchNextPageOfFacilities()); + + return apiRequest + .get(nextPageURL) + .then(({ data }) => dispatch(completeFetchNextPageOfFacilities(data))) + .catch(err => dispatch(logErrorAndDispatchFailure( + err, + 'An error prevented fetching the next page of facilities', + failFetchNextPageOfFacilities, + ))); + }; +} + export function fetchSingleFacility(oarID = null) { return (dispatch) => { dispatch(startFetchSingleFacility()); diff --git a/src/app/src/actions/ui.js b/src/app/src/actions/ui.js index 58c41ce21..18b90740d 100644 --- a/src/app/src/actions/ui.js +++ b/src/app/src/actions/ui.js @@ -4,11 +4,6 @@ export const makeSidebarGuideTabActive = createAction('MAKE_SIDEBAR_GUIDE_TAB_AC export const makeSidebarSearchTabActive = createAction('MAKE_SIDEBAR_SEARCH_TAB_ACTIVE'); export const makeSidebarFacilitiesTabActive = createAction('MAKE_SIDEBAR_FACILITIES_TAB_ACTIVE'); -export const updateSidebarFacilitiesTabTextFilter = - createAction('UPDATE_SIDEBAR_FACILITIES_TAB_TEXT_FILTER'); -export const resetSidebarFacilitiesTabTextFilter = - createAction('RESET_SIDEBAR_FACILITIES_TAB_TEXT_FILTER'); - export const recordSearchTabResetButtonClick = createAction('RECORD_SEARCH_TAB_RESET_BUTTON_CLICK'); diff --git a/src/app/src/components/FilterSidebarFacilitiesTab.jsx b/src/app/src/components/FilterSidebarFacilitiesTab.jsx index a7c5a9864..1c9607080 100644 --- a/src/app/src/components/FilterSidebarFacilitiesTab.jsx +++ b/src/app/src/components/FilterSidebarFacilitiesTab.jsx @@ -17,13 +17,12 @@ import get from 'lodash/get'; import { toast } from 'react-toastify'; import InfiniteAnyHeight from 'react-infinite-any-height'; -import ControlledTextInput from './ControlledTextInput'; - import { makeSidebarSearchTabActive, - updateSidebarFacilitiesTabTextFilter, } from '../actions/ui'; +import { fetchNextPageOfFacilities } from '../actions/facilities'; + import { logDownload } from '../actions/logDownload'; import { facilityCollectionPropType } from '../util/propTypes'; @@ -35,17 +34,13 @@ import { import { makeFacilityDetailLink, - getValueFromEvent, - caseInsensitiveIncludes, - sortFacilitiesAlphabeticallyByName, + pluralizeResultsCount, } from '../util/util'; import COLOURS from '../util/COLOURS'; import { filterSidebarStyles } from '../util/styles'; -const SEARCH_TERM_INPUT = 'SEARCH_TERM_INPUT'; - const facilitiesTabStyles = Object.freeze({ noResultsTextStyles: Object.freeze({ margin: '30px', @@ -95,9 +90,9 @@ function FilterSidebarFacilitiesTab({ logDownloadError, user, returnToSearchTab, - filterText, - updateFilterText, handleDownload, + fetchNextPage, + isInfiniteLoading, }) { const [loginRequiredDialogIsOpen, setLoginRequiredDialogIsOpen] = useState(false); const [requestedDownload, setRequestedDownload] = useState(false); @@ -188,27 +183,9 @@ function FilterSidebarFacilitiesTab({ ); } - const filteredFacilities = filterText - ? facilities - .filter(({ - properties: { - address, - name, - country_name: countryName, - }, - }) => caseInsensitiveIncludes(address, filterText) - || caseInsensitiveIncludes(name, filterText) - || caseInsensitiveIncludes(countryName, filterText)) - : facilities; - - const orderedFacilities = - sortFacilitiesAlphabeticallyByName(filteredFacilities); - const facilitiesCount = get(data, 'count', null); - const headerDisplayString = facilitiesCount && (facilitiesCount !== filteredFacilities.length) - ? `Displaying ${filteredFacilities.length} facilities of ${facilitiesCount} results` - : `Displaying ${filteredFacilities.length} facilities`; + const headerDisplayString = pluralizeResultsCount(facilitiesCount); const LoginLink = props => ; const RegisterLink = props => ; @@ -242,19 +219,6 @@ function FilterSidebarFacilitiesTab({ -
- - -
); @@ -265,6 +229,15 @@ function FilterSidebarFacilitiesTab({ const resultListHeight = windowHeight - nonResultListComponentHeight; + const loadingElement = (facilities.length !== facilitiesCount) && ( + + + + + + + ); + return ( {listHeaderInsetComponent} @@ -272,8 +245,12 @@ function FilterSidebarFacilitiesTab({ dispatch(makeSidebarSearchTabActive()), - updateFilterText: e => - dispatch((updateSidebarFacilitiesTabTextFilter(getValueFromEvent(e)))), handleDownload: () => dispatch(logDownload()), + fetchNextPage: () => dispatch(fetchNextPageOfFacilities()), }; } diff --git a/src/app/src/reducers/FacilitiesReducer.js b/src/app/src/reducers/FacilitiesReducer.js index b356a0003..4d01f6872 100644 --- a/src/app/src/reducers/FacilitiesReducer.js +++ b/src/app/src/reducers/FacilitiesReducer.js @@ -13,6 +13,9 @@ import { failFetchSingleFacility, completeFetchSingleFacility, resetSingleFacility, + startFetchNextPageOfFacilities, + failFetchNextPageOfFacilities, + completeFetchNextPageOfFacilities, } from '../actions/facilities'; import { @@ -33,6 +36,8 @@ const initialState = Object.freeze({ data: null, fetching: false, error: null, + nextPageURL: null, + isInfiniteLoading: false, }), singleFacility: Object.freeze({ data: null, @@ -100,6 +105,30 @@ export default createReducer({ fetching: { $set: false }, error: { $set: null }, data: { $set: payload }, + nextPageURL: { $set: get(payload, 'next', null) }, + }, + }), + [startFetchNextPageOfFacilities]: state => update(state, { + facilities: { + isInfiniteLoading: { $set: true }, + }, + }), + [failFetchNextPageOfFacilities]: state => update(state, { + facilities: { + isInfiniteLoading: { $set: false }, + }, + }), + [completeFetchNextPageOfFacilities]: (state, payload) => update(state, { + facilities: { + fetching: { $set: false }, + error: { $set: null }, + data: { + features: { + $push: get(payload, 'features', []), + }, + }, + nextPageURL: { $set: get(payload, 'next', null) }, + isInfiniteLoading: { $set: false }, }, }), [resetFacilities]: state => update(state, { diff --git a/src/app/src/reducers/UIReducer.js b/src/app/src/reducers/UIReducer.js index 10ac000b8..cf35e6a58 100644 --- a/src/app/src/reducers/UIReducer.js +++ b/src/app/src/reducers/UIReducer.js @@ -6,8 +6,6 @@ import { makeSidebarGuideTabActive, makeSidebarSearchTabActive, makeSidebarFacilitiesTabActive, - updateSidebarFacilitiesTabTextFilter, - resetSidebarFacilitiesTabTextFilter, recordSearchTabResetButtonClick, reportWindowResize, } from '../actions/ui'; @@ -19,7 +17,6 @@ import { filterSidebarTabsEnum } from '../util/constants'; const initialState = Object.freeze({ activeFilterSidebarTab: filterSidebarTabsEnum.search, facilitiesSidebarTabSearch: Object.freeze({ - filterText: '', resetButtonClickCount: 0, }), window: Object.freeze({ @@ -66,20 +63,6 @@ export default createReducer({ }, }, }), - [updateSidebarFacilitiesTabTextFilter]: (state, payload) => update(state, { - facilitiesSidebarTabSearch: { - filterText: { - $set: payload, - }, - }, - }), - [resetSidebarFacilitiesTabTextFilter]: state => update(state, { - facilitiesSidebarTabSearch: { - filterText: { - $set: initialState.facilitiesSidebarTabSearch.filterText, - }, - }, - }), [reportWindowResize]: (state, payload) => update(state, { window: { $merge: payload }, }), diff --git a/src/app/src/util/constants.js b/src/app/src/util/constants.js index 6ea5dd526..1bb70a392 100644 --- a/src/app/src/util/constants.js +++ b/src/app/src/util/constants.js @@ -1,5 +1,7 @@ export const OTHER = 'Other'; +export const FACILITIES_REQUEST_PAGE_SIZE = 25; + // This choices must be kept in sync with the identical list // kept in the Django API's models.py file export const contributorTypeOptions = Object.freeze([ diff --git a/src/app/src/util/util.js b/src/app/src/util/util.js index 457ea380a..8eb1e900a 100644 --- a/src/app/src/util/util.js +++ b/src/app/src/util/util.js @@ -10,6 +10,7 @@ import negate from 'lodash/negate'; import omitBy from 'lodash/omitBy'; import isEmpty from 'lodash/isEmpty'; import isNumber from 'lodash/isNumber'; +import isNil from 'lodash/isNil'; import values from 'lodash/values'; import flow from 'lodash/flow'; import noop from 'lodash/noop'; @@ -18,8 +19,6 @@ import startsWith from 'lodash/startsWith'; import head from 'lodash/head'; import replace from 'lodash/replace'; import trimEnd from 'lodash/trimEnd'; -import includes from 'lodash/includes'; -import lowerCase from 'lodash/lowerCase'; import range from 'lodash/range'; import ceil from 'lodash/ceil'; import toInteger from 'lodash/toInteger'; @@ -34,6 +33,7 @@ import env from './env'; import { OTHER, + FACILITIES_REQUEST_PAGE_SIZE, FEATURE_COLLECTION, inputTypesEnum, registrationFieldsEnum, @@ -93,7 +93,7 @@ export const makeGetCountriesURL = () => '/api/countries/'; export const makeGetFacilitiesURL = () => '/api/facilities/'; export const makeGetFacilityByOARIdURL = oarId => `/api/facilities/${oarId}/`; -export const makeGetFacilitiesURLWithQueryString = qs => `/api/facilities/?${qs}`; +export const makeGetFacilitiesURLWithQueryString = qs => `/api/facilities/?${qs}&pageSize=${FACILITIES_REQUEST_PAGE_SIZE}`; export const makeClaimFacilityAPIURL = oarId => `/api/facilities/${oarId}/claim/`; export const makeSplitFacilityAPIURL = oarID => `/api/facilities/${oarID}/split/`; export const makePromoteFacilityMatchAPIURL = oarID => `/api/facilities/${oarID}/promote/`; @@ -485,33 +485,6 @@ export const joinDataIntoCSVString = data => data ); }, ''); -export const caseInsensitiveIncludes = (target, test) => - includes(lowerCase(target), lowerCase(test)); - -export const sortFacilitiesAlphabeticallyByName = data => data - .slice() - .sort(( - { - properties: { - name: firstFacilityName, - }, - }, - { - properties: { - name: secondFacilityName, - }, - }, - ) => { - const a = lowerCase(firstFacilityName); - const b = lowerCase(secondFacilityName); - - if (a === b) { - return 0; - } - - return (a < b) ? -1 : 1; - }); - // Given a list where each item is like { label: 'ABCD', value: 123 }, and // a payload which is a list of items like { label: '123', value: 123 }, // returns a list of items from the payload with their labels replaced with @@ -632,3 +605,15 @@ export const claimFacilityFacilityInfoStepIsValid = ({ ]); export const anyListItemMatchesAreInactive = ({ matches }) => some(matches, ['is_active', false]); + +export const pluralizeResultsCount = (count) => { + if (isNil(count)) { + return null; + } + + if (count === 1) { + return '1 result'; + } + + return `${count} results`; +}; diff --git a/src/django/api/views.py b/src/django/api/views.py index 7777cd566..890e6cc91 100644 --- a/src/django/api/views.py +++ b/src/django/api/views.py @@ -558,7 +558,8 @@ def list(self, request): queryset = Facility \ .objects \ - .filter_by_query_params(request.query_params) + .filter_by_query_params(request.query_params) \ + .order_by('name') page_queryset = self.paginate_queryset(queryset)