Skip to content

Commit

Permalink
Refactor filter (search query) event handling
Browse files Browse the repository at this point in the history
Refactor filter event handling to a unified event with visitor pattern
to simplify the code, avoid future bugs and provide better test
coverage.

This commit shifts from using separate `filtered` and `filterRemoved`
events to a singular, more expressive `filterChanged` event. The new
approach emits a detailed payload that explicitly indicates the filter
action and the associated filter data. The event object unifies the way
the presentation layer reacts to the events.

Benefits with this approach include:

- Simplifying event listeners by reducing the number of events to
  handle.
- Increasing code clarity and reduces potential for oversight by
  providing explicit action details in the event payload.
- Offering extensibility for future actions without introducing new
  events.
- Providing visitor pattern to handle different kind of events in easy
  and robust manner without code repetition.

Other changes:

- Refactor components handling of events to follow DRY and KISS
  principles better.
- Refactor `UserFilter.spec.ts` to:
  - Make it easier to add new tests.
  - Increase code coverage by running all event-based tests on the
    current property.
  • Loading branch information
undergroundwires committed Aug 16, 2023
1 parent ae75059 commit 6a20d80
Show file tree
Hide file tree
Showing 17 changed files with 488 additions and 229 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export enum FilterActionType {
Apply,
Clear,
}
37 changes: 37 additions & 0 deletions src/application/Context/State/Filter/Event/FilterChange.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { IFilterResult } from '@/application/Context/State/Filter/IFilterResult';
import { FilterActionType } from './FilterActionType';
import { IFilterChangeDetails, IFilterChangeDetailsVisitor } from './IFilterChangeDetails';

export class FilterChange implements IFilterChangeDetails {
public static forApply(filter: IFilterResult) {
if (!filter) {
throw new Error('missing filter');
}
return new FilterChange(FilterActionType.Apply, filter);
}

public static forClear() {
return new FilterChange(FilterActionType.Clear);
}

private constructor(
public readonly actionType: FilterActionType,
public readonly filter?: IFilterResult,
) { }

public visit(visitor: IFilterChangeDetailsVisitor): void {
if (!visitor) {
throw new Error('missing visitor');
}
switch (this.actionType) {
case FilterActionType.Apply:
visitor.onApply(this.filter);
break;
case FilterActionType.Clear:
visitor.onClear();
break;
default:
throw new Error(`Unknown action type: ${this.actionType}`);
}
}
}
14 changes: 14 additions & 0 deletions src/application/Context/State/Filter/Event/IFilterChangeDetails.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { IFilterResult } from '@/application/Context/State/Filter/IFilterResult';
import { FilterActionType } from './FilterActionType';

export interface IFilterChangeDetails {
readonly actionType: FilterActionType;
readonly filter?: IFilterResult;

visit(visitor: IFilterChangeDetailsVisitor): void;
}

export interface IFilterChangeDetailsVisitor {
onClear(): void;
onApply(filter: IFilterResult): void;
}
8 changes: 4 additions & 4 deletions src/application/Context/State/Filter/IUserFilter.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { IEventSource } from '@/infrastructure/Events/IEventSource';
import { IFilterResult } from './IFilterResult';
import { IFilterChangeDetails } from './Event/IFilterChangeDetails';

export interface IReadOnlyUserFilter {
readonly currentFilter: IFilterResult | undefined;
readonly filtered: IEventSource<IFilterResult>;
readonly filterRemoved: IEventSource<void>;
readonly filterChanged: IEventSource<IFilterChangeDetails>;
}

export interface IUserFilter extends IReadOnlyUserFilter {
setFilter(filter: string): void;
removeFilter(): void;
applyFilter(filter: string): void;
clearFilter(): void;
}
16 changes: 8 additions & 8 deletions src/application/Context/State/Filter/UserFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,21 @@ import { ICategoryCollection } from '@/domain/ICategoryCollection';
import { FilterResult } from './FilterResult';
import { IFilterResult } from './IFilterResult';
import { IUserFilter } from './IUserFilter';
import { IFilterChangeDetails } from './Event/IFilterChangeDetails';
import { FilterChange } from './Event/FilterChange';

