Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MM-22555] Auto-fill server URLs when deep linking into the Desktop App and the server isn't configured #3200

Merged
merged 6 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,6 @@
"main.tray.tray.expired": "Session Expired: Please sign in to continue receiving notifications.",
"main.tray.tray.mention": "You have been mentioned",
"main.tray.tray.unread": "You have unread channels",
"main.views.viewManager.handleDeepLink.error.body": "There is no configured server in the app that matches the requested url: {url}",
"main.views.viewManager.handleDeepLink.error.title": "No matching server",
"main.windows.mainWindow.closeApp.dialog.checkboxLabel": "Don't ask again",
"main.windows.mainWindow.closeApp.dialog.detail": "You will no longer receive notifications for messages. If you want to leave {appName} running in the system tray, you can enable this in Settings.",
"main.windows.mainWindow.closeApp.dialog.message": "Are you sure you want to quit?",
Expand Down
2 changes: 1 addition & 1 deletion src/app/serverViewState.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ describe('app/serverViewState', () => {
serverViewState.showNewServerModal();
await promise;

expect(ServerManager.addServer).toHaveBeenCalledWith(data);
expect(ServerManager.addServer).toHaveBeenCalledWith(data, undefined);
expect(serversCopy).toContainEqual(expect.objectContaining({
id: 'server-1',
name: 'new-server',
Expand Down
26 changes: 20 additions & 6 deletions src/app/serverViewState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export class ServerViewState {

constructor() {
ipcMain.on(SWITCH_SERVER, (event, serverId) => this.switchServer(serverId));
ipcMain.on(SHOW_NEW_SERVER_MODAL, this.showNewServerModal);
ipcMain.on(SHOW_NEW_SERVER_MODAL, this.handleShowNewServerModal);
ipcMain.on(SHOW_EDIT_SERVER_MODAL, this.showEditServerModal);
ipcMain.on(SHOW_REMOVE_SERVER_MODAL, this.showRemoveServerModal);
ipcMain.handle(VALIDATE_SERVER_URL, this.handleServerURLValidation);
Expand Down Expand Up @@ -123,25 +123,32 @@ export class ServerViewState {
* Server Modals
*/

showNewServerModal = () => {
log.debug('showNewServerModal');
showNewServerModal = (prefillURL?: string) => {
log.debug('showNewServerModal', {prefillURL});

const mainWindow = MainWindow.get();
if (!mainWindow) {
return;
}

const modalPromise = ModalManager.addModal<null, Server>(
const modalPromise = ModalManager.addModal<{prefillURL?: string}, Server>(
'newServer',
'mattermost-desktop://renderer/newServer.html',
getLocalPreload('internalAPI.js'),
null,
{prefillURL},
mainWindow,
!ServerManager.hasServers(),
);

modalPromise.then((data) => {
const newServer = ServerManager.addServer(data);
let initialLoadURL;
if (prefillURL) {
const parsedServerURL = parseURL(data.url);
if (parsedServerURL) {
initialLoadURL = parseURL(`${parsedServerURL.origin}${prefillURL.substring(prefillURL.indexOf('/'))}`);
}
}
const newServer = ServerManager.addServer(data, initialLoadURL);
this.switchServer(newServer.id, true);
}).catch((e) => {
// e is undefined for user cancellation
Expand All @@ -151,6 +158,8 @@ export class ServerViewState {
});
};

private handleShowNewServerModal = () => this.showNewServerModal();

private showEditServerModal = (e: IpcMainEvent, id: string) => {
log.debug('showEditServerModal', id);

Expand Down Expand Up @@ -281,6 +290,11 @@ export class ServerViewState {
// If the original URL was invalid, don't replace that as they probably have a typo somewhere
// Also strip the trailing slash if it's there so that the user can keep typing
if (!remoteInfo) {
// If the URL provided has a path, try to validate the server with parts of the path removed, until we reach the root and then return a failure
if (parsedURL.pathname !== '/') {
return this.handleServerURLValidation(e, parsedURL.toString().substring(0, parsedURL.toString().lastIndexOf('/')), currentId);
}

return {status: URLValidationStatus.NotMattermost, validatedURL: parsedURL.toString().replace(/\/$/, '')};
}

Expand Down
4 changes: 3 additions & 1 deletion src/common/servers/MattermostServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,16 @@ export class MattermostServer {
name: string;
url!: URL;
isPredefined: boolean;
initialLoadURL?: URL;

constructor(server: Server, isPredefined: boolean) {
constructor(server: Server, isPredefined: boolean, initialLoadURL?: URL) {
this.id = uuid();

this.name = server.name;
this.updateURL(server.url);

this.isPredefined = isPredefined;
this.initialLoadURL = initialLoadURL;
}

updateURL = (url: string) => {
Expand Down
4 changes: 2 additions & 2 deletions src/common/servers/serverManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ export class ServerManager extends EventEmitter {
this.persistServers();
};

addServer = (server: Server) => {
const newServer = new MattermostServer(server, false);
addServer = (server: Server, initialLoadURL?: URL) => {
const newServer = new MattermostServer(server, false, initialLoadURL);

if (this.servers.has(newServer.id)) {
throw new Error('ID Collision detected. Cannot add server.');
Expand Down
2 changes: 1 addition & 1 deletion src/main/app/intercom.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe('main/app/intercom', () => {
ModalManager.addModal.mockReturnValue(promise);

handleWelcomeScreenModal();
expect(ModalManager.addModal).toHaveBeenCalledWith('welcomeScreen', 'mattermost-desktop://renderer/welcomeScreen.html', '/some/preload.js', null, {}, true);
expect(ModalManager.addModal).toHaveBeenCalledWith('welcomeScreen', 'mattermost-desktop://renderer/welcomeScreen.html', '/some/preload.js', {prefillURL: undefined}, {}, true);
});
});

Expand Down
14 changes: 11 additions & 3 deletions src/main/app/intercom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import ServerViewState from 'app/serverViewState';
import {Logger} from 'common/log';
import ServerManager from 'common/servers/serverManager';
import {ping} from 'common/utils/requests';
import {parseURL} from 'common/utils/url';
import NotificationManager from 'main/notifications';
import {getLocalPreload} from 'main/utils';
import ModalManager from 'main/views/modalManager';
Expand Down Expand Up @@ -85,7 +86,7 @@ export function handleMainWindowIsShown() {
}
}

export function handleWelcomeScreenModal() {
export function handleWelcomeScreenModal(prefillURL?: string) {
log.debug('handleWelcomeScreenModal');

const html = 'mattermost-desktop://renderer/welcomeScreen.html';
Expand All @@ -96,10 +97,17 @@ export function handleWelcomeScreenModal() {
if (!mainWindow) {
return;
}
const modalPromise = ModalManager.addModal<null, UniqueServer>('welcomeScreen', html, preload, null, mainWindow, !ServerManager.hasServers());
const modalPromise = ModalManager.addModal<{prefillURL?: string}, UniqueServer>('welcomeScreen', html, preload, {prefillURL}, mainWindow, !ServerManager.hasServers());
if (modalPromise) {
modalPromise.then((data) => {
const newServer = ServerManager.addServer(data);
let initialLoadURL;
if (prefillURL) {
const parsedServerURL = parseURL(data.url);
if (parsedServerURL) {
initialLoadURL = parseURL(`${parsedServerURL.origin}${prefillURL.substring(prefillURL.indexOf('/'))}`);
}
}
const newServer = ServerManager.addServer(data, initialLoadURL);
ServerViewState.switchServer(newServer.id, true);
}).catch((e) => {
// e is undefined for user cancellation
Expand Down
12 changes: 12 additions & 0 deletions src/main/views/modalManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,18 @@ export class ModalManager {
return this.modalPromises.get(key) as Promise<T2>;
};

removeModal = (key: string) => {
const modalView = this.modalQueue.find((modal) => modal.key === key);
if (!modalView) {
return;
}

modalView.hide();
modalView.resolve(null);
this.modalPromises.delete(key);
this.filterActive();
};

findModalByCaller = (event: IpcMainInvokeEvent) => {
if (this.modalQueue.length) {
const requestModal = this.modalQueue.find((modal) => {
Expand Down
21 changes: 12 additions & 9 deletions src/main/views/viewManager.test.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import {dialog} from 'electron';

import ServerViewState from 'app/serverViewState';
import {BROWSER_HISTORY_PUSH, LOAD_SUCCESS, SET_ACTIVE_VIEW} from 'common/communication';
import ServerManager from 'common/servers/serverManager';
Expand All @@ -20,9 +18,6 @@ jest.mock('electron', () => ({
getAppPath: () => '/path/to/app',
getPath: jest.fn(() => '/valid/downloads/path'),
},
dialog: {
showErrorBox: jest.fn(),
},
ipcMain: {
emit: jest.fn(),
on: jest.fn(),
Expand All @@ -33,6 +28,7 @@ jest.mock('app/serverViewState', () => ({
getCurrentServer: jest.fn(),
updateCurrentView: jest.fn(),
init: jest.fn(),
showNewServerModal: jest.fn(),
}));
jest.mock('common/views/View', () => ({
getViewName: jest.fn((a, b) => `${a}-${b}`),
Expand Down Expand Up @@ -62,6 +58,10 @@ jest.mock('main/app/utils', () => ({
flushCookiesStore: jest.fn(),
}));

jest.mock('main/app/intercom', () => ({
handleWelcomeScreenModal: jest.fn(),
}));

jest.mock('main/i18nManager', () => ({
localizeMessage: jest.fn(),
}));
Expand Down Expand Up @@ -116,8 +116,9 @@ jest.mock('./MattermostWebContentsView', () => ({
MattermostWebContentsView: jest.fn(),
}));

jest.mock('./modalManager', () => ({
jest.mock('main/views/modalManager', () => ({
showModal: jest.fn(),
removeModal: jest.fn(),
isModalDisplayed: jest.fn(),
}));
jest.mock('./webContentEvents', () => ({}));
Expand Down Expand Up @@ -321,6 +322,7 @@ describe('main/views/viewManager', () => {
isOpen: true,
url: new URL('http://server1.com/view'),
},
undefined,
);
makeSpy.mockRestore();
});
Expand Down Expand Up @@ -692,11 +694,12 @@ describe('main/views/viewManager', () => {
expect(view.load).not.toHaveBeenCalled();
});

it('should throw dialog when cannot find the view', () => {
it('should open new server modal when using a server that does not exist', () => {
ServerManager.hasServers.mockReturnValue(true);
const view = {...baseView};
viewManager.handleDeepLink('mattermost://server-1.com/deep/link?thing=yes');
viewManager.handleDeepLink('mattermost://server-2.com/deep/link?thing=yes');
expect(view.load).not.toHaveBeenCalled();
expect(dialog.showErrorBox).toHaveBeenCalled();
expect(ServerViewState.showNewServerModal).toHaveBeenCalled();
});

it('should reopen closed view if called upon', () => {
Expand Down
22 changes: 11 additions & 11 deletions src/main/views/viewManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// See LICENSE.txt for license information.

import type {IpcMainEvent, IpcMainInvokeEvent} from 'electron';
import {WebContentsView, dialog, ipcMain} from 'electron';
import {WebContentsView, ipcMain} from 'electron';
import isDev from 'electron-is-dev';

import ServerViewState from 'app/serverViewState';
Expand Down Expand Up @@ -41,18 +41,18 @@ import {getFormattedPathName, parseURL} from 'common/utils/url';
import Utils from 'common/utils/util';
import type {MattermostView} from 'common/views/View';
import {TAB_MESSAGING} from 'common/views/View';
import {handleWelcomeScreenModal} from 'main/app/intercom';
import {flushCookiesStore} from 'main/app/utils';
import DeveloperMode from 'main/developerMode';
import {localizeMessage} from 'main/i18nManager';
import performanceMonitor from 'main/performanceMonitor';
import PermissionsManager from 'main/permissionsManager';
import ModalManager from 'main/views/modalManager';
import MainWindow from 'main/windows/mainWindow';

import type {DeveloperSettings} from 'types/settings';

import LoadingScreen from './loadingScreen';
import {MattermostWebContentsView} from './MattermostWebContentsView';
import modalManager from './modalManager';

import {getLocalPreload, getAdjustedWindowBoundaries} from '../utils';

Expand Down Expand Up @@ -158,14 +158,14 @@ export class ViewManager {
} else {
this.getViewLogger(viewId).warn(`Couldn't find a view with name: ${viewId}`);
}
modalManager.showModal();
ModalManager.showModal();
};

focusCurrentView = () => {
log.debug('focusCurrentView');

if (modalManager.isModalDisplayed()) {
modalManager.focusCurrentModal();
if (ModalManager.isModalDisplayed()) {
ModalManager.focusCurrentModal();
return;
}

Expand Down Expand Up @@ -227,11 +227,11 @@ export class ViewManager {
webContentsView.once(LOAD_FAILED, this.deeplinkFailed);
}
}
} else if (ServerManager.hasServers()) {
ServerViewState.showNewServerModal(`${parsedURL.host}${getFormattedPathName(parsedURL.pathname)}${parsedURL.search}`);
} else {
dialog.showErrorBox(
localizeMessage('main.views.viewManager.handleDeepLink.error.title', 'No matching server'),
localizeMessage('main.views.viewManager.handleDeepLink.error.body', 'There is no configured server in the app that matches the requested url: {url}', {url: parsedURL.toString()}),
);
ModalManager.removeModal('welcomeScreen');
handleWelcomeScreenModal(`${parsedURL.host}${getFormattedPathName(parsedURL.pathname)}${parsedURL.search}`);
}
}
};
Expand Down Expand Up @@ -439,7 +439,7 @@ export class ViewManager {
} else if (recycle) {
views.set(view.id, recycle);
} else {
views.set(view.id, this.makeView(srv, view));
views.set(view.id, this.makeView(srv, view, srv.initialLoadURL?.toString()));
}
}

Expand Down
11 changes: 9 additions & 2 deletions src/renderer/components/ConfigureServer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import 'renderer/css/components/LoadingScreen.css';

type ConfigureServerProps = {
server?: UniqueServer;
prefillURL?: string;
mobileView?: boolean;
darkMode?: boolean;
messageTitle?: string;
Expand All @@ -33,6 +34,7 @@ type ConfigureServerProps = {

function ConfigureServer({
server,
prefillURL,
mobileView,
darkMode,
messageTitle,
Expand All @@ -53,8 +55,8 @@ function ConfigureServer({

const mounted = useRef(false);
const [transition, setTransition] = useState<'inFromRight' | 'outToLeft'>();
const [name, setName] = useState(prevName || '');
const [url, setUrl] = useState(prevURL || '');
const [name, setName] = useState(prevName ?? '');
const [url, setUrl] = useState(prevURL ?? prefillURL ?? '');
const [nameError, setNameError] = useState('');
const [urlError, setURLError] = useState<{type: STATUS; value: string}>();
const [showContent, setShowContent] = useState(false);
Expand All @@ -71,6 +73,11 @@ function ConfigureServer({
setTransition('inFromRight');
setShowContent(true);
mounted.current = true;

if (url) {
fetchValidationResult(url);
}

return () => {
mounted.current = false;
};
Expand Down
8 changes: 8 additions & 0 deletions src/renderer/components/NewServerModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ type Props = {
currentOrder?: number;
setInputRef?: (inputRef: HTMLInputElement) => void;
intl: IntlShape;
prefillURL?: string;
};

type State = {
Expand Down Expand Up @@ -77,6 +78,13 @@ class NewServerModal extends React.PureComponent<Props, State> {
this.mounted = false;
}

componentDidUpdate(prevProps: Readonly<Props>): void {
if (this.props.prefillURL && this.props.prefillURL !== prevProps.prefillURL) {
this.setState({serverUrl: this.props.prefillURL});
this.validateServerURL(this.props.prefillURL);
}
}

initializeOnShow = async () => {
const cameraDisabled = window.process.platform === 'win32' && await window.desktop.getMediaAccessStatus('camera') !== 'granted';
const microphoneDisabled = window.process.platform === 'win32' && await window.desktop.getMediaAccessStatus('microphone') !== 'granted';
Expand Down
Loading
Loading