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

feat: access client side logging from Test Tools Modal #38803

Merged
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
125f181
add client side logging tool to hidden menu
TMisiukiewicz Mar 21, 2024
576786c
save and share logs from hidden menu
TMisiukiewicz Mar 22, 2024
cc424f7
share logs on android and ios
TMisiukiewicz Mar 22, 2024
a6ea8bd
allow to download logs on web
TMisiukiewicz Mar 22, 2024
e2882ad
download file when disabling logging in troubleshoot page
TMisiukiewicz Mar 22, 2024
851831c
Merge branch 'main' into feat/four-finger-tap-logging
TMisiukiewicz Mar 22, 2024
d0b6195
update types
TMisiukiewicz Mar 22, 2024
e807fce
prevent trying to parse empty logs
TMisiukiewicz Mar 22, 2024
fde28ad
Merge remote-tracking branch 'upstream/main' into feat/four-finger-ta…
TMisiukiewicz Mar 25, 2024
13e7124
code review updates
TMisiukiewicz Mar 25, 2024
4b6ab28
unify platform-agnostic code
TMisiukiewicz Mar 25, 2024
5b54245
update types
TMisiukiewicz Mar 25, 2024
56790ae
code review updates
TMisiukiewicz Mar 25, 2024
d29bf47
hide existing file when enable logging
TMisiukiewicz Mar 26, 2024
a4979ec
Merge remote-tracking branch 'upstream/main' into feat/four-finger-ta…
TMisiukiewicz Mar 26, 2024
0611f14
rename disable switch callback name
TMisiukiewicz Mar 26, 2024
40e4fb3
move types to base component
TMisiukiewicz Mar 26, 2024
25ebd15
Merge branch 'main' into feat/four-finger-tap-logging
TMisiukiewicz Mar 27, 2024
17a8844
code review updates
TMisiukiewicz Mar 27, 2024
eb2184f
Merge branch 'main' into feat/four-finger-tap-logging
TMisiukiewicz Mar 27, 2024
72d348e
Merge remote-tracking branch 'upstream/main' into feat/four-finger-ta…
TMisiukiewicz Apr 2, 2024
155d7ed
update translations
TMisiukiewicz Apr 2, 2024
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import React from 'react';
import {withOnyx} from 'react-native-onyx';
import Button from '@components/Button';
import Switch from '@components/Switch';
import TestToolRow from '@components/TestToolRow';
import Text from '@components/Text';
import useThemeStyles from '@hooks/useThemeStyles';
import ONYXKEYS from '@src/ONYXKEYS';
import type {ClientSideLoggingToolMenuOnyxProps} from './types';

type BaseClientSideLoggingToolMenuOnyxProps = Pick<ClientSideLoggingToolMenuOnyxProps, 'shouldStoreLogs'>;

type BaseClientSideLoggingToolProps = {
file?: {path: string; newFileName: string; size: number};
onShareLogs?: () => void;
onToggleSwitch: () => void;
} & BaseClientSideLoggingToolMenuOnyxProps;

function BaseClientSideLoggingToolMenu({shouldStoreLogs, file, onShareLogs, onToggleSwitch}: BaseClientSideLoggingToolProps) {
const styles = useThemeStyles();
return (
<>
<TestToolRow title="Client side logging">
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a translation for this phrase? I see that it's consistent with other titles in Tools (all are hardcoded English) so NAB, but curious if there's a reason for not translating

<Switch
accessibilityLabel="Client side logging"
isOn={!!shouldStoreLogs}
onToggle={onToggleSwitch}
/>
</TestToolRow>
{!!file && (
<>
<Text style={[styles.textLabelSupporting, styles.mb4]}>{`path: ${file.path}`}</Text>
<TestToolRow title="Logs">
<Button
small
text="Share"
Copy link
Contributor

Choose a reason for hiding this comment

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

There's an existing translation for this, let's use it

onPress={onShareLogs}
Copy link
Contributor

Choose a reason for hiding this comment

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

Pass file as a param in onShareLogs

Copy link
Contributor

Choose a reason for hiding this comment

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

Or better remove the onShareLogs prop and let the button always call Share.open

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this has to be platform-specific, as there is no share option for web/desktop

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's pass the file prop through onShareLogs for now

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually there is no benefit in doing this.

/>
</TestToolRow>
</>
)}
</>
);
}

