MOBILE-4120 messages: Fix wrong messages displayed in split view

src/addons/messages/messages-lazy.module.ts

@@ -18,11 +18,18 @@ import { Route, RouterModule, ROUTES, Routes } from '@angular/router';
 import { buildTabMainRoutes } from '@features/mainmenu/mainmenu-tab-routing.module';
 import { AddonMessagesIndexGuard } from './guards';
 
-export const AddonMessagesDiscussionRoute: Route = {
+export const DISCUSSION_ROUTES: Route[] = [
-    path: 'discussion',
+    {
+        path: 'discussion/user/:userId',
         loadChildren: () => import('./pages/discussion/discussion.module')
             .then(m => m.AddonMessagesDiscussionPageModule),
-};
+    },
+    {
+        path: 'discussion/:conversationId',
+        loadChildren: () => import('./pages/discussion/discussion.module')
+            .then(m => m.AddonMessagesDiscussionPageModule),
+    },
+];
 
 function buildRoutes(injector: Injector): Routes {
     return [
@@ -40,7 +47,7 @@ function buildRoutes(injector: Injector): Routes {
             loadChildren: () => import('./pages/group-conversations/group-conversations.module')
                 .then(m => m.AddonMessagesGroupConversationsPageModule),
         },
-        AddonMessagesDiscussionRoute,
+        ...DISCUSSION_ROUTES,
         {
             path: 'search',
             loadChildren: () => import('./pages/search/search.module')

src/addons/messages/pages/contacts-35/contacts.module.ts

@@ -15,7 +15,7 @@
 import { NgModule } from '@angular/core';
 import { RouterModule, Routes } from '@angular/router';
 import { conditionalRoutes } from '@/app/app-routing.module';
-import { AddonMessagesDiscussionRoute } from '@addons/messages/messages-lazy.module';
+import { DISCUSSION_ROUTES } from '@addons/messages/messages-lazy.module';
 import { CoreScreen } from '@services/screen';
 
 import { CoreSharedModule } from '@/core/shared.module';
@@ -28,16 +28,14 @@ const mobileRoutes: Routes = [
         path: '',
         component: AddonMessagesContacts35Page,
     },
-    AddonMessagesDiscussionRoute,
+    ...DISCUSSION_ROUTES,
 ];
 
 const tabletRoutes: Routes = [
     {
         path: '',
         component: AddonMessagesContacts35Page,
-        children: [
+        children: DISCUSSION_ROUTES,
-            AddonMessagesDiscussionRoute,
-        ],
     },
 ];
 

src/addons/messages/pages/contacts-35/contacts.page.ts

@@ -25,7 +25,7 @@ import {
 import { CoreDomUtils } from '@services/utils/dom';
 import { CoreApp } from '@services/app';
 import { CoreEventObserver, CoreEvents } from '@singletons/events';
-import { ActivatedRoute, Params } from '@angular/router';
+import { ActivatedRoute } from '@angular/router';
 import { Translate } from '@singletons';
 import { CoreScreen } from '@services/screen';
 import { CoreNavigator } from '@services/navigator';
@@ -241,15 +241,10 @@ export class AddonMessagesContacts35Page implements OnInit, OnDestroy {
     gotoDiscussion(discussionUserId: number): void {
         this.discussionUserId = discussionUserId;
 
-        const params: Params = {
+        const path = CoreNavigator.getRelativePathToParent('/messages/contacts-35') + `discussion/user/${discussionUserId}`;
-            userId: discussionUserId,
-        };
-
-        const splitViewLoaded = CoreNavigator.isCurrentPathInTablet('**/messages/contacts-35/discussion');
-        const path = (splitViewLoaded ? '../' : '') + 'discussion';
 
         // @todo Check why this is failing on ngInit.
-        CoreNavigator.navigate(path, { params });
+        CoreNavigator.navigate(path);
     }
 
     /**

src/addons/messages/pages/contacts/contacts.module.ts

@@ -15,7 +15,7 @@
 import { NgModule } from '@angular/core';
 import { RouterModule, Routes } from '@angular/router';
 import { conditionalRoutes } from '@/app/app-routing.module';
-import { AddonMessagesDiscussionRoute } from '@addons/messages/messages-lazy.module';
+import { DISCUSSION_ROUTES } from '@addons/messages/messages-lazy.module';
 import { CoreScreen } from '@services/screen';
 
 import { CoreSharedModule } from '@/core/shared.module';
@@ -27,16 +27,14 @@ const mobileRoutes: Routes = [
         path: '',
         component: AddonMessagesContactsPage,
     },
-    AddonMessagesDiscussionRoute,
+    ...DISCUSSION_ROUTES,
 ];
 
 const tabletRoutes: Routes = [
     {
         path: '',
         component: AddonMessagesContactsPage,
-        children: [
+        children: DISCUSSION_ROUTES,
-            AddonMessagesDiscussionRoute,
-        ],
     },
 ];
 

src/addons/messages/pages/contacts/contacts.page.ts

@@ -291,10 +291,8 @@ export class AddonMessagesContactsPage implements OnInit, OnDestroy {
 
         this.selectedUserId = userId;
 
-        const splitViewLoaded = CoreNavigator.isCurrentPathInTablet('**/messages/contacts/discussion');
+        const path = CoreNavigator.getRelativePathToParent('/messages/contacts') + `discussion/user/${userId}`;
-        const path = (splitViewLoaded ? '../' : '') + 'discussion';
+        CoreNavigator.navigate(path);
-
-        CoreNavigator.navigate(path, { params : { userId } });
     }
 
     /**

src/addons/messages/pages/discussion/discussion.page.ts

@@ -154,28 +154,14 @@ export class AddonMessagesDiscussionPage implements OnInit, OnDestroy, AfterView
      * Setup code for the page.
      */
     async ngOnInit(): Promise<void> {
-
+        this.conversationId = CoreNavigator.getRouteNumberParam('conversationId');
-        this.route.queryParams.subscribe(async (params) => {
+        this.userId = CoreNavigator.getRouteNumberParam('userId');
-            const oldConversationId = this.conversationId;
+        this.showInfo = !CoreNavigator.getRouteBooleanParam('hideInfo');
-            const oldUserId = this.userId;
+        this.showKeyboard = !!CoreNavigator.getRouteBooleanParam('showKeyboard');
-            let forceScrollToBottom = false;
-            this.conversationId = CoreNavigator.getRouteNumberParam('conversationId', { params }) || undefined;
-            this.userId = CoreNavigator.getRouteNumberParam('userId', { params }) || undefined;
-            this.showInfo = !params.hideInfo;
-
-            if (oldConversationId != this.conversationId || oldUserId != this.userId) {
-                // Showing reload again can break animations.
-                this.loaded = false;
-                this.initialized = false;
-                forceScrollToBottom = true;
-            }
-
-            this.showKeyboard = CoreNavigator.getRouteBooleanParam('showKeyboard', { params }) || false;
 
         await this.fetchData();
 
-            this.scrollToBottom(forceScrollToBottom);
+        this.scrollToBottom(true);
-        });
     }
 
     /**
@@ -1265,7 +1251,7 @@ export class AddonMessagesDiscussionPage implements OnInit, OnDestroy, AfterView
             });
 
             if (userId !== undefined) {
-                const splitViewLoaded = CoreNavigator.isCurrentPathInTablet('**/messages/**/discussion');
+                const splitViewLoaded = CoreNavigator.isCurrentPathInTablet('**/messages/**/discussion/**');
 
                 // Open user conversation.
                 if (splitViewLoaded) {
@@ -1277,7 +1263,7 @@ export class AddonMessagesDiscussionPage implements OnInit, OnDestroy, AfterView
                     );
                 } else {
                     // Open the discussion in a new view.
-                    CoreNavigator.navigateToSitePath('/messages/discussion', { params: { userId } });
+                    CoreNavigator.navigateToSitePath(`/messages/discussion/user/${userId}`);
                 }
             }
         } else {

src/addons/messages/pages/discussions-35/discussions.module.ts

@@ -16,7 +16,7 @@ import { NgModule } from '@angular/core';
 import { RouterModule, Routes } from '@angular/router';
 import { CoreScreen } from '@services/screen';
 import { conditionalRoutes } from '@/app/app-routing.module';
-import { AddonMessagesDiscussionRoute } from '@addons/messages/messages-lazy.module';
+import { DISCUSSION_ROUTES } from '@addons/messages/messages-lazy.module';
 
 import { CoreSharedModule } from '@/core/shared.module';
 import { CoreSearchComponentsModule } from '@features/search/components/components.module';
@@ -33,7 +33,7 @@ const mobileRoutes: Routes = [
         },
         component: AddonMessagesDiscussions35Page,
     },
