Skip to content

Commit

Permalink
Merge pull request #31791 from kidroca/kidroca/fix/regressions-relate…
Browse files Browse the repository at this point in the history
…d-to-file-downloads

Refactor and Fix Attachment Handling in File Download and Carousel
  • Loading branch information
NikkiWines authored Dec 5, 2023
2 parents 9cc23b6 + cd60646 commit 3915fb2
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 18 deletions.
4 changes: 2 additions & 2 deletions src/components/Attachments/AttachmentCarousel/CarouselItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ const propTypes = {
/** Additional information about the attachment file */
file: PropTypes.shape({
/** File name of the attachment */
name: PropTypes.string,
}),
name: PropTypes.string.isRequired,
}).isRequired,

/** Whether the attachment has been flagged */
hasBeenFlagged: PropTypes.bool,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {Parser as HtmlParser} from 'htmlparser2';
import _ from 'underscore';
import * as FileUtils from '@libs/fileDownload/FileUtils';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import tryResolveUrlFromApiRoot from '@libs/tryResolveUrlFromApiRoot';
import CONST from '@src/CONST';
Expand All @@ -21,14 +22,16 @@ function extractAttachmentsFromReport(parentReportAction, reportActions) {
}

const expensifySource = attribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE];
const source = tryResolveUrlFromApiRoot(expensifySource || attribs.src);
const fileName = attribs[CONST.ATTACHMENT_ORIGINAL_FILENAME_ATTRIBUTE] || FileUtils.getFileName(`${source}`);

// By iterating actions in chronological order and prepending each attachment
// we ensure correct order of attachments even across actions with multiple attachments.
attachments.unshift({
source,
reportActionID: attribs['data-id'],
source: tryResolveUrlFromApiRoot(expensifySource || attribs.src),
isAuthTokenRequired: Boolean(expensifySource),
file: {name: attribs[CONST.ATTACHMENT_ORIGINAL_FILENAME_ATTRIBUTE]},
file: {name: fileName},
hasBeenFlagged: attribs['data-flagged'] === 'true',
});
},
Expand Down
4 changes: 2 additions & 2 deletions src/components/Attachments/propTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import PropTypes from 'prop-types';

const attachmentSourcePropType = PropTypes.oneOfType([PropTypes.string, PropTypes.func, PropTypes.number]);
const attachmentFilePropType = PropTypes.shape({
name: PropTypes.string,
name: PropTypes.string.isRequired,
});

const attachmentPropType = PropTypes.shape({
Expand All @@ -13,7 +13,7 @@ const attachmentPropType = PropTypes.shape({
source: attachmentSourcePropType.isRequired,

/** File object can be an instance of File or Object */
file: attachmentFilePropType,
file: attachmentFilePropType.isRequired,
});

const attachmentsPropType = PropTypes.arrayOf(attachmentPropType);
Expand Down
22 changes: 16 additions & 6 deletions src/libs/fileDownload/FileUtils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {Alert, Linking, Platform} from 'react-native';
import DateUtils from '@libs/DateUtils';
import * as Localize from '@libs/Localize';
import Log from '@libs/Log';
import CONST from '@src/CONST';
import type {ReadFileAsync, SplitExtensionFromFileName} from './types';

Expand Down Expand Up @@ -75,13 +76,22 @@ function showCameraPermissionsAlert() {
}

/**
* Generate a random file name with timestamp and file extension
* Extracts a filename from a given URL and sanitizes it for file system usage.
*
* This function takes a URL as input and performs the following operations:
* 1. Extracts the last segment of the URL, which could be a file name, a path segment,
* or a query string parameter.
* 2. Decodes the extracted segment from URL encoding to a plain string for better readability.
* 3. Replaces any characters in the decoded string that are illegal in file names
* with underscores.
*/
function getAttachmentName(url: string): string {
if (!url) {
return '';
function getFileName(url: string): string {
const fileName = url.split(/[#?/]/).pop() ?? '';
if (!fileName) {
Log.warn('[FileUtils] Could not get attachment name', {url});
}
return `${DateUtils.getDBTime()}.${url.split(/[#?]/)[0].split('.').pop()?.trim()}`;

return decodeURIComponent(fileName).replace(CONST.REGEX.ILLEGAL_FILENAME_CHARACTERS, '_');
}

function isImage(fileName: string): boolean {
Expand Down Expand Up @@ -231,7 +241,7 @@ export {
showPermissionErrorAlert,
showCameraPermissionsAlert,
splitExtensionFromFileName,
getAttachmentName,
getFileName,
getFileType,
cleanFileName,
appendTimeToFileName,
Expand Down
2 changes: 1 addition & 1 deletion src/libs/fileDownload/index.android.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function handleDownload(url: string, fileName: string): Promise<void> {

// Android files will download to Download directory
const path = dirs.DownloadDir;
const attachmentName = FileUtils.appendTimeToFileName(fileName) || FileUtils.getAttachmentName(url);
const attachmentName = FileUtils.appendTimeToFileName(fileName || FileUtils.getFileName(url));

const isLocalFile = url.startsWith('file://');

Expand Down
2 changes: 1 addition & 1 deletion src/libs/fileDownload/index.ios.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ const fileDownload: FileDownload = (fileUrl, fileName) =>
new Promise((resolve) => {
let fileDownloadPromise;
const fileType = FileUtils.getFileType(fileUrl);
const attachmentName = FileUtils.appendTimeToFileName(fileName) || FileUtils.getAttachmentName(fileUrl);
const attachmentName = FileUtils.appendTimeToFileName(fileName || FileUtils.getFileName(fileUrl));

switch (fileType) {
case CONST.ATTACHMENT_FILE_TYPE.IMAGE:
Expand Down
5 changes: 1 addition & 4 deletions src/libs/fileDownload/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,7 @@ const fileDownload: FileDownload = (url, fileName) => {
// adding href to anchor
link.href = href;
link.style.display = 'none';
link.setAttribute(
'download',
FileUtils.appendTimeToFileName(fileName) || FileUtils.getAttachmentName(url), // generating the file name
);
link.download = FileUtils.appendTimeToFileName(fileName || FileUtils.getFileName(url));

// Append to html link element page
document.body.appendChild(link);
Expand Down

0 comments on commit 3915fb2

Please sign in to comment.