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-55152] Add new Desktop API endpoints, improve preload script, some clean-up #2900

Merged
merged 23 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
37c95d6
Add constants for app info, add to API
devinbinnie Oct 27, 2023
3f03ee3
Migrate history button
devinbinnie Oct 30, 2023
0f21faf
Converted calls API over to context bridge, removed some unnecessary …
devinbinnie Nov 2, 2023
dcd925d
Merge remote-tracking branch 'upstream/master' into MM-55152
devinbinnie Nov 2, 2023
2f00c90
Convert to TS, add types for web app to consume
devinbinnie Nov 2, 2023
4ef50bb
Fix tests, prune
devinbinnie Nov 3, 2023
f64aad0
Fix lint
devinbinnie Nov 3, 2023
8a8d984
Merge remote-tracking branch 'upstream/master' into MM-55152
devinbinnie Nov 10, 2023
8ede07c
More changes to support the legacy API
devinbinnie Nov 13, 2023
1479976
Force legacy code off, add support for unreads/mentions/expired throu…
devinbinnie Nov 14, 2023
f1065c2
Fix issues with cross-tab login, removed need for log in/log out sign…
devinbinnie Nov 14, 2023
6aff875
Merge remote-tracking branch 'upstream/master' into MM-55152
devinbinnie Nov 14, 2023
da4c97a
Fixed test, typos
devinbinnie Nov 14, 2023
96224f8
Change package name for types
devinbinnie Nov 15, 2023
7fbbc81
Add some other stuff to the types
devinbinnie Nov 15, 2023
e7a06fa
PR feedback
devinbinnie Nov 16, 2023
ccf282b
More feedback
devinbinnie Nov 20, 2023
77e5f5b
Use npm package
devinbinnie Nov 20, 2023
52704b0
Change types and API to provide off listeners
devinbinnie Nov 21, 2023
3975133
Version number
devinbinnie Nov 21, 2023
965c49a
Lock
devinbinnie Nov 21, 2023
d9bece0
Fix typo
devinbinnie Nov 23, 2023
02d80b1
Add sessionID for calls
devinbinnie Nov 23, 2023
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: 1 addition & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@
"src/main/downloadURL.ts",
"src/main/SpellChecker.ts",
"src/main/menus/app.ts",
"src/main/preload/mattermost.js",
"src/main/preload/externalAPI.js",
"src/renderer/components/RemoveServerModal.tsx",
"src/renderer/components/MainPage.tsx",
"src/renderer/components/HoveringURL.tsx",
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,5 @@ fastlane/README.md
fastlane/report.xml

*.provisionprofile