-    AddonMessagesDiscussionRoute,
+    ...DISCUSSION_ROUTES,
 ];
 
 const tabletRoutes: Routes = [
@@ -43,9 +43,7 @@ const tabletRoutes: Routes = [
             mainMenuTabRoot: AddonMessagesMainMenuHandlerService.PAGE_NAME,
         },
         component: AddonMessagesDiscussions35Page,
-        children: [
+        children: DISCUSSION_ROUTES,
-            AddonMessagesDiscussionRoute,
-        ],
     },
 ];
 

src/addons/messages/pages/discussions-35/discussions.page.ts

@@ -257,16 +257,13 @@ export class AddonMessagesDiscussions35Page implements OnInit, OnDestroy {
     async gotoDiscussion(discussionUserId: number, messageId?: number): Promise<void> {
         this.discussionUserId = discussionUserId;
 
-        const params: Params = {
+        const params: Params = {};
-            userId: discussionUserId,
-        };
 
         if (messageId) {
             params.message = messageId;
         }
 
-        const splitViewLoaded = CoreNavigator.isCurrentPathInTablet('**/messages/index/discussion');
+        const path = CoreNavigator.getRelativePathToParent('/messages/index') + `discussion/user/${discussionUserId}`;
-        const path = (splitViewLoaded ? '../' : '') + 'discussion';
 
         await CoreNavigator.navigate(path, { params });
     }

src/addons/messages/pages/group-conversations/group-conversations.module.ts

@@ -15,7 +15,7 @@
 import { NgModule } from '@angular/core';
 import { RouterModule, Routes } from '@angular/router';
 import { conditionalRoutes } from '@/app/app-routing.module';
-import { AddonMessagesDiscussionRoute } from '@addons/messages/messages-lazy.module';
+import { DISCUSSION_ROUTES } from '@addons/messages/messages-lazy.module';
 import { CoreScreen } from '@services/screen';
 
 import { CoreSharedModule } from '@/core/shared.module';
@@ -32,7 +32,7 @@ const mobileRoutes: Routes = [
         },
         component: AddonMessagesGroupConversationsPage,
     },