BaseClientSideLoggingToolMenu.displayName = 'BaseClientSideLoggingToolMenu';

export default withOnyx<BaseClientSideLoggingToolProps, BaseClientSideLoggingToolMenuOnyxProps>({
shouldStoreLogs: {
key: ONYXKEYS.SHOULD_STORE_LOGS,
},
})(BaseClientSideLoggingToolMenu);
74 changes: 74 additions & 0 deletions src/components/ClientSideLoggingToolMenu/index.native.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import React, {useState} from 'react';
import {Alert} from 'react-native';
import RNFetchBlob from 'react-native-blob-util';
import {withOnyx} from 'react-native-onyx';
import Share from 'react-native-share';
import * as Console from '@libs/actions/Console';
import {parseStringifyMessages} from '@libs/Console';
import type {Log} from '@libs/Console';
import getOperatingSystem from '@libs/getOperatingSystem';
import localFileCreate from '@libs/localFileCreate';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import BaseClientSideLoggingToolMenu from './BaseClientSideLoggingToolMenu';
import type {ClientSideLoggingToolMenuOnyxProps, ClientSideLoggingToolProps} from './types';

function ClientSideLoggingToolMenu({shouldStoreLogs, capturedLogs}: ClientSideLoggingToolProps) {
const [file, setFile] = useState<{path: string; newFileName: string; size: number}>();

const onToggle = () => {
if (!shouldStoreLogs) {
Console.setShouldStoreLogs(true);
} else {
if (capturedLogs) {
Copy link
Contributor

Choose a reason for hiding this comment

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

these could be simplified with early returns on all the edge cases (like no logs captured)

const logs = Object.values(capturedLogs as Log[]);
const logsWithParsedMessages = parseStringifyMessages(logs);
localFileCreate('logs', JSON.stringify(logsWithParsedMessages, null, 2)).then((localFile) => {
if (getOperatingSystem() === CONST.OS.ANDROID) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid platform specific code. Instead write a index.android.tsx file

RNFetchBlob.MediaCollection.copyToMediaStore(
{
name: localFile.newFileName,
parentFolder: '',
mimeType: 'text/plain',
},
'Download',
localFile.path,
);
}
setFile(localFile);
});
} else {
Alert.alert('No logs to share', 'There are no logs to share');
}
Console.disableLoggingAndFlushLogs();
}
};

const shareLogs = () => {
if (!file) {
return;
}
Share.open({
url: `file://${file.path}`,
});
};

return (
<BaseClientSideLoggingToolMenu
file={file}
onToggleSwitch={onToggle}
onShareLogs={shareLogs}
/>
);
}

ClientSideLoggingToolMenu.displayName = 'ClientSideLoggingToolMenu';

export default withOnyx<ClientSideLoggingToolProps, ClientSideLoggingToolMenuOnyxProps>({
capturedLogs: {
key: ONYXKEYS.LOGS,
},
shouldStoreLogs: {
key: ONYXKEYS.SHOULD_STORE_LOGS,
},
})(ClientSideLoggingToolMenu);
41 changes: 41 additions & 0 deletions src/components/ClientSideLoggingToolMenu/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import React from 'react';
import {Alert} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import * as Console from '@libs/actions/Console';
import {parseStringifyMessages} from '@libs/Console';
import type {Log} from '@libs/Console';
import localFileDownload from '@libs/localFileDownload';
import ONYXKEYS from '@src/ONYXKEYS';
import BaseClientSideLoggingToolMenu from './BaseClientSideLoggingToolMenu';
import type {ClientSideLoggingToolMenuOnyxProps, ClientSideLoggingToolProps} from './types';

