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
Minor update when arrange sorted
  • Loading branch information
manelcecs committed Apr 23, 2024
1 parent 6fb94db commit 44e15c2
Show file tree
Hide file tree
Showing 8 changed files with 123 additions and 97 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 @@ -18,6 +18,7 @@ import {
defaultFlowOrderBy,
intersectUniqueFlowEntities,
mapFlowOrderBy,
mergeUniqueEntities,
prepareFlowConditions,
prepareFlowStatusConditions,
} from './utils';
Expand Down Expand Up @@ -190,11 +191,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 Expand Up @@ -227,12 +229,27 @@ export class SearchFlowByFiltersStrategy implements FlowSearchStrategy {
// We are going to sort the deduplicated flows
// using the sortByFlowIDs if there are any
let sortedFlows: UniqueFlowEntity[] = [];
if (sortByFlowIDs.length > 0) {
// We need to keep only present in the deduplicatedFlows that way the sorted Flows get only the filtered ones but sorted
// There are 3 scenarios:
// 1. While sorting we have more flows 'sorted' than deduplicatedFlows
// That means we can apply the filterSorting pattern to gain a bit of performance
// 2. While sorting we have the same amount or less flows 'sorted' than deduplicatedFlows
// That means we need to keep the sortedFilters and then keep the rest of deduplicatedFlows thar are not in sortedFlows
// If we don't do this it may cause that just changing the orderBy we get different results
// Because we get rid of those flows that are not present in the sortedFlows list
// 3. There are no sortByFlowIDs
// That means we need to keep all the deduplicatedFlows as they come
if (sortByFlowIDs.length > deduplicatedFlows.length) {
sortedFlows = intersectUniqueFlowEntities(
sortByFlowIDs,
deduplicatedFlows
);
} else if (sortByFlowIDs.length <= deduplicatedFlows.length) {
sortedFlows = intersectUniqueFlowEntities(
sortByFlowIDs,
deduplicatedFlows
);

sortedFlows = mergeUniqueEntities(sortedFlows, deduplicatedFlows);
} else {
sortedFlows = deduplicatedFlows;
}
Expand Down
Loading

0 comments on commit 44e15c2

Please sign in to comment.