[Internal QA]: Fix: Logged-In Agents Can Appear to Delete Workspaces in NewDot Without Actual Deletion or Error Message #53333
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: GitHub <noreply@github.com>
twilight2294
commented
Nov 30, 2024
src/languages/es.ts
Outdated
| @@ -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.
@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
Signed-off-by: GitHub <noreply@github.com>
melvin-bot
bot
requested review from
allgandalf
and removed request for
a team
|
@allgandalf Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2024-12-03.at.7.08.01.PM.movAndroid: mWeb ChromeScreen.Recording.2024-12-03.at.7.06.18.PM.moviOS: NativeScreen.Recording.2024-12-03.at.7.09.18.PM.moviOS: mWeb SafariScreen.Recording.2024-12-03.at.7.12.44.PM.movMacOS: Chrome / SafariScreen.Recording.2024-12-03.at.7.02.56.PM.movMacOS: DesktopScreen.Recording.2024-12-03.at.7.11.22.PM.mov |
|
@twilight2294 please update videos on all platforms |
twilight2294
changed the title
|
@allgandalf I updated videos and added test as well |
allgandalf
reviewed
Dec 3, 2024
Co-authored-by: Gandalf <whogandalf@gmail.com>
allgandalf
approved these changes
Dec 3, 2024
Beamanator
requested changes
Dec 3, 2024
| @@ -439,6 +461,10 @@ function WorkspacesListPage() { | |||
| cancelText={translate('common.cancel')} | |||
| danger | |||
| /> | |||
| <SupportalActionModal | |||
There was a problem hiding this comment.
What do y'all think about calling this SupportalActionRestrictedModal or RestrictedSupportalActionModal to make it super clear that this modal specifically tells the user that an action is restricted since they're supportaled?
There was a problem hiding this comment.
I’m down with this suggestion 🙌, @twilight2294 please update this name
| hideSupportalModal: () => void; | ||
| }; | ||
|
|
||
| function SupportalActionModal({isSupportalActionRestrictedModalOpen, hideSupportalModal}: SupportalActionModalProps) { |
There was a problem hiding this comment.
What do y'all think about shortening isSupportalActionRestrictedModalOpen to just isOpen since that's a pretty clear prop name internal to this component? 🤔
There was a problem hiding this comment.
umm, isOpen is quite generic how about isSupportalModalOpen? defines purpose ?!
There was a problem hiding this comment.
Hmmm what about isModalOpen? 😅 I just don't think we need to be TOO specific, but I do agree isOpen is a bit too generic 😅
There was a problem hiding this comment.
worksssss for mee! :team-work:
|
I updated according to the changes @Beamanator |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
github-actions
bot
added
the
DeployBlockerCash
Performance Comparison Report 📊Significant Changes To Duration
Show details
Meaningless Changes To DurationShow entries
Show details
|
|
@Expensify/mobile-deployers 📣 Please look into this performance regression as it's a deploy blocker. |
35 tasks
|
🚀 Deployed to staging by https://github.com/Beamanator in version: 9.0.71-0 🚀
|
Beamanator
removed
the
DeployBlockerCash
|
This is very unlikely the real performance blocker as we're just adding a Modal in 1 scenario |
|
Looking good!! 
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.0.71-2 🚀
|
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explanation of Change
Fixed Issues
$ #52854
PROPOSAL: #52854 (comment)
Tests
Precondition: User is supportal logged in OR set this line/ condition to true
Verify that error modal is shown
Offline tests
Precondition: User is supportal logged in
Verify that error modal is shown
QA Steps
N/A. Internal QA
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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
Screen_recording_20241203_140955.mp4
Android: mWeb Chrome
Screen_recording_20241203_141054.mp4
iOS: Native
Simulator.Screen.Recording.-.Expensify.testing.-.2024-12-03.at.14.04.42.mp4
iOS: mWeb Safari
Simulator.Screen.Recording.-.Expensify.testing.-.2024-12-03.at.14.05.18.mp4
MacOS: Chrome / Safari
Screen.Recording.2024-12-03.at.2.01.13.PM.mov
MacOS: Desktop
Screen.Recording.2024-12-03.at.2.03.43.PM.mov