Desktop, Mobile: Resolves #10664: Allow user to generate deletion logs #11083
Conversation
| if (!Logger.globalLogger) { | ||
| throw new Error('Could not find global logger'); | ||
| } | ||
|
|
||
| const fileTarget = Logger.globalLogger.targets().find(t => t.type === TargetType.File); | ||
|
|
||
| if (!fileTarget || !fileTarget.path) { | ||
| throw new Error('Log file target not found'); | ||
| } |
There was a problem hiding this comment.
If the log file was recently rotated, you won't get the information. Maybe instead just directly get log.txt and the latest log-*.txt from the profile directory?
|
|
||
| await shim.fsDriver().writeFile(deletionLogPath, deletionLog, 'utf8'); | ||
|
|
||
| await void bridge().openItem(Setting.value('profileDir')); |
There was a problem hiding this comment.
Instead please open the file you've just created. Seems a bit random to open the profile directory as the user won't know why we're doing this
| name: 'exportDeletionLogs', | ||
| label: () => _('Export deletion logs'), |
There was a problem hiding this comment.
Should be singular deletion log. Also the filename should be singular
|
I updated the PR getting all the log files, including the rotations one. I also included a function to order by the timestamp in the log filename (while every time I tested it was already in the right order, I thought it would be better to include this logic there in case something changes) Since I'm building the path with pure strings (aka: |
| const logsOrderedByTimestamp = (logFiles: Stat[]) => { | ||
| return logFiles.sort((a, b) => { | ||
| const aTimestamp = a.path.match(/\d+/); | ||
| const bTimestamp = b.path.match(/\d+/); | ||
|
|
||
| if (!aTimestamp) return 1; | ||
| if (!bTimestamp) return -1; | ||
|
|
||
| return parseInt(aTimestamp[0], 10) - parseInt(bTimestamp[0], 10); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Wouldn't it be the same to sort alphabetically?
There was a problem hiding this comment.
You are right, thanks for pointing it out, I'm removing this in favour of sort()
| if (!Logger.globalLogger) { | ||
| throw new Error('Could not find global logger'); | ||
| } | ||
|
|
||
| const fileTarget = Logger.globalLogger.targets().find(t => t.type === TargetType.File); | ||
|
|
||
| if (!fileTarget || !fileTarget.path) { | ||
| throw new Error('Log file target not found'); | ||
| } |
There was a problem hiding this comment.
I don't get why this is still here
| for (const file of orderedLogs) { | ||
|
|
||
| const deletionLines = await getDeletionLines(file.path); | ||
|
|
||
| allDeletionLines += deletionLines; | ||
| } |
There was a problem hiding this comment.
I've suggested taking log.txt and the latest log-*.txt, nothing more. We don't need to look at the last 2 years of logs.
| if (!Logger.globalLogger) { | ||
| throw new Error('Could not find global logger'); | ||
| } |
There was a problem hiding this comment.
Again, why do you need this check for? You don't use the logger anywhere in that code below.
Resolves #10664
Summary
We are adding this new feature on Desktop and Mobile to help users share logs about deletion when they might want some help figuring out what is happening.
To do that, on mobile I created a new Log button on the Tools submenu that filters every DeleteAction event.
On Desktop, I created a Desktop-specific command that reads the log of the active profile and filters every line with the
DeleteActionevent, creating a newdeletion_logs.txtfile on the current profile directory.Testing
I only did manual tests, like:
1 - Create a notebook and a new note
2 - Send the note to the trash
3 - Empty the trash
4 - Create a new note, and send it to the trash
5 - Access the new log screen on mobile or
Export deletion logson the desktop6 - Only the note that isn't in trash anymore (because it was permanently deleted) should be logged