export class UserFilter implements IUserFilter {
public readonly filtered = new EventSource<IFilterResult>();

public readonly filterRemoved = new EventSource<void>();
public readonly filterChanged = new EventSource<IFilterChangeDetails>();

public currentFilter: IFilterResult | undefined;

constructor(private collection: ICategoryCollection) {

}

public setFilter(filter: string): void {
public applyFilter(filter: string): void {
if (!filter) {
throw new Error('Filter must be defined and not empty. Use removeFilter() to remove the filter');
throw new Error('Filter must be defined and not empty. Use clearFilter() to remove the filter');
}
const filterLowercase = filter.toLocaleLowerCase();
const filteredScripts = this.collection.getAllScripts().filter(
Expand All @@ -33,12 +33,12 @@ export class UserFilter implements IUserFilter {
filter,
);
this.currentFilter = matches;
this.filtered.notify(matches);
this.filterChanged.notify(FilterChange.forApply(this.currentFilter));
}

public removeFilter(): void {
public clearFilter(): void {
this.currentFilter = undefined;
this.filterRemoved.notify();
this.filterChanged.notify(FilterChange.forClear());
}
}

Expand Down
20 changes: 11 additions & 9 deletions src/presentation/components/Scripts/Menu/TheScriptsMenu.vue
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
defineComponent, ref, onUnmounted, inject,
} from 'vue';
import { useCollectionStateKey } from '@/presentation/injectionSymbols';
import { IReadOnlyCategoryCollectionState } from '@/application/Context/State/ICategoryCollectionState';
import { IReadOnlyUserFilter } from '@/application/Context/State/Filter/IUserFilter';
import TheOsChanger from './TheOsChanger.vue';
import TheSelector from './Selector/TheSelector.vue';
import TheViewChanger from './View/TheViewChanger.vue';
Expand All @@ -31,20 +31,22 @@ export default defineComponent({
const isSearching = ref(false);
onStateChange((state) => {
subscribe(state);
subscribeToFilterChanges(state.filter);
}, { immediate: true });
onUnmounted(() => {
unsubscribeAll();
});
function subscribe(state: IReadOnlyCategoryCollectionState) {
events.register(state.filter.filterRemoved.on(() => {
isSearching.value = false;
}));
events.register(state.filter.filtered.on(() => {
isSearching.value = true;
}));
function subscribeToFilterChanges(filter: IReadOnlyUserFilter) {
events.register(
filter.filterChanged.on((event) => {
event.visit({
onApply: () => { isSearching.value = true; },
onClear: () => { isSearching.value = false; },
});
}),
);
}
function unsubscribeAll() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
import { useCollectionStateKey } from '@/presentation/injectionSymbols';
import { IScript } from '@/domain/IScript';
import { ICategory } from '@/domain/ICategory';
import { ICategoryCollectionState } from '@/application/Context/State/ICategoryCollectionState';
import { ICategoryCollectionState, IReadOnlyCategoryCollectionState } from '@/application/Context/State/ICategoryCollectionState';
import { IFilterResult } from '@/application/Context/State/Filter/IFilterResult';
import { SelectedScript } from '@/application/Context/State/Selection/SelectedScript';
import {
Expand Down Expand Up @@ -64,9 +64,7 @@ export default defineComponent({
nodes.value = parseAllCategories(state.collection);
}
events.unsubscribeAll();
modifyCurrentState((mutableState) => {
registerStateMutators(mutableState);
});
subscribeToState(state);
}, { immediate: true });
function toggleNodeSelection(event: INodeSelectedEvent) {
Expand Down Expand Up @@ -99,36 +97,33 @@ export default defineComponent({
.map((selected) => getScriptNodeId(selected.script));
}
function registerStateMutators(state: ICategoryCollectionState) {
function subscribeToState(state: IReadOnlyCategoryCollectionState) {
events.register(
state.selection.changed.on((scripts) => handleSelectionChanged(scripts)),
state.filter.filterRemoved.on(() => handleFilterRemoved()),
state.filter.filtered.on((filterResult) => handleFiltered(filterResult)),
state.filter.filterChanged.on((event) => {
event.visit({
onApply: (filter) => {
filterText.value = filter.query;
filtered = filter;
},
onClear: () => {
filterText.value = '';
},
});
}),
);
}
function setCurrentFilter(currentFilter: IFilterResult | undefined) {
if (!currentFilter) {
handleFilterRemoved();
} else {
handleFiltered(currentFilter);
}
filtered = currentFilter;
filterText.value = currentFilter?.query || '';
}
function handleSelectionChanged(selectedScripts: ReadonlyArray<SelectedScript>): void {
selectedNodeIds.value = selectedScripts
.map((node) => node.id);
}
function handleFilterRemoved() {
filterText.value = '';
}
function handleFiltered(result: IFilterResult) {
filterText.value = result.query;
filtered = result;
}
return {
nodes,
selectedNodeIds,
Expand Down
28 changes: 15 additions & 13 deletions src/presentation/components/Scripts/View/TheScriptsView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,8 @@ import { useApplicationKey, useCollectionStateKey } from '@/presentation/injecti
import ScriptsTree from '@/presentation/components/Scripts/View/ScriptsTree/ScriptsTree.vue';
import CardList from '@/presentation/components/Scripts/View/Cards/CardList.vue';
import { ViewType } from '@/presentation/components/Scripts/Menu/View/ViewType';
import { IFilterResult } from '@/application/Context/State/Filter/IFilterResult';
import { IReadOnlyCategoryCollectionState } from '@/application/Context/State/ICategoryCollectionState';
import { IReadOnlyUserFilter } from '@/application/Context/State/Filter/IUserFilter';
/** Shows content of single category or many categories */
export default defineComponent({
components: {
ScriptsTree,
Expand Down Expand Up @@ -74,25 +72,29 @@ export default defineComponent({
onStateChange((newState) => {
events.unsubscribeAll();
subscribeState(newState);
subscribeToFilterChanges(newState.filter);
});
function clearSearchQuery() {
modifyCurrentState((state) => {
const { filter } = state;
filter.removeFilter();
filter.clearFilter();
});
}
function subscribeState(state: IReadOnlyCategoryCollectionState) {
function subscribeToFilterChanges(filter: IReadOnlyUserFilter) {
events.register(
state.filter.filterRemoved.on(() => {
isSearching.value = false;
}),
state.filter.filtered.on((result: IFilterResult) => {
searchQuery.value = result.query;
isSearching.value = true;
searchHasMatches.value = result.hasAnyMatches();
filter.filterChanged.on((event) => {
event.visit({
onApply: (newFilter) => {
searchQuery.value = newFilter.query;
isSearching.value = true;
searchHasMatches.value = newFilter.hasAnyMatches();
},
onClear: () => {
isSearching.value = false;
},
});
}),
);
}
Expand Down
35 changes: 17 additions & 18 deletions src/presentation/components/TheSearchBar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import { useCollectionStateKey } from '@/presentation/injectionSymbols';
import { NonCollapsing } from '@/presentation/components/Scripts/View/Cards/NonCollapsingDirective';
import { IReadOnlyUserFilter } from '@/application/Context/State/Filter/IUserFilter';
import { IFilterResult } from '@/application/Context/State/Filter/IFilterResult';
import { IReadOnlyCategoryCollectionState } from '@/application/Context/State/ICategoryCollectionState';
export default defineComponent({
directives: {
Expand All @@ -44,38 +43,38 @@ export default defineComponent({
modifyCurrentState((state) => {
const { filter } = state;
if (!newFilter) {
filter.removeFilter();
filter.clearFilter();
} else {
filter.setFilter(newFilter);
filter.applyFilter(newFilter);
}
});
}
onStateChange((newState) => {
events.unsubscribeAll();
subscribeSearchQuery(newState);
updateFromInitialFilter(newState.filter.currentFilter);
subscribeToFilterChanges(newState.filter);
}, { immediate: true });
function subscribeSearchQuery(newState: IReadOnlyCategoryCollectionState) {
searchQuery.value = newState.filter.currentFilter ? newState.filter.currentFilter.query : '';
subscribeFilter(newState.filter);
function updateFromInitialFilter(filter?: IFilterResult) {
searchQuery.value = filter?.query || '';
}
function subscribeFilter(filter: IReadOnlyUserFilter) {
function subscribeToFilterChanges(filter: IReadOnlyUserFilter) {
events.register(
filter.filtered.on((result) => handleFiltered(result)),
filter.filterRemoved.on(() => handleFilterRemoved()),
filter.filterChanged.on((event) => {
event.visit({
onApply: (result) => {
searchQuery.value = result.query;
},
onClear: () => {
searchQuery.value = '';
},
});
}),
);
}
function handleFilterRemoved() {
searchQuery.value = '';
}
function handleFiltered(result: IFilterResult) {
searchQuery.value = result.query;
}
return {
searchPlaceholder,
searchQuery,
Expand Down
8 changes: 4 additions & 4 deletions tests/unit/application/Context/ApplicationContext.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('ApplicationContext', () => {
const sut = testContext
.withInitialOs(OperatingSystem.Windows)
.construct();
sut.state.filter.setFilter('filtered');
sut.state.filter.applyFilter('filtered');
sut.changeContext(OperatingSystem.macOS);
// assert
expectEmptyState(sut.state);
Expand All @@ -65,10 +65,10 @@ describe('ApplicationContext', () => {
.withInitialOs(os)
.construct();
const firstState = sut.state;
firstState.filter.setFilter(expectedFilter);
firstState.filter.applyFilter(expectedFilter);
sut.changeContext(os);
sut.changeContext(changedOs);
sut.state.filter.setFilter('second-state');
sut.state.filter.applyFilter('second-state');
sut.changeContext(os);
// assert
const actualFilter = sut.state.filter.currentFilter.query;
Expand Down Expand Up @@ -103,7 +103,7 @@ describe('ApplicationContext', () => {
.withInitialOs(os)
.construct();
const initialState = sut.state;
initialState.filter.setFilter('dirty-state');
initialState.filter.applyFilter('dirty-state');
sut.changeContext(os);
// assert
expect(testContext.firedEvents.length).to.equal(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,11 @@ describe('CategoryCollectionState', () => {
.withAction(new CategoryStub(0).withScript(expectedScript));
const sut = new CategoryCollectionState(collection);
// act
let actualScript: IScript;
sut.filter.filtered.on((result) => {
[actualScript] = result.scriptMatches;
let actualScript: IScript | undefined;
sut.filter.filterChanged.on((result) => {
[actualScript] = result.filter?.scriptMatches ?? [undefined];
});
sut.filter.setFilter(scriptNameFilter);
sut.filter.applyFilter(scriptNameFilter);
// assert
expect(expectedScript).to.equal(actualScript);
});
Expand Down
Loading

0 comments on commit 6a20d80

Please sign in to comment.