sxhundred/ISA-Frontend
1
0
Fork 0
mirror of https://dev.azure.com/hugendubel/ISA/_git/ISA-Frontend synced 2025-12-31 09:37:15 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merged PR 2023: fix(core/tabs): Add logging and fix critical bugs in TabService

Browse Source 
fix(core/tabs): Add logging and fix critical bugs in TabService

- Add comprehensive logging using @isa/core/logging for all TabService methods
- Fix null pointer in patchTab when tab doesn't exist
- Fix activateTab allowing non-existent tabs to be set as active
- Fix removeTab not clearing activatedTabId when removing active tab
- Replace unsafe 'as any' type casts with proper type checking
- Document side effect in getCurrentLocation method

Fixes #5474

Related work items: #5474
This commit is contained in:
Lorenz Hilpert
2025-11-12 13:39:50 +00:00
committed by Nino Righi
parent b827a6f0a0
commit 999f61fcc0
1 changed files with 133 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

153
libs/core/tabs/src/lib/tab.ts
View File

@@ -27,6 +27,7 @@ import { computed, inject } from '@angular/core';
import { withDevtools } from '@angular-architects/ngrx-toolkit';
import { CORE_TAB_ID_GENERATOR } from './tab-id.generator';
import { withStorage, UserStorageProvider } from '@isa/core/storage';
import { logger } from '@isa/core/logging';
export const TabService = signalStore(
{ providedIn: 'root' },
@@ -44,6 +45,7 @@ export const TabService = signalStore(
) => ({
_generateId: idGenerator,
_config: config,
_logger: logger({ service: 'TabService' }),
}),
),
withComputed((store) => ({
@@ -68,16 +70,33 @@ export const TabService = signalStore(
location: { current: -1, locations: [] },
};
patchState(store, addEntity(tab));
store._logger.info('Tab added', () => ({
tabId: tab.id,
name: tab.name,
tags: tab.tags,
}));
return tab;
},
activateTab(id: number) {
const tab = store.entityMap()[id];
if (!tab) {
store._logger.warn('Cannot activate non-existent tab', () => ({ tabId: id }));
return;
}
const changes: Partial<Tab> = { activatedAt: Date.now() };
patchState(store, updateEntity({ id, changes }));
patchState(store, { activatedTabId: id });
store._logger.debug('Tab activated', () => ({ tabId: id }));
},
patchTab(id: number, changes: z.infer<typeof PatchTabSchema>) {
const currentTab = store.entityMap()[id];
if (!currentTab) {
store._logger.warn('Cannot patch non-existent tab', () => ({ tabId: id }));
return;
}
const patchedMetadata = changes.metadata
? { ...currentTab.metadata, ...changes.metadata }
: currentTab.metadata;
@@ -88,16 +107,35 @@ export const TabService = signalStore(
};
patchState(store, updateEntity({ id, changes: entityChanges }));
store._logger.debug('Tab patched', () => ({
tabId: id,
changedFields: Object.keys(changes),
}));
},
patchTabMetadata(id: number, metadata: Record<string, unknown>) {
const currentTab = store.entityMap()[id];
if (!currentTab) {
store._logger.warn('Cannot patch metadata for non-existent tab', () => ({ tabId: id }));
return;
}
const changes: Partial<Tab> = {
metadata: { ...currentTab.metadata, ...metadata },
};
patchState(store, updateEntity({ id, changes }));
store._logger.debug('Tab metadata patched', () => ({
tabId: id,
metadataKeys: Object.keys(metadata),
}));
},
removeTab(id: number) {
const wasActive = store.activatedTabId() === id;
patchState(store, removeEntity(id));
if (wasActive) {
patchState(store, { activatedTabId: null });
}
store._logger.info('Tab removed', () => ({ tabId: id, wasActive }));
},
navigateToLocation(
id: number,
@@ -105,14 +143,18 @@ export const TabService = signalStore(
) {
const parsed: TabLocation = TabLocationSchema.parse(location);
const currentTab = store.entityMap()[id];
if (!currentTab) return null;
if (!currentTab) {
store._logger.warn('Cannot navigate to location for non-existent tab', () => ({ tabId: id }));
return null;
}
const currentLocation = currentTab.location;
// First, limit forward history if configured
const maxForwardHistory =
(currentTab.metadata as any)?.maxForwardHistory ??
store._config.maxForwardHistory;
typeof currentTab.metadata['maxForwardHistory'] === 'number'
? currentTab.metadata['maxForwardHistory']
: store._config.maxForwardHistory;
const { locations: limitedLocations } =
TabHistoryPruner.pruneForwardHistory(
@@ -136,14 +178,16 @@ export const TabService = signalStore(
const pruningResult = TabHistoryPruner.pruneHistory(
newLocationHistory,
store._config,
currentTab.metadata as any,
currentTab.metadata,
);
if (pruningResult.entriesRemoved > 0) {
if (store._config.logPruning) {
console.log(
`Tab ${id}: Pruned ${pruningResult.entriesRemoved} entries using ${pruningResult.strategy} strategy`,
);
store._logger.info('Tab history pruned', () => ({
tabId: id,
entriesRemoved: pruningResult.entriesRemoved,
strategy: pruningResult.strategy,
}));
}
newLocationHistory = {
@@ -160,9 +204,11 @@ export const TabService = signalStore(
);
if (wasInvalid && store._config.enableIndexValidation) {
console.warn(
`Tab ${id}: Invalid location index corrected from ${newLocationHistory.current} to ${validatedCurrent}`,
);
store._logger.warn('Invalid location index corrected', () => ({
tabId: id,
invalidIndex: newLocationHistory.current,
correctedIndex: validatedCurrent,
}));
newLocationHistory.current = validatedCurrent;
}
@@ -171,11 +217,21 @@ export const TabService = signalStore(
};
patchState(store, updateEntity({ id, changes }));
store._logger.debug('Navigated to location', () => ({
tabId: id,
url: parsed.url,
historyLength: newLocationHistory.locations.length,
currentIndex: newLocationHistory.current,
}));
return parsed;
},
navigateBack(id: number) {
const currentTab = store.entityMap()[id];
if (!currentTab) return null;
if (!currentTab) {
store._logger.warn('Cannot navigate back for non-existent tab', () => ({ tabId: id }));
return null;
}
const currentLocation = currentTab.location;
@@ -186,7 +242,10 @@ export const TabService = signalStore(
currentLocation.current,
);
if (validatedCurrent <= 0) return null;
if (validatedCurrent <= 0) {
store._logger.debug('Cannot navigate back - at beginning of history', () => ({ tabId: id }));
return null;
}
const newCurrent = validatedCurrent - 1;
const previousLocation = currentLocation.locations[newCurrent];
@@ -199,11 +258,20 @@ export const TabService = signalStore(
};
patchState(store, updateEntity({ id, changes }));
store._logger.debug('Navigated back', () => ({
tabId: id,
newIndex: newCurrent,
url: previousLocation.url,
}));
return previousLocation;
},
navigateForward(id: number) {
const currentTab = store.entityMap()[id];
if (!currentTab) return null;
if (!currentTab) {
store._logger.warn('Cannot navigate forward for non-existent tab', () => ({ tabId: id }));
return null;
}
const currentLocation = currentTab.location;
@@ -214,7 +282,10 @@ export const TabService = signalStore(
currentLocation.current,
);
if (validatedCurrent >= currentLocation.locations.length - 1) return null;
if (validatedCurrent >= currentLocation.locations.length - 1) {
store._logger.debug('Cannot navigate forward - at end of history', () => ({ tabId: id }));
return null;
}
const newCurrent = validatedCurrent + 1;
const nextLocation = currentLocation.locations[newCurrent];
@@ -227,11 +298,22 @@ export const TabService = signalStore(
};
patchState(store, updateEntity({ id, changes }));
store._logger.debug('Navigated forward', () => ({
tabId: id,
newIndex: newCurrent,
url: nextLocation.url,
}));
return nextLocation;
},
clearLocationHistory(id: number) {
const currentTab = store.entityMap()[id];
if (!currentTab) return;
if (!currentTab) {
store._logger.warn('Cannot clear location history for non-existent tab', () => ({ tabId: id }));
return;
}
const historyLength = currentTab.location.locations.length;
const changes: Partial<Tab> = {
location: {
@@ -240,7 +322,22 @@ export const TabService = signalStore(
},
};
patchState(store, updateEntity({ id, changes }));
store._logger.debug('Location history cleared', () => ({
tabId: id,
clearedEntries: historyLength,
}));
},
/**
* Gets the current location for a tab.
*
* IMPORTANT: This method has a side effect - if index validation is enabled
* and an invalid index is detected, it will automatically correct the index
* in the store, triggering state updates and storage autosave.
*
* @param id - The tab ID
* @returns The current location or null if tab doesn't exist or history is empty
*/
getCurrentLocation(id: number) {
const currentTab = store.entityMap()[id];
if (!currentTab) return null;
@@ -255,11 +352,13 @@ export const TabService = signalStore(
);
if (wasInvalid && store._config.enableIndexValidation) {
console.warn(
`Tab ${id}: Invalid location index corrected in getCurrentLocation from ${currentLocation.current} to ${validatedCurrent}`,
);
store._logger.warn('Invalid location index corrected in getCurrentLocation', () => ({
tabId: id,
invalidIndex: currentLocation.current,
correctedIndex: validatedCurrent,
}));
// Correct the invalid index in store
// Correct the invalid index in store (SIDE EFFECT)
const changes: Partial<Tab> = {
location: {
...currentLocation,
@@ -280,13 +379,21 @@ export const TabService = signalStore(
},
updateCurrentLocation(id: number, updates: Partial<TabLocation>) {
const currentTab = store.entityMap()[id];
if (!currentTab) return null;
if (!currentTab) {
store._logger.warn('Cannot update current location for non-existent tab', () => ({ tabId: id }));
return null;
}
const currentLocation = currentTab.location;
if (
currentLocation.current < 0 ||
currentLocation.current >= currentLocation.locations.length
) {
store._logger.warn('Cannot update current location - invalid index', () => ({
tabId: id,
currentIndex: currentLocation.current,
historyLength: currentLocation.locations.length,
}));
return null;
}
@@ -306,6 +413,12 @@ export const TabService = signalStore(
};
patchState(store, updateEntity({ id, changes }));
store._logger.debug('Current location updated', () => ({
tabId: id,
updatedFields: Object.keys(updates),
url: updatedLocation.url,
}));
return updatedLocation;
},
})),