-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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.
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
[Internal QA]: Fix: Logged-In Agents Can Appear to Delete Workspaces in NewDot Without Actual Deletion or Error Message #53333
Changes from 3 commits
546e8ab
0987959
cd8f777
88337ae
013f5ab
05b01c8
1b5ae43
c2ec2f6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
import React from 'react'; | ||
import useLocalize from '@hooks/useLocalize'; | ||
import ConfirmModal from './ConfirmModal'; | ||
|
||
type SupportalActionModalProps = { | ||
isSupportalActionRestrictedModalOpen: boolean; | ||
hideSupportalModal: () => void; | ||
}; | ||
|
||
function SupportalActionModal({isSupportalActionRestrictedModalOpen, hideSupportalModal}: SupportalActionModalProps) { | ||
const {translate} = useLocalize(); | ||
return ( | ||
<ConfirmModal | ||
title={translate('supportalNoAccess.title')} | ||
isVisible={isSupportalActionRestrictedModalOpen} | ||
onConfirm={hideSupportalModal} | ||
prompt={translate('supportalNoAccess.description')} | ||
confirmText={translate('common.buttonConfirm')} | ||
shouldShowCancelButton={false} | ||
/> | ||
); | ||
} | ||
|
||
SupportalActionModal.displayName = 'SupportalActionModal'; | ||
|
||
export default SupportalActionModal; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -463,6 +463,10 @@ const translations = { | |
links: 'Enlaces', | ||
days: 'días', | ||
}, | ||
supportalNoAccess: { | ||
title: 'No tan rápido', | ||
description: 'No estás autorizado para realizar esta acción mientras el soporte está conectado.', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @allgandalf can you please confirm these translation ? I am still to be added on slack, the above translation for title is already present in the codebase picked it up from there There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Asked on slack here
twilight2294 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, | ||
connectionComplete: { | ||
title: 'Conexión completa', | ||
supportingText: 'Ya puedes cerrar esta página y volver a la App de Expensify.', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ import type {PopoverMenuItem} from '@components/PopoverMenu'; | |
import {PressableWithoutFeedback} from '@components/Pressable'; | ||
import ScreenWrapper from '@components/ScreenWrapper'; | ||
import ScrollView from '@components/ScrollView'; | ||
import SupportalActionModal from '@components/SupportalActionModal'; | ||
import Text from '@components/Text'; | ||
import useActiveWorkspace from '@hooks/useActiveWorkspace'; | ||
import useLocalize from '@hooks/useLocalize'; | ||
|
@@ -126,6 +127,12 @@ function WorkspacesListPage() { | |
const [cardsList] = useOnyx(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${workspaceAccountID}_${CONST.EXPENSIFY_CARD.BANK}`); | ||
const hasCardFeedOrExpensifyCard = !isEmptyObject(cardFeeds) || !isEmptyObject(cardsList); | ||
|
||
const isSupportalAction = Session.isSupportAuthToken(); | ||
|
||
const [isSupportalActionRestrictedModalOpen, setIsSupportalActionRestrictedModalOpen] = useState(false); | ||
const hideSupportalModal = () => { | ||
setIsSupportalActionRestrictedModalOpen(false); | ||
}; | ||
const confirmDeleteAndHideModal = () => { | ||
if (!policyIDToDelete || !policyNameToDelete) { | ||
return; | ||
|
@@ -158,6 +165,10 @@ function WorkspacesListPage() { | |
icon: Expensicons.Trashcan, | ||
text: translate('workspace.common.delete'), | ||
onSelected: () => { | ||
if (isSupportalAction) { | ||
setIsSupportalActionRestrictedModalOpen(true); | ||
return; | ||
} | ||
setPolicyIDToDelete(item.policyID ?? '-1'); | ||
setPolicyNameToDelete(item.title); | ||
setIsDeleteModalOpen(true); | ||
|
@@ -226,7 +237,18 @@ function WorkspacesListPage() { | |
</OfflineWithFeedback> | ||
); | ||
}, | ||
[isLessThanMediumScreen, styles.mb2, styles.mh5, styles.ph5, styles.hoveredComponentBG, translate, styles.offlineFeedback.deleted, session?.accountID, session?.email], | ||
[ | ||
isLessThanMediumScreen, | ||
styles.mb2, | ||
styles.mh5, | ||
styles.ph5, | ||
styles.hoveredComponentBG, | ||
translate, | ||
styles.offlineFeedback.deleted, | ||
session?.accountID, | ||
session?.email, | ||
isSupportalAction, | ||
], | ||
); | ||
|
||
const listHeaderComponent = useCallback(() => { | ||
|
@@ -439,6 +461,10 @@ function WorkspacesListPage() { | |
cancelText={translate('common.cancel')} | ||
danger | ||
/> | ||
<SupportalActionModal | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do y'all think about calling this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’m down with this suggestion 🙌, @twilight2294 please update this name |
||
isSupportalActionRestrictedModalOpen={isSupportalActionRestrictedModalOpen} | ||
hideSupportalModal={hideSupportalModal} | ||
/> | ||
</ScreenWrapper> | ||
); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do y'all think about shortening
isSupportalActionRestrictedModalOpen
to justisOpen
since that's a pretty clear prop name internal to this component? 🤔There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
umm, isOpen is quite generic how about
isSupportalModalOpen
? defines purpose ?!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm what about
isModalOpen
? 😅 I just don't think we need to be TOO specific, but I do agreeisOpen
is a bit too generic 😅There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
worksssss for mee! :team-work: