Skip to content

Commit 053bf59

Browse files
committed
Refactor query to use Sequelize model
1 parent e7db0aa commit 053bf59

File tree

2 files changed

+200
-83
lines changed

2 files changed

+200
-83
lines changed

server/graphql/v2/collection/OrderCollection.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ export const GraphQLOrderCollection = new GraphQLObjectType({
1717
createdByUsers: {
1818
type: new GraphQLNonNull(GraphQLAccountCollection),
1919
description:
20-
'The accounts that created the orders in this collection, regardless of pagination. Returns a paginated and searchable collection.',
20+
'The accounts that created the orders in this collection (respecting the `account`, `host`, `hostContext`, `includeChildrenAccounts`, `expectedFundsFilter` and `status` arguments), regardless of pagination. Returns a paginated and searchable collection.',
2121
args: {
2222
...CollectionArgs,
2323
searchTerm: {

server/graphql/v2/query/collection/OrdersCollectionQuery.ts

Lines changed: 199 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ import assert from 'assert';
33
import express from 'express';
44
import { GraphQLBoolean, GraphQLEnumType, GraphQLInt, GraphQLList, GraphQLNonNull, GraphQLString } from 'graphql';
55
import { GraphQLDateTime } from 'graphql-scalars';
6-
import { cloneDeep, compact, isEmpty, isNil, uniq } from 'lodash';
6+
import { compact, isEmpty, isNil, uniq } from 'lodash';
77
import { Includeable, Order, Utils as SequelizeUtils, WhereOptions } from 'sequelize';
88

99
import OrderStatuses from '../../../../constants/order-status';
10-
import { buildSearchConditions, getSearchTermSQLConditions } from '../../../../lib/sql-search';
10+
import { buildSearchConditions } from '../../../../lib/sql-search';
1111
import models, { AccountingCategory, Collective, Op, sequelize } from '../../../../models';
1212
import { checkScope } from '../../../common/scope-check';
1313
import { Forbidden, NotFound, Unauthorized } from '../../../errors';
@@ -38,76 +38,124 @@ import { getDatabaseIdFromTierReference, GraphQLTierReferenceInput } from '../..
3838
import { CollectionArgs, CollectionReturnType } from '../../interface/Collection';
3939
import { UncategorizedValue } from '../../object/AccountingCategory';
4040

41-
type OrderAssociation = 'fromCollective' | 'collective';
42-
43-
// Returns the join condition for association
44-
const getCollectivesJoinCondition = (
41+
/**
42+
* Builds WHERE conditions for Collective filtering
43+
* Works for both direct table queries and association-based joins.
44+
*/
45+
const buildCollectivesConditions = ({
4546
account,
46-
association: OrderAssociation,
47-
includeChildrenAccounts = false,
48-
hostContext?: 'ALL' | 'INTERNAL' | 'HOSTED', // TODO: make this a constant
49-
limitToHostedAccounts?: Collective[],
50-
): WhereOptions => {
51-
const associationFields = { collective: 'CollectiveId', fromCollective: 'FromCollectiveId' };
52-
const field =
53-
// Foreign Key columns should only be used in isolation. When querying for associated data, it is more performant to also query for the associated id
54-
associationFields[association] && !includeChildrenAccounts && !(hostContext && account.hasMoneyManagement)
55-
? associationFields[association]
56-
: `$${association}.id$`;
57-
const limitToHostedAccountsIds = limitToHostedAccounts?.map(a => a.id).filter(id => id !== account.id) || [];
58-
const allTopAccountIds = uniq([account.id, ...limitToHostedAccountsIds]);
59-
let conditions = [{ [field]: allTopAccountIds }];
47+
limitToHostedAccountsIds,
48+
allTopAccountIds,
49+
includeChildrenAccounts,
50+
hostContext,
51+
getField = field => field,
52+
}: {
53+
account: Collective;
54+
limitToHostedAccountsIds: number[];
55+
allTopAccountIds: number[];
56+
includeChildrenAccounts: boolean;
57+
hostContext?: 'ALL' | 'INTERNAL' | 'HOSTED';
58+
getField?: (fieldName: string) => string;
59+
}): WhereOptions => {
60+
let conditions: WhereOptions[] = [{ [getField('id')]: { [Op.in]: allTopAccountIds } }];
6061
let shouldQueryForChildAccounts = includeChildrenAccounts;
6162

6263
if (hostContext && account.hasMoneyManagement) {
63-
// Skip specifically querying for children when using host context unless you specify specific account ids, since all children collectives also have the HostCollectiveId
64+
// Skip specifically querying for children when using host context unless you specify specific account ids
6465
if (!limitToHostedAccountsIds.length) {
6566
shouldQueryForChildAccounts = false;
6667
}
6768

6869
// Hosted accounts are always approved and have a HostCollectiveId
6970
const hostedAccountCondition: WhereOptions = {
70-
[`$${association}.HostCollectiveId$`]: account.id,
71-
[`$${association}.approvedAt$`]: { [Op.not]: null },
71+
[getField('HostCollectiveId')]: account.id,
72+
[getField('approvedAt')]: { [Op.not]: null },
7273
};
7374

7475
// Handle id filtering: either limit to specific hosted accounts, or exclude host accounts
7576
if (limitToHostedAccountsIds.length) {
76-
conditions = [{ ...hostedAccountCondition, [`$${association}.id$`]: { [Op.in]: limitToHostedAccountsIds } }];
77+
conditions = [{ ...hostedAccountCondition, [getField('id')]: { [Op.in]: limitToHostedAccountsIds } }];
7778
} else if (hostContext === 'ALL') {
7879
conditions = [hostedAccountCondition];
7980
} else if (hostContext === 'HOSTED') {
8081
// Exclude the host account and its children
8182
conditions = [
8283
{
8384
...hostedAccountCondition,
84-
[`$${association}.id$`]: { [Op.ne]: account.id },
85-
[`$${association}.ParentCollectiveId$`]: {
86-
[Op.or]: [{ [Op.is]: null }, { [Op.ne]: account.id }],
87-
},
85+
[getField('id')]: { [Op.ne]: account.id },
86+
[getField('ParentCollectiveId')]: { [Op.or]: [{ [Op.is]: null }, { [Op.ne]: account.id }] },
8887
},
8988
];
9089
} else if (hostContext === 'INTERNAL') {
9190
// Only get internal accounts
9291
conditions = [
9392
{
94-
[Op.or]: [{ [`$${association}.id$`]: account.id }, { [`$${association}.ParentCollectiveId$`]: account.id }],
93+
[Op.or]: [{ [getField('id')]: account.id }, { [getField('ParentCollectiveId')]: account.id }],
9594
},
9695
];
9796
}
9897
}
9998

10099
if (shouldQueryForChildAccounts) {
101-
if (limitToHostedAccountsIds.length) {
102-
conditions.push({ [`$${association}.ParentCollectiveId$`]: limitToHostedAccountsIds });
103-
} else {
104-
conditions.push({ [`$${association}.ParentCollectiveId$`]: allTopAccountIds });
105-
}
100+
const parentIds = limitToHostedAccountsIds.length ? limitToHostedAccountsIds : allTopAccountIds;
101+
conditions.push({ [getField('ParentCollectiveId')]: { [Op.in]: parentIds } });
106102
}
107103

108104
return conditions.length === 1 ? conditions[0] : { [Op.or]: conditions };
109105
};
110106

107+
type OrderAssociation = 'fromCollective' | 'collective';
108+
109+
// Returns the join condition for association
110+
const getCollectivesJoinCondition = (
111+
account: Collective,
112+
association: OrderAssociation,
113+
includeChildrenAccounts = false,
114+
hostContext?: 'ALL' | 'INTERNAL' | 'HOSTED',
115+
limitToHostedAccounts?: Collective[],
116+
): WhereOptions => {
117+
const associationFields = { collective: 'CollectiveId', fromCollective: 'FromCollectiveId' };
118+
const limitToHostedAccountsIds = limitToHostedAccounts?.map(a => a.id).filter(id => id !== account.id) || [];
119+
const allTopAccountIds = uniq([account.id, ...limitToHostedAccountsIds]);
120+
121+
// Use direct FK column when possible for better performance
122+
const canUseDirectFK = !includeChildrenAccounts && !(hostContext && account.hasMoneyManagement);
123+
if (canUseDirectFK && associationFields[association]) {
124+
return { [associationFields[association]]: allTopAccountIds };
125+
}
126+
const associationFieldAccessor =
127+
(association: OrderAssociation): FieldAccessor =>
128+
field =>
129+
`$${association}.${field}$`;
130+
131+
return buildAccountConditions({
132+
account,
133+
limitToHostedAccountsIds,
134+
allTopAccountIds,
135+
includeChildrenAccounts,
136+
hostContext,
137+
getField: associationFieldAccessor(association),
138+
});
139+
};
140+
141+
const getCollectivesCondition = (
142+
account: Collective,
143+
includeChildrenAccounts = false,
144+
hostContext?: 'ALL' | 'INTERNAL' | 'HOSTED',
145+
limitToHostedAccounts?: Collective[],
146+
): WhereOptions => {
147+
const limitToHostedAccountsIds = limitToHostedAccounts?.map(a => a.id).filter(id => id !== account.id) || [];
148+
const allTopAccountIds = uniq([account.id, ...limitToHostedAccountsIds]);
149+
150+
return buildCollectivesConditions({
151+
account,
152+
limitToHostedAccountsIds,
153+
allTopAccountIds,
154+
includeChildrenAccounts,
155+
hostContext,
156+
});
157+
};
158+
111159
export const OrdersCollectionArgs = {
112160
limit: { ...CollectionArgs.limit, defaultValue: 100 },
113161
offset: CollectionArgs.offset,
@@ -588,9 +636,6 @@ export const OrdersCollectionResolver = async (args, req: express.Request) => {
588636
where['status'] = { ...where['status'], [Op.ne]: OrderStatuses.PENDING };
589637
}
590638

591-
// Store the current where as it will be used to fetch createdByUsers (before applying createdBy filter)
592-
const baseWhere = cloneDeep(where);
593-
594639
if (!isEmpty(args.createdBy)) {
595640
assert(args.createdBy.length <= 1000, '"Created by" is limited to 1000 users');
596641
const createdByAccounts = await fetchAccountsWithReferences(args.createdBy, fetchAccountParams);
@@ -619,53 +664,125 @@ export const OrdersCollectionResolver = async (args, req: express.Request) => {
619664
totalCount: () => models.Order.count({ include, where }),
620665
limit: args.limit,
621666
offset: args.offset,
622-
createdByUsers: async (args: { limit?: number; offset?: number; searchTerm?: string } = {}) => {
623-
const { limit = 10, offset = 0, searchTerm } = args;
624-
625-
const searchConditions = getSearchTermSQLConditions(searchTerm, 'c');
626-
627-
const orderWhereClause = sequelize.queryInterface.queryGenerator
628-
.getWhereConditions(baseWhere, 'o', models.Order)
629-
.replace(/"Order"\./g, 'o.');
630-
631-
const result = await sequelize.query<Collective & { __total__: string }>(
632-
`
633-
SELECT
634-
c.*,
635-
COUNT(*) OVER() AS __total__
636-
FROM "Collectives" c
637-
WHERE c."id" IN (
638-
SELECT DISTINCT u."CollectiveId"
639-
FROM "Orders" o
640-
INNER JOIN "Collectives" "fromCollective" ON o."FromCollectiveId" = "fromCollective".id
641-
INNER JOIN "Collectives" "collective" ON o."CollectiveId" = "collective".id
642-
LEFT JOIN "Subscriptions" "Subscription" ON o."SubscriptionId" = "Subscription".id
643-
INNER JOIN "Users" u ON o."CreatedByUserId" = u.id
644-
WHERE o."deletedAt" IS NULL
645-
AND o."CreatedByUserId" IS NOT NULL
646-
AND u."CollectiveId" IS NOT NULL
647-
${orderWhereClause ? `AND ${orderWhereClause}` : ''}
648-
)
649-
AND c."deletedAt" IS NULL
650-
${searchConditions.sqlConditions}
651-
ORDER BY c."name" ASC
652-
OFFSET :offset
653-
LIMIT :limit
654-
`,
655-
{
656-
model: models.Collective,
657-
mapToModel: true,
658-
replacements: {
659-
sanitizedTerm: searchConditions.sanitizedTerm,
660-
sanitizedTermNoWhitespaces: searchConditions.sanitizedTermNoWhitespaces,
661-
offset,
662-
limit,
667+
createdByUsers: async (subArgs: { limit?: number; offset?: number; searchTerm?: string } = {}) => {
668+
const { limit = 10, offset = 0, searchTerm } = subArgs;
669+
670+
const searchConditions = buildSearchConditions(searchTerm, {
671+
slugFields: ['slug'],
672+
textFields: ['name'],
673+
});
674+
675+
const ordersInclude: Includeable[] = [];
676+
677+
const fromCollectiveConditions: WhereOptions[] = [];
678+
const collectiveConditions: WhereOptions[] = [];
679+
680+
if (account) {
681+
const accountConditions = getCollectivesCondition(
682+
account,
683+
args.includeChildrenAccounts,
684+
args.hostContext,
685+
args.hostedAccounts,
686+
);
687+
688+
if (!args.filter || args.filter === 'OUTGOING') {
689+
fromCollectiveConditions.push(accountConditions);
690+
}
691+
692+
if (!args.filter || args.filter === 'INCOMING') {
693+
collectiveConditions.push(accountConditions);
694+
}
695+
}
696+
697+
if (host) {
698+
collectiveConditions.push({
699+
HostCollectiveId: host.id,
700+
approvedAt: { [Op.not]: null },
701+
});
702+
}
703+
704+
if (fromCollectiveConditions.length) {
705+
ordersInclude.push({
706+
association: 'fromCollective',
707+
required: true,
708+
attributes: [],
709+
where: {
710+
[Op.and]:
711+
fromCollectiveConditions.length === 1 ? fromCollectiveConditions : { [Op.or]: fromCollectiveConditions },
663712
},
664-
},
665-
);
713+
});
714+
}
715+
if (collectiveConditions.length) {
716+
ordersInclude.push({
717+
association: 'collective',
718+
required: true,
719+
attributes: [],
720+
where: {
721+
[Op.and]: collectiveConditions.length === 1 ? collectiveConditions : { [Op.or]: collectiveConditions },
722+
},
723+
});
724+
}
725+
726+
const ordersWhere: WhereOptions = {};
666727

667-
const totalCount = parseInt(result[0]?.dataValues?.__total__ || '0', 10);
668-
return { nodes: result, totalCount, limit, offset };
728+
if (args.expectedFundsFilter) {
729+
if (args.expectedFundsFilter === 'ONLY_MANUAL') {
730+
ordersWhere['data.isManualContribution'] = 'true';
731+
} else if (args.expectedFundsFilter === 'ONLY_PENDING') {
732+
ordersWhere['data.isPendingContribution'] = 'true';
733+
} else {
734+
Object.assign(ordersWhere, {
735+
[Op.or]: {
736+
'data.isPendingContribution': 'true',
737+
'data.isManualContribution': 'true',
738+
},
739+
});
740+
}
741+
}
742+
if (args.status && args.status.length > 0) {
743+
ordersWhere['status'] = { [Op.in]: args.status };
744+
}
745+
746+
const queryOptions = {
747+
where: {
748+
deletedAt: null,
749+
...(searchConditions.length ? { [Op.or]: searchConditions } : {}),
750+
},
751+
distinct: true,
752+
include: [
753+
{
754+
association: 'user',
755+
required: true,
756+
attributes: [],
757+
include: [
758+
{
759+
association: 'orders',
760+
required: true,
761+
attributes: [],
762+
where: ordersWhere,
763+
include: ordersInclude,
764+
},
765+
],
766+
},
767+
],
768+
};
769+
770+
return {
771+
nodes: models.Collective.findAll({
772+
...queryOptions,
773+
order: [['name', 'ASC']],
774+
offset,
775+
limit,
776+
subQuery: false,
777+
}),
778+
totalCount: models.Collective.count({
779+
...queryOptions,
780+
distinct: true,
781+
col: 'id',
782+
}),
783+
limit,
784+
offset,
785+
};
669786
},
670787
};
671788
};

0 commit comments

Comments
 (0)