From 346a456f413e4bfa0d082770942a207271ee2ff4 Mon Sep 17 00:00:00 2001 From: Rovack <6605826+Rovack@users.noreply.github.com> Date: Fri, 13 Mar 2026 23:58:26 +0200 Subject: [PATCH 01/10] feature: limit IDs passed to location assocations query - When locationFieldsOnly is true, get all datafields in the 1st query, so the 2nd one doesn't need a giant IN with countless IDs. - When locationFieldsOnly is false, disallow fetching >200 locations. --- src/controllers/locations.js | 7 ++++- src/models/location.js | 60 +++++++++++++++++------------------- 2 files changed, 35 insertions(+), 32 deletions(-) diff --git a/src/controllers/locations.js b/src/controllers/locations.js index d817482..ac43404 100644 --- a/src/controllers/locations.js +++ b/src/controllers/locations.js @@ -232,7 +232,12 @@ export default { const taxonomyIds = taxonomyId.split(','); filterParameters.taxonomyIds = await models.Taxonomy.getAllIdsWithinTaxonomies(taxonomyIds); } - const limit = pageSize || maxResults; + + const requestedLimit = pageSize || maxResults; + const maxDetailedResults = 200; + const limit = locationFieldsOnly + ? requestedLimit + : Math.min(requestedLimit, maxDetailedResults); const offset = pageNumber !== undefined && pageSize !== undefined ? pageNumber * pageSize : undefined; diff --git a/src/models/location.js b/src/models/location.js index 6360657..1bd0f16 100644 --- a/src/models/location.js +++ b/src/models/location.js @@ -390,14 +390,15 @@ module.exports = (sequelize, DataTypes, Op) => { sequelize.models.Organization, sequelize.models.PhysicalAddress, sequelize.models.Phone, + sequelize.models.EventRelatedInfo, { model: sequelize.models.Service, required: !noServices, include: [ sequelize.models.Taxonomy, + sequelize.models.HolidaySchedule, ...(areRequiredDocsSpecified ? [sequelize.models.RequiredDocument] : []), ...((openAt && !occasion) ? [sequelize.models.RegularSchedule] : []), - ...(occasion ? [sequelize.models.HolidaySchedule] : []), ...(servesZipcode ? [sequelize.models.ServiceArea] : []), ...(shouldJoinEligibilities ? [{ model: sequelize.models.Eligibility, @@ -585,37 +586,34 @@ module.exports = (sequelize, DataTypes, Op) => { }, selectedAttributeForOrderBy); } - const additionalLocationData = locationFieldsOnly ? [ - sequelize.models.EventRelatedInfo, - { - model: sequelize.models.Service, - include: [ - sequelize.models.HolidaySchedule, - ], - }, - ] : [ - sequelize.models.Organization, - sequelize.models.EventRelatedInfo, - { - model: sequelize.models.Service, - include: [ - sequelize.models.Taxonomy, - sequelize.models.RequiredDocument, - sequelize.models.HolidaySchedule, - ], - }, - sequelize.models.Phone, - sequelize.models.PhysicalAddress, - ]; + let locationsWithAssociations; + if (locationFieldsOnly) { + locationsWithAssociations = locationIds; + } else { + const additionalLocationData = [ + sequelize.models.Organization, + sequelize.models.EventRelatedInfo, + { + model: sequelize.models.Service, + include: [ + sequelize.models.Taxonomy, + sequelize.models.RequiredDocument, + sequelize.models.HolidaySchedule, + ], + }, + sequelize.models.Phone, + sequelize.models.PhysicalAddress, + ]; - const locationsWithAssociations = await Location.findAll({ - attributes: { - include: selectedAttributeForOrderBy ? [selectedAttributeForOrderBy] : undefined, - }, - where: { id: { [Op.in]: locationIds } }, - include: additionalLocationData, - order, - }); + locationsWithAssociations = await Location.findAll({ + attributes: { + include: selectedAttributeForOrderBy ? [selectedAttributeForOrderBy] : undefined, + }, + where: { id: { [Op.in]: locationIds } }, + include: additionalLocationData, + order, + }); + } function sortByLocationIds(a, b) { return locationIds.indexOf(a.id) - locationIds.indexOf(b.id); From bb912756e156ae45285800c131f763ac7ea1ec31 Mon Sep 17 00:00:00 2001 From: Rovack <6605826+Rovack@users.noreply.github.com> Date: Sat, 14 Mar 2026 00:07:00 +0200 Subject: [PATCH 02/10] Actually return the location stubs fetched in the 1st query --- src/models/location.js | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/models/location.js b/src/models/location.js index 1bd0f16..bb58374 100644 --- a/src/models/location.js +++ b/src/models/location.js @@ -297,7 +297,7 @@ module.exports = (sequelize, DataTypes, Op) => { return sequelize.and(requiredDocumentCondition, notRequiredDocumentCondition); }; - Location.findUniqueLocationIds = async (filterParameters, + Location.findUniqueLocationStubs = async (filterParameters, additionalConditions, originalQueryProps = {}, selectedAttributeForOrderBy, noServices) => { @@ -499,7 +499,7 @@ module.exports = (sequelize, DataTypes, Op) => { // apply limit and offset in memory here locations = locations.slice(offset || 0, limit ? (offset || 0) + limit : undefined); - return locations.map(location => location.id); + return locations; }; Location.search = async ({ @@ -513,7 +513,7 @@ module.exports = (sequelize, DataTypes, Op) => { sortBy, noServices, }) => { - let locationIds; + let locationStubs; let distance; let totalNumLocations; // order is used to specify the attribute referenced in the ORDER BY @@ -547,12 +547,12 @@ module.exports = (sequelize, DataTypes, Op) => { if (radius && position) { const distanceCondition = sequelize.where(distance, { [Op.lte]: radius }); - totalNumLocations = (await Location.findUniqueLocationIds( + totalNumLocations = (await Location.findUniqueLocationStubs( filterParameters, [distanceCondition], )).length; - locationIds = await Location.findUniqueLocationIds( + locationStubs = await Location.findUniqueLocationStubs( filterParameters, [distanceCondition].filter(Boolean), { order, @@ -569,26 +569,28 @@ module.exports = (sequelize, DataTypes, Op) => { // However, filtering by window functions requires nested queries, which // aren't natively supported by sequelize and would require a raw query. // For now, the simplicity and security of sequelize seems worth the slight performance hit. - if (minResults && locationIds.length < minResults) { - totalNumLocations = (await Location.findUniqueLocationIds(filterParameters, [])).length; - locationIds = await Location.findUniqueLocationIds(filterParameters, [], { + if (minResults && locationStubs.length < minResults) { + totalNumLocations = (await Location.findUniqueLocationStubs(filterParameters, [])).length; + locationStubs = await Location.findUniqueLocationStubs(filterParameters, [], { order, limit: minResults, offset, }, selectedAttributeForOrderBy, noServices); } } else { - totalNumLocations = (await Location.findUniqueLocationIds(filterParameters, [])).length; - locationIds = await Location.findUniqueLocationIds(filterParameters, [], { + totalNumLocations = (await Location.findUniqueLocationStubs(filterParameters, [])).length; + locationStubs = await Location.findUniqueLocationStubs(filterParameters, [], { limit, offset, order, }, selectedAttributeForOrderBy); } + const locationIds = locationStubs.map(location => location.id); + let locationsWithAssociations; if (locationFieldsOnly) { - locationsWithAssociations = locationIds; + locationsWithAssociations = locationStubs; } else { const additionalLocationData = [ sequelize.models.Organization, From 2d3737644e648f8d1cfa91da57b7028f29b66e99 Mon Sep 17 00:00:00 2001 From: Rovack <6605826+Rovack@users.noreply.github.com> Date: Sat, 14 Mar 2026 00:14:33 +0200 Subject: [PATCH 03/10] Fix pagination to take the updated limit into account --- src/controllers/locations.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/controllers/locations.js b/src/controllers/locations.js index ac43404..ca9df00 100644 --- a/src/controllers/locations.js +++ b/src/controllers/locations.js @@ -239,8 +239,8 @@ export default { ? requestedLimit : Math.min(requestedLimit, maxDetailedResults); - const offset = pageNumber !== undefined && pageSize !== undefined ? - pageNumber * pageSize : undefined; + const offset = pageNumber !== undefined && limit !== undefined ? + pageNumber * limit : undefined; const { locations, From e39f2f156d4895156c5e3edd002d56bb2c0324c3 Mon Sep 17 00:00:00 2001 From: Rovack <6605826+Rovack@users.noreply.github.com> Date: Sat, 14 Mar 2026 00:21:52 +0200 Subject: [PATCH 04/10] Skip redundant plain-ification for already-raw locations --- src/controllers/locations.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controllers/locations.js b/src/controllers/locations.js index ca9df00..0f2856f 100644 --- a/src/controllers/locations.js +++ b/src/controllers/locations.js @@ -257,7 +257,7 @@ export default { sortBy, }); const plainLocations = await locations - .map(location => location.get({ plain: true })); + .map(location => (location.get ? location.get({ plain: true }) : location)); const paginationCount = Math.ceil(totalNumLocations / pageSize); const formattedLocations = plainLocations.map((location) => { From bd996a66b44f1d9adb08dde9e01cfb2111ca2d21 Mon Sep 17 00:00:00 2001 From: Rovack <6605826+Rovack@users.noreply.github.com> Date: Sat, 14 Mar 2026 00:43:50 +0200 Subject: [PATCH 05/10] Actually return all fetched fields in stubs, not just IDs - Remove attribute filtering from the query for location stubs. - Dedupe in memory, as the query doesn't do DISTINCT anymore. --- src/models/location.js | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/models/location.js b/src/models/location.js index bb58374..9f12aa7 100644 --- a/src/models/location.js +++ b/src/models/location.js @@ -297,10 +297,12 @@ module.exports = (sequelize, DataTypes, Op) => { return sequelize.and(requiredDocumentCondition, notRequiredDocumentCondition); }; - Location.findUniqueLocationStubs = async (filterParameters, + Location.findUniqueLocationStubs = async ( + filterParameters, additionalConditions, originalQueryProps = {}, - selectedAttributeForOrderBy, noServices) => { + noServices, + ) => { const queryProps = { order: originalQueryProps.order }; // eslint-disable-next-line prefer-destructuring const limit = originalQueryProps.limit; @@ -371,11 +373,6 @@ module.exports = (sequelize, DataTypes, Op) => { return Location.findAll({ ...queryProps, where: sequelize.and(..._whereConditions, ...additionalConditions), - attributes: [ - sequelize.fn('DISTINCT', sequelize.col('Location.id')), - // For SELECT DISTINCT, ORDER BY expressions must appear in select list. - ...(selectedAttributeForOrderBy ? [selectedAttributeForOrderBy] : []), - ], raw: true, // Not like associations and grouping work perfectly out of the box either though... // https://github.com/sequelize/sequelize/issues/5481 @@ -490,14 +487,15 @@ module.exports = (sequelize, DataTypes, Op) => { ].reduce((a, b) => a.concat(b)); - // Remove duplicates - locations = Array.from(new Map(searchResults.map(item => [item.id, item])).values()); + locations = searchResults; } else { locations = await findAll(whereConditions); } - // apply limit and offset in memory here - locations = locations.slice(offset || 0, limit ? (offset || 0) + limit : undefined); + // Remove duplicates + locations = Array.from(new Map(locations.map(item => [item.id, item])).values()) + // apply limit and offset in memory here + .slice(offset || 0, limit ? (offset || 0) + limit : undefined); return locations; }; @@ -559,7 +557,6 @@ module.exports = (sequelize, DataTypes, Op) => { limit, offset, }, - selectedAttributeForOrderBy, noServices, ); @@ -575,7 +572,7 @@ module.exports = (sequelize, DataTypes, Op) => { order, limit: minResults, offset, - }, selectedAttributeForOrderBy, noServices); + }, noServices); } } else { totalNumLocations = (await Location.findUniqueLocationStubs(filterParameters, [])).length; @@ -583,7 +580,7 @@ module.exports = (sequelize, DataTypes, Op) => { limit, offset, order, - }, selectedAttributeForOrderBy); + }); } const locationIds = locationStubs.map(location => location.id); From d7aa7f16850007d6c0699c7c0f1e70ba48cd61d4 Mon Sep 17 00:00:00 2001 From: Rovack <6605826+Rovack@users.noreply.github.com> Date: Sat, 14 Mar 2026 00:52:14 +0200 Subject: [PATCH 06/10] Fix remaining pageSize vs. limit discrepancies - Revert to using pageSize as before, but capping both it _and_ maxResults (not limit directly) if asking for location details. --- src/controllers/locations.js | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/controllers/locations.js b/src/controllers/locations.js index 0f2856f..774a115 100644 --- a/src/controllers/locations.js +++ b/src/controllers/locations.js @@ -169,8 +169,15 @@ export default { sortBy, } = req.query; + const capDetailedLocations = (requestedLimit) => { + const maxDetailedLocations = 200; + return locationFieldsOnly + ? requestedLimit + : Math.min(requestedLimit, maxDetailedLocations); + }; + const pageNumber = _pageNumber ? parseInt(_pageNumber, 10) : undefined; - const pageSize = _pageNumber ? parseInt(_pageSize, 10) : undefined; + const pageSize = _pageNumber ? capDetailedLocations(parseInt(_pageSize, 10)) : undefined; const age = _age ? parseInt(_age, 10) : undefined; const ageMin = _ageMin ? parseInt(_ageMin, 10) : undefined; const ageMax = _ageMax ? parseInt(_ageMax, 10) : undefined; @@ -232,15 +239,10 @@ export default { const taxonomyIds = taxonomyId.split(','); filterParameters.taxonomyIds = await models.Taxonomy.getAllIdsWithinTaxonomies(taxonomyIds); } + const limit = pageSize || capDetailedLocations(maxResults); - const requestedLimit = pageSize || maxResults; - const maxDetailedResults = 200; - const limit = locationFieldsOnly - ? requestedLimit - : Math.min(requestedLimit, maxDetailedResults); - - const offset = pageNumber !== undefined && limit !== undefined ? - pageNumber * limit : undefined; + const offset = pageNumber !== undefined && pageSize !== undefined ? + pageNumber * pageSize : undefined; const { locations, From 3b92c5e951db160496bf1672e22a1b7696c30ed8 Mon Sep 17 00:00:00 2001 From: Rovack <6605826+Rovack@users.noreply.github.com> Date: Sat, 14 Mar 2026 00:58:43 +0200 Subject: [PATCH 07/10] Cap minResults too (though highly unlikely to exceed 200) --- src/controllers/locations.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controllers/locations.js b/src/controllers/locations.js index 774a115..bdab72b 100644 --- a/src/controllers/locations.js +++ b/src/controllers/locations.js @@ -250,7 +250,7 @@ export default { } = await models.Location.search({ position: (longitude && latitude) ? geometry.createPoint(longitude, latitude) : null, radius, - minResults, + minResults: capDetailedLocations(minResults), filterParameters, locationFieldsOnly, noServices: parseBoolean(noServices), From 6ffc84ee25df1ab9d45dfc54751ab958a1156004 Mon Sep 17 00:00:00 2001 From: Rovack <6605826+Rovack@users.noreply.github.com> Date: Sat, 14 Mar 2026 01:16:43 +0200 Subject: [PATCH 08/10] Fix sorting by mostServices - Re-add selectedAttributeForOrderBy to findAll, so when it's a computed column it's still selected and therefore exists for WHERE. --- src/models/location.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/models/location.js b/src/models/location.js index 9f12aa7..4c97e48 100644 --- a/src/models/location.js +++ b/src/models/location.js @@ -301,6 +301,7 @@ module.exports = (sequelize, DataTypes, Op) => { filterParameters, additionalConditions, originalQueryProps = {}, + selectedAttributeForOrderBy, noServices, ) => { const queryProps = { order: originalQueryProps.order }; @@ -373,6 +374,9 @@ module.exports = (sequelize, DataTypes, Op) => { return Location.findAll({ ...queryProps, where: sequelize.and(..._whereConditions, ...additionalConditions), + attributes: { + include: selectedAttributeForOrderBy ? [selectedAttributeForOrderBy] : undefined, + }, raw: true, // Not like associations and grouping work perfectly out of the box either though... // https://github.com/sequelize/sequelize/issues/5481 @@ -557,6 +561,7 @@ module.exports = (sequelize, DataTypes, Op) => { limit, offset, }, + selectedAttributeForOrderBy, noServices, ); @@ -572,7 +577,7 @@ module.exports = (sequelize, DataTypes, Op) => { order, limit: minResults, offset, - }, noServices); + }, selectedAttributeForOrderBy, noServices); } } else { totalNumLocations = (await Location.findUniqueLocationStubs(filterParameters, [])).length; @@ -580,7 +585,7 @@ module.exports = (sequelize, DataTypes, Op) => { limit, offset, order, - }); + }, selectedAttributeForOrderBy); } const locationIds = locationStubs.map(location => location.id); From bcb3f77df8cd24304313348a3ae6c2dcf86eec9f Mon Sep 17 00:00:00 2001 From: Rovack <6605826+Rovack@users.noreply.github.com> Date: Sat, 14 Mar 2026 02:28:02 +0200 Subject: [PATCH 09/10] Restore event info to the locationFieldsOnly case - Even when using the `raw: true` query, without associations, use a literal to add a column with the event list from EventRelatedInfos. - Standardize the response structure by returning it as objects resembling the (relevant parts of) EventRelatedInfos. --- src/models/location.js | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/models/location.js b/src/models/location.js index 4c97e48..a77c73f 100644 --- a/src/models/location.js +++ b/src/models/location.js @@ -39,7 +39,6 @@ module.exports = (sequelize, DataTypes, Op) => { }); const SERVICE_COUNT_COLUMN_ALIAS = 'service_count'; - const SERVICE_COUNT_SUBQUERY = [ sequelize.literal(`( SELECT cast(COUNT(*) as integer) @@ -49,6 +48,16 @@ module.exports = (sequelize, DataTypes, Op) => { SERVICE_COUNT_COLUMN_ALIAS, ]; + const EVENTS_WITH_INFO_COLUMN_ALIAS = 'events_with_info'; + const EVENTS_WITH_INFO_COLUMN_SUBQUERY = [ + sequelize.literal(`( + SELECT JSON_AGG(DISTINCT event_related_info.event) + FROM event_related_info + WHERE event_related_info.location_id = "Location".id + )`), + EVENTS_WITH_INFO_COLUMN_ALIAS, + ]; + Location.associate = (models) => { Location.belongsTo(models.Organization, { foreignKey: 'organization_id' }); Location.belongsToMany(models.Service, { @@ -375,7 +384,10 @@ module.exports = (sequelize, DataTypes, Op) => { ...queryProps, where: sequelize.and(..._whereConditions, ...additionalConditions), attributes: { - include: selectedAttributeForOrderBy ? [selectedAttributeForOrderBy] : undefined, + include: [ + EVENTS_WITH_INFO_COLUMN_SUBQUERY, + ...(selectedAttributeForOrderBy ? [selectedAttributeForOrderBy] : []), + ], }, raw: true, // Not like associations and grouping work perfectly out of the box either though... @@ -496,10 +508,15 @@ module.exports = (sequelize, DataTypes, Op) => { locations = await findAll(whereConditions); } + const reconstructEvents = ({ + [EVENTS_WITH_INFO_COLUMN_ALIAS]: events, ...rest + }) => ({ ...rest, EventRelatedInfos: (events || []).map(event => ({ event })) }); + // Remove duplicates locations = Array.from(new Map(locations.map(item => [item.id, item])).values()) // apply limit and offset in memory here - .slice(offset || 0, limit ? (offset || 0) + limit : undefined); + .slice(offset || 0, limit ? (offset || 0) + limit : undefined) + .map(reconstructEvents); return locations; }; From 6bc827a61ebfc576114c6e9a1844f034a31e07a1 Mon Sep 17 00:00:00 2001 From: Rovack <6605826+Rovack@users.noreply.github.com> Date: Sat, 14 Mar 2026 02:41:35 +0200 Subject: [PATCH 10/10] Stop passing the unused Services to isLocationClosed - Remove services from the signature and calls to isLocationClosed. - To still be able to omit it from the returned location objects, configure varsIgnorePattern for eslint. (Doesn't really has to be part of this PR, but both changes are good to have regardless, and should make Codex less confused.) --- .eslintrc.js | 2 +- src/controllers/locations.js | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 7e9ef33..3e4c9bb 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -10,7 +10,7 @@ module.exports = { 'max-len': [2, 100], 'no-console': 2, 'no-multiple-empty-lines': [2, { max: 1, maxBOF: 1, maxEOF: 1 }], - 'no-unused-vars': [2, { args: 'none' }], + 'no-unused-vars': [2, { args: 'none', varsIgnorePattern: '^_' }], 'no-restricted-syntax': ['off', 'ForOfStatement'], 'no-await-in-loop': ['off'], }, diff --git a/src/controllers/locations.js b/src/controllers/locations.js index bdab72b..fd41d2d 100644 --- a/src/controllers/locations.js +++ b/src/controllers/locations.js @@ -15,7 +15,7 @@ import { NotFoundError, ValidationError } from '../utils/errors'; const DEFAULT_MAX_LOCATIONS_RETURNED = 1000; -const isLocationClosed = (occasion, eventRelatedInfos, services) => { +const isLocationClosed = (occasion, eventRelatedInfos) => { if (!occasion) { return false; } @@ -111,7 +111,7 @@ async function handleGetInfoResponse(location, locationWithServices, excludeMeta const { EventRelatedInfos } = location; // FIXME: we should not be hard-coding the COVID19 event here // this is logic that needs ot be revisited in this codebase - const closed = isLocationClosed('COVID19', EventRelatedInfos, services); + const closed = isLocationClosed('COVID19', EventRelatedInfos); if (excludeMetadata) { const [{ lastValidatedDateForLocation }] = await getLastValidatedDateForLocation(location.id); @@ -263,8 +263,8 @@ export default { const paginationCount = Math.ceil(totalNumLocations / pageSize); const formattedLocations = plainLocations.map((location) => { - const { EventRelatedInfos, Services, ...simplifiedLocation } = location; - const closed = isLocationClosed(occasion, EventRelatedInfos, Services); + const { EventRelatedInfos, Services: _, ...simplifiedLocation } = location; + const closed = isLocationClosed(occasion, EventRelatedInfos); if (locationFieldsOnly) { return {