Refactor filter (search query) event handling
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.
This commit is contained in:
@@ -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);
|
||||
@@ -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;
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
import 'mocha';
|
||||
import { expect } from 'chai';
|
||||
import { FilterChange } from '@/application/Context/State/Filter/Event/FilterChange';
|
||||
import { itEachAbsentObjectValue } from '@tests/unit/shared/TestCases/AbsentTests';
|
||||
import { FilterResultStub } from '@tests/unit/shared/Stubs/FilterResultStub';
|
||||
import { FilterActionType } from '@/application/Context/State/Filter/Event/FilterActionType';
|
||||
import { FilterChangeDetailsVisitorStub } from '@tests/unit/shared/Stubs/FilterChangeDetailsVisitorStub';
|
||||
|
||||
describe('FilterChange', () => {
|
||||
describe('forApply', () => {
|
||||
describe('throws when filter is absent', () => {
|
||||
itEachAbsentObjectValue((absentValue) => {
|
||||
// arrange
|
||||
const expectedError = 'missing filter';
|
||||
const filterValue = absentValue;
|
||||
// act
|
||||
const act = () => FilterChange.forApply(filterValue);
|
||||
// assert
|
||||
expect(act).to.throw(expectedError);
|
||||
});
|
||||
});
|
||||
it('sets filter result', () => {
|
||||
// arrange
|
||||
const expectedFilter = new FilterResultStub();
|
||||
// act
|
||||
const sut = FilterChange.forApply(expectedFilter);
|
||||
// assert
|
||||
const actualFilter = sut.filter;
|
||||
expect(actualFilter).to.equal(expectedFilter);
|
||||
});
|
||||
it('sets action as expected', () => {
|
||||
// arrange
|
||||
const expectedAction = FilterActionType.Apply;
|
||||
// act
|
||||
const sut = FilterChange.forApply(new FilterResultStub());
|
||||
// assert
|
||||
const actualAction = sut.actionType;
|
||||
expect(actualAction).to.equal(expectedAction);
|
||||
});
|
||||
});
|
||||
describe('forClear', () => {
|
||||
it('does not set filter result', () => {
|
||||
// arrange
|
||||
const expectedFilter = undefined;
|
||||
// act
|
||||
const sut = FilterChange.forClear();
|
||||
// assert
|
||||
const actualFilter = sut.filter;
|
||||
expect(actualFilter).to.equal(expectedFilter);
|
||||
});
|
||||
it('sets action as expected', () => {
|
||||
// arrange
|
||||
const expectedAction = FilterActionType.Clear;
|
||||
// act
|
||||
const sut = FilterChange.forClear();
|
||||
// assert
|
||||
const actualAction = sut.actionType;
|
||||
expect(actualAction).to.equal(expectedAction);
|
||||
});
|
||||
});
|
||||
describe('visit', () => {
|
||||
describe('throws when visitor is absent', () => {
|
||||
itEachAbsentObjectValue((absentValue) => {
|
||||
// arrange
|
||||
const expectedError = 'missing visitor';
|
||||
const visitorValue = absentValue;
|
||||
const sut = FilterChange.forClear();
|
||||
// act
|
||||
const act = () => sut.visit(visitorValue);
|
||||
// assert
|
||||
expect(act).to.throw(expectedError);
|
||||
});
|
||||
});
|
||||
describe('onClear', () => {
|
||||
itVisitsOnce(
|
||||
() => FilterChange.forClear(),
|
||||
);
|
||||
});
|
||||
describe('onApply', () => {
|
||||
itVisitsOnce(
|
||||
() => FilterChange.forApply(new FilterResultStub()),
|
||||
);
|
||||
|
||||
it('visits with expected filter', () => {
|
||||
// arrange
|
||||
const expectedFilter = new FilterResultStub();
|
||||
const sut = FilterChange.forApply(expectedFilter);
|
||||
const visitor = new FilterChangeDetailsVisitorStub();
|
||||
// act
|
||||
sut.visit(visitor);
|
||||
// assert
|
||||
expect(visitor.visitedResults).to.have.lengthOf(1);
|
||||
expect(visitor.visitedResults).to.include(expectedFilter);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
function itVisitsOnce(sutFactory: () => FilterChange) {
|
||||
it('visits', () => {
|
||||
// arrange
|
||||
const sut = sutFactory();
|
||||
const expectedType = sut.actionType;
|
||||
const visitor = new FilterChangeDetailsVisitorStub();
|
||||
// act
|
||||
sut.visit(visitor);
|
||||
// assert
|
||||
expect(visitor.visitedEvents).to.include(expectedType);
|
||||
});
|
||||
it('visits once', () => {
|
||||
// arrange
|
||||
const sut = sutFactory();
|
||||
const expectedType = sut.actionType;
|
||||
const visitor = new FilterChangeDetailsVisitorStub();
|
||||
// act
|
||||
sut.visit(visitor);
|
||||
// assert
|
||||
expect(
|
||||
visitor.visitedEvents.filter((action) => action === expectedType),
|
||||
).to.have.lengthOf(1);
|
||||
});
|
||||
}
|
||||
@@ -5,173 +5,182 @@ import { UserFilter } from '@/application/Context/State/Filter/UserFilter';
|
||||
import { CategoryStub } from '@tests/unit/shared/Stubs/CategoryStub';
|
||||
import { ScriptStub } from '@tests/unit/shared/Stubs/ScriptStub';
|
||||
import { CategoryCollectionStub } from '@tests/unit/shared/Stubs/CategoryCollectionStub';
|
||||
import { FilterChange } from '@/application/Context/State/Filter/Event/FilterChange';
|
||||
import { IFilterChangeDetails } from '@/application/Context/State/Filter/Event/IFilterChangeDetails';
|
||||
import { ICategoryCollection } from '@/domain/ICategoryCollection';
|
||||
|
||||
describe('UserFilter', () => {
|
||||
describe('removeFilter', () => {
|
||||
describe('clearFilter', () => {
|
||||
it('signals when removing filter', () => {
|
||||
// arrange
|
||||
let isCalled = false;
|
||||
const expectedChange = FilterChange.forClear();
|
||||
let actualChange: IFilterChangeDetails;
|
||||
const sut = new UserFilter(new CategoryCollectionStub());
|
||||
sut.filterRemoved.on(() => {
|
||||
isCalled = true;
|
||||
sut.filterChanged.on((change) => {
|
||||
actualChange = change;
|
||||
});
|
||||
// act
|
||||
sut.removeFilter();
|
||||
sut.clearFilter();
|
||||
// assert
|
||||
expect(isCalled).to.be.equal(true);
|
||||
expect(actualChange).to.deep.equal(expectedChange);
|
||||
});
|
||||
it('sets currentFilter to undefined', () => {
|
||||
// arrange
|
||||
const sut = new UserFilter(new CategoryCollectionStub());
|
||||
// act
|
||||
sut.setFilter('non-important');
|
||||
sut.removeFilter();
|
||||
sut.applyFilter('non-important');
|
||||
sut.clearFilter();
|
||||
// assert
|
||||
expect(sut.currentFilter).to.be.equal(undefined);
|
||||
});
|
||||
});
|
||||
describe('setFilter', () => {
|
||||
it('signals when no matches', () => {
|
||||
// arrange
|
||||
let actual: IFilterResult;
|
||||
const nonMatchingFilter = 'non matching filter';
|
||||
const sut = new UserFilter(new CategoryCollectionStub());
|
||||
sut.filtered.on((filterResult) => {
|
||||
actual = filterResult;
|
||||
});
|
||||
// act
|
||||
sut.setFilter(nonMatchingFilter);
|
||||
// assert
|
||||
expect(actual.hasAnyMatches()).be.equal(false);
|
||||
expect(actual.query).to.equal(nonMatchingFilter);
|
||||
});
|
||||
it('sets currentFilter as expected when no matches', () => {
|
||||
// arrange
|
||||
const nonMatchingFilter = 'non matching filter';
|
||||
const sut = new UserFilter(new CategoryCollectionStub());
|
||||
// act
|
||||
sut.setFilter(nonMatchingFilter);
|
||||
// assert
|
||||
const actual = sut.currentFilter;
|
||||
expect(actual.hasAnyMatches()).be.equal(false);
|
||||
expect(actual.query).to.equal(nonMatchingFilter);
|
||||
});
|
||||
describe('signals when matches', () => {
|
||||
describe('signals when script matches', () => {
|
||||
it('code matches', () => {
|
||||
// arrange
|
||||
const code = 'HELLO world';
|
||||
const filter = 'Hello WoRLD';
|
||||
let actual: IFilterResult;
|
||||
const script = new ScriptStub('id').withCode(code);
|
||||
const category = new CategoryStub(33).withScript(script);
|
||||
const sut = new UserFilter(new CategoryCollectionStub()
|
||||
.withAction(category));
|
||||
sut.filtered.on((filterResult) => {
|
||||
actual = filterResult;
|
||||
});
|
||||
// act
|
||||
sut.setFilter(filter);
|
||||
// assert
|
||||
expect(actual.hasAnyMatches()).be.equal(true);
|
||||
expect(actual.categoryMatches).to.have.lengthOf(0);
|
||||
expect(actual.scriptMatches).to.have.lengthOf(1);
|
||||
expect(actual.scriptMatches[0]).to.deep.equal(script);
|
||||
expect(actual.query).to.equal(filter);
|
||||
expect(sut.currentFilter).to.deep.equal(actual);
|
||||
});
|
||||
it('revertCode matches', () => {
|
||||
// arrange
|
||||
const revertCode = 'HELLO world';
|
||||
const filter = 'Hello WoRLD';
|
||||
let actual: IFilterResult;
|
||||
const script = new ScriptStub('id').withRevertCode(revertCode);
|
||||
const category = new CategoryStub(33).withScript(script);
|
||||
const sut = new UserFilter(new CategoryCollectionStub()
|
||||
.withAction(category));
|
||||
sut.filtered.on((filterResult) => {
|
||||
actual = filterResult;
|
||||
});
|
||||
// act
|
||||
sut.setFilter(filter);
|
||||
// assert
|
||||
expect(actual.hasAnyMatches()).be.equal(true);
|
||||
expect(actual.categoryMatches).to.have.lengthOf(0);
|
||||
expect(actual.scriptMatches).to.have.lengthOf(1);
|
||||
expect(actual.scriptMatches[0]).to.deep.equal(script);
|
||||
expect(actual.query).to.equal(filter);
|
||||
expect(sut.currentFilter).to.deep.equal(actual);
|
||||
});
|
||||
it('name matches', () => {
|
||||
// arrange
|
||||
const name = 'HELLO world';
|
||||
const filter = 'Hello WoRLD';
|
||||
let actual: IFilterResult;
|
||||
const script = new ScriptStub('id').withName(name);
|
||||
const category = new CategoryStub(33).withScript(script);
|
||||
const sut = new UserFilter(new CategoryCollectionStub()
|
||||
.withAction(category));
|
||||
sut.filtered.on((filterResult) => {
|
||||
actual = filterResult;
|
||||
});
|
||||
// act
|
||||
sut.setFilter(filter);
|
||||
// assert
|
||||
expect(actual.hasAnyMatches()).be.equal(true);
|
||||
expect(actual.categoryMatches).to.have.lengthOf(0);
|
||||
expect(actual.scriptMatches).to.have.lengthOf(1);
|
||||
expect(actual.scriptMatches[0]).to.deep.equal(script);
|
||||
expect(actual.query).to.equal(filter);
|
||||
expect(sut.currentFilter).to.deep.equal(actual);
|
||||
});
|
||||
});
|
||||
it('signals when category matches', () => {
|
||||
// arrange
|
||||
describe('applyFilter', () => {
|
||||
interface IApplyFilterTestCase {
|
||||
readonly name: string;
|
||||
readonly filter: string;
|
||||
readonly collection: ICategoryCollection;
|
||||
readonly assert: (result: IFilterResult) => void;
|
||||
}
|
||||
const testCases: readonly IApplyFilterTestCase[] = [
|
||||
(() => {
|
||||
const nonMatchingFilter = 'non matching filter';
|
||||
return {
|
||||
name: 'given no matches',
|
||||
filter: nonMatchingFilter,
|
||||
collection: new CategoryCollectionStub(),
|
||||
assert: (filter) => {
|
||||
expect(filter.hasAnyMatches()).be.equal(false);
|
||||
expect(filter.query).to.equal(nonMatchingFilter);
|
||||
},
|
||||
};
|
||||
})(),
|
||||
(() => {
|
||||
const code = 'HELLO world';
|
||||
const matchingFilter = 'Hello WoRLD';
|
||||
const script = new ScriptStub('id').withCode(code);
|
||||
return {
|
||||
name: 'given script match with case-insensitive code',
|
||||
filter: matchingFilter,
|
||||
collection: new CategoryCollectionStub()
|
||||
.withAction(new CategoryStub(33).withScript(script)),
|
||||
assert: (filter) => {
|
||||
expect(filter.hasAnyMatches()).be.equal(true);
|
||||
expect(filter.categoryMatches).to.have.lengthOf(0);
|
||||
expect(filter.scriptMatches).to.have.lengthOf(1);
|
||||
expect(filter.scriptMatches[0]).to.deep.equal(script);
|
||||
expect(filter.query).to.equal(matchingFilter);
|
||||
},
|
||||
};
|
||||
})(),
|
||||
(() => {
|
||||
const revertCode = 'HELLO world';
|
||||
const matchingFilter = 'Hello WoRLD';
|
||||
const script = new ScriptStub('id').withRevertCode(revertCode);
|
||||
return {
|
||||
name: 'given script match with case-insensitive revertCode',
|
||||
filter: matchingFilter,
|
||||
collection: new CategoryCollectionStub()
|
||||
.withAction(new CategoryStub(33).withScript(script)),
|
||||
assert: (filter) => {
|
||||
expect(filter.hasAnyMatches()).be.equal(true);
|
||||
expect(filter.categoryMatches).to.have.lengthOf(0);
|
||||
expect(filter.scriptMatches).to.have.lengthOf(1);
|
||||
expect(filter.scriptMatches[0]).to.deep.equal(script);
|
||||
expect(filter.query).to.equal(matchingFilter);
|
||||
},
|
||||
};
|
||||
})(),
|
||||
(() => {
|
||||
const name = 'HELLO world';
|
||||
const matchingFilter = 'Hello WoRLD';
|
||||
const script = new ScriptStub('id').withName(name);
|
||||
return {
|
||||
name: 'given script match with case-insensitive name',
|
||||
filter: matchingFilter,
|
||||
collection: new CategoryCollectionStub()
|
||||
.withAction(new CategoryStub(33).withScript(script)),
|
||||
assert: (filter) => {
|
||||
expect(filter.hasAnyMatches()).be.equal(true);
|
||||
expect(filter.categoryMatches).to.have.lengthOf(0);
|
||||
expect(filter.scriptMatches).to.have.lengthOf(1);
|
||||
expect(filter.scriptMatches[0]).to.deep.equal(script);
|
||||
expect(filter.query).to.equal(matchingFilter);
|
||||
},
|
||||
};
|
||||
})(),
|
||||
(() => {
|
||||
const categoryName = 'HELLO world';
|
||||
const filter = 'Hello WoRLD';
|
||||
let actual: IFilterResult;
|
||||
const matchingFilter = 'Hello WoRLD';
|
||||
const category = new CategoryStub(55).withName(categoryName);
|
||||
const sut = new UserFilter(new CategoryCollectionStub()
|
||||
.withAction(category));
|
||||
sut.filtered.on((filterResult) => {
|
||||
actual = filterResult;
|
||||
});
|
||||
// act
|
||||
sut.setFilter(filter);
|
||||
// assert
|
||||
expect(actual.hasAnyMatches()).be.equal(true);
|
||||
expect(actual.categoryMatches).to.have.lengthOf(1);
|
||||
expect(actual.categoryMatches[0]).to.deep.equal(category);
|
||||
expect(actual.scriptMatches).to.have.lengthOf(0);
|
||||
expect(actual.query).to.equal(filter);
|
||||
expect(sut.currentFilter).to.deep.equal(actual);
|
||||
});
|
||||
it('signals when category and script matches', () => {
|
||||
// arrange
|
||||
return {
|
||||
name: 'given category match with case-insensitive name',
|
||||
filter: matchingFilter,
|
||||
collection: new CategoryCollectionStub()
|
||||
.withAction(category),
|
||||
assert: (filter) => {
|
||||
expect(filter.hasAnyMatches()).be.equal(true);
|
||||
expect(filter.categoryMatches).to.have.lengthOf(1);
|
||||
expect(filter.categoryMatches[0]).to.deep.equal(category);
|
||||
expect(filter.scriptMatches).to.have.lengthOf(0);
|
||||
expect(filter.query).to.equal(matchingFilter);
|
||||
},
|
||||
};
|
||||
})(),
|
||||
(() => {
|
||||
const matchingText = 'HELLO world';
|
||||
const filter = 'Hello WoRLD';
|
||||
let actual: IFilterResult;
|
||||
const matchingFilter = 'Hello WoRLD';
|
||||
const script = new ScriptStub('script')
|
||||
.withName(matchingText);
|
||||
const category = new CategoryStub(55)
|
||||
.withName(matchingText)
|
||||
.withScript(script);
|
||||
const collection = new CategoryCollectionStub()
|
||||
.withAction(category);
|
||||
const sut = new UserFilter(collection);
|
||||
sut.filtered.on((filterResult) => {
|
||||
actual = filterResult;
|
||||
return {
|
||||
name: 'given category and script matches with case-insensitive names',
|
||||
filter: matchingFilter,
|
||||
collection: new CategoryCollectionStub()
|
||||
.withAction(category),
|
||||
assert: (filter) => {
|
||||
expect(filter.hasAnyMatches()).be.equal(true);
|
||||
expect(filter.categoryMatches).to.have.lengthOf(1);
|
||||
expect(filter.categoryMatches[0]).to.deep.equal(category);
|
||||
expect(filter.scriptMatches).to.have.lengthOf(1);
|
||||
expect(filter.scriptMatches[0]).to.deep.equal(script);
|
||||
expect(filter.query).to.equal(matchingFilter);
|
||||
},
|
||||
};
|
||||
})(),
|
||||
];
|
||||
describe('sets currentFilter as expected', () => {
|
||||
testCases.forEach(({
|
||||
name, filter, collection, assert,
|
||||
}) => {
|
||||
it(name, () => {
|
||||
// arrange
|
||||
const sut = new UserFilter(collection);
|
||||
// act
|
||||
sut.applyFilter(filter);
|
||||
// assert
|
||||
const actual = sut.currentFilter;
|
||||
assert(actual);
|
||||
});
|
||||
});
|
||||
});
|
||||
describe('signals as expected', () => {
|
||||
testCases.forEach(({
|
||||
name, filter, collection, assert,
|
||||
}) => {
|
||||
it(name, () => {
|
||||
// arrange
|
||||
const sut = new UserFilter(collection);
|
||||
let actualFilterResult: IFilterResult;
|
||||
sut.filterChanged.on((filterResult) => {
|
||||
actualFilterResult = filterResult.filter;
|
||||
});
|
||||
// act
|
||||
sut.applyFilter(filter);
|
||||
// assert
|
||||
assert(actualFilterResult);
|
||||
});
|
||||
// act
|
||||
sut.setFilter(filter);
|
||||
// assert
|
||||
expect(actual.hasAnyMatches()).be.equal(true);
|
||||
expect(actual.categoryMatches).to.have.lengthOf(1);
|
||||
expect(actual.categoryMatches[0]).to.deep.equal(category);
|
||||
expect(actual.scriptMatches).to.have.lengthOf(1);
|
||||
expect(actual.scriptMatches[0]).to.deep.equal(script);
|
||||
expect(actual.query).to.equal(filter);
|
||||
expect(sut.currentFilter).to.deep.equal(actual);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user