Skip to content

Commit

Permalink
Fix incorrect usage of map for categories shortcut
Browse files Browse the repository at this point in the history
Simplified how category shortcut are map to the id reference
  • Loading branch information
manelcecs committed Apr 22, 2024
1 parent 6fb94db commit 956dda2
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 82 deletions.
1 change: 1 addition & 0 deletions src/domain-services/categories/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,5 @@ export type CategoryGroup =
export type ShortcutCategoryFilter = {
category: string;
operation: typeof Op.IN | typeof Op.NOT_IN;
id: number;
};
17 changes: 9 additions & 8 deletions src/domain-services/flows/flow-search-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,21 +319,22 @@ export class FlowSearchService {
isStandardFlows: boolean
): ShortcutCategoryFilter[] | null {
const filters = [
{ flag: isPendingFlows, category: 'Pending' },
{ flag: isCommitmentFlows, category: 'Commitment' },
{ flag: isPaidFlows, category: 'Paid' },
{ flag: isPledgedFlows, category: 'Pledge' },
{ flag: isCarryoverFlows, category: 'Carryover' },
{ flag: isParkedFlows, category: 'Parked' },
{ flag: isPassThroughFlows, category: 'Pass Through' },
{ flag: isStandardFlows, category: 'Standard' },
{ flag: isPendingFlows, category: 'Pending', id: 45 },
{ flag: isCommitmentFlows, category: 'Commitment', id: 47 },
{ flag: isPaidFlows, category: 'Paid', id: 48 },
{ flag: isPledgedFlows, category: 'Pledge', id: 46 },
{ flag: isCarryoverFlows, category: 'Carryover', id: 137 },
{ flag: isParkedFlows, category: 'Parked', id: 1252 },
{ flag: isPassThroughFlows, category: 'Pass Through', id: 136 },
{ flag: isStandardFlows, category: 'Standard', id: 133 },
];

const shortcutFilters: ShortcutCategoryFilter[] = filters
.filter((filter) => filter.flag !== undefined)
.map((filter) => ({
category: filter.category,
operation: filter.flag ? Op.IN : Op.NOT_IN,
id: filter.id,
}));

return shortcutFilters.length > 0 ? shortcutFilters : null;
Expand Down
23 changes: 5 additions & 18 deletions src/domain-services/flows/flow-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,7 @@ export class FlowService {
async getFlowsAsUniqueFlowEntity(
args: GetFlowsArgs
): Promise<UniqueFlowEntity[]> {
const {
databaseConnection,
orderBy,
conditions,
offset,
limit,
whereClauses,
} = args;
const { databaseConnection, orderBy, conditions, whereClauses } = args;

let query = databaseConnection!
.queryBuilder()
Expand All @@ -59,6 +52,8 @@ export class FlowService {
.whereNull('deletedAt');
if (orderBy.raw) {
query = query.orderByRaw(orderBy.raw);
} else if (orderBy.entity !== 'flow') {
query = query.orderBy('id', orderBy.order);
} else {
query = query.orderBy(orderBy.column, orderBy.order);
}
Expand All @@ -70,17 +65,7 @@ export class FlowService {
query = query.andWhere(prepareCondition(whereClauses));
}

// Because this list can be really large (+330K entries), we need to split it in chunks
// Using limit and offset as cursors
const databaseParsingLimit = 10_000;
if (limit && offset) {
query = query.limit(databaseParsingLimit + offset).offset(offset);
} else if (limit) {
query = query.limit(limit + databaseParsingLimit);
}

const flows = await query;

const mapFlowsToUniqueFlowEntities = flows.map((flow) => ({
id: flow.id,
versionID: flow.versionID,
Expand All @@ -106,6 +91,8 @@ export class FlowService {
.whereNull('deletedAt');
if (orderBy.raw) {
query = query.orderByRaw(orderBy.raw);
} else if (orderBy.entity !== 'flow') {
query = query.orderBy('id', orderBy.order);
} else {
query = query.orderBy(orderBy.column, orderBy.order);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,6 @@ export class GetFlowIdsFromCategoryConditionsStrategyImpl
{
constructor(private readonly categoryService: CategoryService) {}

private readonly categoryIDsMap: Map<string, number> = new Map<
string,
number
>([
['Pending', 45],
['Pledge', 46],
['Commitment', 47],
['Paid', 48],
['Standard', 133],
['Pass through', 136],
['Carryover', 137],
['Parked', 1252],
]);

async search(
args: FlowIdSearchStrategyArgs
): Promise<FlowIdSearchStrategyResponse> {
Expand All @@ -41,19 +27,21 @@ export class GetFlowIdsFromCategoryConditionsStrategyImpl
databaseConnection,
} = args;

const categoriesIds: CategoryId[] = [];

const whereClause = mapFlowCategoryConditionsToWhereClause(
flowCategoryConditions!
);
let categoriesIds: CategoryId[] = [];

let whereClause = null;
if (flowCategoryConditions) {
whereClause = mapFlowCategoryConditionsToWhereClause(
flowCategoryConditions
);
}
if (whereClause) {
const categories = await this.categoryService.findCategories(
models,
whereClause
);

categories.map((category) => category.id);
categoriesIds = categories.map((category) => category.id);
}

// Add category IDs from shortcut filter
Expand All @@ -63,17 +51,14 @@ export class GetFlowIdsFromCategoryConditionsStrategyImpl

if (shortcutFilter) {
for (const shortcut of shortcutFilter) {
const shortcutCategoryID = this.categoryIDsMap.get(shortcut.category);
if (shortcutCategoryID) {
if (shortcut.operation === Op.IN) {
categoriesIdsFromShortcutFilterIN.push(
createBrandedValue(shortcutCategoryID)
);
} else {
categoriesIdsFromShortcutFilterNOTIN.push(
createBrandedValue(shortcutCategoryID)
);
}
if (shortcut.operation === Op.IN) {
categoriesIdsFromShortcutFilterIN.push(
createBrandedValue(shortcut.id)
);
} else {
categoriesIdsFromShortcutFilterNOTIN.push(
createBrandedValue(shortcut.id)
);
}
}
}
Expand All @@ -91,15 +76,15 @@ export class GetFlowIdsFromCategoryConditionsStrategyImpl
});

if (categoriesIds.length > 0) {
joinQuery = joinQuery.orWhere(function () {
joinQuery = joinQuery.andWhere(function () {
this.where('categoryRef.categoryID', 'IN', categoriesIds)
.andWhere('categoryRef.objectType', 'flow')
.andWhere('flow.deletedAt', null);
});
}

if (categoriesIdsFromShortcutFilterIN.length > 0) {
joinQuery = joinQuery.orWhere(function () {
joinQuery = joinQuery.andWhere(function () {
this.where(
'categoryRef.categoryID',
'IN',
Expand All @@ -111,7 +96,7 @@ export class GetFlowIdsFromCategoryConditionsStrategyImpl
}

if (categoriesIdsFromShortcutFilterNOTIN.length > 0) {
joinQuery = joinQuery.orWhere(function () {
joinQuery = joinQuery.andWhere(function () {
this.where(
'categoryRef.categoryID',
'NOT IN',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,27 @@ export class GetFlowIdsFromNestedFlowFiltersStrategyImpl
flowsLegacyId
);

if (flowIDsFromNestedFlowFilters.length === 0) {
return { flows: [] };
}
// Once gathered and disjoined the flowIDs from the nestedFlowFilters
// Look after this uniqueFlows in the flow table
// To verify the flow is not deleted
const getFlowArgs = {
databaseConnection,
orderBy: defaultFlowOrderBy(),
whereClauses: buildSearchFlowsConditions(flowIDsFromNestedFlowFilters),
};
const uniqueFlowsNotDeleted =
await this.flowService.getFlowsAsUniqueFlowEntity(getFlowArgs);
return { flows: uniqueFlowsNotDeleted };
const uniqueFlowEntitiesNotDeleted = [];

// TEMP fix
for (let i = 0; i < flowIDsFromNestedFlowFilters.length; i += 1000) {
const getFlowArgs = {
databaseConnection,
orderBy: defaultFlowOrderBy(),
whereClauses: buildSearchFlowsConditions(
flowIDsFromNestedFlowFilters.slice(i, i + 1000)
),
};
const uniqueFlowsNotDeleted =
await this.flowService.getFlowsAsUniqueFlowEntity(getFlowArgs);
uniqueFlowEntitiesNotDeleted.push(...uniqueFlowsNotDeleted);
}
return { flows: uniqueFlowEntitiesNotDeleted };
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,21 @@ export class GetFlowIdsFromObjectConditionsStrategyImpl
// 3. Once we have the flowIDs
// Search in the Flow table for the flowIDs and versionIDs
// To verify that the flows are not deleted
const uniqueFlowsNotDeleted: UniqueFlowEntity[] = [];
// TEMP fix
for (let i = 0; i < flowsFromFilteredFlowObjects.length; i += 1000) {
const getFlowArgs = {
databaseConnection,
orderBy: defaultFlowOrderBy(),
whereClauses: buildSearchFlowsConditions(
flowsFromFilteredFlowObjects.slice(i, i + 1000)
),
};
const uniqueFlowsNotDeletedSlice =
await this.flowService.getFlowsAsUniqueFlowEntity(getFlowArgs);
uniqueFlowsNotDeleted.push(...uniqueFlowsNotDeletedSlice);
}

const searchFlowArgs = {
databaseConnection,
orderBy: defaultFlowOrderBy(),
whereClauses: buildSearchFlowsConditions(flowsFromFilteredFlowObjects),
};
const uniqueFlowsNotDeleted =
await this.flowService.getFlowsAsUniqueFlowEntity(searchFlowArgs);
return { flows: uniqueFlowsNotDeleted };
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,12 @@ export class SearchFlowByFiltersStrategy implements FlowSearchStrategy {
statusFilter
);

const orderByForFlow = isSortByEntity ? defaultFlowOrderBy() : orderBy;
const flows: UniqueFlowEntity[] =
await this.flowService.getFlowsAsUniqueFlowEntity({
databaseConnection,
conditions: flowConditions,
orderBy,
orderBy: orderByForFlow,
});

// If after this filter we have no flows, we can return an empty array
Expand Down
8 changes: 2 additions & 6 deletions src/domain-services/flows/strategy/impl/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export function mapFlowCategoryConditionsToWhereClause(
return whereClause;
}

return {};
return null;
}

export function mergeFlowIDsFromFilteredFlowObjectsAndFlowCategories(
Expand Down Expand Up @@ -287,11 +287,7 @@ export function buildSearchFlowsConditions(
flowFilters?: SearchFlowsFilters
): any {
const whereClauses = uniqueFlowEntities.map((flow) => ({
[Cond.AND]: [
{ id: flow.id },
{ versionID: flow.versionID },
{ deletedAt: null },
],
[Cond.AND]: [{ id: flow.id }, { versionID: flow.versionID }],
}));

if (flowFilters) {
Expand Down

0 comments on commit 956dda2

Please sign in to comment.