-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
update correct field #54501
update correct field #54501
Conversation
@mananjadhav 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] |
@jayeshmangwani Could you review this PR? |
@DylanDylann I'll |
You already saw that in the original PR but I missed committing the suggestion for the failure data :( |
Haha, we sometimes miss things. I’m reviewing PR in 5 minutes. |
@DylanDylann Upon pressing downgrade, nothing happens for me—it just reloads the page. Are you facing the same issue? downgrade-flickers.mov |
As mentioned here, It is a BE bug. currently, the BE only allows the owner of policy downgrade |
@DylanDylann , it seems we need to adjust the permissions here. We should display the Plan Type if the user is the owner of the workspace, rather than the Admin. App/src/pages/workspace/WorkspaceProfilePage.tsx Lines 331 to 334 in 8b7096f
|
I think admin should have permission to downgrade and upgrade the workspace as design docs |
Okay then BE needs to be adjusted as I am getting BE error {
"code": 666,
"jsonCode": 666,
"type": "Expensify\\Libs\\Error\\ExpError",
"UUID": "4d7b2da5-d585-403a-aee7-793621b49e49",
"message": "Only policy owner can downgrade",
"title": "",
"data": [],
"htmlMessage": "",
"onyxData": [],
"requestID": "8f6fa64f085df29a-BOM"
} |
@jayeshmangwani I have added a new commit to solve this bug too |
Thanks, testing that issue too |
@jayeshmangwani There is a minor problem. The App already updated the selected option when another device updated it. But the background focus still be outdated see the below video Screen.Recording.2024-12-24.at.16.53.52.movBut this is a bug in SelectionList that will happen to all places where we use SelectionList. Now, we already have a PR to address it separately. I think we can ignore it here |
Sounds good to me |
@DylanDylann PR changes looks good, but can we please update the QA Steps to include the testing steps instead of doc link? It seems the tester doesn’t have access to the document , as they’ve commented on this PR #54356 (comment). Please update that PR with QA Steps as well. |
@jayeshmangwani Done. I updated test step for #54495 only |
To test #54493, I think we need to wait for the BE change |
Yes we can wait, I think we can update this steps once BE will be fixed |
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid.movAndroid: mWeb Chromemweb-chrome.moviOS: NativeiOS.moviOS: mWeb Safarimweb-safari.movMacOS: Chrome / Safariweb.movMacOS: Desktopdesktop.mov |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/techievivek in version: 9.0.79-0 🚀
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.0.79-5 🚀
|
Explanation of Change
Fixed Issues
$ #54493
#54495
PROPOSAL:
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Screen.Recording.2024-12-20.at.15.58.13.mov
Android: mWeb Chrome
Screen.Recording.2024-12-20.at.16.20.42.mov
iOS: Native
Screen.Recording.2024-12-20.at.16.05.10.mov
iOS: mWeb Safari
Screen.Recording.2024-12-19.at.17.31.41.mov
MacOS: Chrome / Safari
Screen.Recording.2024-12-19.at.17.30.07.mov
MacOS: Desktop
Screen.Recording.2024-12-19.at.17.33.24.mov