Domain control 3.4 - set vacation delegate#80100
Conversation
…etails_page' into war-in/domains/add-vacation-delegate-page # Conflicts: # src/libs/Navigation/AppNavigator/ModalStackNavigators/index.tsx # src/pages/domain/BaseDomainMemberDetailsComponent.tsx
# Conflicts: # src/ROUTES.ts # src/SCREENS.ts # src/libs/Navigation/AppNavigator/ModalStackNavigators/index.tsx # src/libs/Navigation/linkingConfig/RELATIONS/DOMAIN_TO_RHP.ts # src/libs/Navigation/linkingConfig/config.ts # src/libs/actions/Domain.ts
|
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 |
# Conflicts: # src/ROUTES.ts # src/languages/en.ts # src/libs/Navigation/AppNavigator/ModalStackNavigators/index.tsx # src/libs/Navigation/linkingConfig/RELATIONS/DOMAIN_TO_RHP.ts # src/libs/Navigation/linkingConfig/config.ts # src/libs/Navigation/types.ts # src/libs/actions/Domain.ts # src/selectors/Domain.ts # src/types/onyx/DomainErrors.ts # tests/unit/DomainSelectorsTest.ts
# Conflicts: # src/pages/settings/Profile/CustomStatus/VacationDelegatePage.tsx
| const parameters: SetVacationDelegateParams = { | ||
| creator, | ||
| vacationerEmail: vacationer, | ||
| vacationDelegateEmail: delegate, | ||
| overridePolicyDiffWarning: true, | ||
| }; |
There was a problem hiding this comment.
Let's add domainAccountID parameter to SetVacationDelegate and DeleteVacationDelegate
| onPress={() => Navigation.navigate(ROUTES.DOMAIN_VACATION_DELEGATE.getRoute(domainAccountID, accountID))} | ||
| pendingAction={domainPendingActions?.member?.[accountID]?.vacationDelegate} | ||
| errors={getLatestError(domainErrors?.memberErrors?.[accountID]?.vacationDelegateErrors)} | ||
| onCloseError={() => clearVacationDelegateError(domainAccountID, accountID, domainPendingActions?.member?.[accountID]?.vacationDelegate)} |
There was a problem hiding this comment.
We agreed on using the member's email instead of accountID here, so let's update this. Thanks
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66be0a8214
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5769c21e4b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5769c21e4b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5769c21e4b
ℹ️ 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".
mountiny
left a comment
There was a problem hiding this comment.
Let's add tests for actions too
Also I dont think the design review was done here, please make sure that all the PRs especially if they release (no beta) have design review done as well before we can merge.
@dubielzyk-expensify can you please review this one?
| API.write(WRITE_COMMANDS.DELETE_DOMAIN_MEMBER, parameters, {optimisticData, successData, failureData}); | ||
| } | ||
|
|
||
| function setDomainVacationDelegate(domainAccountID: number, domainMemberAccountID: number, creator: string, vacationer: string, delegate: string, vacationDelegate?: BaseVacationDelegate) { |
There was a problem hiding this comment.
+1 still missing tests for the actions
| @@ -0,0 +1,15 @@ | |||
| import {vacationDelegateSelector} from '@selectors/Domain'; | |||
There was a problem hiding this comment.
Yeah I dont really think that we need this if its just useOnyx with selector
|
Can we run a test build so I can try it out? |
|
@mountiny both comments resolved |
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
@dubielzyk-expensify Here you go |
|
That looks good to me 👍 Awesome work |
|
🚧 @mountiny 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! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
Hi @sumo-slonik. Has this option been implemented already? Can the QA team test it? |
Yes 2026-02-20.17.27.39.mov |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.3.24-1 🚀
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.3.24-3 🚀
|
Explanation of Change
Fixed Issues
$ #79570
PROPOSAL:
Tests
(For now this works only in Onyx — there is no backend call yet because the API is not ready.)
Offline tests
For now unnesesary (waiting for endpoint)
QA Steps
Same as test
// TODO: These must be filled out, or the issue title must include "[No QA]."
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: mWeb Chrome
web.mov