Add DualEntry integration - #97373
Conversation
|
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 |
|
@lakchote Please run the translate workflow #97373 (comment) |
|
@Expensify/design Can you please attach the icon for DualEntry |
|
@truph01 The BE PRs are not ready yet so please test with mock data (there is one in the design doc) |
![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: 67ba974604
ℹ️ 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".
| key={key} | ||
| /> | ||
| ), | ||
| onImportPagePress: () => null, |
There was a problem hiding this comment.
After a DualEntry connection is verified, PolicyAccountingPage renders the Import, Export, and Advanced rows using these callbacks, but this callback (and the Export/Advanced callbacks immediately below) just returns null. Admins will see tappable rows with chevrons, pending/error indicators, and no navigation, so they cannot view or fix any DualEntry settings from the accounting page; please navigate to implemented DualEntry settings pages or omit/disable these rows until those pages exist.
Useful? React with 👍 / 👎.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| function ConnectToDualEntryFlow({policyID}: ConnectToDualEntryFlowProps) { | ||
| const hasReusablePoliciesConnectedToDualEntry = useHasReusablePoliciesConnectedTo(CONST.POLICY.CONNECTIONS.NAME.DUALENTRY, policyID); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
❌ PERF-16 (docs)
This useEffect runs with an empty dependency array [] and its only work is a Navigation.navigate(...) call. Navigation is explicitly non-idempotent — under React Strict Mode this effect is intentionally double-invoked in development, which can fire the navigation twice and push a duplicate route. The eslint-disable/"run once" comment only silences the exhaustive-deps lint; it is not an execution guard.
Add a real one-time guard so the navigation runs exactly once:
let didInit = false;
function ConnectToDualEntryFlow({policyID}: ConnectToDualEntryFlowProps) {
const hasReusablePoliciesConnectedToDualEntry = useHasReusablePoliciesConnectedTo(CONST.POLICY.CONNECTIONS.NAME.DUALENTRY, policyID);
useEffect(() => {
if (didInit) {
return;
}
didInit = true;
if (hasReusablePoliciesConnectedToDualEntry) {
Navigation.navigate(ROUTES.POLICY_ACCOUNTING_DUALENTRY_EXISTING_CONNECTIONS.getRoute(policyID));
return;
}
Navigation.navigate(ROUTES.POLICY_ACCOUNTING_DUALENTRY_SETUP.getRoute(policyID));
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
return null;
}Reviewed at: 67ba974 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
|
|
||
| import {useEffect} from 'react'; | ||
|
|
||
| type ConnectToDualEntryFlowProps = { |
There was a problem hiding this comment.
❌ CONSISTENCY-13 (docs)
This newly added component props type declares its own prop but none of its members carry a /** ... */ block comment. Per STYLE.md, every component prop should be documented with a JSDoc block comment at its definition site.
type ConnectToDualEntryFlowProps = {
/** The ID of the policy being connected to DualEntry */
policyID: string;
};Reviewed at: 67ba974 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
Here you go: dualentry.svg.zip |
2 participants
Explanation of Change
Fixed Issues
$ #97234
$ #97235
$ #97236
PROPOSAL:
Tests
Offline tests
Same as 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 in all cases)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