-    AddonMessagesDiscussionRoute,
+    ...DISCUSSION_ROUTES,
 ];
 
 const tabletRoutes: Routes = [
@@ -42,9 +42,7 @@ const tabletRoutes: Routes = [
             mainMenuTabRoot: AddonMessagesMainMenuHandlerService.PAGE_NAME,
         },
         component: AddonMessagesGroupConversationsPage,
-        children: [
+        children: DISCUSSION_ROUTES,
-            AddonMessagesDiscussionRoute,
-        ],
     },
 ];
 

src/addons/messages/pages/group-conversations/group-conversations.page.ts

@@ -519,18 +519,12 @@ export class AddonMessagesGroupConversationsPage implements OnInit, OnDestroy {
         this.selectedUserId = userId;
 
         const params: Params = {};
-        if (conversationId) {
-            params.conversationId = conversationId;
-        }
-        if (userId) {
-            params.userId = userId;
-        }
         if (messageId) {
             params.message = messageId;
         }
 
-        const splitViewLoaded = CoreNavigator.isCurrentPathInTablet('**/messages/group-conversations/discussion');
+        const path = CoreNavigator.getRelativePathToParent('/messages/group-conversations') + 'discussion/' +
-        const path = (splitViewLoaded ? '../' : '') + 'discussion';
+            (conversationId ? conversationId : `user/${userId}`);
 
         await CoreNavigator.navigate(path, { params });
     }
