[$500] [MEDIUM] Scan - No thumbnail is shown for the corrupted file in receipt preview
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: v1.4.44-8 Reproducible in staging?: Y Reproducible in production?: Y Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause-Internal Team
Action Performed:
- Go to staging.new.expensify.com.
- Go to workspace chat.
- Go to + > Request money > Scan.
- Upload a corrupted file (attached below).
- Proceed to confirmation page.
Expected Result:
The confirmation page will show thumbnail for the corrupted file.
Actual Result:
The confirmation page shows blank area for the corrupted file.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
- [x] Android: Native
- [x] Android: mWeb Chrome
- [x] iOS: Native
- [x] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [x] MacOS: Desktop
Screenshots/Videos
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~01ddfcfb2a8a486a23
- Upwork Job ID: 1763726750178574336
- Last Price Increase: 2024-03-30
Triggered auto assignment to @greg-schroeder (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
@greg-schroeder I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.
We think that this bug might be related to #wave5-free-submitters CC @dylanexpensify
Proposal
Please re-state the problem that we are trying to solve in this issue.
We are not showing a fallback image when image loading fails.
What is the root cause of that problem?
in the Image component here and here we don't have a mechanism to display a fallback image when image loading fails.
What changes do you think we should make in order to solve the problem?
we should make usage of the onError prop of react-native Image component expo-image Image component.
https://reactnative.dev/docs/image#onerror
https://docs.expo.dev/versions/latest/sdk/image/#onerror
add a prop to our Image component to accept a fallbackSource for the fallback image.
add a new state to the component:
const [hasImageError, setHasImageError] = useState(false);
add this function:
const handleImageError = () => {
setHasImageError(true);
};
and pass it as onError prop for react-native and expo-image Image components here and here
example:
return (
<RNImage
// eslint-disable-next-line react/jsx-props-no-spreading
{...forwardedProps}
source={hasImageError ? fallbackSource : source}
onError={handleImageError}
/>
);
based on the hasImageError we determine if we should use fallbackSource or source.
and then, here:
https://github.com/Expensify/App/blob/3e74ddf0d2b408faa270ff243cf333065b177a87/src/components/MoneyRequestConfirmationList.tsx#L627-L634
we should pass fallbackSource for the receipt.
we should ask design team to design the fallback error image for the receipt.
we can also design a generic fallback error image and use it as a fallbackSource default prop value for Image component.
Result:
for demo, I used @assets/images/eReceiptBGs/eReceiptBG_pink.png image as a fallback image:
https://github.com/Expensify/App/assets/77965000/51c44b5c-6dbd-4bd8-885c-360544ba4ede
What alternative solutions did you explore? (Optional)
we can also use defaultsource prop for react-native Image component and placeholder for expo-image Image component.
https://reactnative.dev/docs/image#defaultsource
https://docs.expo.dev/versions/latest/sdk/image/#placeholder
but the placeholder image will also show when loading.
Triaged
Job added to Upwork: https://www.upwork.com/jobs/~01ddfcfb2a8a486a23
Triggered auto assignment to Contributor-plus team member for initial proposal review - @situchan (External)
Initial proposal
### ProposalPlease re-state the problem that we are trying to solve in this issue.
[MEDIUM] Scan - No thumbnail is shown for the corrupted file in receipt preview
What is the root cause of that problem?
We don't handle image errors in MoneyTemporaryForRefactorRequestConfirmationList & we don't check for errors before returning the thumbnail & image.
https://github.com/Expensify/App/blob/a5092f5661478dda230204dfaf6be9e2b66e6e27/src/libs/ReceiptUtils.ts#L36
What changes do you think we should make in order to solve the problem?
For showing the default svgs like ReceiptGeneric, we need to follow steps below.
- Create a state for image error and set that to true when
onErrortriggers on <Image/> component. - Pass the error state to
getThumbnailAndImageURIsutil function. - Before returning
thumbnail&imageurl check for error which is passed. https://github.com/Expensify/App/blob/a5092f5661478dda230204dfaf6be9e2b66e6e27/src/libs/ReceiptUtils.ts#L54 https://github.com/Expensify/App/blob/a5092f5661478dda230204dfaf6be9e2b66e6e27/src/libs/ReceiptUtils.ts#L58
// For local files, we won't have a thumbnail yet
if (isReceiptImage && !error && typeof path === 'string' && (path.startsWith('blob:') || path.startsWith('file:'))) {
return {thumbnail: null, image: path, isLocalFile: true, filename};
}
if (isReceiptImage && !error) {
return {thumbnail: `${path}.1024.jpg`, image: path, filename};
}
Result
https://github.com/Expensify/App/assets/85894871/0d884d71-7467-4406-8686-707d787e9d7f
Alternatively
We can directly use ReceiptGeneric image as source when image error is true.
source={{uri: imageError ? ReceiptGeneric : receiptThumbnail || receiptImage}}
Proposal
Please re-state the problem that we are trying to solve in this issue.
Android - IOU - Nothing happens when you select corrupted jpg file in SmartScan/no error message
What is the root cause of that problem?
ImageSize.getSize fails to get the size of the image when its corrupted and throws error, but we don't handle it to show alert incase it fails.
https://github.com/Expensify/App/blob/e9c112a2b89217401dfef3e248fa8e93d4c4e418/src/components/AttachmentPicker/index.native.js#L271-L274
What changes do you think we should make in order to solve the problem?
We need to use third parameter of ImageSize.getSize which receives a failure callback and handle the error accordingly. Incase of corrupt image it throws unknown image format, we can use it and show corrupt image alert else we will show general alert.
For no-preview files we cannot detect if corrupted or not so we will use the image suggested by design team and pass it to fallbackSource.
https://github.com/Expensify/App/blob/3e74ddf0d2b408faa270ff243cf333065b177a87/src/components/MoneyRequestConfirmationList.tsx#L627
ImageSize.getSize(
fileData.fileCopyUri || fileData.uri,
(width, height) => {
fileData.width = width;
fileData.height = height;
return validateAndCompleteAttachmentSelection(fileData);
},
(error) => {
if (error.message === 'unknown image format') {
showImageCorruptionAlert();
} else {
showGeneralAlert();
}
},
);
If we want to block users from adding corrupted images on web also we can use ImageSize.getSize and block user and show modal when failure callback is called.
ImageSize.getSize(
fileCopy.fileCopyUri || fileCopy.uri,
() => {},
(e) => {
/// Block users and show message.
},
);
This is just a pseudo-code, it will require more changes but approach will be to use RNImage.getSize to check if the image file is corrupted or not. Will only add this check for images by using Str.isImage(fileCopy.fileName || fileCopy.name)
Pseudo code for web
function checkIfImageIsCorrupted(file) {
return new Promise((resolve, reject) => {
if (!Str.isImage(file.name)) {
resolve(true)
}
ImageSize.getSize(file.uri)
.then((r) => {
resolve(true);
})
.catch(() => {
reject(new Error(`'Error reading file: The file is corrupted`));
});
});
}
function validateReceipt(file) {
return checkIfImageIsCorrupted(file)
.then(() => {
const {fileExtension} = FileUtils.splitExtensionFromFileName(lodashGet(file, 'name', ''));
if (!CONST.API_ATTACHMENT_VALIDATIONS.ALLOWED_RECEIPT_EXTENSIONS.includes(fileExtension.toLowerCase())) {
setUploadReceiptError(true, 'attachmentPicker.wrongFileType', 'attachmentPicker.notAllowedExtension');
return false;
}
if (lodashGet(file, 'size', 0) > CONST.API_ATTACHMENT_VALIDATIONS.MAX_SIZE) {
setUploadReceiptError(true, 'attachmentPicker.attachmentTooLarge', 'attachmentPicker.sizeExceeded');
return false;
}
if (lodashGet(file, 'size', 0) < CONST.API_ATTACHMENT_VALIDATIONS.MIN_SIZE) {
setUploadReceiptError(true, 'attachmentPicker.attachmentTooSmall', 'attachmentPicker.sizeNotMet');
return false;
}
return true;
})
.catch(() => {
setUploadReceiptError(true, 'attachmentPicker.attachmentError', 'attachmentPicker.errorWhileSelectingCorruptedImage');
return false;
});
}
const setReceiptAndNavigate = (file) => {
validateReceipt(file).then((isFileValid) => {
if (!isFileValid) {
return;
}
// Store the receipt on the transaction object in Onyx
const source = URL.createObjectURL(file);
IOU.setMoneyRequestReceipt(transactionID, source, file.name, action !== CONST.IOU.ACTION.EDIT);
if (action === CONST.IOU.ACTION.EDIT) {
updateScanAndNavigate(file, source);
return;
}
navigateToConfirmationStep();
});
};
Fix for native bug here: /issues/38389
RNImage.getSize(
fileData.fileCopyUri || fileData.uri,
(width, height) => {
fileData.width = width;
fileData.height = height;
return validateAndCompleteAttachmentSelection(fileData);
},
(error) => {
validateAndCompleteAttachmentSelection(fileData);
},
);
Result
https://github.com/Expensify/App/assets/85894871/e1b1151a-b7d6-4d85-9096-a8458b87f371
Alternative
Or simply use fileData (without modifications) when error callback is triggered.
RNImage.getSize(
fileData.fileCopyUri || fileData.uri,
(width, height) => {
fileData.width = width;
fileData.height = height;
return validateAndCompleteAttachmentSelection(fileData);
},
(error) => {
validateAndCompleteAttachmentSelection(fileData);
},
);
The corrupted file used to create this issue seems to be a very small image, only 16 x 16 pixels. A thumbnail of this image might appear blank, possibly. The corrupted file I am using works as expected. I see a thumbnail of it.
proposals are in review
@greg-schroeder, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
eta Monday
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@greg-schroeder, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
Proposal review
@greg-schroeder @situchan this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
@Expensify/design do we agree to show thumbnail when attached receipt is not previewable (i.e. corrupted image/pdf)?
I think showing a fallback image makes sense—but I'm not sure I would show that type of thumbnail. I feel like it kinda makes more sense to show our little fallback image thingy:
What do the other designers think?
Yup, I agree with Danny here.
Agree with Shawn who agrees with Danny
Thanks for feedback. Please update proposals.
@situchan, I guess we shouldn't allow users to upload corrupted images when creating a scan request, it will eventually fail. We can hold on this issue if we agree to block users from creating a scan request. In native we can't even upload corrupted images.
@Krishna2323 the expected behavior should be discussed with design team. Why don't we just use this GH if we don't allow corrupted images? I see the the linked issue created just today with no context.
Yeah, we can handle that here since we already have context. But I think we should confirm with the design team before we try to hold that issue because it has some differences.
@shawnborton can you pls help
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@greg-schroeder, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!
@greg-schroeder @situchan this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!
We're going to prevent uploading corrupted PDF as scan receipt in #34032. So to be consistent, we should not allow corrupted images. cc: @Expensify/design to confirm the expected behavior.
^ is only when we're able to detect if file is corrupted or not. For no-preview files we cannot detect if corrupted or not, I think it still makes sense to show fallback image like this.
We're going to prevent uploading corrupted PDF as scan receipt in https://github.com/Expensify/App/issues/34032. So to be consistent, we should not allow corrupted images.
@situchan that makes sense to me.
For no-preview files we cannot detect if corrupted or not, I think it still makes sense to show fallback image like https://github.com/Expensify/App/issues/37435#issuecomment-1995579903.
Agree with this as well!