fix: PDF preview not working

[OtavioStasiak]

Proposed changes

PDF was not downloading correctly because the expo file system was not sending the headers properly.

Issue(s)

https://rocketchat.atlassian.net/browse/CORE-1811

How to test or reproduce

• Open the app;
• Go to a room with a message with PDF attachment;
• Open the PDF;

Screenshots

Before	After

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• Improvement (non-breaking change which improves a current function)
• New feature (non-breaking change which adds functionality)
• Documentation update (if none of the other choices apply)

Checklist

• I have read the CONTRIBUTING doc
• I have signed the CLA
• Lint and unit tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works (if applicable)
• I have added necessary documentation (if applicable)
• Any dependent changes have been merged and published in downstream modules

Further comments

Summary by CodeRabbit

• New Features
  • Enhanced file download functionality to include HTTP headers, improving compatibility and handling of downloaded files.

[coderabbitai]

Walkthrough

A single file modification adds HTTP headers parameter to file download requests via FileSystem.downloadAsync, enabling downloaded files to include specified headers. No control flow or error handling changes—only request option modifications.

Changes

Cohort / File(s)	Summary
File Download Enhancement app/lib/methods/helpers/fileDownload.ts	Added HTTP headers parameter to FileSystem.downloadAsync call to include headers in file download requests.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A PDF that once would vanish in the air,
Now travels down with headers, safe and fair,
The rabbit hops through fetch and streams divine,
And downloads bloom like carrots in a line! 📄✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR partially addresses the linked issue CORE-1811 by implementing a fix for PDF downloads via headers, but does not address the broader requirements of library migration evaluation or Android testing.	Document whether the current fix resolves the core issue or if further work (library migration, Android testing) is still required per CORE-1811 scope.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: PDF preview not working' clearly summarizes the main change: fixing PDF preview functionality by adding missing headers to the download flow.
Out of Scope Changes check	✅ Passed	The change (adding headers to file download) is directly in-scope for fixing the PDF preview issue described in CORE-1811; no out-of-scope modifications detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

app/lib/methods/helpers/fileDownload.ts (1)

22-22: Type assertion as Record<string, string> may mask issues if headers is undefined or has unexpected shape.

If headers is undefined (e.g., before the user logs in or if the export is missing), the cast silently passes { headers: undefined } to downloadAsync, which won't fail but won't send auth headers either. Consider adding a guard or at least a runtime check so that a missing-headers scenario surfaces visibly during development.

💡 Optional: add a dev-time guard

-	const file = await FileSystem.downloadAsync(url, path, { headers: headers as Record<string, string> });
+	if (__DEV__ && !headers) {
+		console.warn('fileDownload: headers are not set; the download may fail authentication.');
+	}
+	const file = await FileSystem.downloadAsync(url, path, { headers: headers as Record<string, string> });

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between acf48d4 and a38319d.

📒 Files selected for processing (1)

• app/lib/methods/helpers/fileDownload.ts

🧰 Additional context used

🧬 Code graph analysis (1)

app/lib/methods/helpers/fileDownload.ts (1)

app/lib/methods/helpers/fetch.ts (1)

• url (42-54)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: E2E Build iOS / ios-build
• GitHub Check: E2E Build Android / android-build

🔇 Additional comments (1)

app/lib/methods/helpers/fileDownload.ts (1)

10-10: The headers export from ./fetch.ts exists and the import is valid.

The named export headers is defined on line 23 of fetch.ts as export const headers: CustomHeaders = {...}. The import on line 10 of fileDownload.ts correctly resolves to this export. The concern in the original comment was based on an incomplete code snippet that didn't include the export definition.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

iOS Build Available

Rocket.Chat 4.70.0.108257