function ClientSideLoggingToolMenu({capturedLogs, shouldStoreLogs}: ClientSideLoggingToolProps) {
const onToggle = () => {
if (!shouldStoreLogs) {
Console.setShouldStoreLogs(true);
} else {
if (!capturedLogs) {
Alert.alert('No logs to share', 'There are no logs to share');
Console.disableLoggingAndFlushLogs();
return;
}
const logs = Object.values(capturedLogs as Log[]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const logs = Object.values(capturedLogs as Log[]);
const logs = Object.values(capturedLogs);

const logsWithParsedMessages = parseStringifyMessages(logs);

localFileDownload('logs', JSON.stringify(logsWithParsedMessages, null, 2));
Console.disableLoggingAndFlushLogs();
}
};
return <BaseClientSideLoggingToolMenu onToggleSwitch={onToggle} />;
}

ClientSideLoggingToolMenu.displayName = 'ClientSideLoggingToolMenu';

export default withOnyx<ClientSideLoggingToolProps, ClientSideLoggingToolMenuOnyxProps>({
capturedLogs: {
key: ONYXKEYS.LOGS,
},
shouldStoreLogs: {
key: ONYXKEYS.SHOULD_STORE_LOGS,
},
})(ClientSideLoggingToolMenu);
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need to subscribe to onyx keys twice. Only subscribe in BaseClientSideLoggingToolMenu and pass the required values through the props. (or the other way around)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, now i am subscribing to Onyx only in ClientSideLogginToolMenu and passing it down to base component

Copy link
Contributor

Choose a reason for hiding this comment

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

Due to recent change I see no use in subscribing in ClientSideLogginToolMenu. Let's subscribe in the base component instead

16 changes: 16 additions & 0 deletions src/components/ClientSideLoggingToolMenu/types.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

Seeing that the types are only used in BaseClientSideLoggingToolMenu we should move them there

Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import type {OnyxEntry} from 'react-native-onyx';
import type {Log} from '@libs/Console';

type CapturedLogs = Record<number, Log>;
Copy link
Contributor

Choose a reason for hiding this comment

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

In ONYXKEYS please use that type

Copy link
Contributor

Choose a reason for hiding this comment

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

Change

[ONYXKEYS.LOGS]: Record<number, OnyxTypes.Log>;

to:

[ONYXKEYS.LOGS]: OnyxTypes.CapturedLogs;


type ClientSideLoggingToolMenuOnyxProps = {
/** Logs captured on the current device */
capturedLogs: OnyxEntry<CapturedLogs>;

/** Whether or not logs should be stored */
shouldStoreLogs: OnyxEntry<boolean>;
};

type ClientSideLoggingToolProps = ClientSideLoggingToolMenuOnyxProps;

export type {CapturedLogs, ClientSideLoggingToolMenuOnyxProps, ClientSideLoggingToolProps};
25 changes: 10 additions & 15 deletions src/components/ProfilingToolMenu/index.native.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -142,29 +142,24 @@ function ProfilingToolMenu({isProfilingInProgress = false}: ProfilingToolMenuPro

return (
<>
<Text
style={[styles.textLabelSupporting, styles.mt4, styles.mb3]}
numberOfLines={1}
>
Release options
</Text>

<TestToolRow title="Use Profiling">
<Switch
accessibilityLabel="Use Profiling"
isOn={!!isProfilingInProgress}
onToggle={onToggleProfiling}
/>
</TestToolRow>
<Text style={[styles.textLabelSupporting, styles.mb4]}>{!!pathIOS && `path: ${pathIOS}`}</Text>
{!!pathIOS && (
<TestToolRow title="Profile trace">
<Button
small
text="Share"
onPress={onDownloadProfiling}
/>
</TestToolRow>
<>
<Text style={[styles.textLabelSupporting, styles.mb4]}>{`path: ${pathIOS}`}</Text>
<TestToolRow title="Profile trace">
<Button
small
text="Share"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above

onPress={onDownloadProfiling}
/>
</TestToolRow>
</>
)}
</>
);
Expand Down
11 changes: 11 additions & 0 deletions src/components/TestToolsModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import useEnvironment from '@hooks/useEnvironment';
import useStyleUtils from '@hooks/useStyleUtils';
import useThemeStyles from '@hooks/useThemeStyles';
import useWindowDimensions from '@hooks/useWindowDimensions';
import toggleTestToolsModal from '@userActions/TestTool';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ClientSideLoggingToolMenu from './ClientSideLoggingToolMenu';
import Modal from './Modal';
import ProfilingToolMenu from './ProfilingToolMenu';
import TestToolMenu from './TestToolMenu';
import Text from './Text';

type TestToolsModalOnyxProps = {
/** Whether the test tools modal is open */
Expand All @@ -23,6 +26,7 @@ function TestToolsModal({isTestToolsModalOpen = false}: TestToolsModalProps) {
const {isDevelopment} = useEnvironment();
const {windowWidth} = useWindowDimensions();
const StyleUtils = useStyleUtils();
const styles = useThemeStyles();

return (
<Modal
Expand All @@ -32,7 +36,14 @@ function TestToolsModal({isTestToolsModalOpen = false}: TestToolsModalProps) {
>
<View style={[StyleUtils.getTestToolsModalStyle(windowWidth)]}>
{isDevelopment && <TestToolMenu />}
<Text
style={[styles.textLabelSupporting, styles.mt4, styles.mb3]}
numberOfLines={1}
>
Release options
</Text>
<ProfilingToolMenu />
<ClientSideLoggingToolMenu />
</View>
</Modal>
);
Expand Down
27 changes: 26 additions & 1 deletion src/libs/Console/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-disable @typescript-eslint/naming-convention */
import isEmpty from 'lodash/isEmpty';
import Onyx from 'react-native-onyx';
import {addLog} from '@libs/actions/Console';
import CONFIG from '@src/CONFIG';
Expand Down Expand Up @@ -118,5 +119,29 @@ function createLog(text: string) {
}
}

export {sanitizeConsoleInput, createLog, shouldAttachLog};
/**
* Loops through all the logs and parses the message if it's a stringified JSON
* @param logs Logs captured on the current device
* @returns CapturedLogs with parsed messages
*/
const parseStringifyMessages = (logs: Log[]) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const parseStringifyMessages = (logs: Log[]) => {
function parseStringifyMessages(logs: Log[]): Log[] {

if (isEmpty(logs)) {
return;
}

return logs.map((log) => {
try {
const parsedMessage = JSON.parse(log.message);
return {
...log,
message: parsedMessage,
};
} catch {
// If the message can't be parsed, just return the original log
return log;
}
});
};

export {sanitizeConsoleInput, createLog, shouldAttachLog, parseStringifyMessages};
export type {Log};
27 changes: 1 addition & 26 deletions src/pages/settings/AboutPage/ConsolePage.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {format} from 'date-fns';
import isEmpty from 'lodash/isEmpty';
import React, {useCallback, useEffect, useMemo, useState} from 'react';
import {FlatList, View} from 'react-native';
import type {ListRenderItem, ListRenderItemInfo} from 'react-native';
Expand All @@ -16,7 +15,7 @@ import useKeyboardShortcut from '@hooks/useKeyboardShortcut';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import {addLog} from '@libs/actions/Console';
import {createLog, sanitizeConsoleInput} from '@libs/Console';
import {createLog, parseStringifyMessages, sanitizeConsoleInput} from '@libs/Console';
import type {Log} from '@libs/Console';
import localFileCreate from '@libs/localFileCreate';
import localFileDownload from '@libs/localFileDownload';
Expand All @@ -37,30 +36,6 @@ type ConsolePageOnyxProps = {

type ConsolePageProps = ConsolePageOnyxProps;

/**
* Loops through all the logs and parses the message if it's a stringified JSON
* @param logs Logs captured on the current device
* @returns CapturedLogs with parsed messages
*/
const parseStringifyMessages = (logs: Log[]) => {
if (isEmpty(logs)) {
return;
}

return logs.map((log) => {
try {
const parsedMessage = JSON.parse(log.message);
return {
...log,
message: parsedMessage,
};
} catch {
// If the message can't be parsed, just return the original log
return log;
}
});
};

function ConsolePage({capturedLogs, shouldStoreLogs}: ConsolePageProps) {
const [input, setInput] = useState('');
const [logs, setLogs] = useState(capturedLogs);
Expand Down
Loading
Loading