Repository navigation
Add imported vendor enable and disable controls - #103549
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
I reverted the main PR, sorry 🙈 Let's add those changes back to this one? |
|
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 bun ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51739a2ed5
ℹ️ 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".
|
@ShridharGoel is this ready for review / testing yet? Once it's ready, i can make an adhoc build for @heyjennahay to test 🙏 |
|
Yes it is ready |
…-main # Conflicts: # src/libs/Violations/ViolationsUtils.ts # src/pages/iou/request/step/IOURequestStepVendor.tsx # tests/unit/ViolationUtilsTest.ts
|
@Pujan92 Fixed |
|
@MelvinBot review |
Code reviewThis PR is close. I found one bug to fix before merge and a few smaller items. CI passes, apart from the approval check. The earlier bot comments look addressed: Should fix1. Read-only users never see the read-only modal. The table passes Minor2. The vendor picker shows "not loaded yet" as "Disabled". 3. The fetch logic is duplicated. The 4. A comment is now out of date. UpdateMoneyRequest.ts:866-870 says the picker offers vendors from 5. The zh-hans wording is inconsistent. I didn't run UI tests. They need a workspace with a connected accounting integration and imported vendors, plus the paired Auth and Web-Expensify changes. |
|
@MelvinBot review |
Code review (follow-up)The latest commits fix every item from my previous review. I found no new bugs. CI passes, apart from the approval check.
Two optional follow-ups:
|
|
@Beamanator It's ready for your review |
|
🚧 Beamanator 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/Beamanator in version: 9.5.20-0 🚀
|
|
A help site update is needed. I opened a draft PR: Docs updates for E/A#103549 Remove read-only vendor list wording, add disable step This PR adds an Enabled column to Workspaces > [workspace name] > Vendors. That makes this sentence wrong in four vendor matching articles:
The docs PR changes that sentence in each article to say the list refreshes on sync and that admins can disable a vendor in the Enabled column to hide it from the Vendor field. Scope
@ShridharGoel, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
Deploy Blocker #104891 was identified to be related to this PR. |
|
Deploy Blocker #104892 was identified to be related to this PR. |
|
Deploy Blocker #104894 was identified to be related to this PR. |
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.5.20-5 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
This PR adds enable and disable controls for imported accounting vendors (re-applying Add imported vendor enable and disable controls) and fixes the two issues found during staging QA:
enabledtoggle column withSwitchand lock support.EnableandDisableactions (ButtonWithDropdownMenu).canWriteVendorstoselectionEnabledso long-press bulk selection works on mobile layouts.policyVendors_{policyID}Onyx collection and fetch it on page open and reconnect viaOpenPolicyVendorsPageso stale sessions load the vendor list.setPolicyVendorsEnabledcallingSetPolicyVendorsEnabledwith optimistic update, success merge, and failure rollback.clearVendorErrorsfor error dismissal.openPolicyVendorsPagecallingOpenPolicyVendorsPage.IOURequestStepVendor):Disabledsubtitle so the user can pick an enabled vendor or clear it with "None".policyVendors_{policyID}viaOpenPolicyVendorsPagewhen the collection is missing in Onyx.Paired PRs:
Fixed Issues
$ #98355
$ https://github.com/Expensify/Expensify/issues/678247
$ #103529
$ #103556
PROPOSAL: N/A
Tests
Offline tests
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected)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
iOS: mWeb Safari
MacOS: Chrome / Safari