Refactor executable IDs to use strings #262
This commit unifies executable ID structure across categories and scripts, paving the way for more complex ID solutions for #262. It also refactors related code to adapt to the changes. Key changes: - Change numeric IDs to string IDs for categories - Use named types for string IDs to improve code clarity - Add unit tests to verify ID uniqueness Other supporting changes: - Separate concerns in entities for data access and executables by using separate abstractions (`Identifiable` and `RepositoryEntity`) - Simplify usage and construction of entities. - Remove `BaseEntity` for simplicity. - Move creation of categories/scripts to domain layer - Refactor CategoryCollection for better validation logic isolation - Rename some categories to keep the names (used as pseudo-IDs) unique on Windows.
This commit is contained in:
undergroundwires
@@ -2,14 +2,13 @@ import { CategoryStub } from '@tests/unit/shared/Stubs/CategoryStub';
|
||||
import { CategoryCollectionStub } from '@tests/unit/shared/Stubs/CategoryCollectionStub';
|
||||
import { ScriptStub } from '@tests/unit/shared/Stubs/ScriptStub';
|
||||
import type { ScriptSelection } from '@/application/Context/State/Selection/Script/ScriptSelection';
|
||||
import type { ICategoryCollection } from '@/domain/ICategoryCollection';
|
||||
import type { ICategoryCollection } from '@/domain/Collection/ICategoryCollection';
|
||||
import { ScriptToCategorySelectionMapper } from '@/application/Context/State/Selection/Category/ScriptToCategorySelectionMapper';
|
||||
import { ScriptSelectionStub } from '@tests/unit/shared/Stubs/ScriptSelectionStub';
|
||||
import type { CategorySelectionChange } from '@/application/Context/State/Selection/Category/CategorySelectionChange';
|
||||
import type { ScriptSelectionChange, ScriptSelectionChangeCommand } from '@/application/Context/State/Selection/Script/ScriptSelectionChange';
|
||||
import { expectExists } from '@tests/shared/Assertions/ExpectExists';
|
||||
import type { Category } from '@/domain/Executables/Category/Category';
|
||||
import type { Script } from '@/domain/Executables/Script/Script';
|
||||
import type { ExecutableId } from '@/domain/Executables/Identifiable';
|
||||
|
||||
describe('ScriptToCategorySelectionMapper', () => {
|
||||
describe('areAllScriptsSelected', () => {
|
||||
@@ -65,18 +64,18 @@ describe('ScriptToCategorySelectionMapper', () => {
|
||||
readonly description: string;
|
||||
readonly changes: readonly CategorySelectionChange[];
|
||||
readonly categories: ReadonlyArray<{
|
||||
readonly categoryId: Category['id'],
|
||||
readonly scriptIds: readonly Script['id'][],
|
||||
readonly categoryId: ExecutableId,
|
||||
readonly scriptIds: readonly ExecutableId[],
|
||||
}>;
|
||||
readonly expected: readonly ScriptSelectionChange[],
|
||||
}> = [
|
||||
{
|
||||
description: 'single script: select without revert',
|
||||
categories: [
|
||||
{ categoryId: 1, scriptIds: ['single-script'] },
|
||||
{ categoryId: 'category-1', scriptIds: ['single-script'] },
|
||||
],
|
||||
changes: [
|
||||
{ categoryId: 1, newStatus: { isSelected: true, isReverted: false } },
|
||||
{ categoryId: 'category-1', newStatus: { isSelected: true, isReverted: false } },
|
||||
],
|
||||
expected: [
|
||||
{ scriptId: 'single-script', newStatus: { isSelected: true, isReverted: false } },
|
||||
@@ -85,12 +84,12 @@ describe('ScriptToCategorySelectionMapper', () => {
|
||||
{
|
||||
description: 'multiple scripts: select without revert',
|
||||
categories: [
|
||||
{ categoryId: 1, scriptIds: ['script1-cat1', 'script2-cat1'] },
|
||||
{ categoryId: 2, scriptIds: ['script3-cat2'] },
|
||||
{ categoryId: 'category-1', scriptIds: ['script1-cat1', 'script2-cat1'] },
|
||||
{ categoryId: 'category-2', scriptIds: ['script3-cat2'] },
|
||||
],
|
||||
changes: [
|
||||
{ categoryId: 1, newStatus: { isSelected: true, isReverted: false } },
|
||||
{ categoryId: 2, newStatus: { isSelected: true, isReverted: false } },
|
||||
{ categoryId: 'category-1', newStatus: { isSelected: true, isReverted: false } },
|
||||
{ categoryId: 'category-2', newStatus: { isSelected: true, isReverted: false } },
|
||||
],
|
||||
expected: [
|
||||
{ scriptId: 'script1-cat1', newStatus: { isSelected: true, isReverted: false } },
|
||||
@@ -101,10 +100,10 @@ describe('ScriptToCategorySelectionMapper', () => {
|
||||
{
|
||||
description: 'single script: select with revert',
|
||||
categories: [
|
||||
{ categoryId: 1, scriptIds: ['single-script'] },
|
||||
{ categoryId: 'category-1', scriptIds: ['single-script'] },
|
||||
],
|
||||
changes: [
|
||||
{ categoryId: 1, newStatus: { isSelected: true, isReverted: true } },
|
||||
{ categoryId: 'category-1', newStatus: { isSelected: true, isReverted: true } },
|
||||
],
|
||||
expected: [
|
||||
{ scriptId: 'single-script', newStatus: { isSelected: true, isReverted: true } },
|
||||
@@ -113,14 +112,14 @@ describe('ScriptToCategorySelectionMapper', () => {
|
||||
{
|
||||
description: 'multiple scripts: select with revert',
|
||||
categories: [
|
||||
{ categoryId: 1, scriptIds: ['script-1-cat-1'] },
|
||||
{ categoryId: 2, scriptIds: ['script-2-cat-2'] },
|
||||
{ categoryId: 3, scriptIds: ['script-3-cat-3'] },
|
||||
{ categoryId: 'category-1', scriptIds: ['script-1-cat-1'] },
|
||||
{ categoryId: 'category-2', scriptIds: ['script-2-cat-2'] },
|
||||
{ categoryId: 'category-3', scriptIds: ['script-3-cat-3'] },
|
||||
],
|
||||
changes: [
|
||||
{ categoryId: 1, newStatus: { isSelected: true, isReverted: true } },
|
||||
{ categoryId: 2, newStatus: { isSelected: true, isReverted: true } },
|
||||
{ categoryId: 3, newStatus: { isSelected: true, isReverted: true } },
|
||||
{ categoryId: 'category-1', newStatus: { isSelected: true, isReverted: true } },
|
||||
{ categoryId: 'category-2', newStatus: { isSelected: true, isReverted: true } },
|
||||
{ categoryId: 'category-3', newStatus: { isSelected: true, isReverted: true } },
|
||||
],
|
||||
expected: [
|
||||
{ scriptId: 'script-1-cat-1', newStatus: { isSelected: true, isReverted: true } },
|
||||
@@ -131,10 +130,10 @@ describe('ScriptToCategorySelectionMapper', () => {
|
||||
{
|
||||
description: 'single script: deselect',
|
||||
categories: [
|
||||
{ categoryId: 1, scriptIds: ['single-script'] },
|
||||
{ categoryId: 'category-1', scriptIds: ['single-script'] },
|
||||
],
|
||||
changes: [
|
||||
{ categoryId: 1, newStatus: { isSelected: false } },
|
||||
{ categoryId: 'category-1', newStatus: { isSelected: false } },
|
||||
],
|
||||
expected: [
|
||||
{ scriptId: 'single-script', newStatus: { isSelected: false } },
|
||||
@@ -143,12 +142,12 @@ describe('ScriptToCategorySelectionMapper', () => {
|
||||
{
|
||||
description: 'multiple scripts: deselect',
|
||||
categories: [
|
||||
{ categoryId: 1, scriptIds: ['script-1-cat1'] },
|
||||
{ categoryId: 2, scriptIds: ['script-2-cat2'] },
|
||||
{ categoryId: 'category-1', scriptIds: ['script-1-cat1'] },
|
||||
{ categoryId: 'category-2', scriptIds: ['script-2-cat2'] },
|
||||
],
|
||||
changes: [
|
||||
{ categoryId: 1, newStatus: { isSelected: false } },
|
||||
{ categoryId: 2, newStatus: { isSelected: false } },
|
||||
{ categoryId: 'category-1', newStatus: { isSelected: false } },
|
||||
{ categoryId: 'category-2', newStatus: { isSelected: false } },
|
||||
],
|
||||
expected: [
|
||||
{ scriptId: 'script-1-cat1', newStatus: { isSelected: false } },
|
||||
@@ -158,14 +157,14 @@ describe('ScriptToCategorySelectionMapper', () => {
|
||||
{
|
||||
description: 'mixed operations (select, revert, deselect)',
|
||||
categories: [
|
||||
{ categoryId: 1, scriptIds: ['to-revert'] },
|
||||
{ categoryId: 2, scriptIds: ['not-revert'] },
|
||||
{ categoryId: 3, scriptIds: ['to-deselect'] },
|
||||
{ categoryId: 'category-1', scriptIds: ['to-revert'] },
|
||||
{ categoryId: 'category-2', scriptIds: ['not-revert'] },
|
||||
{ categoryId: 'category-3', scriptIds: ['to-deselect'] },
|
||||
],
|
||||
changes: [
|
||||
{ categoryId: 1, newStatus: { isSelected: true, isReverted: true } },
|
||||
{ categoryId: 2, newStatus: { isSelected: true, isReverted: false } },
|
||||
{ categoryId: 3, newStatus: { isSelected: false } },
|
||||
{ categoryId: 'category-1', newStatus: { isSelected: true, isReverted: true } },
|
||||
{ categoryId: 'category-2', newStatus: { isSelected: true, isReverted: false } },
|
||||
{ categoryId: 'category-3', newStatus: { isSelected: false } },
|
||||
],
|
||||
expected: [
|
||||
{ scriptId: 'to-revert', newStatus: { isSelected: true, isReverted: true } },
|
||||
@@ -176,12 +175,12 @@ describe('ScriptToCategorySelectionMapper', () => {
|
||||
{
|
||||
description: 'affecting selected categories only',
|
||||
categories: [
|
||||
{ categoryId: 1, scriptIds: ['relevant-1', 'relevant-2'] },
|
||||
{ categoryId: 2, scriptIds: ['not-relevant-1', 'not-relevant-2'] },
|
||||
{ categoryId: 3, scriptIds: ['not-relevant-3', 'not-relevant-4'] },
|
||||
{ categoryId: 'category-1', scriptIds: ['relevant-1', 'relevant-2'] },
|
||||
{ categoryId: 'category-2', scriptIds: ['not-relevant-1', 'not-relevant-2'] },
|
||||
{ categoryId: 'category-3', scriptIds: ['not-relevant-3', 'not-relevant-4'] },
|
||||
],
|
||||
changes: [
|
||||
{ categoryId: 1, newStatus: { isSelected: true, isReverted: true } },
|
||||
{ categoryId: 'category-1', newStatus: { isSelected: true, isReverted: true } },
|
||||
],
|
||||
expected: [
|
||||
{ scriptId: 'relevant-1', newStatus: { isSelected: true, isReverted: true } },
|
||||
@@ -198,7 +197,7 @@ describe('ScriptToCategorySelectionMapper', () => {
|
||||
const sut = new ScriptToCategorySelectionMapperBuilder()
|
||||
.withScriptSelection(scriptSelectionStub)
|
||||
.withCollection(new CategoryCollectionStub().withAction(
|
||||
new CategoryStub(99)
|
||||
new CategoryStub('single-parent-category-action')
|
||||
// Register scripts to test for nested items
|
||||
.withAllScriptIdsRecursively(...categories.flatMap((c) => c.scriptIds))
|
||||
.withCategories(...categories.map(
|
||||
@@ -256,7 +255,7 @@ function setupTestWithPreselectedScripts(options: {
|
||||
new ScriptStub('third-script'),
|
||||
];
|
||||
const preselectedScripts = options.preselect(allScripts);
|
||||
const category = new CategoryStub(1)
|
||||
const category = new CategoryStub('single-parent-category-action')
|
||||
.withAllScriptsRecursively(...allScripts); // Register scripts to test for nested items
|
||||
const collection = new CategoryCollectionStub().withAction(category);
|
||||
const sut = new ScriptToCategorySelectionMapperBuilder()
|
||||
|
||||
@@ -4,7 +4,7 @@ import { CategoryStub } from '@tests/unit/shared/Stubs/CategoryStub';
|
||||
import { CategoryCollectionStub } from '@tests/unit/shared/Stubs/CategoryCollectionStub';
|
||||
import { SelectedScriptStub } from '@tests/unit/shared/Stubs/SelectedScriptStub';
|
||||
import { ScriptStub } from '@tests/unit/shared/Stubs/ScriptStub';
|
||||
import type { ICategoryCollection } from '@/domain/ICategoryCollection';
|
||||
import type { ICategoryCollection } from '@/domain/Collection/ICategoryCollection';
|
||||
import type { SelectedScript } from '@/application/Context/State/Selection/Script/SelectedScript';
|
||||
import { BatchedDebounceStub } from '@tests/unit/shared/Stubs/BatchedDebounceStub';
|
||||
import type { ScriptSelectionChange, ScriptSelectionChangeCommand } from '@/application/Context/State/Selection/Script/ScriptSelectionChange';
|
||||
@@ -104,7 +104,7 @@ describe('DebouncedScriptSelection', () => {
|
||||
const { scriptSelection, unselectedScripts } = setupTestWithPreselectedScripts({
|
||||
preselect: (allScripts) => [allScripts[0]],
|
||||
});
|
||||
const scriptIdToCheck = unselectedScripts[0].id;
|
||||
const scriptIdToCheck = unselectedScripts[0].executableId;
|
||||
// act
|
||||
const actual = scriptSelection.isSelected(scriptIdToCheck);
|
||||
// assert
|
||||
@@ -300,7 +300,10 @@ describe('DebouncedScriptSelection', () => {
|
||||
preselect: (allScripts) => [allScripts[0], allScripts[1]]
|
||||
.map((s) => s.toSelectedScript()),
|
||||
getChanges: (allScripts) => [
|
||||
{ scriptId: allScripts[2].id, newStatus: { isReverted: true, isSelected: true } },
|
||||
{
|
||||
scriptId: allScripts[2].executableId,
|
||||
newStatus: { isReverted: true, isSelected: true },
|
||||
},
|
||||
],
|
||||
getExpectedFinalSelection: (allScripts) => [
|
||||
allScripts[0].toSelectedScript(),
|
||||
@@ -313,7 +316,10 @@ describe('DebouncedScriptSelection', () => {
|
||||
preselect: (allScripts) => [allScripts[0], allScripts[1]]
|
||||
.map((s) => s.toSelectedScript()),
|
||||
getChanges: (allScripts) => [
|
||||
{ scriptId: allScripts[2].id, newStatus: { isReverted: false, isSelected: true } },
|
||||
{
|
||||
scriptId: allScripts[2].executableId,
|
||||
newStatus: { isReverted: false, isSelected: true },
|
||||
},
|
||||
],
|
||||
getExpectedFinalSelection: (allScripts) => [
|
||||
allScripts[0].toSelectedScript(),
|
||||
@@ -326,7 +332,7 @@ describe('DebouncedScriptSelection', () => {
|
||||
preselect: (allScripts) => [allScripts[0], allScripts[1]]
|
||||
.map((s) => s.toSelectedScript()),
|
||||
getChanges: (allScripts) => [
|
||||
{ scriptId: allScripts[0].id, newStatus: { isSelected: false } },
|
||||
{ scriptId: allScripts[0].executableId, newStatus: { isSelected: false } },
|
||||
],
|
||||
getExpectedFinalSelection: (allScripts) => [
|
||||
allScripts[1].toSelectedScript(),
|
||||
@@ -339,7 +345,10 @@ describe('DebouncedScriptSelection', () => {
|
||||
allScripts[1].toSelectedScript(),
|
||||
],
|
||||
getChanges: (allScripts) => [
|
||||
{ scriptId: allScripts[0].id, newStatus: { isSelected: true, isReverted: true } },
|
||||
{
|
||||
scriptId: allScripts[0].executableId,
|
||||
newStatus: { isSelected: true, isReverted: true },
|
||||
},
|
||||
],
|
||||
getExpectedFinalSelection: (allScripts) => [
|
||||
allScripts[0].toSelectedScript().withRevert(true),
|
||||
@@ -353,7 +362,10 @@ describe('DebouncedScriptSelection', () => {
|
||||
allScripts[1].toSelectedScript(),
|
||||
],
|
||||
getChanges: (allScripts) => [
|
||||
{ scriptId: allScripts[0].id, newStatus: { isSelected: true, isReverted: false } },
|
||||
{
|
||||
scriptId: allScripts[0].executableId,
|
||||
newStatus: { isSelected: true, isReverted: false },
|
||||
},
|
||||
],
|
||||
getExpectedFinalSelection: (allScripts) => [
|
||||
allScripts[0].toSelectedScript().withRevert(false),
|
||||
@@ -367,9 +379,18 @@ describe('DebouncedScriptSelection', () => {
|
||||
allScripts[2].toSelectedScript(), // remove
|
||||
],
|
||||
getChanges: (allScripts) => [
|
||||
{ scriptId: allScripts[0].id, newStatus: { isSelected: true, isReverted: false } },
|
||||
{ scriptId: allScripts[1].id, newStatus: { isSelected: true, isReverted: true } },
|
||||
{ scriptId: allScripts[2].id, newStatus: { isSelected: false } },
|
||||
{
|
||||
scriptId: allScripts[0].executableId,
|
||||
newStatus: { isSelected: true, isReverted: false },
|
||||
},
|
||||
{
|
||||
scriptId: allScripts[1].executableId,
|
||||
newStatus: { isSelected: true, isReverted: true },
|
||||
},
|
||||
{
|
||||
scriptId: allScripts[2].executableId,
|
||||
newStatus: { isSelected: false },
|
||||
},
|
||||
],
|
||||
getExpectedFinalSelection: (allScripts) => [
|
||||
allScripts[0].toSelectedScript().withRevert(false),
|
||||
@@ -408,7 +429,10 @@ describe('DebouncedScriptSelection', () => {
|
||||
description: 'does not change selection for an already selected script',
|
||||
preselect: (allScripts) => [allScripts[0].toSelectedScript().withRevert(true)],
|
||||
getChanges: (allScripts) => [
|
||||
{ scriptId: allScripts[0].id, newStatus: { isReverted: true, isSelected: true } },
|
||||
{
|
||||
scriptId: allScripts[0].executableId,
|
||||
newStatus: { isReverted: true, isSelected: true },
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
@@ -416,15 +440,21 @@ describe('DebouncedScriptSelection', () => {
|
||||
preselect: (allScripts) => [allScripts[0], allScripts[1]]
|
||||
.map((s) => s.toSelectedScript()),
|
||||
getChanges: (allScripts) => [
|
||||
{ scriptId: allScripts[2].id, newStatus: { isSelected: false } },
|
||||
{ scriptId: allScripts[2].executableId, newStatus: { isSelected: false } },
|
||||
],
|
||||
},
|
||||
{
|
||||
description: 'handles no mutations for mixed unchanged operations',
|
||||
preselect: (allScripts) => [allScripts[0].toSelectedScript().withRevert(false)],
|
||||
getChanges: (allScripts) => [
|
||||
{ scriptId: allScripts[0].id, newStatus: { isSelected: true, isReverted: false } },
|
||||
{ scriptId: allScripts[1].id, newStatus: { isSelected: false } },
|
||||
{
|
||||
scriptId: allScripts[0].executableId,
|
||||
newStatus: { isSelected: true, isReverted: false },
|
||||
},
|
||||
{
|
||||
scriptId: allScripts[1].executableId,
|
||||
newStatus: { isSelected: false },
|
||||
},
|
||||
],
|
||||
},
|
||||
];
|
||||
@@ -459,7 +489,7 @@ describe('DebouncedScriptSelection', () => {
|
||||
.build();
|
||||
const expectedCommand: ScriptSelectionChangeCommand = {
|
||||
changes: [
|
||||
{ scriptId: script.id, newStatus: { isReverted: true, isSelected: true } },
|
||||
{ scriptId: script.executableId, newStatus: { isReverted: true, isSelected: true } },
|
||||
],
|
||||
};
|
||||
// act
|
||||
@@ -481,7 +511,7 @@ describe('DebouncedScriptSelection', () => {
|
||||
// act
|
||||
selection.processChanges({
|
||||
changes: [
|
||||
{ scriptId: script.id, newStatus: { isReverted: true, isSelected: true } },
|
||||
{ scriptId: script.executableId, newStatus: { isReverted: true, isSelected: true } },
|
||||
],
|
||||
});
|
||||
// assert
|
||||
@@ -502,7 +532,7 @@ describe('DebouncedScriptSelection', () => {
|
||||
// act
|
||||
selection.processChanges({
|
||||
changes: [
|
||||
{ scriptId: script.id, newStatus: { isReverted: true, isSelected: true } },
|
||||
{ scriptId: script.executableId, newStatus: { isReverted: true, isSelected: true } },
|
||||
],
|
||||
});
|
||||
debounceStub.execute();
|
||||
@@ -525,7 +555,7 @@ describe('DebouncedScriptSelection', () => {
|
||||
for (const script of scripts) {
|
||||
selection.processChanges({
|
||||
changes: [
|
||||
{ scriptId: script.id, newStatus: { isReverted: true, isSelected: true } },
|
||||
{ scriptId: script.executableId, newStatus: { isReverted: true, isSelected: true } },
|
||||
],
|
||||
});
|
||||
}
|
||||
@@ -539,7 +569,7 @@ describe('DebouncedScriptSelection', () => {
|
||||
});
|
||||
|
||||
function createCollectionWithScripts(...scripts: Script[]): CategoryCollectionStub {
|
||||
const category = new CategoryStub(1).withScripts(...scripts);
|
||||
const category = new CategoryStub('parent-category').withScripts(...scripts);
|
||||
const collection = new CategoryCollectionStub().withAction(category);
|
||||
return collection;
|
||||
}
|
||||
@@ -572,7 +602,7 @@ function setupTestWithPreselectedScripts(options: {
|
||||
return initialSelection;
|
||||
})();
|
||||
const unselectedScripts = allScripts.filter(
|
||||
(s) => !preselectedScripts.map((selected) => selected.id).includes(s.id),
|
||||
(s) => !preselectedScripts.map((selected) => selected.id).includes(s.executableId),
|
||||
);
|
||||
const collection = createCollectionWithScripts(...allScripts);
|
||||
const scriptSelection = new DebouncedScriptSelectionBuilder()
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { describe, it } from 'vitest';
|
||||
import type { SelectedScript } from '@/application/Context/State/Selection/Script/SelectedScript';
|
||||
import { UserSelectionFacade } from '@/application/Context/State/Selection/UserSelectionFacade';
|
||||
import type { ICategoryCollection } from '@/domain/ICategoryCollection';
|
||||
import type { ICategoryCollection } from '@/domain/Collection/ICategoryCollection';
|
||||
import { CategoryCollectionStub } from '@tests/unit/shared/Stubs/CategoryCollectionStub';
|
||||
import type { ScriptsFactory, CategoriesFactory } from '@/application/Context/State/Selection/UserSelectionFacade';
|
||||
import { ScriptSelectionStub } from '@tests/unit/shared/Stubs/ScriptSelectionStub';
|
||||
|
||||
Reference in New Issue
Block a user