Skip to content

Limit IDs passed to location assocations query #189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Rovack wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

Limit IDs passed to location assocations query #189

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
346a456
feature: limit IDs passed to location assocations query
Rovack Mar 13, 2026
bb91275
Actually return the location stubs fetched in the 1st query
Rovack Mar 13, 2026
2d37376
Fix pagination to take the updated limit into account
Rovack Mar 13, 2026
e39f2f1
Skip redundant plain-ification for already-raw locations
Rovack Mar 13, 2026
bd996a6
Actually return all fetched fields in stubs, not just IDs
Rovack Mar 13, 2026
d7aa7f1
Fix remaining pageSize vs. limit discrepancies
Rovack Mar 13, 2026
3b92c5e
Cap minResults too (though highly unlikely to exceed 200)
Rovack Mar 13, 2026
6ffc84e
Fix sorting by mostServices
Rovack Mar 13, 2026
bcb3f77
Restore event info to the locationFieldsOnly case
Rovack Mar 14, 2026
6bc827a
Stop passing the unused Services to isLocationClosed
Rovack Mar 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/controllers/locations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Comment thread
Rovack marked this conversation as resolved.
Outdated
Comment thread
Rovack marked this conversation as resolved.
Outdated
Comment thread
Rovack marked this conversation as resolved.
Outdated

const offset = pageNumber !== undefined && pageSize !== undefined ?
pageNumber * pageSize : undefined;
Expand Down
60 changes: 29 additions & 31 deletions src/models/location.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Comment thread
Rovack marked this conversation as resolved.
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);
Expand Down
Loading