fix(controller): bugfix for controller store loading / loaded states

packages/snap-controller/src/Autocomplete/AutocompleteController.test.ts

@@ -158,20 +158,27 @@ describe('Autocomplete Controller', () => {
 			tracker: new Tracker(globals),
 		});
 
-		controller.init();
+		// calling init to ensure event timings line up for asserting loading and loaded states
+		await controller.init();
 
 		const query = 'wh';
 		controller.urlManager = controller.urlManager.reset().set('query', query);
 		expect(controller.urlManager.state.query).toBe(query);
 
 		(controller.client as MockClient).mockData.updateConfig({ autocomplete: 'autocomplete.query.wh' });
-		await controller.search();
+		const searchPromise = controller.search();
 
-		await waitFor(() => {
-			expect(controller.store.results.length).toBeGreaterThan(0);
-			expect(controller.store.results.length).toBe(acConfig.globals!.pagination!.pageSize);
-			expect(controller.store.terms.length).toBe(acConfig.globals!.suggestions!.count);
-		});
+		expect(controller.store.loaded).toBe(false);
+		expect(controller.store.loading).toBe(true);
+
+		await searchPromise;
+
+		expect(controller.store.loaded).toBe(true);
+		expect(controller.store.loading).toBe(false);
+
+		expect(controller.store.results.length).toBeGreaterThan(0);
+		expect(controller.store.results.length).toBe(acConfig.globals!.pagination!.pageSize);
+		expect(controller.store.terms.length).toBe(acConfig.globals!.suggestions!.count);
 	});
 
 	it('has no results if query is blank', async () => {

packages/snap-controller/src/Autocomplete/AutocompleteController.ts

@@ -6,7 +6,7 @@ import { getSearchParams } from '../utils/getParams';
 import { ControllerTypes } from '../types';
 
 import { AutocompleteStore } from '@searchspring/snap-store-mobx';
-import type { AutocompleteControllerConfig, BeforeSearchObj, AfterSearchObj, AfterStoreObj, ControllerServices, ContextVariables } from '../types';
+import type { AutocompleteControllerConfig, AfterSearchObj, AfterStoreObj, ControllerServices, ContextVariables } from '../types';
 import type { Next } from '@searchspring/snap-event-manager';
 import type { AutocompleteRequestModel } from '@searchspring/snapi-types';
 
@@ -80,31 +80,16 @@ export class AutocompleteController extends AbstractController {
 			key: `ss-controller-${this.config.id}`,
 		});
 
-		// add 'beforeSearch' middleware
-		this.eventManager.on('beforeSearch', async (ac: BeforeSearchObj, next: Next): Promise<void | boolean> => {
-			ac.controller.store.loading = true;
-
-			await next();
-		});
-
 		// add 'afterSearch' middleware
 		this.eventManager.on('afterSearch', async (ac: AfterSearchObj, next: Next): Promise<void | boolean> => {
 			await next();
 
 			// cancel search if no input or query doesn't match current urlState
 			if (ac.response.autocomplete.query != ac.controller.urlManager.state.query) {
-				ac.controller.store.loading = false;
 				return false;
 			}
 		});
 
-		// add 'afterStore' middleware
-		this.eventManager.on('afterStore', async (ac: AfterStoreObj, next: Next): Promise<void | boolean> => {
-			await next();
-
-			ac.controller.store.loading = false;
-		});
-
 		this.eventManager.on('beforeSubmit', async (ac: AfterStoreObj, next: Next): Promise<void | boolean> => {
 			await next();
 
@@ -540,19 +525,25 @@ export class AutocompleteController extends AbstractController {
 	};
 
 	search = async (): Promise<void> => {
-		// if urlManager has no query, there will be no need to get params and no query
-		if (!this.urlManager.state.query) {
-			return;
-		}
+		try {
+			if (!this.initialized) {
+				await this.init();
+			}
 
-		const params = this.params;
+			// if urlManager has no query, there will be no need to get params and no query
+			if (!this.urlManager.state.query) {
+				return;
+			}
 
-		// if params have no query do not search
-		if (!params?.search?.query?.string) {
-			return;
-		}
+			const params = this.params;
+
+			// if params have no query do not search
+			if (!params?.search?.query?.string) {
+				return;
+			}
+
+			this.store.loading = true;
 
-		try {
 			try {
 				await this.eventManager.fire('beforeSearch', {
 					controller: this,
@@ -667,8 +658,9 @@ export class AutocompleteController extends AbstractController {
 					this.log.error(err);
 					this.handleError(err);
 				}
-				this.store.loading = false;
 			}
+		} finally {
+			this.store.loading = false;
 		}
 	};
 }

packages/snap-controller/src/Finder/FinderController.test.ts

@@ -152,6 +152,34 @@ describe('Finder Controller', () => {
 				});
 			});
 
+			it(`sets proper loading states`, async function () {
+				const controller = new FinderController(config, {
+					client: new MockClient(globals, {}),
+					store: new FinderStore(config, services),
+					urlManager,
+					eventManager: new EventManager(),
+					profiler: new Profiler(),
+					logger: new Logger(),
+					tracker: new Tracker(globals),
+				});
+
+				// calling init to ensure event timings line up for asserting loading and loaded states
+				await controller.init();
+
+				expect(controller.store.loaded).toBe(false);
+				expect(controller.store.loading).toBe(false);
+
+				const searchPromise = controller.search();
+
+				expect(controller.store.loaded).toBe(false);
+				expect(controller.store.loading).toBe(true);
+
+				await searchPromise;
+
+				expect(controller.store.loaded).toBe(true);
+				expect(controller.store.loading).toBe(false);
+			});
+
 			it(`sets root URL params`, async () => {
 				config.url = '/search/accessories';
 				const controller = new FinderController(config, {

packages/snap-controller/src/Finder/FinderController.ts

@@ -7,7 +7,7 @@ import { getSearchParams } from '../utils/getParams';
 import { ControllerTypes } from '../types';
 import type { FinderStore } from '@searchspring/snap-store-mobx';
 import type { Next } from '@searchspring/snap-event-manager';
-import type { FinderControllerConfig, BeforeSearchObj, AfterSearchObj, ControllerServices, ContextVariables } from '../types';
+import type { FinderControllerConfig, AfterSearchObj, ControllerServices, ContextVariables } from '../types';
 
 const defaultConfig: FinderControllerConfig = {
 	id: 'finder',
@@ -50,13 +50,7 @@ export class FinderController extends AbstractController {
 			});
 		}
 
-		this.eventManager.on('beforeSearch', async (finder: BeforeSearchObj, next: Next): Promise<void | boolean> => {
-			finder.controller.store.loading = true;
-
-			await next();
-		});
-
-		// TODO: move this to afterStore
+		// TODO: remove this aftersearch when store interface changes
 		this.eventManager.on('afterSearch', async (finder: AfterSearchObj, next: Next): Promise<void | boolean> => {
 			await next();
 
@@ -129,17 +123,18 @@ export class FinderController extends AbstractController {
 	};
 
 	search = async (): Promise<void> => {
-		if (!this.initialized) {
-			await this.init();
-		}
+		try {
+			if (!this.initialized) {
+				await this.init();
+			}
 
-		if (this.store.state.persisted) {
-			return;
-		}
+			if (this.store.state.persisted) {
+				return;
+			}
 
-		const params = this.params;
+			const params = this.params;
 
-		try {
+			this.store.loading = true;
 			try {
 				await this.eventManager.fire('beforeSearch', {
 					controller: this,
@@ -255,8 +250,9 @@ export class FinderController extends AbstractController {
 					this.log.error(err);
 					this.handleError(err);
 				}
-				this.store.loading = false;
 			}
+		} finally {
+			this.store.loading = false;
 		}
 	};
 }

packages/snap-controller/src/Recommendation/RecommendationController.test.ts

@@ -118,6 +118,34 @@ describe('Recommendation Controller', () => {
 		});
 	});
 
+	it(`sets proper loading states`, async function () {
+		const controller = new RecommendationController(recommendConfig, {
+			client: new MockClient(globals, {}),
+			store: new RecommendationStore(recommendConfig, services),
+			urlManager,
+			eventManager: new EventManager(),
+			profiler: new Profiler(),
+			logger: new Logger(),
+			tracker: new Tracker(globals),
+		});
+
+		// calling init to ensure event timings line up for asserting loading and loaded states
+		await controller.init();
+
+		expect(controller.store.loaded).toBe(false);
+		expect(controller.store.loading).toBe(false);
+
+		const searchPromise = controller.search();
+
+		expect(controller.store.loaded).toBe(false);
+		expect(controller.store.loading).toBe(true);
+
+		await searchPromise;
+
+		expect(controller.store.loaded).toBe(true);
+		expect(controller.store.loading).toBe(false);
+	});
+
 	it(`tests searchOnPageShow triggers search on persisted pageshow event `, async function () {
 		const controller = new RecommendationController(recommendConfig, {
 			client: new MockClient(globals, {}),

packages/snap-controller/src/Recommendation/RecommendationController.ts

@@ -9,7 +9,7 @@ import type { ProductViewEvent } from '@searchspring/snap-tracker';
 import type { RecommendationStore } from '@searchspring/snap-store-mobx';
 import type { Next } from '@searchspring/snap-event-manager';
 import type { RecommendRequestModel } from '@searchspring/snap-client';
-import type { RecommendationControllerConfig, BeforeSearchObj, AfterStoreObj, ControllerServices, ContextVariables } from '../types';
+import type { RecommendationControllerConfig, AfterStoreObj, ControllerServices, ContextVariables } from '../types';
 
 type RecommendationTrackMethods = {
 	product: {
@@ -75,13 +75,6 @@ export class RecommendationController extends AbstractController {
 		this.config = deepmerge(defaultConfig, this.config);
 		this.store.setConfig(this.config);
 
-		// add 'beforeSearch' middleware
-		this.eventManager.on('beforeSearch', async (recommend: BeforeSearchObj, next: Next): Promise<void | boolean> => {
-			recommend.controller.store.loading = true;
-
-			await next();
-		});
-
 		// add 'afterStore' middleware
 		this.eventManager.on('afterStore', async (recommend: AfterStoreObj, next: Next): Promise<void | boolean> => {
 			await next();
@@ -98,8 +91,6 @@ export class RecommendationController extends AbstractController {
 					this.track.product.removedFromBundle(item);
 				});
 			});
-
-			recommend.controller.store.loading = false;
 		});
 
 		// attach config plugins and event middleware
@@ -434,13 +425,15 @@ export class RecommendationController extends AbstractController {
 	}
 
 	search = async (): Promise<void> => {
-		if (!this.initialized) {
-			await this.init();
-		}
+		try {
+			if (!this.initialized) {
+				await this.init();
+			}
 
-		const params = this.params;
+			const params = this.params;
+
+			this.store.loading = true;
 
-		try {
 			try {
 				await this.eventManager.fire('beforeSearch', {
 					controller: this,
@@ -549,8 +542,9 @@ export class RecommendationController extends AbstractController {
 					this.log.error(err);
 					this.handleError(err);
 				}
-				this.store.loading = false;
 			}
+		} finally {
+			this.store.loading = false;
 		}
 	};
 }

packages/snap-controller/src/Search/SearchController.test.ts

@@ -49,6 +49,9 @@ describe('Search Controller', () => {
 
 		expect(initfn).not.toHaveBeenCalled();
 		controller.init();
+		expect(controller.store.loaded).toBe(false);
+		expect(controller.store.loading).toBe(false);
+
 		expect(initfn).toHaveBeenCalled();
 
 		expect(controller instanceof SearchController).toBeTruthy();
@@ -63,7 +66,19 @@ describe('Search Controller', () => {
 
 		expect(controller.store.results.length).toBe(0);
 		expect(searchfn).not.toHaveBeenCalled();
-		await controller.search();
+
+		// calling init to ensure event timings line up for asserting loading and loaded states
+		await controller.init();
+
+		const searchPromise = controller.search();
+
+		expect(controller.store.loaded).toBe(false);
+		expect(controller.store.loading).toBe(true);
+
+		await searchPromise;
+		expect(controller.store.loaded).toBe(true);
+		expect(controller.store.loading).toBe(false);
+
 		expect(searchfn).toHaveBeenCalled();
 		expect(controller.store.results.length).toBeGreaterThan(0);