Skip to content

Commit 9239ed6

Browse files
Francesco MautoAndrea Barbasso
Francesco Mauto
authored and
Andrea Barbasso
committed
Merged in task/dspace-cris-2025_02_x/DSC-2887 (pull request DSpace#4640)
Task/dspace cris 2025 02 x/DSC-2887 Approved-by: Andrea Barbasso
2 parents 29e178b + 4db2ee9 commit 9239ed6

6 files changed

Lines changed: 256 additions & 29 deletions

File tree

‎src/app/app.metareducers.spec.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import { universalMetaReducer } from './app.metareducers';
2+
import { StoreActionTypes } from './store.actions';
3+
4+
describe('universalMetaReducer', () => {
5+
const mockReducer = (state: any, action: any) => state;
6+
7+
it('should pass state through for unknown action', () => {
8+
const state = { core: { route: { queryParams: { a: '1' }, params: {} } } };
9+
const result = universalMetaReducer(mockReducer)(state, { type: 'UNKNOWN' });
10+
expect(result).toEqual(state);
11+
});
12+
13+
describe('REHYDRATE', () => {
14+
it('should merge payload into state', () => {
15+
const state = { otherProp: 'keep-me' };
16+
const payload = { core: { route: { queryParams: { f: ['x'] }, params: {} } } };
17+
const result = universalMetaReducer(mockReducer)(state, {
18+
type: StoreActionTypes.REHYDRATE,
19+
payload,
20+
});
21+
expect(result.otherProp).toBe('keep-me');
22+
expect(result.core.route).toEqual({ queryParams: {}, params: {} });
23+
});
24+
25+
it('should reset core.route to empty after rehydration', () => {
26+
const state = { core: { route: { queryParams: { f: ['x'] }, params: { id: '123' } } } };
27+
const payload = { core: { route: { queryParams: { stale: ['val'] }, params: { stale: 'val' } } } };
28+
const result = universalMetaReducer(mockReducer)(state, {
29+
type: StoreActionTypes.REHYDRATE,
30+
payload,
31+
});
32+
expect(result.core.route).toEqual({ queryParams: {}, params: {} });
33+
});
34+
35+
it('should not crash when state.core is undefined', () => {
36+
const state = {};
37+
const payload = { core: { route: { queryParams: { a: '1' }, params: {} } } };
38+
const result = universalMetaReducer(mockReducer)(state, {
39+
type: StoreActionTypes.REHYDRATE,
40+
payload,
41+
});
42+
expect(result.core.route).toEqual({ queryParams: {}, params: {} });
43+
});
44+
45+
it('should preserve other core properties when resetting route', () => {
46+
const state = { core: { route: {} } };
47+
const payload = { core: { route: { queryParams: { x: '1' }, params: {} }, otherProp: 'keep-me' } };
48+
const result = universalMetaReducer(mockReducer)(state, {
49+
type: StoreActionTypes.REHYDRATE,
50+
payload,
51+
});
52+
expect(result.core.otherProp).toBe('keep-me');
53+
expect(result.core.route).toEqual({ queryParams: {}, params: {} });
54+
});
55+
56+
it('should handle REPLAY action (no-op)', () => {
57+
const state = { core: { route: { queryParams: { a: '1' }, params: {} } } };
58+
const result = universalMetaReducer(mockReducer)(state, {
59+
type: StoreActionTypes.REPLAY,
60+
});
61+
expect(result).toEqual(state);
62+
});
63+
});
64+
});

‎src/app/app.metareducers.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ export function universalMetaReducer(reducer) {
1919
switch (action.type) {
2020
case StoreActionTypes.REHYDRATE:
2121
state = Object.assign({}, state, action.payload);
22+
if (state.core) {
23+
state.core = Object.assign({}, state.core, {
24+
route: { queryParams: {}, params: {} },
25+
});
26+
}
2227
break;
2328
case StoreActionTypes.REPLAY:
2429
default:

‎src/app/core/services/route.reducer.spec.ts‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
import { Params } from '@angular/router';
2+
3+
import {
4+
AddParameterAction,
5+
AddQueryParameterAction,
6+
ResetRouteStateAction,
7+
SetParameterAction,
8+
SetParametersAction,
9+
SetQueryParameterAction,
10+
SetQueryParametersAction,
11+
} from './route.actions';
12+
import {
13+
routeReducer,
14+
RouteState,
15+
} from './route.reducer';
16+
17+
describe('routeReducer', () => {
18+
const initialState: RouteState = {
19+
queryParams: {},
20+
params: {},
21+
};
22+
23+
it('should return initial state for unknown action', () => {
24+
const state = routeReducer(undefined, { type: 'UNKNOWN' } as any);
25+
expect(state).toEqual(initialState);
26+
});
27+
28+
describe('RESET', () => {
29+
it('should reset state to initialState', () => {
30+
const state: RouteState = {
31+
queryParams: { f: { author: ['Smith'] } },
32+
params: { id: '123', configuration: 'default' },
33+
};
34+
const result = routeReducer(state, new ResetRouteStateAction());
35+
expect(result).toEqual(initialState);
36+
});
37+
});
38+
39+
describe('SET_PARAMETERS', () => {
40+
it('should replace all params with action payload', () => {
41+
const state: RouteState = { ...initialState };
42+
const payload: Params = { id: '123', configuration: 'default' };
43+
const result = routeReducer(state, new SetParametersAction(payload));
44+
expect(result.params).toEqual(payload);
45+
expect(result.queryParams).toEqual(state.queryParams);
46+
});
47+
48+
it('should clear params when payload is empty', () => {
49+
const state: RouteState = {
50+
queryParams: {},
51+
params: { id: '123' },
52+
};
53+
const result = routeReducer(state, new SetParametersAction({}));
54+
expect(result.params).toEqual({});
55+
expect(result.queryParams).toEqual(state.queryParams);
56+
});
57+
});
58+
59+
describe('SET_QUERY_PARAMETERS', () => {
60+
it('should replace all queryParams with action payload', () => {
61+
const state: RouteState = { ...initialState };
62+
const payload: Params = { 'f.author': ['Smith'], 'spc.page': '1' };
63+
const result = routeReducer(state, new SetQueryParametersAction(payload));
64+
expect(result.queryParams).toEqual(payload);
65+
expect(result.params).toEqual(state.params);
66+
});
67+
68+
it('should clear queryParams when payload is empty', () => {
69+
const state: RouteState = {
70+
queryParams: { 'f.author': ['Smith'] },
71+
params: {},
72+
};
73+
const result = routeReducer(state, new SetQueryParametersAction({}));
74+
expect(result.queryParams).toEqual({});
75+
expect(result.params).toEqual(state.params);
76+
});
77+
});
78+
79+
describe('SET_PARAMETER', () => {
80+
it('should set a single param key-value pair', () => {
81+
const state: RouteState = {
82+
queryParams: {},
83+
params: { id: '123' },
84+
};
85+
const result = routeReducer(state, new SetParameterAction('configuration', 'default'));
86+
expect(result.params).toEqual({ id: '123', configuration: 'default' });
87+
});
88+
});
89+
90+
describe('SET_QUERY_PARAMETER', () => {
91+
it('should set a single query param key-value pair', () => {
92+
const state: RouteState = {
93+
queryParams: { 'f.author': ['Smith'] },
94+
params: {},
95+
};
96+
const result = routeReducer(state, new SetQueryParameterAction('tab', 'publications'));
97+
expect(result.queryParams).toEqual({ 'f.author': ['Smith'], tab: 'publications' });
98+
});
99+
});
100+
101+
describe('ADD_PARAMETER', () => {
102+
it('should append value to existing param key', () => {
103+
const state: RouteState = {
104+
queryParams: {},
105+
params: { tag: ['a'] },
106+
};
107+
const result = routeReducer(state, new AddParameterAction('tag', 'b'));
108+
expect(result.params.tag).toEqual(['a', 'b']);
109+
});
110+
111+
it('should create new param key with value array when key does not exist', () => {
112+
const state: RouteState = { ...initialState };
113+
const result = routeReducer(state, new AddParameterAction('tag', 'new'));
114+
expect(result.params.tag).toEqual(['new']);
115+
});
116+
});
117+
118+
describe('ADD_QUERY_PARAMETER', () => {
119+
it('should append value to existing query param key', () => {
120+
const state: RouteState = {
121+
queryParams: { 'f.author': ['Smith'] },
122+
params: {},
123+
};
124+
const result = routeReducer(state, new AddQueryParameterAction('f.author', 'Jones'));
125+
expect(result.queryParams['f.author']).toEqual(['Smith', 'Jones']);
126+
});
127+
128+
it('should create new query param key with value array when key does not exist', () => {
129+
const state: RouteState = { ...initialState };
130+
const result = routeReducer(state, new AddQueryParameterAction('tag', 'new'));
131+
expect(result.queryParams.tag).toEqual(['new']);
132+
});
133+
});
134+
});

‎src/app/core/services/route.reducer.ts‎

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@ import {
77
RouteActions,
88
RouteActionTypes,
99
SetParameterAction,
10-
SetParametersAction,
1110
SetQueryParameterAction,
12-
SetQueryParametersAction,
1311
} from './route.actions';
1412

1513
/**
@@ -39,10 +37,14 @@ export function routeReducer(state = initialState, action: RouteActions): RouteS
3937
return initialState;
4038
}
4139
case RouteActionTypes.SET_PARAMETERS: {
42-
return setParameters(state, action as SetParametersAction, 'params');
40+
return Object.assign({}, state, {
41+
params: isNotEmpty(action.payload) ? action.payload : {},
42+
});
4343
}
4444
case RouteActionTypes.SET_QUERY_PARAMETERS: {
45-
return setParameters(state, action as SetQueryParametersAction, 'queryParams');
45+
return Object.assign({}, state, {
46+
queryParams: isNotEmpty(action.payload) ? action.payload : {},
47+
});
4648
}
4749
case RouteActionTypes.ADD_PARAMETER: {
4850
return addParameter(state, action as AddParameterAction, 'params');
@@ -68,7 +70,7 @@ export function routeReducer(state = initialState, action: RouteActions): RouteS
6870
* @param action The add action to perform on the current state
6971
* @param paramType The type of parameter to add: route or query parameter
7072
*/
71-
function addParameter(state: RouteState, action: AddParameterAction | AddQueryParameterAction, paramType: string): RouteState {
73+
function addParameter(state: RouteState, action: AddParameterAction | AddQueryParameterAction, paramType: 'params' | 'queryParams'): RouteState {
7274
const subState = state[paramType];
7375
const existingValues = subState[action.payload.key] || [];
7476
const newValues = [...existingValues, action.payload.value];
@@ -82,18 +84,7 @@ function addParameter(state: RouteState, action: AddParameterAction | AddQueryPa
8284
* @param action The set action to perform on the current state
8385
* @param paramType The type of parameter to set: route or query parameter
8486
*/
85-
function setParameters(state: RouteState, action: SetParametersAction | SetQueryParametersAction, paramType: string): RouteState {
86-
const param = isNotEmpty(action.payload) ? { [paramType]: { [action.payload.key]: action.payload.value } } : {};
87-
return Object.assign({}, state, param);
88-
}
89-
90-
/**
91-
* Set a route or query parameter in the store
92-
* @param state The current state
93-
* @param action The set action to perform on the current state
94-
* @param paramType The type of parameter to set: route or query parameter
95-
*/
96-
function setParameter(state: RouteState, action: SetParameterAction | SetQueryParameterAction, paramType: string): RouteState {
87+
function setParameter(state: RouteState, action: SetParameterAction | SetQueryParameterAction, paramType: 'params' | 'queryParams'): RouteState {
9788
const subState = state[paramType];
9889
const newSubstate = Object.assign({}, subState, { [action.payload.key]: action.payload.value });
9990
return Object.assign({}, state, { [paramType]: newSubstate });

‎src/app/item-page/cris-item-page-tab.resolver.spec.ts‎

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -136,19 +136,50 @@ describe('CrisItemPageTabResolver', () => {
136136
});
137137
});
138138

139-
it('Should handle tab shortnames with "::" correctly', () => {
140-
const tabRD = createSuccessfulRemoteDataObject(createPaginatedList([{
139+
it('Should handle tab shortnames with "::" correctly', (done) => {
140+
const tabPageList = createPaginatedList([{
141141
...tabPublicationsTest,
142142
shortname: 'publication::details',
143-
}]));
144-
tabService.findByItem.and.returnValue(createSuccessfulRemoteDataObject$(tabRD) as any);
143+
}]);
144+
const tabRD = createSuccessfulRemoteDataObject(tabPageList);
145+
tabService.findByItem.and.returnValue(createSuccessfulRemoteDataObject$(tabPageList) as any);
145146

146-
TestBed.runInInjectionContext(() => {
147-
return crisItemPageTabResolver({ params: { id: uuid } } as any, { url: '/entities/publication/1234-65487-12354-1235/publication::details' } as any);
147+
const obs = TestBed.runInInjectionContext(() => {
148+
return crisItemPageTabResolver({ params: { id: uuid } } as any, { url: '/entities/publication/1234-65487-12354-1235/details' } as any);
149+
}) as Observable<RemoteData<PaginatedList<CrisLayoutTab>>>;
150+
151+
obs.pipe(take(1)).subscribe((resolved) => {
152+
expect(router.navigateByUrl).not.toHaveBeenCalled();
153+
expect(hardRedirectService.redirect).not.toHaveBeenCalled();
154+
expect(resolved).toEqual(tabRD);
155+
done();
148156
});
157+
});
158+
159+
it('should NOT redirect to 404 when query params are present in the URL', (done) => {
160+
const obs = TestBed.runInInjectionContext(() => {
161+
return crisItemPageTabResolver({ params: { id: uuid } } as any, { url: '/entities/publication/1234-65487-12354-1235?f.subject=value&f.date=2024' } as any);
162+
}) as Observable<RemoteData<PaginatedList<CrisLayoutTab>>>;
149163

150-
expect(router.navigateByUrl).not.toHaveBeenCalled();
151-
expect(hardRedirectService.redirect).not.toHaveBeenCalled();
164+
obs.pipe(take(1)).subscribe((resolved) => {
165+
expect(router.navigateByUrl).not.toHaveBeenCalled();
166+
expect(hardRedirectService.redirect).not.toHaveBeenCalled();
167+
expect(resolved).toEqual(tabsRD);
168+
done();
169+
});
170+
});
171+
172+
it('should NOT redirect to 404 when query params are present with a valid tab', (done) => {
173+
const obs = TestBed.runInInjectionContext(() => {
174+
return crisItemPageTabResolver({ params: { id: uuid } } as any, { url: '/entities/publication/1234-65487-12354-1235/details?f.subject=value' } as any);
175+
}) as Observable<RemoteData<PaginatedList<CrisLayoutTab>>>;
176+
177+
obs.pipe(take(1)).subscribe((resolved) => {
178+
expect(router.navigateByUrl).not.toHaveBeenCalled();
179+
expect(hardRedirectService.redirect).not.toHaveBeenCalled();
180+
expect(resolved).toEqual(tabsRD);
181+
done();
182+
});
152183
});
153184
});
154185

‎src/app/item-page/cris-item-page-tab.resolver.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,16 @@ export const crisItemPageTabResolver: ResolveFn<RemoteData<PaginatedList<CrisLay
5757
map((tabsRD: RemoteData<PaginatedList<CrisLayoutTab>>) => {
5858
if (tabsRD.hasSucceeded && tabsRD?.payload?.page?.length > 0) {
5959
// By splitting the url with uuid we can understand if the item is primary item page or a tab
60-
const urlSplit = state.url.split(route.params.id);
61-
const givenTab = urlSplit[1];
60+
const urlWithoutQuery = state.url.split('?')[0];
61+
const urlSplit = urlWithoutQuery.split(route.params.id);
62+
const tabArguments = urlSplit[1]?.split('/');
63+
const givenTab = tabArguments?.[1];
6264
const itemPageRoute = getItemPageRoute(itemRD.payload);
6365

6466
const isValidTab = !givenTab || tabsRD.payload.page.some((tab) => {
6567
const shortnameSplit = tab.shortname.split('::');
6668
const shortname = shortnameSplit[shortnameSplit.length - 1];
67-
return `/${shortname}` === givenTab;
69+
return shortname === givenTab;
6870
});
6971

7072
const mainTab = tabsRD.payload.page.length === 1
@@ -74,7 +76,7 @@ export const crisItemPageTabResolver: ResolveFn<RemoteData<PaginatedList<CrisLay
7476
if (!isValidTab) {
7577
// If wrong tab is given redirect to 404 page
7678
router.navigateByUrl(getPageNotFoundRoute(), { skipLocationChange: true, replaceUrl: false });
77-
} else if (givenTab === `/${mainTab.shortname}`) {
79+
} else if (givenTab === mainTab.shortname) {
7880
if (isPlatformServer(platformId)) {
7981
// If first tab is given redirect to root item page
8082
hardRedirectService.redirect(itemPageRoute, 302);

0 commit comments

Comments
 (0)