Skip to content

Commit

Permalink
fix(elasticsearch-plugin): Correctly update index on variant deletion
Browse files Browse the repository at this point in the history 
Fixes #266
  • Loading branch information
@michaelbromley
michaelbromley committed Feb 26, 2020
1 parent 401c236 commit 8b91a59
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 6 deletions.
37 changes: 34 additions & 3 deletions packages/elasticsearch-plugin/e2e/elasticsearch-plugin.e2e-spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ describe('Elasticsearch plugin', () => {
groupByProduct: false,
});

expect(search2.items.map(i => i.sku)).toEqual([
expect(search2.items.map(i => i.sku).sort()).toEqual([
'IHD455T2_updated',
'IHD455T3_updated',
'IHD455T4_updated',
Expand Down Expand Up @@ -469,7 +469,13 @@ describe('Elasticsearch plugin', () => {

it('updates index when a Product is deleted', async () => {
const { search } = await doAdminSearchQuery({ facetValueIds: ['T_2'], groupByProduct: true });
expect(search.items.map(i => i.productId)).toEqual(['T_3', 'T_5', 'T_6', 'T_2', 'T_4']);
expect(search.items.map(i => i.productId).sort()).toEqual([
'T_2',
'T_3',
'T_4',
'T_5',
'T_6',
]);
await adminClient.query<DeleteProduct.Mutation, DeleteProduct.Variables>(DELETE_PRODUCT, {
id: 'T_5',
});
Expand All @@ -478,7 +484,7 @@ describe('Elasticsearch plugin', () => {
facetValueIds: ['T_2'],
groupByProduct: true,
});
expect(search2.items.map(i => i.productId)).toEqual(['T_3', 'T_6', 'T_2', 'T_4']);
expect(search2.items.map(i => i.productId).sort()).toEqual(['T_2', 'T_3', 'T_4', 'T_6']);
});

it('updates index when a Collection is changed', async () => {
Expand Down Expand Up @@ -631,6 +637,31 @@ describe('Elasticsearch plugin', () => {
expect(search2.items[0].productAsset!.focalPoint).toEqual({ x: 0.42, y: 0.42 });
});

it('does not include deleted ProductVariants in index', async () => {
const { search: s1 } = await doAdminSearchQuery({
term: 'hard drive',
groupByProduct: false,
});

const variantToDelete = s1.items.find(i => i.sku === 'IHD455T2_updated')!;

const { deleteProductVariant } = await adminClient.query<
DeleteProductVariant.Mutation,
DeleteProductVariant.Variables
>(DELETE_PRODUCT_VARIANT, { id: variantToDelete.productVariantId });

await awaitRunningJobs(adminClient);

const { search } = await adminClient.query<SearchGetPrices.Query, SearchGetPrices.Variables>(
SEARCH_GET_PRICES,
{ input: { term: 'hard drive', groupByProduct: true } },
);
expect(search.items[0].price).toEqual({
min: 7896,
max: 13435,
});
});

it('returns disabled field when not grouped', async () => {
const result = await doAdminSearchQuery({ groupByProduct: false, term: 'laptop' });
expect(result.search.items.map(pick(['productVariantId', 'enabled']))).toEqual([
Expand Down
23 changes: 20 additions & 3 deletions packages/elasticsearch-plugin/src/indexer.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,13 @@ export class ElasticsearchIndexerController implements OnModuleInit, OnModuleDes
}: DeleteVariantMessage['data']): Observable<DeleteVariantMessage['response']> {
const ctx = RequestContext.fromObject(rawContext);
return asyncObservable(async () => {
const variants = await this.connection
.getRepository(ProductVariant)
.findByIds(variantIds, { relations: ['product'] });
const productIds = unique(variants.map(v => v.product.id));
for (const productId of productIds) {
await this.updateProductInternal(ctx, productId, ctx.channelId);
}
await this.deleteVariantsInternal(variantIds, ctx.channelId);
return true;
});
Expand Down Expand Up @@ -262,7 +269,7 @@ export class ElasticsearchIndexerController implements OnModuleInit, OnModuleDes
}

const qb = this.getSearchIndexQueryBuilder(ctx.channelId);
const count = await qb.andWhere('variants__product.deletedAt IS NULL').getCount();
const count = await qb.getCount();
Logger.verbose(`Reindexing ${count} ProductVariants`, loggerCtx);

const batches = Math.ceil(count / batchSize);
Expand Down Expand Up @@ -403,6 +410,9 @@ export class ElasticsearchIndexerController implements OnModuleInit, OnModuleDes

const productVariants = await this.connection.getRepository(ProductVariant).findByIds(variantIds, {
relations: variantRelations,
where: {
deletedAt: null,
},
order: {
id: 'ASC',
},
Expand Down Expand Up @@ -441,6 +451,9 @@ export class ElasticsearchIndexerController implements OnModuleInit, OnModuleDes
.getRepository(ProductVariant)
.findByIds(product.variants.map(v => v.id), {
relations: variantRelations,
where: {
deletedAt: null,
},
});
if (product.enabled === false) {
updatedProductVariants.forEach(v => (v.enabled = false));
Expand Down Expand Up @@ -526,7 +539,9 @@ export class ElasticsearchIndexerController implements OnModuleInit, OnModuleDes
FindOptionsUtils.joinEagerRelations(qb, qb.alias, this.connection.getMetadata(ProductVariant));
qb.leftJoin('variants.product', '__product')
.leftJoin('__product.channels', '__channel')
.where('__channel.id = :channelId', { channelId });
.where('__channel.id = :channelId', { channelId })
.andWhere('variants__product.deletedAt IS NULL')
.andWhere('variants.deletedAt IS NULL');
return qb;
}

Expand All @@ -538,7 +553,6 @@ export class ElasticsearchIndexerController implements OnModuleInit, OnModuleDes
const { batchSize } = this.options;
const i = Number.parseInt(batchNumber.toString(), 10);
const variants = await qb
.andWhere('variants__product.deletedAt IS NULL')
.take(batchSize)
.skip(i * batchSize)
.addOrderBy('variants.id', 'ASC')
Expand All @@ -550,6 +564,9 @@ export class ElasticsearchIndexerController implements OnModuleInit, OnModuleDes
private async getVariantsByIds(ctx: RequestContext, ids: ID[]) {
const variants = await this.connection.getRepository(ProductVariant).findByIds(ids, {
relations: variantRelations,
where: {
deletedAt: null,
},
order: {
id: 'ASC',
},
Expand Down

0 comments on commit 8b91a59

Please sign in to comment.