Add workspace rules PDF document upload, view, and management UI#87684
Add workspace rules PDF document upload, view, and management UI#87684ishpaul777 wants to merge 18 commits intoExpensify:mainfrom
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
I'd like to review this PR when the time comes, so going to add myself proactively. |
ishpaul777
force-pushed
the
ishpaul/workspace-rules-document-upload
branch
from
159f567 to
340ab6f
Compare
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
|
@MelvinBot please review that this follows our code style and patterns |
|
Overall the PR follows our code style and patterns well — the Onyx optimistic/failure/finally structure, navigation wiring, modal content architecture, and API parameter types all match established conventions. A few things worth addressing: 1. 2. Inline style objects on every render — These create new object references each render: style={{width: variables.rulesDocumentThumbnailWidth, height: variables.rulesDocumentThumbnailHeight}}and {top: variables.spacing2, right: variables.spacing2}and {borderRadius: variables.componentSizeNormal / 2, backgroundColor: theme.cardBG}Consider extracting these to 3. 4. Minor — The Everything else (translations, route/screen registration, |
melvin-bot
Bot
requested review from
abzokhattab and
flaviadefaria
and removed request for
a team and
flaviadefaria
|
@abzokhattab 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] |
|
I think the modal title should match the label that we use above the thumbnail (which is "Policy document"): |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c41aa38280
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| onPicked: (files) => { | ||
| if (!policyID || !files.length) { | ||
| return; | ||
| } | ||
| updatePolicyRulesDocument(policyID, files.at(0) as File, policy?.rulesDocumentURL); |
There was a problem hiding this comment.
The selected file is sent directly to updatePolicyRulesDocument without checking MIME/extension, which means iOS/Android can submit non-PDF files here (native AttachmentPicker does not enforce acceptedFileTypes and uses types.allFiles for FILE mode). In that case the command may upload invalid content or fail server-side, breaking the “PDF rules document” flow for mobile users.
Useful? React with 👍 / 👎.
| subtitleTextStyles={[styles.textNormal, styles.colorMuted, styles.mr5]} | ||
| containerStyles={shouldUseNarrowLayout ? styles.p5 : styles.p8} | ||
| > | ||
| <OfflineWithFeedback pendingAction={policy?.pendingFields?.rulesDocumentURL}> |
There was a problem hiding this comment.
This wrapper only tracks pendingAction, but the new actions write failures into policy.errorFields.rulesDocumentURL. Because errors/onClose are not wired here, failed uploads/deletes silently revert with no visible feedback, so users cannot tell why the document update failed (e.g., rejected file type, network/server error).
Useful? React with 👍 / 👎.
ishpaul777
force-pushed
the
ishpaul/workspace-rules-document-upload
branch
from
c41aa38 to
a79a1df
Compare
fixed 
|
|
Thanks! Will run a test build now 🚀 |
|
@shawnborton BE is not merged so upload will not work right now 😅 will have to wait for BE PRs to be merged meanwhile here are screenshots 👤 Employee
🛠️ Admin
|
|
@MelvinBot Review the PR |
|
@codex review |
PR ReviewOverall this is well-structured and follows existing codebase patterns (Onyx optimistic/failure/finally, navigation wiring, Issues1. 2. 3. Section visibility condition is complex — consider extracting isBetaEnabled(CONST.BETAS.CUSTOM_RULES) && (isPolicyAdmin || !!policy?.rulesDocumentURL || !StringUtils.isEmptyString(policy?.customRules ?? ''))This condition is evaluated once but is hard to parse at a glance. A named boolean (e.g. 4. Minor: Looks good
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ba51e224d
ℹ️ 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".
| const contentProps = useMemo<AttachmentModalBaseContentProps>( | ||
| () => ({ | ||
| source: rulesDocumentSourceURL, | ||
| headerTitle: translate('workspace.rules.customRules.policyDocument'), | ||
| originalFileName: `${policyID}-policy-document.pdf`, | ||
| shouldShowNotFoundPage, | ||
| isLoading, | ||
| shouldCloseOnSwipeDown: true, |
There was a problem hiding this comment.
WorkspaceDocumentModalContent only passes source/metadata and never provides an onDownloadAttachment path (or a context that enables the built-in download control), so the new policy PDF is effectively view-only in-app. In AttachmentModalBaseContent, the header download button is gated by that callback/context, which means members cannot reliably download the rules document from this screen (especially on native, where browser save controls are unavailable).
Useful? React with 👍 / 👎.
|
Okay that looks pretty good, but let us know when the BE is ready so we can test with adhoc builds. Thanks! |
|
@shawnborton @abzokhattab BE PRs merged and deployed, can we trigger Adhoc build |
|
I have two weird issues.. On iOS when clicking on replace nothing happens ... also after uploading the PDF and opening it .. it keeps reloading ... is it the same for you? Screen.Recording.2026-04-22.at.16.27.30.movScreen.Recording.2026-04-22.at.16.28.33.mov |
|
@MelvinBot please investigate the all issues pointed out |
|
Here's my investigation of all four issues raised on this PR: 1. Validate rules document type before sending upload (Codex P2)Verdict: Already handled, but UX could be better on native. There is a post-pick PDF validation in On web, On native, the 2. Show rules document upload failures in the UI (Codex P2)Verdict: Not an issue — errors are already wired up. The
Both 3. Wire a download action into workspace document modal (Codex P2)Verdict: Valid issue — download button is hidden despite the callback being provided.
The download callback works; it's just unreachable. A fix would be to either pass a prop that satisfies the 4. Forward
|
| Issue | Valid? | Severity |
|---|---|---|
| File type validation | Partially — server-safe but native UX gap | Low |
| Error display in UI | No — already wired correctly | N/A |
| Download button hidden | Yes — button never renders | Medium |
Native acceptedFileTypes |
Yes — prop is ignored on native | Low |
Allow workspace admins to upload, replace, and remove a PDF document on the workspace overview page. Non-admins can view and download it. PDFs are stored in a private S3 bucket and served through an authenticated streaming endpoint.
- Rename field: policyDocumentURL → rulesDocumentURL across types, actions, utils, and pages - Rename API commands: UpdatePolicyRulesDocument, DeletePolicyRulesDocument - Rename param types and files to match - Rename utility: getPolicyDocumentSourceURL → getRulesDocumentSourceURL
Renamed updateWorkspaceRulesDocument -> updatePolicyRulesDocument and deleteWorkspaceRulesDocument -> deletePolicyRulesDocument. Updated GetPolicyDocument -> GetPolicyRulesDocument streaming endpoint.
- Add policyDocument and policyText translation keys to all locales - Use named constants for thumbnail dimensions instead of magic numbers - Position three-dots menu on thumbnail with circular background - Wrap Choose file button to prevent full-width stretch - Use policyText label for custom rules text field
- Add policyDocument and policyText to es, fr, de, it, ja, nl, pl - Add sentryLabel to PressableWithoutFeedback for PDF thumbnail - Add RULES_DOCUMENT sentry label constant
Extract inline style objects for the PDF thumbnail container, PDFThumbnail, ThreeDotsMenu position, and icon into useMemo to avoid creating new references on every render.
- Change rulesDocumentThumbnailWidth to rulesDocumentThumbnailMaxWidth for clarity. - Adjust thumbnail style to use maxWidth and set width to 100% for better responsiveness. - Update PDFThumbnail component to use a flex style for improved layout.
- Introduce file type validation to ensure only PDF documents are accepted for policy rules. - Refactor document selection logic to improve readability and maintainability. - Update UI to handle errors related to invalid file types more effectively.
- Update conditional rendering for the custom rules section to improve clarity and maintainability. - Adjust subtitle styles based on user permissions and document availability. - Enhance document handling UI for better user experience, including improved layout and accessibility features.
ishpaul777
force-pushed
the
ishpaul/workspace-rules-document-upload
branch
from
6dae0b8 to
00b1593
Compare
… document - Show download button in workspace document modal by including onDownloadAttachment in isValidContext check - Use receiptActionButton style for three-dots menu to match receipt preview icon buttons - Add shouldCallAfterModalHide to Replace menu item so native picker opens after popover closes
fixed this Screen.Recording.2026-04-22.at.9.15.42.PM.mov
i can't reproduce this but may be its because your pdf size is large its taking time to download Screen.Recording.2026-04-22.at.9.00.30.PM.mov |
- Map acceptedFileTypes extensions to native picker types (UTIs on iOS, MIME on Android) - Add shouldSkipAttachmentTypeModal prop to bypass Camera/Gallery/Document modal - Use shouldSkipAttachmentTypeModal for workspace rules document picker - Remove all type assertions in favor of String() coercion
|
@MelvinBot resolve conflicts |
|
|
|
@MelvinBot can you try again to fix conflicts |
…nflicts Co-authored-by: Ishpaul Singh <ishpaul777@users.noreply.github.com>
|
Merge conflicts resolved. The only conflict was in |
JmillsExpensify
left a comment
JmillsExpensify
left a comment
There was a problem hiding this comment.
LGTM. Not going to hold up this PR based on my NAB comment above.
| import launchCamera from './launchCamera/launchCamera'; | ||
| import type AttachmentPickerProps from './types'; | ||
|
|
||
| const EXTENSION_TO_NATIVE_TYPE: Record<string, string> = { |
There was a problem hiding this comment.
I think msword and text are not file extensions .. i think we can remove the msword
const EXTENSION_TO_NATIVE_TYPE: Record<string, string> = {
pdf: String(types.pdf),
doc: String(types.doc),
docx: String(types.docx),
zip: String(types.zip),
txt: String(types.plainText),
json: String(types.json),
xls: String(types.xls),
xlsx: String(types.xlsx),
jpg: String(types.images),
jpeg: String(types.images),
png: String(types.images),
gif: String(types.images),
heif: String(types.images),
heic: String(types.images),
tif: String(types.images),
tiff: String(types.images),
};6 participants
Explanation of Change
Adds frontend support for workspace admins to upload, replace, view, and delete a PDF rules document on the workspace overview page. Non-admins can view/download the document. This is the frontend (App) portion of a multi-repo change.
Key changes:
rulesDocumentURLfield on the Policy Onyx typeupdatePolicyRulesDocument/deletePolicyRulesDocumentaction functions with optimistic/failure/finally Onyx updatesWORKSPACE_DOCUMENTroute andWorkspaceDocumentModalContentgetRulesDocumentSourceURLutility routing remote URLs through authenticatedGetPolicyRulesDocumentstreaming endpoint with cache-bustingKey decisions:
rulesDocumentURL(notpolicyDocumentURL) —policyprefix is redundant inside a policy objectfile.uri(local blob URL) for instant display — same pattern as workspace avatar&cacheBuster=encoding the fullrulesDocumentURL— ensures unique streaming URL per PDF versionWORKSPACE_DOCUMENTroute instead of reusingREPORT_ATTACHMENTSCompanion PRs:
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/610806
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Designlabel and/or tagged@Expensify/designso the design team can review the changes.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
Android: mWeb Chrome
iOS: Native
Screen.Recording.2026-04-16.at.1.10.39.AM.mov
iOS: mWeb Safari
Screen.Recording.2026-04-16.at.1.10.02.AM.mov
MacOS: Chrome / Safari
Screen.Recording.2026-04-15.at.11.42.24.PM.mov
Screen.Recording.2026-04-15.at.11.42.24.PM.mov