-
Notifications
You must be signed in to change notification settings - Fork 3.5k
fix: Auto-complete setup tags task when creating first tag #78921
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
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@thesahindia 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] |
|
@thesahindia kindly bump on this |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-01-13.at.3.38.26.PM.movAndroid: mWeb ChromeScreen.Recording.2026-01-13.at.3.43.45.PM.moviOS: HybridAppScreen.Recording.2026-01-17.at.10.31.26.AM.moviOS: mWeb SafariScreen.Recording.2026-01-17.at.10.38.54.AM.movMacOS: Chrome / SafariScreen.Recording.2026-01-13.at.10.20.11.AM.mov |
|
@mukhrr, it works well but I have found one inconsistency. "Set up categories" task gets completed on disabling any category but the same doesn't work for "Set up categories and tags" task. I added a tag and then disabled a category. The task didn't get completed. |
|
@thesahindia could you pls check it once more? |
|
Tested well. @mukhrr, could you resolve the conflicts? |
|
@thesahindia it's ready |
|
Looks good, can we add test coverage though so that this check passes? Looks like |
|
@blimpich It's covered now. And the eslint-check error doesn't seem to related to this PR. |
|
Hmmm, I would like the linter to pass. I've run it a few times and it still fails so it's consistent. Can we merge main again and see if that fixes it? |
|
I merged twice. And it didn't help. Seems we have problem with files I touched on main too |
|
I see. Yeah this is completely unrelated to your PR. lets just move forward. Not your responsibility to deal with that. |
|
@blimpich looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
|
not emergency, eslint failure is unrelated to this PR |
|
On second thought I think this did break eslint. See this slack thread: https://expensify.slack.com/archives/C01GTK53T8Q/p1768954074730379 |
|
🚀 Deployed to staging by https://github.com/blimpich in version: 9.3.6-0 🚀
|
|
@mukhrr PR has failed with an original issue 1769045609980.bandicam_2026-01-22_04-32-17-246.mp4 |
|
@IuliiaHerets pls, follow OP steps, i.e, add at least one category to make it enabled
|
|
@IuliiaHerets btw, also this PR got reverted, thus, you may still reproduce issue. New PR is waiting for a review. |
|
🚀 Deployed to production by https://github.com/Beamanator in version: 9.3.6-4 🚀
|
Explanation of Change
Fixed Issues
$ #78639
$ #78935
PROPOSAL: #78639 (comment)
Tests
Test for #78639
Test for #78935
Verify "Set up categories and tags" task is completed
Verify that no errors appear in the JS console
Offline tests
QA Steps
The same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)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
android_native.mp4
Android: mWeb Chrome
android_mWeb.mp4
iOS: Native
IOS_native.mp4
iOS: mWeb Safari
IOS_mWeb.mp4
MacOS: Chrome / Safari
web.mp4