[Release 3.3] [Domain Control] Report suspicious activity#78372
[Release 3.3] [Domain Control] Report suspicious activity#78372mountiny merged 51 commits intoExpensify:mainfrom
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 |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
| } | ||
|
|
||
| setIsLoading(true); | ||
| const response = await lockAccount(accountID); |
There was a problem hiding this comment.
Let's add a domainAccountID parameter to LockAccount so that we can return Onyx data for errors
src/CONST/index.ts
Outdated
| /** Onyx prefix for domain security groups */ | ||
| DOMAIN_SECURITY_GROUP_PREFIX: 'domain_securityGroup_', | ||
| /** Onyx prefix for lock account IDs */ | ||
| EXPENSIFY_LOCKED_ACCOUNT_PREFIX: 'expensify_lockAccountDetails_', |
There was a problem hiding this comment.
| EXPENSIFY_LOCKED_ACCOUNT_PREFIX: 'expensify_lockAccountDetails_', | |
| PRIVATE_LOCKED_ACCOUNT_PREFIX: 'private_lockAccountDetails_', |
| description={translate('common.vacationDelegate')} | ||
| shouldShowRightIcon | ||
| onPress={onPress} | ||
| containerStyle={styles.pr2} |
There was a problem hiding this comment.
Based on the discussion on this PR I'd say the custom mr on Clear after should be adjusted instead ? cc. @Expensify/design
There was a problem hiding this comment.
@Expensify/design What do you think?
There was a problem hiding this comment.
The staging screenshot looks more correct to me - but maybe I've missed a discussion somewhere? Let's see what the rest of @Expensify/design thinks.
| resetDomainExplanation: ({domainName}: {domainName?: string}) => `Please type <strong>${domainName}</strong> to confirm the domain reset.`, | ||
| enterDomainName: 'Enter your domain name here', | ||
| resetDomainInfo: `This action is <strong>permanent</strong> and the following data will be deleted: <br/> <ul><li>Company card connections and any unreported expenses from those cards</li> <li>SAML and group settings</li> </ul> All accounts, workspaces, reports, expenses, and other data will remain. <br/><br/>Note: You can clear this domain from your domains list by removing the associated email from your <a href="#">contact methods</a>.`, | ||
| resetDomainInfo: `This action is <strong>permanent</strong> and the following data will be deleted: <br/> <bullet-list><bullet-item>Company card connections and any unreported expenses from those cards</bullet-item><bullet-item>SAML and group settings</bullet-item></bullet-list> All accounts, workspaces, reports, expenses, and other data will remain. <br/><br/>Note: You can clear this domain from your domains list by removing the associated email from your <a href="#">contact methods</a>.`, |
There was a problem hiding this comment.
Do you know why it worked for closeAccountInfo?
There was a problem hiding this comment.
Yes, when you pass a link to the RenderHTML it is using a different RenderHTMLConfigProvider. It does not have custom rendereres passed to it.
Before it was ok, bc the bullet list was using plain html
| closeAccountInfo: () => ({ | ||
| one: 'We recommend closing the account safely to skip closing it in case there are: <ul><li>Pending approvals</li><li>Active reimbursements</li><li>No alternative login methods</li></ul>Otherwise, you can ignore the safety precautions above and force close the selected account.', | ||
| other: 'We recommend closing the accounts safely to skip closing it in case there are: <ul><li>Pending approvals</li><li>Active reimbursements</li><li>No alternative login methods</li></ul>Otherwise, you can ignore the safety precautions above and force close the selected accounts.', | ||
| one: 'We recommend closing the account safely to skip closing it in case there are: <bullet-list><bullet-item>Pending approvals</bullet-item><bullet-item>Active reimbursements</bullet-item><bullet-item>No alternative login methods</bullet-item></bullet-list>Otherwise, you can ignore the safety precautions above and force close the selected account.', |
There was a problem hiding this comment.
|
Just waiting for @Expensify/design on #78372 (comment) |
|
Visually this looks great! Approved on my end |
@dubielzyk-expensify are you saying that this inconsistency is fine?
|
|
No that's not fine, but that's the status screen. What's that got to do with the domain control part? Sorry if I've missed something |
|
@dubielzyk-expensify ok then, is this inconsistency fine? Right padding is different between same components.
|
|
@dubielzyk-expensify we had a conversation with @shawnborton above about the right padding on arrows #78372 (comment) And the decision was to not modify the standard They are modified (less margin right) in one other place in the app, and unlucky for us this place is |
|
@jmusial ok let's do what we landed and push the fix. |
Waiting for @Expensify/design to confirm before making changes |
|
I'm a bit confused, but if Shawn suggested a way forward then lets go with that. In general we don't wanna specifically tweak these components per screen. They should work the same everywhere since they're just a bunch of list components. |
mountiny
left a comment
There was a problem hiding this comment.
Changes look good to me, I dont want to hold this one any further, we can clean up the padding on the menu item later in case we will decide that way
|
🚧 @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! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.3.31-0 🚀
|
|
Deploy Blocker #84214 was identified to be related to this PR. |
Opened a PR with a fix |
|
Fix cped |
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.3.31-12 🚀
|










Explanation of Change
Part for Domain Control project. This PR adds reporting suspicious activity (locking users account).
Based on the design doc & discussion it resembles Report Suspicious Activity from the Settings/Account page.
Fixed Issues
$ #79568
PROPOSAL:
Tests
Pre requisites:
Have a domain and be an admin there.
Offline tests
QA Steps
Same as tests.
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: Native
0093.android.native.mov
Android: mWeb Chrome
0093.android.chrome.mov
iOS: Native
0093.ios.native.mov
iOS: mWeb Safari
0093.ios.safari.mov
MacOS: Chrome / Safari
0093.desktop.chrome.mov