Fix: "Attachment can't be downloaded" error when downloading receipt#81652
Conversation
| import type PDFThumbnailProps from './types'; | ||
|
|
||
| function PDFThumbnail({previewSourceURL, style, isAuthTokenRequired = false, enabled = true, fitPolicy = 0, onPassword, onLoadError, onLoadSuccess}: PDFThumbnailProps) { | ||
| function PDFThumbnail({previewSourceURL, style, enabled = true, fitPolicy = 0, onPassword, onLoadError, onLoadSuccess}: PDFThumbnailProps) { |
There was a problem hiding this comment.
I checked the other places where the PDFThumbnail component is used, and we no longer use isAuthTokenRequired. Since it’s no longer needed, I removed this prop and addEncryptedAuthTokenToURL
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@mananjadhav Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 875f359aad
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
mananjadhav
left a comment
There was a problem hiding this comment.
I reviewed the code.
Can we trigger an adhoc build for this one?
|
I'll trigger an ad-hoc build |
This comment has been minimized.
This comment has been minimized.
|
🚧 @Valforte has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
Please merge main so the ad hoc build works |
|
🚧 @Valforte has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
mananjadhav
left a comment
There was a problem hiding this comment.
The code changes are fine. Just the QA is taking a bit longer. I'll finish this tomorrow. The method call changed at 15+ places so need to test it.
|
@linhvovan29546 Can you please resolve conflicts? |
|
Resolving |
Done, just waiting for the workflow to complete. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid-attachment-download.movAndroid: mWeb Chromemweb-chrome-attachment-download.moviOS: HybridAppios-attachment-download.MP4iOS: mWeb Safarimweb-safari-attachment-download.MP4MacOS: Chrome / Safariweb-attachment-download.mov |
|
@linhvovan29546 Can you fix the type in the PR title |
Done. Previously I was unable to update PR titles across my PR, this is now resolved |
|
🚧 @Valforte has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/Valforte in version: 9.3.19-0 🚀
|
| const styles = useThemeStyles(); | ||
| const session = useSession(); | ||
| const encryptedAuthToken = session?.encryptedAuthToken ?? ''; | ||
| const addAuthTokenToImageURL = (url: string) => addEncryptedAuthTokenToURL(url, encryptedAuthToken); |
There was a problem hiding this comment.
This component is not react compilable so auto memoization didn't work. So it caused #82465.
In the future, please carefully check if component is react compilable or not when remove manual memoization.
For this check, install react-compiler-marker extension.
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.3.19-5 🚀
|
Explanation of Change
Fixed Issues
$ #80860
PROPOSAL: #80860 (comment)
Tests
Offline tests
No impact offline mode.
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen.Recording.2026-02-06.at.18.51.45.mov
Android: mWeb Chrome
Screen.Recording.2026-02-06.at.18.36.06.mov
iOS: Native
Screen.Recording.2026-02-06.at.19.29.14.mov
iOS: mWeb Safari
Screen.Recording.2026-02-06.at.18.54.01.mov
MacOS: Chrome / Safari
Screen.Recording.2026-02-06.at.18.33.59.mov