@@ -539,7 +533,7 @@ export class AddonMessagesGroupConversationsPage implements OnInit, OnDestroy {
      * Navigate to message settings.
      */
     gotoSettings(): void {
-        CoreNavigator.navigateToSitePath('../message-settings');
+        CoreNavigator.navigateToSitePath('message-settings');
     }
 
     /**

src/addons/messages/pages/search/search.module.ts

@@ -16,7 +16,7 @@ import { NgModule } from '@angular/core';
 import { RouterModule, Routes } from '@angular/router';
 import { CoreScreen } from '@services/screen';
 import { conditionalRoutes } from '@/app/app-routing.module';
-import { AddonMessagesDiscussionRoute } from '@addons/messages/messages-lazy.module';
+import { DISCUSSION_ROUTES } from '@addons/messages/messages-lazy.module';
 
 import { CoreSharedModule } from '@/core/shared.module';
 import { CoreSearchComponentsModule } from '@features/search/components/components.module';
@@ -28,16 +28,14 @@ const mobileRoutes: Routes = [
         path: '',
         component: AddonMessagesSearchPage,
     },
-    AddonMessagesDiscussionRoute,
+    ...DISCUSSION_ROUTES,
 ];
 
 const tabletRoutes: Routes = [
     {
         path: '',
         component: AddonMessagesSearchPage,
-        children: [
+        children: DISCUSSION_ROUTES,
-            AddonMessagesDiscussionRoute,
-        ],
     },
 ];
 

src/addons/messages/pages/search/search.page.ts

@@ -24,7 +24,6 @@ import {
 import { CoreDomUtils } from '@services/utils/dom';
 import { CoreApp } from '@services/app';
 import { CoreNavigator } from '@services/navigator';
-import { Params } from '@angular/router';
 import { CoreScreen } from '@services/screen';
 
 /**
@@ -107,9 +106,9 @@ export class AddonMessagesSearchPage implements OnDestroy {
         this.displayResults = false;
 
         // Empty details.
-        const splitViewLoaded = CoreNavigator.isCurrentPathInTablet('**/messages/search/discussion');
+        const path = CoreNavigator.getRelativePathToParent('/messages/search');
-        if (splitViewLoaded) {
+        if (path) {
-            CoreNavigator.navigate('../');
+            CoreNavigator.navigate(path);
         }
     }
 
@@ -250,16 +249,18 @@ export class AddonMessagesSearchPage implements OnDestroy {
         if (!onInit || CoreScreen.isTablet) {
             this.selectedResult = result;
 
-            const params: Params = {};
+            let conversationId: number | undefined;
+            let userId: number | undefined;
             if ('conversationid' in result) {
-                params.conversationId = result.conversationid;
+                conversationId = result.conversationid;
             } else {
-                params.userId = result.id;
+                userId = result.id;
             }
 
-            const splitViewLoaded = CoreNavigator.isCurrentPathInTablet('**/messages/search/discussion');
+            const path = CoreNavigator.getRelativePathToParent('/messages/search') + 'discussion/' +
-            const path = (splitViewLoaded ? '../' : '') + 'discussion';
+                (conversationId ? conversationId : `user/${userId}`);
-            CoreNavigator.navigate(path, { params });
+
+            CoreNavigator.navigate(path);
         }
     }
 

src/addons/messages/services/handlers/discussion-link.ts

@@ -46,10 +46,8 @@ export class AddonMessagesDiscussionLinkHandlerService extends CoreContentLinksH
     ): CoreContentLinksAction[] | Promise<CoreContentLinksAction[]> {
         return [{
             action: (siteId): void => {
-                const stateParams = {
+                const userId = parseInt(params.id || params.user2, 10);
-                    userId: parseInt(params.id || params.user2, 10),
+                CoreNavigator.navigateToSitePath(`/messages/discussion/user/${userId}`, { siteId });
-                };
-                CoreNavigator.navigateToSitePath('/messages/discussion', { params: stateParams, siteId });
             },
         }];
     }

src/addons/messages/services/handlers/push-click.ts

@@ -13,7 +13,6 @@
 // limitations under the License.
 
 import { Injectable } from '@angular/core';
-import { Params } from '@angular/router';
 import { CorePushNotificationsClickHandler } from '@features/pushnotifications/services/push-delegate';
 import { CorePushNotificationsNotificationBasicData } from '@features/pushnotifications/services/pushnotifications';
 import { CoreNavigator } from '@services/navigator';
@@ -63,28 +62,22 @@ export class AddonMessagesPushClickHandlerService implements CorePushNotificatio
         // Check if group messaging is enabled, to determine which page should be loaded.
         const enabled = await AddonMessages.isGroupMessagingEnabledInSite(notification.site);
 
