Skip to content

Commit

Permalink
DAH 2488 Decouple Validations from List Retrieval (#656)
Browse files Browse the repository at this point in the history
* Empty commit to force CircleCI run

* DAH-2488 decouple validations from list call

* DAH-2488 do not call general when not needed

* DAH-2488 rename variable

---------

Co-authored-by: Andrea Egan <[email protected]>
  • Loading branch information
tallulahkay and akegan authored Aug 13, 2024
1 parent 1f2fc29 commit 930f266
Show file tree
Hide file tree
Showing 9 changed files with 104 additions and 26 deletions.
11 changes: 9 additions & 2 deletions app/javascript/apiService.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,12 @@ const fetchApplications = async ({ page, filters }) =>
true
)

const fetchLeaseUpApplications = async (listingId, page, { filters }) => {
const fetchLeaseUpApplications = async (
listingId,
page,
{ filters },
includeGeneralApps = true
) => {
const generalApps = {
records: [],
pages: 0
Expand All @@ -86,7 +91,9 @@ const fetchLeaseUpApplications = async (listingId, page, { filters }) => {
// Fetch application preferences associated with a lease up listing.
const appPrefs = await getLeaseUpApplications(listingId, filters)

if (appPrefs.listing_type !== LISTING_TYPE_FIRST_COME_FIRST_SERVED) {
// don't need to include general applications for first come fist served listings
// or when getting applications for layered preferences
if (appPrefs.listing_type !== LISTING_TYPE_FIRST_COME_FIRST_SERVED && includeGeneralApps) {
// Fetch general applications associated with a lease up listing.
const generalAppsResponse = await getLeaseUpApplications(listingId, filters, true)
generalApps.records = generalAppsResponse.records
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ const textCell = ({ value }) => {

const LeaseUpApplicationsTable = ({
dataSet,
listingType,
prefMap,
onLeaseUpStatusChange,
pages,
rowsPerPage,
Expand Down Expand Up @@ -101,7 +101,9 @@ const LeaseUpApplicationsTable = ({
Cell: (cell) => (
<PreferenceRankCell
preferenceRank={cell.original.preference_rank}
preferenceValidation={cell.original.layered_validation}
preferenceValidation={
prefMap[`${cell.original.application_id}-${cell.original.preference_name}`]
}
/>
)
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import React from 'react'
import React, { useEffect, useState } from 'react'

import { capitalize, compact, map, cloneDeep } from 'lodash'

import StatusModalWrapper from 'components/organisms/StatusModalWrapper'
import { LISTING_TYPE_FIRST_COME_FIRST_SERVED } from 'utils/consts'

import { withContext } from './context'
import LeaseUpApplicationsFilterContainer from './LeaseUpApplicationsFilterContainer'
import LeaseUpApplicationsTable from './LeaseUpApplicationsTable'
import { getApplications } from './utils/leaseUpRequestUtils'

const getRank = (prefKey, prefLotteryRank) => {
return prefLotteryRank ? `${prefKey} ${prefLotteryRank}` : 'Unranked'
Expand Down Expand Up @@ -68,22 +70,39 @@ const LeaseUpTableContainer = ({
statusModal
}
}) => {
const [prefMap, setPrefMap] = useState(null)

useEffect(() => {
// don't need layered validation for fcfs
if (listingType !== LISTING_TYPE_FIRST_COME_FIRST_SERVED) {
getApplications(listingId, 0, {}, true, false).then(({ records }) => {
const prefMap = {}
records.forEach((preference) => {
prefMap[`${preference.application_id}-${preference.preference_name}`] =
preference.layered_validation
})
setPrefMap(prefMap)
})
}
}, [listingId, listingType])

return (
<>
<LeaseUpApplicationsFilterContainer
listingType={listingType}
preferences={preferences}
onSubmit={onFilter}
loading={loading}
loading={loading || (!prefMap && listingType !== LISTING_TYPE_FIRST_COME_FIRST_SERVED)}
bulkCheckboxesState={bulkCheckboxesState}
onClearSelectedApplications={onClearSelectedApplications}
onSelectAllApplications={onSelectAllApplications}
onBulkLeaseUpStatusChange={(val) => onLeaseUpStatusChange(val, null, false)}
onBulkLeaseUpCommentChange={(val) => onLeaseUpStatusChange(null, null, true)}
/>
{!loading && (
{!loading && (prefMap || listingType === LISTING_TYPE_FIRST_COME_FIRST_SERVED) && (
<LeaseUpApplicationsTable
dataSet={map(applications, buildRowData)}
prefMap={prefMap}
listingId={listingId}
listingType={listingType}
onLeaseUpStatusChange={onLeaseUpStatusChange}
Expand Down
13 changes: 10 additions & 3 deletions app/javascript/components/lease_ups/utils/leaseUpRequestUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,18 @@ export const sanitizeAndFormatSearch = (str) => {
return convertToCommaSeparatedList(str.replace(/["']/g, ''))
}

export const getApplications = async (listingId, page, filters) => {
export const getApplications = async (
listingId,
page,
filters,
withLayeredValidation = false,
includeGeneralApps = true
) => {
if (filters?.search) {
filters = { ...filters, search: sanitizeAndFormatSearch(filters?.search) }
}
return apiService
.fetchLeaseUpApplications(listingId, page, { filters })
.fetchLeaseUpApplications(listingId, page, { filters }, includeGeneralApps)
.then(({ records, pages, listing_type }) => {
let apps
if (listing_type === LISTING_TYPE_FIRST_COME_FIRST_SERVED) {
Expand All @@ -40,7 +46,8 @@ export const getApplications = async (listingId, page, filters) => {
})
} else {
const preferences = map(records, buildLeaseUpAppPrefModel)
apps = addLayeredValidation(preferences)
// only do the layered validation loop if necessary
apps = withLayeredValidation ? addLayeredValidation(preferences) : preferences
}
return {
records: apps,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ describe('LeaseUpApplicationsPage', () => {
})

test('calls get applications with the listing id and page number = 0', () => {
expect(mockFetchLeaseUpApplications.mock.calls).toHaveLength(1)
expect(mockFetchLeaseUpApplications.mock.calls).toHaveLength(2)
expect(mockFetchLeaseUpApplications.mock.calls[0][0]).toEqual(mockListing.id)
expect(mockFetchLeaseUpApplications.mock.calls[0][1]).toBe(0)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ import LeaseUpApplicationsTable from 'components/lease_ups/LeaseUpApplicationsTa
import Provider from 'context/Provider'
import * as customHooks from 'utils/customHooks'

import { mockBulkCheckboxesState, mockDataSet } from '../../fixtures/lease_up_applications'
import {
mockBulkCheckboxesState,
mockDataSet,
mockPrefMap
} from '../../fixtures/lease_up_applications'

describe('LeaseUpApplicationsTable', () => {
let spy
Expand All @@ -30,6 +34,7 @@ describe('LeaseUpApplicationsTable', () => {
<BrowserRouter>
<Provider value={{ applicationsListData: {} }}>
<LeaseUpApplicationsTable
prefMap={mockPrefMap}
dataSet={mockDataSet}
listingId='a0W4U00000Hm6qRUAR'
onLeaseUpStatusChange={jest.fn()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -436,12 +436,12 @@ exports[`LeaseUpApplicationsTable should render succesfully when not loading 1`]
L_W 1
</div>
<svg
data-testid="preference-rank-x-icon"
style="width: 1rem; height: 1rem; fill: #e31c3d;"
data-testid="preference-rank-check-icon"
style="width: 1.25rem; height: 1.25rem; fill: #2e8540;"
>
<use
style="fill: #e31c3d;"
xlink:href="#i-close"
style="fill: #2e8540;"
xlink:href="#i-check"
/>
</svg>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,14 @@ describe('leaseUpActions', () => {

test('it makes the expected apiService request when no filters are provided', () => {
getApplications(fakeListingId, 0)
expect(apiService.fetchLeaseUpApplications).toHaveBeenCalledWith(fakeListingId, 0, {
filters: undefined
})
expect(apiService.fetchLeaseUpApplications).toHaveBeenCalledWith(
fakeListingId,
0,
{
filters: undefined
},
true
)
})
test('it formats returned data as expected', async () => {
const expectedRowData = {
Expand Down Expand Up @@ -108,15 +113,20 @@ describe('leaseUpActions', () => {
}

const expectedResults = { records: [expectedRowData], pages: 10 }
expect(await getApplications(fakeListingId, 0)).toEqual(expectedResults)
expect(await getApplications(fakeListingId, 0, {}, true)).toEqual(expectedResults)
})

test('it passes filters to the apiService as expected', () => {
const fakeFilters = { filter1: 'something', filter2: 'something else' }
getApplications(fakeListingId, 0, fakeFilters)
expect(apiService.fetchLeaseUpApplications).toHaveBeenCalledWith(fakeListingId, 0, {
filters: fakeFilters
})
expect(apiService.fetchLeaseUpApplications).toHaveBeenCalledWith(
fakeListingId,
0,
{
filters: fakeFilters
},
true
)
})
test('it reformats search strings as expected', () => {
const fakeFilters = { filter1: 'something', search: 'word1 word2' }
Expand All @@ -125,9 +135,14 @@ describe('leaseUpActions', () => {
search: convertToCommaSeparatedList('word1 word2')
}
getApplications(fakeListingId, 0, fakeFilters)
expect(apiService.fetchLeaseUpApplications).toHaveBeenCalledWith(fakeListingId, 0, {
filters: expectedFilters
})
expect(apiService.fetchLeaseUpApplications).toHaveBeenCalledWith(
fakeListingId,
0,
{
filters: expectedFilters
},
true
)
})
})

Expand Down
23 changes: 23 additions & 0 deletions spec/javascript/fixtures/lease_up_applications.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,29 @@ export const mockBulkCheckboxesState = {
a0o4U00000KIUAsQAP: false
}

export const mockPrefMap = {
"a0o4U00000KIYp1QAH-Displaced Tenant Housing Preference (DTHP)": "Confirmed",
"a0o4U00000KIKuwQAH-Live or Work in San Francisco Preference": "Confirmed",
"a0o4U00000KIYpGQAX-Live or Work in San Francisco Preference": "Confirmed",
"a0o4U00000KIK4SQAX-Live or Work in San Francisco Preference": "Confirmed",
"a0o4U00000KIJxEQAX-Live or Work in San Francisco Preference": "Confirmed",
"a0o4U00000KIchuQAD-Live or Work in San Francisco Preference": "Confirmed",
"a0o4U00000KIKRaQAP-Live or Work in San Francisco Preference": "Confirmed",
"a0o4U00000KIXY4QAP-Live or Work in San Francisco Preference": "Confirmed",
"a0o4U00000KIYrNQAX-Live or Work in San Francisco Preference": "Confirmed",
"a0o4U00000KIJmNQAX-Live or Work in San Francisco Preference": "Confirmed",
"a0o4U00000KIKfnQAH-Live or Work in San Francisco Preference": "Confirmed",
"a0o4U00000KIKD8QAP-Live or Work in San Francisco Preference": "Confirmed",
"a0o4U00000KIXj7QAH-Live or Work in San Francisco Preference": "Confirmed",
"a0o4U00000KIJoTQAX-Live or Work in San Francisco Preference": "Confirmed",
"a0o4U00000KIKfEQAX-Live or Work in San Francisco Preference": "Confirmed",
"a0o4U00000KIK3jQAH-Live or Work in San Francisco Preference": "Confirmed",
"a0o4U00000KIXTuQAP-Live or Work in San Francisco Preference": "Confirmed",
"a0o4U00000KIL49QAH-Live or Work in San Francisco Preference": "Confirmed",
"a0o4U00000KIPTJQA5-Live or Work in San Francisco Preference": "Confirmed",
"a0o4U00000KIUAsQAP-Live or Work in San Francisco Preference": "Confirmed"
}

export const mockDataSet = [
{
application_id: 'a0o4U00000KIYp1QAH',
Expand Down

0 comments on commit 930f266

Please sign in to comment.