From 77d3ac9d4348fdb595c8d636b91ebcc6b98d9e48 Mon Sep 17 00:00:00 2001 From: Noel De Martin Date: Tue, 14 May 2024 11:38:34 +0200 Subject: [PATCH] MOBILE-4470 angular: Fix snapshot types Route snapshots are typed as non-optional, but we found a situation where it was undefined. Looking at the Angular source code, it seems like indeed snapshots can be undefined but they have been declared with a definite assignment assertion: https://github.com/angular/angular/blob/17.3.0/packages/router/src/router_state.ts#L231 --- patches/@angular+router+17.3.7.patch | 13 +++ src/addons/calendar/pages/event/event.ts | 4 +- .../pages/competency/competency.page.ts | 4 +- .../submission-review/submission-review.ts | 4 +- .../mod/feedback/pages/attempt/attempt.ts | 4 +- .../mod/forum/pages/discussion/discussion.ts | 6 +- .../pages/new-discussion/new-discussion.ts | 6 +- src/addons/mod/glossary/pages/entry/entry.ts | 6 +- .../mod/wiki/services/handlers/create-link.ts | 8 +- .../pages/notification/notification.ts | 4 +- .../items-management/list-items-manager.ts | 6 +- .../swipe-navigation-items-manager.ts | 4 +- .../swipe-navigation-items-manager.test.ts | 7 +- src/core/components/split-view/split-view.ts | 2 +- src/core/features/course/pages/index/index.ts | 4 +- src/core/features/course/services/course.ts | 2 +- .../grades/pages/course/course.page.ts | 6 +- .../features/user/pages/profile/profile.ts | 6 +- src/core/services/navigator.ts | 97 +++++++++++++++++-- 19 files changed, 135 insertions(+), 58 deletions(-) create mode 100644 patches/@angular+router+17.3.7.patch diff --git a/patches/@angular+router+17.3.7.patch b/patches/@angular+router+17.3.7.patch new file mode 100644 index 000000000..06367c788 --- /dev/null +++ b/patches/@angular+router+17.3.7.patch @@ -0,0 +1,13 @@ +diff --git a/node_modules/@angular/router/index.d.ts b/node_modules/@angular/router/index.d.ts +index b8d7cc8..6511edf 100755 +--- a/node_modules/@angular/router/index.d.ts ++++ b/node_modules/@angular/router/index.d.ts +@@ -58,7 +58,7 @@ export declare class ActivatedRoute { + /** The component of the route, a constant. */ + component: Type | null; + /** The current snapshot of this route */ +- snapshot: ActivatedRouteSnapshot; ++ snapshot?: ActivatedRouteSnapshot; + /** An Observable of the resolved route title */ + readonly title: Observable; + /** An observable of the URL segments matched by this route. */ diff --git a/src/addons/calendar/pages/event/event.ts b/src/addons/calendar/pages/event/event.ts index c4d62ace6..490aec001 100644 --- a/src/addons/calendar/pages/event/event.ts +++ b/src/addons/calendar/pages/event/event.ts @@ -675,9 +675,7 @@ class AddonCalendarEventsSwipeItemsManager extends CoreSwipeNavigationItemsManag * @inheritdoc */ protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { - const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; - - return snapshot.params.id; + return CoreNavigator.getRouteParams(route).id; } } diff --git a/src/addons/competency/pages/competency/competency.page.ts b/src/addons/competency/pages/competency/competency.page.ts index c40a22c13..f7482a6ee 100644 --- a/src/addons/competency/pages/competency/competency.page.ts +++ b/src/addons/competency/pages/competency/competency.page.ts @@ -351,9 +351,7 @@ class AddonCompetencyCompetenciesSwipeManager * @inheritdoc */ protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { - const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; - - return snapshot.params.competencyId; + return CoreNavigator.getRouteParams(route).competencyId; } } diff --git a/src/addons/mod/assign/pages/submission-review/submission-review.ts b/src/addons/mod/assign/pages/submission-review/submission-review.ts index 0c65440a7..a50111338 100644 --- a/src/addons/mod/assign/pages/submission-review/submission-review.ts +++ b/src/addons/mod/assign/pages/submission-review/submission-review.ts @@ -246,9 +246,7 @@ class AddonModAssignSubmissionSwipeItemsManager extends CoreSwipeNavigationItems * @inheritdoc */ protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { - const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; - - return snapshot.params.submitId; + return CoreNavigator.getRouteParams(route).submitId; } } diff --git a/src/addons/mod/feedback/pages/attempt/attempt.ts b/src/addons/mod/feedback/pages/attempt/attempt.ts index 58c612a2d..540ee2f81 100644 --- a/src/addons/mod/feedback/pages/attempt/attempt.ts +++ b/src/addons/mod/feedback/pages/attempt/attempt.ts @@ -188,9 +188,7 @@ class AddonModFeedbackAttemptsSwipeManager extends CoreSwipeNavigationItemsManag * @inheritdoc */ protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { - const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; - - return snapshot.params.attemptId; + return CoreNavigator.getRouteParams(route).attemptId; } } diff --git a/src/addons/mod/forum/pages/discussion/discussion.ts b/src/addons/mod/forum/pages/discussion/discussion.ts index 6828327f7..a4635b9dd 100644 --- a/src/addons/mod/forum/pages/discussion/discussion.ts +++ b/src/addons/mod/forum/pages/discussion/discussion.ts @@ -134,7 +134,7 @@ export class AddonModForumDiscussionPage implements OnInit, AfterViewInit, OnDes async ngOnInit(): Promise { try { - const routeData = this.route.snapshot.data; + const routeData = CoreNavigator.getRouteData(this.route); this.courseId = CoreNavigator.getRouteNumberParam('courseId'); this.cmId = CoreNavigator.getRouteNumberParam('cmId'); this.forumId = CoreNavigator.getRouteNumberParam('forumId'); @@ -893,9 +893,9 @@ class AddonModForumDiscussionDiscussionsSwipeManager extends AddonModForumDiscus * @inheritdoc */ protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { - const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; + const params = CoreNavigator.getRouteParams(route); - return this.getSource().DISCUSSIONS_PATH_PREFIX + snapshot.params.discussionId; + return this.getSource().DISCUSSIONS_PATH_PREFIX + params.discussionId; } } diff --git a/src/addons/mod/forum/pages/new-discussion/new-discussion.ts b/src/addons/mod/forum/pages/new-discussion/new-discussion.ts index 052397773..54e6512e7 100644 --- a/src/addons/mod/forum/pages/new-discussion/new-discussion.ts +++ b/src/addons/mod/forum/pages/new-discussion/new-discussion.ts @@ -125,7 +125,7 @@ export class AddonModForumNewDiscussionPage implements OnInit, OnDestroy, CanLea */ async ngOnInit(): Promise { try { - const routeData = this.route.snapshot.data; + const routeData = CoreNavigator.getRouteData(this.route); this.courseId = CoreNavigator.getRequiredRouteNumberParam('courseId'); this.cmId = CoreNavigator.getRequiredRouteNumberParam('cmId'); this.forumId = CoreNavigator.getRequiredRouteNumberParam('forumId'); @@ -700,9 +700,9 @@ class AddonModForumNewDiscussionDiscussionsSwipeManager extends AddonModForumDis * @inheritdoc */ protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { - const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; + const params = CoreNavigator.getRouteParams(route); - return `${this.getSource().DISCUSSIONS_PATH_PREFIX}new/${snapshot.params.timeCreated}`; + return `${this.getSource().DISCUSSIONS_PATH_PREFIX}new/${params.timeCreated}`; } } diff --git a/src/addons/mod/glossary/pages/entry/entry.ts b/src/addons/mod/glossary/pages/entry/entry.ts index f4b946f77..4daad9772 100644 --- a/src/addons/mod/glossary/pages/entry/entry.ts +++ b/src/addons/mod/glossary/pages/entry/entry.ts @@ -103,7 +103,7 @@ export class AddonModGlossaryEntryPage implements OnInit, OnDestroy { this.cmId = CoreNavigator.getRequiredRouteNumberParam('cmId'); const entrySlug = CoreNavigator.getRequiredRouteParam('entrySlug'); - const routeData = this.route.snapshot.data; + const routeData = CoreNavigator.getRouteData(this.route); const source = CoreRoutedItemsManagerSourcesTracker.getOrCreateSource( AddonModGlossaryEntriesSource, [this.courseId, this.cmId, routeData.glossaryPathPrefix ?? ''], @@ -368,9 +368,9 @@ class AddonModGlossaryEntryEntriesSwipeManager * @inheritdoc */ protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { - const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; + const params = CoreNavigator.getRouteParams(route); - return `${this.getSource().GLOSSARY_PATH_PREFIX}entry/${snapshot.params.entrySlug}`; + return `${this.getSource().GLOSSARY_PATH_PREFIX}entry/${params.entrySlug}`; } } diff --git a/src/addons/mod/wiki/services/handlers/create-link.ts b/src/addons/mod/wiki/services/handlers/create-link.ts index c25c37723..25f604ef9 100644 --- a/src/addons/mod/wiki/services/handlers/create-link.ts +++ b/src/addons/mod/wiki/services/handlers/create-link.ts @@ -49,8 +49,8 @@ export class AddonModWikiCreateLinkHandlerService extends CoreContentLinksHandle return false; } - const params = route.snapshot.params; - const queryParams = route.snapshot.queryParams; + const params = CoreNavigator.getRouteParams(route); + const queryParams = CoreNavigator.getRouteQueryParams(route); if (queryParams.subwikiId == subwikiId) { // Same subwiki, so it's same wiki. @@ -116,7 +116,9 @@ export class AddonModWikiCreateLinkHandlerService extends CoreContentLinksHandle if (isSameWiki) { // User is seeing the wiki, we can get the module from the wiki params. - path = path + `/${route.snapshot.params.courseId}/${route.snapshot.params.cmId}/edit`; + const routeParams = CoreNavigator.getRouteParams(route); + + path = path + `/${routeParams.courseId}/${routeParams.cmId}/edit`; } else if (wikiId) { // The URL specifies which wiki it belongs to. Get the module. const module = await CoreCourse.getModuleBasicInfoByInstance( diff --git a/src/addons/notifications/pages/notification/notification.ts b/src/addons/notifications/pages/notification/notification.ts index 79e674175..1a8c9ddc2 100644 --- a/src/addons/notifications/pages/notification/notification.ts +++ b/src/addons/notifications/pages/notification/notification.ts @@ -212,9 +212,7 @@ class AddonNotificationSwipeItemsManager extends CoreSwipeNavigationItemsManager * @inheritdoc */ protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { - const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; - - return snapshot.params.id; + return CoreNavigator.getRouteParams(route).id; } } diff --git a/src/core/classes/items-management/list-items-manager.ts b/src/core/classes/items-management/list-items-manager.ts index 035d0b845..4761374d7 100644 --- a/src/core/classes/items-management/list-items-manager.ts +++ b/src/core/classes/items-management/list-items-manager.ts @@ -245,9 +245,7 @@ export class CoreListItemsManager< while (route.firstChild) { route = route.firstChild; - const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; - - segments.push(...snapshot.url); + segments.push(...CoreNavigator.getRouteUrl(route)); } return segments.map(segment => segment.path).join('/').replace(/\/+/, '/').trim() || null; @@ -276,7 +274,7 @@ export class CoreListItemsManager< */ private buildRouteMatcher(): (route: ActivatedRouteSnapshot) => boolean { if (this.pageRouteLocator instanceof ActivatedRoute) { - const pageRoutePath = CoreNavigator.getRouteFullPath(this.pageRouteLocator.snapshot); + const pageRoutePath = CoreNavigator.getRouteFullPath(this.pageRouteLocator); return route => CoreNavigator.getRouteFullPath(route) === pageRoutePath; } diff --git a/src/core/classes/items-management/swipe-navigation-items-manager.ts b/src/core/classes/items-management/swipe-navigation-items-manager.ts index 71e9cb201..66ec17612 100644 --- a/src/core/classes/items-management/swipe-navigation-items-manager.ts +++ b/src/core/classes/items-management/swipe-navigation-items-manager.ts @@ -85,9 +85,7 @@ export class CoreSwipeNavigationItemsManager< const segments: UrlSegment[] = []; while (route) { - const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; - - segments.push(...snapshot.url); + segments.push(...CoreNavigator.getRouteUrl(route)); if (!route.firstChild) { break; diff --git a/src/core/classes/tests/swipe-navigation-items-manager.test.ts b/src/core/classes/tests/swipe-navigation-items-manager.test.ts index 07ae31810..19cb1b9a6 100644 --- a/src/core/classes/tests/swipe-navigation-items-manager.test.ts +++ b/src/core/classes/tests/swipe-navigation-items-manager.test.ts @@ -13,10 +13,11 @@ // limitations under the License. import { mock, mockSingleton } from '@/testing/utils'; -import { ActivatedRoute, ActivatedRouteSnapshot, UrlSegment } from '@angular/router'; +import { ActivatedRoute, UrlSegment } from '@angular/router'; import { CoreRoutedItemsManagerSource } from '@classes/items-management/routed-items-manager-source'; import { CoreSwipeNavigationItemsManager } from '@classes/items-management/swipe-navigation-items-manager'; import { CoreNavigator } from '@services/navigator'; +import { BehaviorSubject } from 'rxjs'; interface Item { path: string; @@ -61,9 +62,7 @@ describe('CoreSwipeNavigationItemsManager', () => { mockSingleton(CoreNavigator, { navigate: jest.fn(), getCurrentRoute: () => mock({ - snapshot: mock({ - url: [mock({ path: currentPath })], - }), + url: new BehaviorSubject([mock({ path: currentPath })]), }), }); diff --git a/src/core/components/split-view/split-view.ts b/src/core/components/split-view/split-view.ts index fa23064be..e7d8fc6af 100644 --- a/src/core/components/split-view/split-view.ts +++ b/src/core/components/split-view/split-view.ts @@ -95,7 +95,7 @@ export class CoreSplitViewComponent implements AfterViewInit, OnDestroy { this.updateClasses(); - this.outletRouteSubject.next(outletRoute); + this.outletRouteSubject.next(outletRoute ?? null); } /** diff --git a/src/core/features/course/pages/index/index.ts b/src/core/features/course/pages/index/index.ts index fe16b6e3b..b784c977d 100644 --- a/src/core/features/course/pages/index/index.ts +++ b/src/core/features/course/pages/index/index.ts @@ -111,7 +111,7 @@ export class CoreCourseIndexPage implements OnInit, OnDestroy { */ async ngOnInit(): Promise { // Increase route depth. - const path = CoreNavigator.getRouteFullPath(this.route.snapshot); + const path = CoreNavigator.getRouteFullPath(this.route); CoreNavigator.increaseRouteDepth(path.replace(/(\/deep)+/, '')); @@ -247,7 +247,7 @@ export class CoreCourseIndexPage implements OnInit, OnDestroy { * @inheritdoc */ ngOnDestroy(): void { - const path = CoreNavigator.getRouteFullPath(this.route.snapshot); + const path = CoreNavigator.getRouteFullPath(this.route); CoreNavigator.decreaseRouteDepth(path.replace(/(\/deep)+/, '')); this.selectTabObserver?.off(); diff --git a/src/core/features/course/services/course.ts b/src/core/features/course/services/course.ts index 6542edd7f..4f8e01138 100644 --- a/src/core/features/course/services/course.ts +++ b/src/core/features/course/services/course.ts @@ -307,7 +307,7 @@ export class CoreCourseProvider { return false; } - return Number(route.snapshot.params.courseId) == courseId; + return Number(CoreNavigator.getRouteParams(route).courseId) == courseId; } /** diff --git a/src/core/features/grades/pages/course/course.page.ts b/src/core/features/grades/pages/course/course.page.ts index a715394c3..16f34780d 100644 --- a/src/core/features/grades/pages/course/course.page.ts +++ b/src/core/features/grades/pages/course/course.page.ts @@ -79,7 +79,7 @@ export class CoreGradesCoursePage implements AfterViewInit, OnDestroy { this.collapseLabel = Translate.instant('core.collapse'); this.useLegacyLayout = !CoreSites.getRequiredCurrentSite().isVersionGreaterEqualThan('4.1'); - switch (route.snapshot.data.swipeManagerSource ?? route.snapshot.parent?.data.swipeManagerSource) { + switch (route.snapshot?.data.swipeManagerSource ?? route.snapshot?.parent?.data.swipeManagerSource) { case 'courses': this.swipeManager = new CoreGradesCourseCoursesSwipeManager( CoreRoutedItemsManagerSourcesTracker.getOrCreateSource(CoreGradesCoursesSource, []), @@ -331,9 +331,7 @@ class CoreGradesCourseParticipantsSwipeManager extends CoreSwipeNavigationItemsM * @inheritdoc */ protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { - const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; - - return snapshot.params.userId; + return CoreNavigator.getRouteParams(route).userId; } } diff --git a/src/core/features/user/pages/profile/profile.ts b/src/core/features/user/pages/profile/profile.ts index 8e61e47b9..1e5538dbb 100644 --- a/src/core/features/user/pages/profile/profile.ts +++ b/src/core/features/user/pages/profile/profile.ts @@ -117,7 +117,7 @@ export class CoreUserProfilePage implements OnInit, OnDestroy { this.courseId = undefined; } - if (this.courseId && this.route.snapshot.data.swipeManagerSource === 'participants') { + if (this.courseId && CoreNavigator.getRouteData(this.route).swipeManagerSource === 'participants') { const search = CoreNavigator.getRouteParam('search'); const source = CoreRoutedItemsManagerSourcesTracker.getOrCreateSource( CoreUserParticipantsSource, @@ -252,9 +252,7 @@ class CoreUserSwipeItemsManager extends CoreSwipeNavigationItemsManager { * @inheritdoc */ protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { - const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; - - return snapshot.params.userId; + return CoreNavigator.getRouteParams(route).userId; } } diff --git a/src/core/services/navigator.ts b/src/core/services/navigator.ts index c94faf4ed..1e1001cd9 100644 --- a/src/core/services/navigator.ts +++ b/src/core/services/navigator.ts @@ -13,7 +13,7 @@ // limitations under the License. import { Injectable } from '@angular/core'; -import { ActivatedRoute, ActivatedRouteSnapshot, NavigationEnd, Params } from '@angular/router'; +import { ActivatedRoute, ActivatedRouteSnapshot, Data, NavigationEnd, Params, UrlSegment } from '@angular/router'; import { NavigationOptions } from '@ionic/angular/common/providers/nav-controller'; @@ -32,6 +32,7 @@ import { CoreMainMenuDelegate } from '@features/mainmenu/services/mainmenu-deleg import { CorePlatform } from '@services/platform'; import { filter } from 'rxjs/operators'; import { CorePromisedValue } from '@classes/promised-value'; +import { BehaviorSubject } from 'rxjs'; /** * Redirect payload. @@ -459,7 +460,7 @@ export class CoreNavigatorService { return route; } - if (routeData && CoreUtils.basicLeftCompare(routeData, route.snapshot.data, 3)) { + if (routeData && CoreUtils.basicLeftCompare(routeData, this.getRouteData(route), 3)) { return route; } @@ -477,11 +478,11 @@ export class CoreNavigatorService { * @returns Whether the route is active or not. */ isRouteActive(route: ActivatedRoute): boolean { - const routePath = this.getRouteFullPath(route.snapshot); + const routePath = this.getRouteFullPath(route); let activeRoute: ActivatedRoute | null = Router.routerState.root; while (activeRoute) { - if (this.getRouteFullPath(activeRoute.snapshot) === routePath) { + if (this.getRouteFullPath(activeRoute) === routePath) { return true; } @@ -650,13 +651,13 @@ export class CoreNavigatorService { * @param route Route snapshot. * @returns Path. */ - getRouteFullPath(route: ActivatedRouteSnapshot | null): string { + getRouteFullPath(route: ActivatedRouteSnapshot | ActivatedRoute | null): string { if (!route) { return ''; } - const parentPath = this.getRouteFullPath(route.parent); - const routePath = route.url.join('/'); + const parentPath = this.getRouteFullPath(this.getRouteParent(route)); + const routePath = this.getRouteUrl(route).join('/'); if (!parentPath && !routePath) { return ''; @@ -669,13 +670,63 @@ export class CoreNavigatorService { } } + /** + * Given a route, get url segments. + * + * @param route Route. + * @returns Url segments. + */ + getRouteUrl(route: ActivatedRouteSnapshot | ActivatedRoute): UrlSegment[] { + return this.getRouteProperty(route, 'url', []); + } + + /** + * Given a route, get its parent. + * + * @param route Route. + * @returns Parent. + */ + getRouteParent(route: ActivatedRouteSnapshot | ActivatedRoute): ActivatedRouteSnapshot | ActivatedRoute | null { + return this.getRouteProperty(route, 'parent', null); + } + + /** + * Given a route, get its data. + * + * @param route Route. + * @returns Data. + */ + getRouteData(route: ActivatedRouteSnapshot | ActivatedRoute): Data { + return this.getRouteProperty(route, 'data', {}); + } + + /** + * Given a route, get its params. + * + * @param route Route. + * @returns Params. + */ + getRouteParams(route: ActivatedRouteSnapshot | ActivatedRoute): Params { + return this.getRouteProperty(route, 'params', {}); + } + + /** + * Given a route, get its query params. + * + * @param route Route. + * @returns Query params. + */ + getRouteQueryParams(route: ActivatedRouteSnapshot | ActivatedRoute): Params { + return this.getRouteProperty(route, 'queryParams', {}); + } + /** * Check if the current route page can block leaving the route. * * @returns Whether the current route page can block leaving the route. */ currentRouteCanBlockLeave(): boolean { - return !!this.getCurrentRoute().snapshot.routeConfig?.canDeactivate?.length; + return !!this.getCurrentRoute().snapshot?.routeConfig?.canDeactivate?.length; } /** @@ -725,6 +776,36 @@ export class CoreNavigatorService { return '../'.repeat(depth); } + /** + * Given a route, get one of its properties. + * + * @param route Route. + * @param property Route property. + * @param defaultValue Fallback value if the property is not set. + * @returns Property value. + */ + private getRouteProperty( + route: ActivatedRouteSnapshot | ActivatedRoute, + property: T, + defaultValue: ActivatedRouteSnapshot[T], + ): ActivatedRouteSnapshot[T] { + if (route instanceof ActivatedRouteSnapshot) { + return route[property]; + } + + if (route.snapshot instanceof ActivatedRouteSnapshot) { + return route.snapshot[property]; + } + + const propertyObservable = route[property]; + + if (propertyObservable instanceof BehaviorSubject) { + return propertyObservable.value; + } + + return defaultValue; + } + } export const CoreNavigator = makeSingleton(CoreNavigatorService);