-
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
fix: QBO - Required remains enabled when all tags are disabled #43813
Conversation
Signed-off-by: Krishna Gupta <[email protected]>
@Krishna2323 I've requested for access to the beta. Please fix the merge conflicts in the mean time. |
Signed-off-by: Krishna Gupta <[email protected]>
@akinwale, any updates? conflicts have been resolved. |
@akinwale bump |
Thanks for the bump. Missed the previous update. I will review soon. |
@Krishna2323 Could you please double-check the test steps? The interface seems to be different from what I have. Is there anything else that I need to enable? Screen.Recording.2024-06-29.at.03.10.02.mp4 |
@akinwale, you need to import tags from QBO. |
Yeah, I already did this by connecting the QBO account. The tags in the video were created in QBO Classes. |
@akinwale, I think you can try importing tags again in any other workspace. |
@akinwale, any updates? |
Bump @akinwale. Let us know if you're slammed and we'll find another reviewer |
I'm taking another look today. |
Reviewer Checklist
Screenshots/VideosAndroid: Native43183-android-native.mp4Android: mWeb Chrome43813-android-chrome.mp4iOS: Native43813-ios-native.mp4iOS: mWeb Safari43813-ios-safari.mp4MacOS: Chrome / Safari43813-web.mp4MacOS: Desktop43813-desktop.mp4 |
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.
LGTM.
@akinwale please add evidence of testing in all platforms |
My Internet was sluggish earlier. I'll upload the remaining videos in a bit. |
Thank you 🙇 |
@cead22 All done! |
@cead22 bump |
@akinwale I noticed a console error on your iOS video "function components cannot be given ref...". I assume this wasn't fixed in this PR, can you report this on slack and make sure somebody is working on the fix? Let's be disciplined with this item of the checklist so we're always running the app without any console errors or warnings |
✋ 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/cead22 in version: 9.0.7-3 🚀
|
🚀 Cherry-picked to staging by https://github.com/thienlnam in version: 9.0.7-4 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
🚀 Deployed to production by https://github.com/thienlnam in version: 9.0.7-8 🚀
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.0.8-6 🚀
|
Details
Fixed Issues
$ #43666
PROPOSAL: #43666 (comment)
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.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
android_native.mp4
Android: mWeb Chrome
android_chrome.mp4
iOS: Native
ios_native.mp4
iOS: mWeb Safari
ios_safari.mp4
MacOS: Chrome / Safari
web_chrome.mp4
MacOS: Desktop
desktop_app.mp4