-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Changes from 8 commits
125f181
576786c
cc424f7
a6ea8bd
e2882ad
851831c
d0b6195
e807fce
fde28ad
13e7124
4b6ab28
5b54245
56790ae
d29bf47
a4979ec
0611f14
40e4fb3
25ebd15
17a8844
eb2184f
72d348e
155d7ed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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"> | ||
<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" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pass There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or better remove the There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's pass the file prop through onShareLogs for now There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); |
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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid platform specific code. Instead write a |
||
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); |
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[]); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
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); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks, now i am subscribing to Onyx only in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Due to recent change I see no use in subscribing in |
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In ONYXKEYS please use that type There was a problem hiding this comment. Choose a reason for hiding this commentThe 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}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as above |
||
onPress={onDownloadProfiling} | ||
/> | ||
</TestToolRow> | ||
</> | ||
)} | ||
</> | ||
); | ||
|
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'; | ||||||
|
@@ -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[]) => { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
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}; |
There was a problem hiding this comment.
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