Skip to content

Commit f9244e9

Browse files
committed
fix: add safer default order
1 parent a304473 commit f9244e9

File tree

2 files changed

+18
-9
lines changed

2 files changed

+18
-9
lines changed

services/libs/data-access-layer/src/members/base.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,6 @@ export async function queryMembersAdvanced(
184184
{ pgPromiseFormat: true },
185185
)
186186

187-
// Build queries
188187
const countQuery = buildCountQuery({
189188
withAggregates,
190189
searchConfig,
@@ -212,6 +211,9 @@ export async function queryMembersAdvanced(
212211
return { alias: f, ...mappedField }
213212
})
214213
.filter((mappedField) => mappedField.queryable !== false)
214+
// Exclude fields from SELECT if their source table isn't joined:
215+
// - skip msa.* when aggregates aren't included (no join with memberSegmentsAgg)
216+
// - skip mo.* when member organizations aren't included (no join with member_orgs)
215217
.filter((mappedField) => {
216218
if (!withAggregates && mappedField.name.includes('msa.')) return false
217219
if (!include.memberOrganizations && mappedField.name.includes('mo.')) return false
@@ -233,7 +235,6 @@ export async function queryMembersAdvanced(
233235

234236
log.info(`main query: ${mainQuery} with params ${JSON.stringify(params)}`)
235237

236-
// Execute queries in parallel
237238
const [rows, countResult] = await Promise.all([
238239
qx.select(mainQuery, params),
239240
qx.selectOne(countQuery, params),
@@ -253,7 +254,6 @@ export async function queryMembersAdvanced(
253254
include.maintainers ? findMaintainerRoles(qx, memberIds) : Promise.resolve([]),
254255
])
255256

256-
// Second parallel batch - fetch related data
257257
const [orgExtra, segmentsInfo, maintainerSegmentsInfo] = await Promise.all([
258258
include.memberOrganizations
259259
? fetchOrganizationData(qx, memberOrganizations)
@@ -264,8 +264,6 @@ export async function queryMembersAdvanced(
264264
: Promise.resolve([]),
265265
])
266266

267-
// Data processing section
268-
269267
if (include.memberOrganizations) {
270268
const { orgs = [], lfx = [] } = orgExtra
271269

@@ -350,7 +348,6 @@ export async function queryMembersAdvanced(
350348
return { rows, count, limit, offset }
351349
}
352350

353-
// ...existing code... (resto delle funzioni rimangono uguali)
354351
export async function queryMembers<T extends MemberField>(
355352
qx: QueryExecutor,
356353
opts: QueryOptions<T>,

services/libs/data-access-layer/src/members/queryBuilder.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,13 +103,27 @@ const getOrderClause = (
103103
direction: OrderDirection,
104104
withAggregates: boolean,
105105
): string => {
106+
// Default sort:
107+
// - when aggregates are included → sort by activityCount (from msa)
108+
// - otherwise → sort by joinedAt (from members)
106109
const defaultOrder = withAggregates ? 'msa."activityCount" DESC' : 'm."joinedAt" DESC'
107110

111+
// If no specific order field is provided, use the default one
108112
if (!parsedField) return defaultOrder
109113

110114
const fieldExpr = ORDER_FIELD_MAP[parsedField]
115+
116+
// If the requested field is not mapped, fall back to default order
111117
if (!fieldExpr) return defaultOrder
112118

119+
// Safety check:
120+
// If the order field refers to msa.* but aggregates are not included,
121+
// fallback to the default safe order instead of generating invalid SQL.
122+
if (!withAggregates && fieldExpr.includes('msa.')) {
123+
return defaultOrder
124+
}
125+
126+
// Return the valid ORDER BY clause
113127
return `${fieldExpr} ${direction}`
114128
}
115129

@@ -128,6 +142,7 @@ export const buildQuery = ({
128142
const { field: sortField, direction } = parseOrderBy(orderBy, fallbackDir)
129143

130144
// Detect if filters reference extra aliases.
145+
// TODO: capire che cosa sono questi mo. me.
131146
const filterHasMo = filterString.includes('mo.')
132147
const filterHasMe = filterString.includes('me.')
133148

@@ -145,9 +160,6 @@ export const buildQuery = ({
145160
if (useActivityCountOptimized) {
146161
const ctes: string[] = []
147162

148-
// For optimized path:
149-
// - We MAY include member_orgs CTE only if includeMemberOrgs is true.
150-
// - But filterString is guaranteed not to reference mo/me here.
151163
if (includeMemberOrgs) {
152164
const memberOrgsCTE = buildMemberOrgsCTE(true)
153165
ctes.push(memberOrgsCTE.trim())

0 commit comments

Comments
 (0)