-        let nextPageParams: Params | undefined;
+        let conversationId: number | undefined;
+        let userId: number | undefined;
 
         // Check if we have enough information to open the conversation.
         if (notification.convid && enabled) {
-            nextPageParams = {
+            conversationId = Number(notification.convid);
-                conversationId: Number(notification.convid),
-            };
         } else if (notification.userfromid) {
-            nextPageParams = {
+            userId = Number(notification.userfromid);
-                userId: Number(notification.userfromid),
-            };
         }
 
         await CoreNavigator.navigateToSitePath(AddonMessagesMainMenuHandlerService.PAGE_NAME, {
             siteId: notification.site,
             preferCurrentTab: false,
-            nextNavigation: nextPageParams ?
+            nextNavigation: conversationId ?
-                {
+                { path: `discussion/${conversationId}` } :
-                    path: 'discussion',
+                (userId ? { path: `discussion/user/${userId}` } : undefined),
-                    options: { params: nextPageParams },
-                } :
-                undefined,
         });
     }
 

src/addons/messages/services/handlers/user-send-message.ts

@@ -71,10 +71,9 @@ export class AddonMessagesSendMessageUserHandlerService implements CoreUserProfi
 
                 const pageParams: Params = {
                     showKeyboard: true,
-                    userId: user.id,
                     hideInfo: true,
                 };
-                CoreNavigator.navigateToSitePath('/messages/discussion', { params: pageParams });
+                CoreNavigator.navigateToSitePath(`/messages/discussion/user/${user.id}`, { params: pageParams });
             },
         };
     }

src/core/services/navigator.ts

@@ -700,6 +700,29 @@ export class CoreNavigatorService {
         return promise;
     }
 
+    /**
+     * Get the relative path to a parent path.
+     * E.g. if parent path is '/foo' and current path is '/foo/bar/baz' it will return '../../'.
+     *
+     * @param parentPath Parent path.
+     * @return Relative path to the parent, empty if same path or parent path not found.
+     * @todo If messaging is refactored to use list managers, this function might not be needed anymore.
+     */
+    getRelativePathToParent(parentPath: string): string {
+        // Add an ending slash to avoid collisions with other routes (e.g. /foo and /foobar).
+        parentPath = CoreTextUtils.addEndingSlash(parentPath);
+
+        const path = this.getCurrentPath();
+        const parentRouteIndex = path.indexOf(parentPath);
+        if (parentRouteIndex === -1) {
+            return '';
+        }
+
+        const depth = (path.substring(parentRouteIndex + parentPath.length - 1).match(/\//g) ?? []).length;
+
+        return '../'.repeat(depth);
+    }
+
 }
 
 export const CoreNavigator = makeSingleton(CoreNavigatorService);

src/core/services/tests/navigator.test.ts

@@ -14,7 +14,7 @@
 
 import { NavController as NavControllerService } from '@ionic/angular';
 
-import { mockSingleton } from '@/testing/utils';
+import { mock, mockSingleton } from '@/testing/utils';
 
 import { CoreNavigatorService } from '@services/navigator';
 import { NavController, Router } from '@singletons';
@@ -156,6 +156,22 @@ describe('CoreNavigator', () => {
         expect(navControllerMock.navigateRoot).toHaveBeenCalledWith(['/main/initialpage'], {});
     });
 
+    it('calculates relative paths to parent paths', () => {
+        navigator = mock(navigator, {
+            getCurrentPath: () => '/foo/bar/baz/xyz',
+        });
+
+        expect(navigator.getRelativePathToParent('/foo/bar/baz/xyz')).toEqual('');
+        expect(navigator.getRelativePathToParent('/foo/bar/baz')).toEqual('../');
+        expect(navigator.getRelativePathToParent('/foo/bar')).toEqual('../../');
+        expect(navigator.getRelativePathToParent('/bar')).toEqual('../../');
+        expect(navigator.getRelativePathToParent('/foo')).toEqual('../../../');
+        expect(navigator.getRelativePathToParent('/foo/')).toEqual('../../../');
+        expect(navigator.getRelativePathToParent('foo')).toEqual('../../../');
+        expect(navigator.getRelativePathToParent('/invalid')).toEqual('');
+        expect(navigator.getRelativePathToParent('/fo')).toEqual('');
+    });
+
     it.todo('navigates to a different site');
     it.todo('navigates to login credentials');
     it.todo('navigates to NO_SITE_ID site');