-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[HOLD for payment 2024-11-01] [$500] Tags - Background report changes to Tags page after clicking Custom tag name in RHP #49883
Comments
Triggered auto assignment to @Christinadobrzyn ( |
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
|
Triggered auto assignment to @stitesExpensify ( |
Looks pretty NAB to me |
ProposalPlease re-state the problem that we are trying to solve in this issue.Tags - Background report changes to Tags page after clicking Custom tag name in RHP What is the root cause of that problem?We're not passing the
Lines 847 to 850 in 3047c1b
And same when navigating to the Settings page App/src/pages/workspace/tags/WorkspaceTagsPage.tsx Lines 174 to 176 in 3047c1b
Lines 843 to 846 in 3047c1b
What changes do you think we should make in order to solve the problem?We can add the WORKSPACE_TAGS_SETTINGS: {
route: 'settings/workspaces/:policyID/tags/settings',
getRoute: (policyID: string, backTo?: string) => getUrlWithBackToParam(`settings/workspaces/${policyID}/tags/settings` as const, backTo),
},
WORKSPACE_EDIT_TAGS: {
route: 'settings/workspaces/:policyID/tags/:orderWeight/edit',
getRoute: (policyID: string, orderWeight: number, backTo?: string) => getUrlWithBackToParam(`settings/workspaces/${policyID}/tags/${orderWeight}/edit` as const, backTo),
}, WorkspaceTagsPage.tsx when navigating to Settings page const backTo = route.params.backTo;
const navigateToTagsSettings = () => {
Navigation.navigate(ROUTES.WORKSPACE_TAGS_SETTINGS.getRoute(policyID, backTo));
};
// ...types for route.params
[SCREENS.WORKSPACE.TAGS]: {
policyID: string;
backTo?: Routes;
}; And same for WorkspaceTagsSettingsPage when navigating to Custom tag name page <MenuItemWithTopDescription
title={policyTagLists[0]?.name}
description={translate(`workspace.tags.customTagName`)}
onPress={() => Navigation.navigate(ROUTES.WORKSPACE_EDIT_TAGS.getRoute(policyID, policyTagLists[0].orderWeight, backTo))}
shouldShowRightIcon
/> ResultScreen.Recording.2024-09-28.at.22.57.04.movWhat alternative solutions did you explore? (Optional) |
What does |
@Christinadobrzyn |
ProposalPlease re-state the problem that we are trying to solve in this issue.The report in the background changes to Tags page after clicking Custom tag name What is the root cause of that problem?
App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts Lines 146 to 153 in d187665
hence we push a new full screen navigator to stacks. What changes do you think we should make in order to solve the problem?
logic: App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts Lines 113 to 114 in d187665
is called instead of pushing a new full screen navigator to stacks: App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts Lines 146 to 147 in d187665
Lines 843 to 850 in d187665
and:
It will add new backTo param to the route when we click "Custom tag name" What alternative solutions did you explore? (Optional) |
Ah thanks @NJ-2020! That makes sense. Given that, I think this can be external so adding the label. |
Job added to Upwork: https://www.upwork.com/jobs/~021840631550279932147 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @paultsimura ( |
Upwork job price has been updated to $500 |
Doubling bounty because we are fixing 2 extra bugs |
Just an update for melvin that we're working on the PRs |
@stitesExpensify the PR is ready for your review |
PR in the works! #50416 |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.53-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-11-01. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
@paultsimura Do we need a regression for this? Payouts due:
|
I am paid via Upwork, can you please send me an offer
…On Fri, Nov 1, 2024 at 7:37 AM Christina Dobrzynski < ***@***.***> wrote:
@paultsimura <https://github.com/paultsimura> Do we need a regression for
this?
Payouts due:
- Contributor: $500 @twilight2294 <https://github.com/twilight2294>
(feel free to send a request to NewDot)
- Reviewer: $500 @paultsimura <https://github.com/paultsimura> (Upwork
offer - https://www.upwork.com/nx/wm/offer/104272735)
—
Reply to this email directly, view it on GitHub
<#49883 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BLXFUO2H4T2KI4Z3U2DEQO3Z6LO7RAVCNFSM6AAAAABPAVY3U2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINJRGE2TQMJQGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Ah thanks for letting me know @twilight2294 - can you please accept the offer here |
I’ve accepted the offer, thank you so much
…On Fri, Nov 1, 2024 at 11:02 AM Christina Dobrzynski < ***@***.***> wrote:
Ah thanks for letting me know @twilight2294
<https://github.com/twilight2294> - can you please accept the offer here
<https://www.upwork.com/nx/wm/offer/104689344>
—
Reply to this email directly, view it on GitHub
<#49883 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BLXFUO2447ATSA4K7XCLD2LZ6MG5DAVCNFSM6AAAAABPAVY3U2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINJRGMZDCMZWGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Regression Test ProposalPrecondition:
Test:
Do we agree 👍 or 👎 |
Regression test - https://github.com/Expensify/Expensify/issues/441268 Paid out based on payment summary - #49883 (comment) But I just checked and there was discussion after the PR went to production- is there a regression that we need to track for this? @twilight2294 @paultsimura |
Thank you @Christinadobrzyn. That one bug was related to a super edge case that is not even possible to reproduce fully in ND as the multilevel tags are not currently supported, therefore it was missed while testing. |
hi @paultsimura thank you for the additional information. I think it sounds fair to keep the payment as is. Closing this as complete! |
@stitesExpensify @Christinadobrzyn Be sure to fill out the Contact List! |
Thank you @Christinadobrzyn. |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 9.0.41-2
Reproducible in staging?: Y
Reproducible in production?: N/A Unable to check
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com
Issue reported by: Applause - Internal Team
Action Performed:
Precondition:
Expected Result:
The report in the background will remain after clicking Custom tag name
Actual Result:
The report in the background changes to Tags page after clicking Custom tag name
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6617841_1727469118066.20240928_042600.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @ChristinadobrzynThe text was updated successfully, but these errors were encountered: