-
Notifications
You must be signed in to change notification settings - Fork 297
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
[DDW-248] Refactor compress/download logs handling #995
Changes from 12 commits
ffd61bd
3f4af8c
88b9c50
c8f2a81
15c2a1e
5976494
e39f322
2d5a251
7ab1f8f
afd1343
242710e
81a444d
9a3a123
344d5d5
37d9bf5
0d42e24
74ada87
6913da9
a7af4f1
af76c11
27fc619
43a75fc
d68a724
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,8 @@ | ||
// @flow | ||
import moment from 'moment'; | ||
|
||
export const filenameWithTimestamp = (prefix:string = 'logs', filetype:string = 'zip') => | ||
`${prefix}-${moment.utc().format('YYYY-MM-DDThhmmss.0SSS')}.${filetype}`; | ||
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. @daniloprates please use the following format 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. Oh one thing I forgot - please append 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. Hey @nikolaglumac! I'll add the 'Z', however, it's not possible to add the colons, as it's an invalid character for filenames. 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. ✔︎ |
||
|
||
export const getFilenameWithTimestamp = (prefix:string = 'logs', filetype:string = 'zip') => (fileName:string) => | ||
fileName.match(RegExp(`(${prefix}-)([0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{6}.0[0-9]{3})(.${filetype})`)); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,10 @@ | ||
import fs from 'fs'; | ||
import path from 'path'; | ||
import log from 'electron-log'; | ||
import moment from 'moment'; | ||
import ensureDirectoryExists from './ensureDirectoryExists'; | ||
import { pubLogsFolderPath, APP_NAME } from '../config'; | ||
import { pubLogsFolderPath, appLogsFolderPath, APP_NAME } from '../config'; | ||
import { getFilenameWithTimestamp } from '../../common/fileName'; | ||
|
||
const isTest = process.env.NODE_ENV === 'test'; | ||
|
||
|
@@ -19,4 +21,19 @@ export const setupLogging = () => { | |
const formattedDate = moment.utc(msg.date).format('YYYY-MM-DDTHH:mm:ss.0SSS'); | ||
return `[${formattedDate}Z] [${msg.level}] ${msg.data}`; | ||
}; | ||
|
||
// Removes existing logs | ||
fs.readdir(appLogsFolderPath, (err, files) => { | ||
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. @daniloprates this part works great 🎉 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. 🙂 |
||
files | ||
.filter(getFilenameWithTimestamp()) | ||
.forEach((logFileName) => { | ||
const logFile = path.join(appLogsFolderPath, logFileName); | ||
try { | ||
fs.unlinkSync(logFile); | ||
} catch (error) { | ||
console.error(error); | ||
} | ||
}); | ||
}); | ||
|
||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -118,22 +118,19 @@ const messages = defineMessages({ | |
|
||
type Props = { | ||
logFiles: LogFiles, | ||
compressedLog: ?string, | ||
onCancel: Function, | ||
onSubmit: Function, | ||
onSubmitManually: Function, | ||
onDownload: Function, | ||
onGetLogs: Function, | ||
onCompressLogs: Function, | ||
onDeleteCompressedLogs: Function, | ||
isSubmitting: boolean, | ||
isCompressing: boolean, | ||
onRequestFreshCompressedLogs: Function, | ||
isDownloading?: boolean, | ||
error: ?LocalizableError, | ||
}; | ||
|
||
type State = { | ||
showLogs: boolean, | ||
compressedLog: ?string, | ||
isSubmitting: boolean | ||
}; | ||
|
||
@observer | ||
|
@@ -145,19 +142,23 @@ export default class BugReportDialog extends Component<Props, State> { | |
|
||
state = { | ||
showLogs: true, | ||
compressedLog: null, | ||
isSubmitting: false, | ||
}; | ||
|
||
componentWillMount() { | ||
this.props.onGetLogs(); | ||
this.props.onDeleteCompressedLogs(); | ||
} | ||
|
||
componentWillReceiveProps(nextProps: Object) { | ||
const commpressionFilesChanged = this.props.compressedLog !== nextProps.compressedLog; | ||
|
||
const commpressionFilesChanged = !this.props.compressedLog && !!nextProps.compressedLog; | ||
const { compressedLog } = this.state; | ||
|
||
if (compressedLog) { | ||
return false; | ||
} | ||
|
||
if (nextProps.compressedLog && commpressionFilesChanged && !nextProps.isDownloading) { | ||
// proceed to submit when ipc rendered successfully return compressed files | ||
this.submit(nextProps.compressedLog); | ||
this.setState({ compressedLog: nextProps.compressedLog }, this.submit); | ||
} | ||
|
||
} | ||
|
||
form = new ReactToolboxMobxForm({ | ||
|
@@ -206,24 +207,26 @@ export default class BugReportDialog extends Component<Props, State> { | |
}, | ||
}); | ||
|
||
submit = (compressedLog: ?string) => { | ||
submit = () => { | ||
|
||
this.setState({ isSubmitting: true }); | ||
|
||
const { showLogs, compressedLog } = this.state; | ||
|
||
if (showLogs && !compressedLog) { | ||
this.props.onRequestFreshCompressedLogs(); | ||
return false; | ||
} | ||
|
||
this.form.submit({ | ||
onSuccess: (form) => { | ||
const { logFiles } = this.props; | ||
const logsExist = get(logFiles, ['files'], []).length > 0; | ||
|
||
const { email, subject, problem } = form.values(); | ||
const data = { | ||
email, subject, problem, compressedLog | ||
}; | ||
|
||
if (this.state.showLogs && logsExist && !compressedLog) { | ||
// submit request with commpressed logs files | ||
this.props.onCompressLogs(this.props.logFiles); | ||
} else { | ||
// regular submit | ||
this.props.onSubmit(data); | ||
} | ||
this.props.onSubmit(data); | ||
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. @daniloprates we shouldn't send the log files if the user has set the "attach-logs" switch to off state. 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. ✔︎ |
||
}, | ||
onError: () => {}, | ||
}); | ||
|
@@ -235,10 +238,10 @@ export default class BugReportDialog extends Component<Props, State> { | |
|
||
render() { | ||
const { intl } = this.context; | ||
const { showLogs } = this.state; | ||
const { showLogs, isSubmitting } = this.state; | ||
const { form } = this; | ||
const { | ||
onCancel, isSubmitting, isCompressing, | ||
onCancel, | ||
logFiles, error, onDownload, isDownloading, | ||
} = this.props; | ||
|
||
|
@@ -255,7 +258,7 @@ export default class BugReportDialog extends Component<Props, State> { | |
|
||
const submitButtonClasses = classnames([ | ||
'submitButton', | ||
(isSubmitting || isCompressing) ? styles.isSubmitting : null, | ||
isSubmitting ? styles.isSubmitting : null, | ||
]); | ||
|
||
const downloadButtonClasses = classnames([ | ||
|
@@ -273,7 +276,7 @@ export default class BugReportDialog extends Component<Props, State> { | |
label: this.context.intl.formatMessage(messages.submitButtonLabel), | ||
primary: true, | ||
disabled: isSubmitting, | ||
onClick: this.submit.bind(this, null), | ||
onClick: this.submit, | ||
}, | ||
]; | ||
|
||
|
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.
@daniloprates please add
// @flow
to the top of this file.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.
✔︎