Skip to content

Show no-access modal when delegate tries to update copilots #78110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Valforte merged 6 commits into from
Dec 29, 2025
Merged

Show no-access modal when delegate tries to update copilots #78110

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dd81f0b
Show no-access modal when delegate tries to update copilots
ShridharGoel Dec 19, 2025
fd4616f
Show no-access modal when delegate tries to update copilots
ShridharGoel Dec 19, 2025
57a4108
remove extra deps
ShridharGoel Dec 19, 2025
84d4786
Merge branch 'main' of https://github.com/Expensify/App into delegate…
ShridharGoel Dec 19, 2025
58bcdc6
Merge branch 'main' of https://github.com/Expensify/App into delegate…
ShridharGoel Dec 23, 2025
f84f6b5
Change check for access level
ShridharGoel Dec 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/pages/settings/Security/SecuritySettingsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ function SecuritySettingsPage() {
});

const {isAccountLocked, showLockedAccountModal} = useContext(LockedAccountContext);
const {isDelegateAccessRestricted, showDelegateNoAccessModal} = useContext(DelegateNoAccessContext);
const {isActingAsDelegate, isDelegateAccessRestricted, showDelegateNoAccessModal} = useContext(DelegateNoAccessContext);
const delegates = account?.delegatedAccess?.delegates ?? [];
const delegators = account?.delegatedAccess?.delegators ?? [];

Expand Down Expand Up @@ -320,7 +320,7 @@ function SecuritySettingsPage() {
text: translate('delegate.removeCopilot'),
icon: Expensicons.Trashcan,
onPress: () => {
if (isDelegateAccessRestricted) {
if (isActingAsDelegate) {
modalClose(() => showDelegateNoAccessModal());
return;
}
Expand Down Expand Up @@ -453,6 +453,11 @@ function SecuritySettingsPage() {
prompt={translate('delegate.removeCopilotConfirmation')}
danger
onConfirm={() => {
if (isActingAsDelegate) {
setShouldShowRemoveDelegateModal(false);
showDelegateNoAccessModal();
return;
}
Comment on lines +456 to +460

@truph01 truph01 Dec 19, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this logic? We already prevent the user from clicking the "Remove Copilot" button, so is there any chance this logic could still be triggered?

@ShridharGoel ShridharGoel Dec 19, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the session flips to "acting as delegate" after the modal is already open (e.g., user switches to a delegator account in another tab and returns, or future code opens the modal from some other entry point), this check can be useful. What do you think?

@grgia grgia Dec 19, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ShridharGoel why inside the onConfirm though?

@ShridharGoel ShridharGoel Dec 19, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@grgia Because if the user somehow views the modal and clicks on confirm, but they are a delegate, then the remove-copilot modal will close and instead the "no-access" one will show.

@grgia grgia Dec 19, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a video of this flow on your branch

@ShridharGoel ShridharGoel Dec 20, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screen.Recording.2025-12-20.at.3.51.12.PM.mov
Screen.Recording.2025-12-20.at.3.49.46.PM.mov

removeDelegate({email: selectedDelegate?.email ?? '', delegatedAccess: account?.delegatedAccess});
setShouldShowRemoveDelegateModal(false);
setSelectedDelegate(undefined);
Expand Down
Loading