-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Append current time to the downloaded file name #23531
Changes from all commits
5f9c6f6
512d6b9
b721be4
9ae3e9b
ec6de1b
9913064
0625721
371cd24
55e3a8c
fa463f0
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 | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -41,7 +41,7 @@ function handleDownload(url, fileName) { | |||||||||||||
|
||||||||||||||
// Android files will download to Download directory | ||||||||||||||
const path = dirs.DownloadDir; | ||||||||||||||
const attachmentName = fileName || FileUtils.getAttachmentName(url); | ||||||||||||||
const attachmentName = FileUtils.appendTimeToFileName(fileName) || FileUtils.getAttachmentName(url); | ||||||||||||||
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. The current logic for determining const attachmentName = FileUtils.appendTimeToFileName(fileName) || FileUtils.getAttachmentName(url);
The previous version of the function handled both of these cases 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. You're right. I think we should do const attachmentName = fileName ? FileUtils.appendTimeToFileName(fileName) : FileUtils.getAttachmentName(url); instead. Or should Btw, are you coming from an issue that is caused by this change? I'm curious to know in what case the cc: @parasharrajat 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. I've stumbled on the problem, when I was testing my PR on iOS: #25556 I've already included a fix in my PR It seems 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. I see. Thanks @kidroca for handling that. Let's know if you want any refactors. 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.
I see. What a unique case. Thanks for catching and handling it! |
||||||||||||||
|
||||||||||||||
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. If the App/src/libs/fileDownload/FileUtils.js Lines 56 to 61 in ecf0ec2
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 follow the same pattern for date. 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. which one to follow, the format from new: [name] - 2023-07-26T03:58:14.737Z.[extension], Also, I don't found any case for 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. I would say let's do the old one (24102000163544). But I don't have any preference. Both works. Feel free to use the one that look best to you. 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. I decided to use the date format from |
||||||||||||||
const isLocalFile = url.startsWith('file://'); | ||||||||||||||
|
||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
import DateUtils from '../../src/libs/DateUtils'; | ||
import * as FileUtils from '../../src/libs/fileDownload/FileUtils'; | ||
|
||
describe('FileUtils', () => { | ||
describe('splitExtensionFromFileName', () => { | ||
it('should return correct file name and extension', () => { | ||
const file = FileUtils.splitExtensionFromFileName('image.jpg'); | ||
expect(file.fileName).toEqual('image'); | ||
expect(file.fileExtension).toEqual('jpg'); | ||
}); | ||
|
||
it('should return correct file name and extension even with multiple dots on the file name', () => { | ||
const file = FileUtils.splitExtensionFromFileName('image.pdf.jpg'); | ||
expect(file.fileName).toEqual('image.pdf'); | ||
expect(file.fileExtension).toEqual('jpg'); | ||
}); | ||
|
||
it('should return empty extension if the file name does not have it', () => { | ||
const file = FileUtils.splitExtensionFromFileName('image'); | ||
expect(file.fileName).toEqual('image'); | ||
expect(file.fileExtension).toEqual(''); | ||
}); | ||
}); | ||
|
||
describe('appendTimeToFileName', () => { | ||
it('should append current time to the end of the file name', () => { | ||
const actualFileName = FileUtils.appendTimeToFileName('image.jpg'); | ||
const expectedFileName = `image-${DateUtils.getDBTime()}.jpg`; | ||
expect(actualFileName).toEqual(expectedFileName); | ||
}); | ||
|
||
it('should append current time to the end of the file name without extension', () => { | ||
const actualFileName = FileUtils.appendTimeToFileName('image'); | ||
const expectedFileName = `image-${DateUtils.getDBTime()}`; | ||
expect(actualFileName).toEqual(expectedFileName); | ||
}); | ||
}); | ||
}); |
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.
The change is to fix the file name that is returned as the extension when the file name does not have an extension, for example, if the file name is
anyfilename
, thefileExtension
isanyfilename
and thefileName
is empty.This function is being used for attachment validation. Before this change, we can upload a file name
jpg
becausejpg
is a valid extension.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.
Lovely. Thanks.