src/types/api/lib/*
2 changes: 1 addition & 1 deletion src/app/serverViewState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export class ServerViewState {
}

getCurrentServer = () => {
log.debug('getCurrentServer');
log.silly('getCurrentServer');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These logs kept just getting in the way as some of these methods are called very frequently (ie. on every click) so I changed the logging level on them.


if (!this.currentServerId) {
throw new Error('No server set as current');
Expand Down
2 changes: 1 addition & 1 deletion src/common/appState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class AppState extends EventEmitter {
updateExpired = (viewId: string, expired: boolean) => {
ServerManager.getViewLog(viewId, 'AppState').silly('updateExpired', expired);

this.unreads.set(viewId, expired);
this.expired.set(viewId, expired);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was just wrong :P

this.emitStatusForView(viewId);
}

Expand Down
15 changes: 12 additions & 3 deletions src/common/communication.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

export const GET_APP_INFO = 'get-app-info';

export const SWITCH_SERVER = 'switch-server';
export const SWITCH_TAB = 'switch-tab';
export const CLOSE_VIEW = 'close-view';
Expand Down Expand Up @@ -58,9 +60,9 @@ export const GET_DOWNLOAD_LOCATION = 'get_download_location';
export const UPDATE_MENTIONS = 'update_mentions';
export const IS_UNREAD = 'is_unread';
export const UNREAD_RESULT = 'unread_result';
export const MENTIONS_RESULT = 'mentions-changed';
export const SESSION_EXPIRED = 'session_expired';

export const SET_VIEW_OPTIONS = 'set-view-name';
export const REACT_APP_INITIALIZED = 'react-app-initialized';

export const TOGGLE_BACK_BUTTON = 'toggle-back-button';
Expand Down Expand Up @@ -93,7 +95,6 @@ export const START_UPGRADE = 'start-upgrade';
export const CHECK_FOR_UPDATES = 'check-for-updates';
export const NO_UPDATE_AVAILABLE = 'no-update-available';

export const BROWSER_HISTORY_BUTTON = 'browser-history-button';
export const BROWSER_HISTORY_PUSH = 'browser-history-push';
export const APP_LOGGED_IN = 'app-logged-in';
export const APP_LOGGED_OUT = 'app-logged-out';
Expand All @@ -118,7 +119,7 @@ export const GET_AVAILABLE_LANGUAGES = 'get-available-languages';
export const VIEW_FINISHED_RESIZING = 'view-finished-resizing';

// Calls
export const DISPATCH_GET_DESKTOP_SOURCES = 'dispatch-get-desktop-sources';
export const GET_DESKTOP_SOURCES = 'get-desktop-sources';
export const DESKTOP_SOURCES_RESULT = 'desktop-sources-result';
export const DESKTOP_SOURCES_MODAL_REQUEST = 'desktop-sources-modal-request';
export const CALLS_JOIN_CALL = 'calls-join-call';
Expand Down Expand Up @@ -177,3 +178,11 @@ export const VALIDATE_SERVER_URL = 'validate-server-url';
export const GET_IS_DEV_MODE = 'get-is-dev-mode';

export const TOGGLE_SECURE_INPUT = 'toggle-secure-input';

export const REQUEST_BROWSER_HISTORY_STATUS = 'request-browser-history-status';
export const BROWSER_HISTORY_STATUS_UPDATED = 'browser-history-status-updated';

export const NOTIFICATION_CLICKED = 'notification-clicked';

// Legacy code remove signal
export const LEGACY_OFF = 'legacy-off';
1 change: 1 addition & 0 deletions src/common/utils/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export const isUrlType = (urlType: string, serverURL: URL, inputURL: URL) => {
getFormattedPathName(inputURL.pathname).startsWith(`/${urlType}/`));
};

export const isLoginUrl = (serverURL: URL, inputURL: URL) => isUrlType('login', serverURL, inputURL);
export const isHelpUrl = (serverURL: URL, inputURL: URL) => isUrlType('help', serverURL, inputURL);
export const isImageProxyUrl = (serverURL: URL, inputURL: URL) => isUrlType('api/v4/image', serverURL, inputURL);
export const isPublicFilesUrl = (serverURL: URL, inputURL: URL) => isUrlType('files', serverURL, inputURL);
Expand Down
5 changes: 3 additions & 2 deletions src/main/app/initialize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
WINDOW_RESTORE,
DOUBLE_CLICK_ON_WINDOW,
TOGGLE_SECURE_INPUT,
GET_APP_INFO,
} from 'common/communication';
import Config from 'common/config';
import {Logger} from 'common/log';
Expand Down Expand Up @@ -249,7 +250,7 @@ function initializeBeforeAppReady() {

function initializeInterCommunicationEventListeners() {
ipcMain.on(NOTIFY_MENTION, handleMentionNotification);
ipcMain.handle('get-app-version', handleAppVersion);
ipcMain.handle(GET_APP_INFO, handleAppVersion);
ipcMain.on(UPDATE_SHORTCUT_MENU, handleUpdateMenuEvent);
ipcMain.on(FOCUS_BROWSERVIEW, ViewManager.focusCurrentView);

Expand Down Expand Up @@ -366,7 +367,7 @@ async function initializeAfterAppReady() {
// listen for status updates and pass on to renderer
UserActivityMonitor.on('status', (status) => {
log.debug('UserActivityMonitor.on(status)', status);
ViewManager.sendToAllViews(USER_ACTIVITY_UPDATE, status);
ViewManager.sendToAllViews(USER_ACTIVITY_UPDATE, status.userIsActive, status.idleTime, status.isSystemEvent);
});

// start monitoring user activity (needs to be started after the app is ready)
Expand Down
7 changes: 3 additions & 4 deletions src/main/app/intercom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import {app, IpcMainEvent, IpcMainInvokeEvent, Menu} from 'electron';

import {UniqueServer} from 'types/config';
import {MentionData} from 'types/notification';

import ServerViewState from 'app/serverViewState';

Expand Down Expand Up @@ -114,9 +113,9 @@ export function handleWelcomeScreenModal() {
}
}

export function handleMentionNotification(event: IpcMainEvent, title: string, body: string, channel: {id: string}, teamId: string, url: string, silent: boolean, data: MentionData) {
log.debug('handleMentionNotification', {title, body, channel, teamId, url, silent, data});
NotificationManager.displayMention(title, body, channel, teamId, url, silent, event.sender, data);
export function handleMentionNotification(event: IpcMainEvent, title: string, body: string, channelId: string, teamId: string, url: string, silent: boolean, soundName: string) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplified this since we should be more specific about what these calls expect.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does changing this cause any issues for old web apps, or is it this just called from the desktop app itself?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just called from the Desktop itself, I do some "magic" in the preload script to account for the legacy case so that we could fix it properly here.

log.debug('handleMentionNotification', {title, body, channelId, teamId, url, silent, soundName});
NotificationManager.displayMention(title, body, channelId, teamId, url, silent, event.sender, soundName);
}

export function handleOpenAppMenu() {
Expand Down
8 changes: 4 additions & 4 deletions src/main/notifications/Mention.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,25 @@ const DEFAULT_WIN7 = 'Ding';

export class Mention extends Notification {
customSound: string;
channel: {id: string}; // TODO: Channel from mattermost-redux
channelId: string;
teamId: string;
uId: string;

constructor(customOptions: MentionOptions, channel: {id: string}, teamId: string) {
constructor(customOptions: MentionOptions, channelId: string, teamId: string) {
const options = {...defaultOptions, ...customOptions};
if (process.platform === 'darwin' || (process.platform === 'win32' && Utils.isVersionGreaterThanOrEqualTo(os.release(), '10.0'))) {
// Notification Center shows app's icon, so there were two icons on the notification.
Reflect.deleteProperty(options, 'icon');
}
const isWin7 = (process.platform === 'win32' && !Utils.isVersionGreaterThanOrEqualTo(os.release(), '6.3') && DEFAULT_WIN7);
const customSound = String(!options.silent && ((options.data && options.data.soundName !== 'None' && options.data.soundName) || isWin7));
const customSound = String(!options.silent && ((options.soundName !== 'None' && options.soundName) || isWin7));
if (customSound) {
options.silent = true;
}
super(options);

this.customSound = customSound;
this.channel = channel;
this.channelId = channelId;
this.teamId = teamId;
this.uId = uuid();
}
Expand Down
49 changes: 25 additions & 24 deletions src/main/notifications/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ jest.mock('../views/viewManager', () => ({
name: 'server_name',
url: new URL('http://someurl.com'),
},
shouldNotify: true,
},
}),
showById: jest.fn(),
Expand Down Expand Up @@ -144,12 +145,12 @@ describe('main/notifications', () => {
await NotificationManager.displayMention(
'test',
'test body',
{id: 'channel_id'},
'channel_id',
'team_id',
'http://server-1.com/team_id/channel_id',
false,
{id: 1} as WebContents,
{soundName: ''},
'',
);
expect(MainWindow.show).not.toBeCalled();
});
Expand All @@ -164,12 +165,12 @@ describe('main/notifications', () => {
await NotificationManager.displayMention(
'test',
'test body',
{id: 'channel_id'},
'channel_id',
'team_id',
'http://server-1.com/team_id/channel_id',
false,
{id: 1} as WebContents,
{soundName: ''},
'',
);
expect(MainWindow.show).not.toBeCalled();

Expand All @@ -188,12 +189,12 @@ describe('main/notifications', () => {
await NotificationManager.displayMention(
'test',
'test body',
{id: 'channel_id'},
'channel_id',
'team_id',
'http://server-1.com/team_id/channel_id',
false,
{id: 1} as WebContents,
{soundName: ''},
'',
);
expect(MainWindow.show).not.toBeCalled();

Expand All @@ -207,12 +208,12 @@ describe('main/notifications', () => {
await NotificationManager.displayMention(
'test',
'test body',
{id: 'channel_id'},
'channel_id',
'team_id',
'http://server-1.com/team_id/channel_id',
false,
{id: 1} as WebContents,
{soundName: ''},
'',
);
expect(MainWindow.show).not.toBeCalled();
});
Expand All @@ -221,12 +222,12 @@ describe('main/notifications', () => {
await NotificationManager.displayMention(
'test',
'test body',
{id: 'channel_id'},
'channel_id',
'team_id',
'http://server-1.com/team_id/channel_id',
false,
{id: 1} as WebContents,
{soundName: 'test_sound'},
'test_sound',
);
expect(MainWindow.sendToRenderer).toHaveBeenCalledWith(PLAY_SOUND, 'test_sound');
});
Expand All @@ -240,12 +241,12 @@ describe('main/notifications', () => {
await NotificationManager.displayMention(
'test',
'test body',
{id: 'channel_id'},
'channel_id',
'team_id',
'http://server-1.com/team_id/channel_id',
false,
{id: 1} as WebContents,
{soundName: ''},
'',
);

// convert to any to access private field
Expand All @@ -257,12 +258,12 @@ describe('main/notifications', () => {
await NotificationManager.displayMention(
'test',
'test body 2',
{id: 'channel_id'},
'channel_id',
'team_id',
'http://server-1.com/team_id/channel_id',
false,
{id: 1} as WebContents,
{soundName: ''},
'',
);

expect(mentionsPerChannel.delete).toHaveBeenCalled();
Expand All @@ -277,12 +278,12 @@ describe('main/notifications', () => {
await NotificationManager.displayMention(
'click_test',
'mention_click_body',
{id: 'channel_id'},
'channel_id',
'team_id',
'http://server-1.com/team_id/channel_id',
false,
{id: 1, send: jest.fn()} as unknown as WebContents,
{soundName: ''},
'',
);
const mention = mentions.find((m) => m.body === 'mention_click_body');
mention?.value.click();
Expand All @@ -298,12 +299,12 @@ describe('main/notifications', () => {
await NotificationManager.displayMention(
'click_test',
'mention_click_body',
{id: 'channel_id'},
'channel_id',
'team_id',
'http://server-1.com/team_id/channel_id',
false,
{id: 1, send: jest.fn()} as unknown as WebContents,
{soundName: ''},
'',
);
Object.defineProperty(process, 'platform', {
value: originalPlatform,
Expand All @@ -324,12 +325,12 @@ describe('main/notifications', () => {
await NotificationManager.displayMention(
'click_test',
'mention_click_body',
{id: 'channel_id'},
'channel_id',
'team_id',
'http://server-1.com/team_id/channel_id',
false,
{id: 1, send: jest.fn()} as unknown as WebContents,
{soundName: ''},
'',
);
Object.defineProperty(process, 'platform', {
value: originalPlatform,
Expand All @@ -345,12 +346,12 @@ describe('main/notifications', () => {
await NotificationManager.displayMention(
'click_test',
'mention_click_body',
{id: 'channel_id'},
'channel_id',
'team_id',
'http://server-1.com/team_id/channel_id',
false,
{id: 1, send: jest.fn()} as unknown as WebContents,
{soundName: ''},
'',
);
Object.defineProperty(process, 'platform', {
value: originalPlatform,
Expand All @@ -371,12 +372,12 @@ describe('main/notifications', () => {
await NotificationManager.displayMention(
'click_test',
'mention_click_body',
{id: 'channel_id'},
'channel_id',
'team_id',
'http://server-1.com/team_id/channel_id',
false,
{id: 1, send: jest.fn()} as unknown as WebContents,
{soundName: ''},
'',
);
Object.defineProperty(process, 'platform', {
value: originalPlatform,
Expand Down
Loading
Loading