feat(operators): Add Migrate Organization Tool #6748
Conversation
src/operator/MigrateOrg.tsx
Outdated
| </> | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
Would you mind removing the default exports? 🙇♂️ Makes it harder to track down references than using a named export.
| } = useContext(AccountContext) | ||
|
|
||
| const migrateAccountOrgs = () => { | ||
| if (account?.type !== 'cancelled') { |
There was a problem hiding this comment.
Is this just an extra guard in case there's a race condition, like the account being canceled while the overlay is being opened - since we only render an empty fragment instead of the MigrateOrgsOverlay if the account is canceled?
Since this try / catch block is only hit if the account type isn't canceled, might still want a notification here so that support doesn't presume the migration works if they hit the migration button and nothing happens.
| } | ||
|
|
||
| setMigrateOverlayVisible(false) | ||
| setMigrateStatus(RemoteDataState.Done) |
There was a problem hiding this comment.
I might reverse the order of lns 196 and 197 here, since migrateStatus is used in the migration overlay. By the time migrateStatus is Done I suspect the overlay will be removed from the DOM.
There was a problem hiding this comment.
That's a good point. Changing it.
src/operator/MigrateOrgOverlay.tsx
Outdated
| ) | ||
| } | ||
|
|
||
| export default MigrateOrgOverlay |
There was a problem hiding this comment.
Same comment here re: default exports. Let's make this a named import so that it's easier to trace and tree shake.
|
|
||
| const submit = async () => { | ||
| if (toAccountId === '') { | ||
| return |
There was a problem hiding this comment.
Do we need a notification here - like "please enter an account to migrate to"?
There was a problem hiding this comment.
Yeah, that's a good point. This is a pretty blind failure. It seems like I can pull use the useDispatch() hook with notify to put up an error. Does that make sense?
Also, remocal is failing me... so I'm pushing up these changes to run in CI 😅 🤞
| try { | ||
| const resp = await getOperatorAccount({accountId: toAccountId}) | ||
| if (resp.status !== 200) { | ||
| return |
There was a problem hiding this comment.
Same - could we add a notification letting the user know there was an error retrieving the ToAccount?
abshierjoel
force-pushed
the
add-migrate-to-operator-ui
branch
from
3fd3214
to
8cbfddf
Compare
abshierjoel
force-pushed
the
add-migrate-to-operator-ui
branch
from
8cbfddf
to
3c379c8
Compare
Allows operators to migrate all of an account's organizations or a specific organization to a different account.
Checklist
Authors and Reviewer(s), please verify the following: