-
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 2023-10-25] [$500] iOS - WS Invite - Invite message link does not lead to invite message page #29163
Comments
Triggered auto assignment to @mallenexpensify ( |
Job added to Upwork: https://www.upwork.com/jobs/~0142f9bb42ec3d0112 |
Bug0 Triage Checklist (Main S/O)
|
Triggered auto assignment to Contributor-plus team member for initial proposal review - @sobitneupane ( |
Not reproduced on Prod. It redirects to browser. 20231010_121241.mp4 |
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open
|
Triggered auto assignment to @iwiznia ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Invite message link lead to wrong page. What is the root cause of that problem?We had a check here if App/src/pages/workspace/WorkspaceInviteMessagePage.js Lines 84 to 87 in a63f853
It was introduced from this PR (#22262) What changes do you think we should make in order to solve the problem?Instead of <FullPageNotFoundView
shouldShow={_.isEmpty(this.props.policy) || !PolicyUtils.isPolicyAdmin(this.props.policy) || PolicyUtils.isPendingDeletePolicy(this.props.policy) || _.isEmpty(this.props.invitedEmailsToAccountIDsDraft)} And remove this block of code What alternative solutions did you explore? (Optional)Instead of navigating to InitialPage, we should goBack to WorkspaceInvitePage instead. |
ProposalPlease re-state the problem that we are trying to solve in this issue.I think the expectation should be updated, when we navigate to that invite message page via link, we'll not be able to display the invite message page since we didn't select any users to invite yet. So the best UX here would be for the user to navigate to the This is a regression from this PR refactor change (although not a deploy blocker since it was deployed to prod a long time ago). The refactor has changed the intended behavior from the Workspace Invite page to Workspace Initial page. What is the root cause of that problem?Currently we're navigating to the initial workspace page here if the invite members draft is not found. What changes do you think we should make in order to solve the problem?Here, we should update to navigate the "Invite new members" page instead.
When investigating this, I found there's an existing bug here where the
What alternative solutions did you explore? (Optional)In that draft members empty case, we could navigate to the |
For deploy blocker issues, offending PR should be identified in root cause unless coming from staging backend |
Just seeing this now, sorry I missed it (also weird it's an hourly and hasn't gone overdue in 16 hours). Asking about in #qa here |
Personally, I think we should just close this out. This doesn't seem like behavior anyone would ever engage in. Even if they were to do this, I think the current behavior makes sense. It wouldn't make sense to take you back to the invite-members flow, since you have technically already exited that flow. Going to close this out. |
I don't think it's a deploy blocker. It is reproducible in Prod as well. And the other thing is it is reproducible in all platforms. We navigate users to Invite Message Page if there are any invited emails present in draft for the workspace. But if there is no invited emails in the draft, we redirect users to Workspace Initial Settings Page. We used to initially redirect it to Workspace Invite Page (implemented by this PR) which was later changed to Workspace Initial Page by this PR which doesn't look like intended change. |
Thanks @jasperhuangg and @sobitneupane ! |
@sobitneupane is right that this is a regression from this PR refactor change (although not a deploy blocker since it was deployed to prod a long time ago). The refactor has changed the intended behavior from the Workspace Invite page to Workspace Initial page. so @jasperhuangg IMO we should reopen this issue |
📣 @sobitneupane Please request via NewDot manual requests for the Reviewer role ($500) |
📣 @dukenv0307 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
@sobitneupane The PR is ready for review. |
Yes, sorry, this was a regression that was not intended and the constant just needs to be swapped. I had a conflict come up with that constant in the PR and didn't resolve it properly. |
@dukenv0307 Can you please explain about the issue you are mentioning here. I remember having an issue where app crashes on navigating request money page through link. But I believe it was resolved by some other PR. |
@sobitneupane You're right. I've bugged like that in the past and I remember the issue you said, please ignore it and continue reviewing. |
Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:
On to the next one 🚀 |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.86-5 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 2023-10-25. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
For reference, here are some details about the assignees on this issue:
As a reminder, here are the bonuses/penalties that should be applied for any External 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:
|
Lil confused but .... Issue reporter: Applause I see regression mentioned above, my assumption is that this issue came from another regression, not that there was one here, please correct me if i'm wrong. Unsure if we want a regression test specifically for this if the issue came from another, what cha think @sobitneupane ? |
@mallenexpensify You are right. This issue was regression from #28050. There is no regression from the PR associated with the issue. |
This issue was caused by some merge conflict during a large refactor.
Yes.
|
Regression Test Proposal
Do we agree 👍 or 👎 |
Requested payment on newDot. |
Thanks @sobitneupane ! Regression test GH created Payment breakdown above is up to date. |
$750 payment approved for @sobitneupane based on this summary. |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Issue found when executing PR #25543
Version Number: 1.3.80-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause-Internal Team
Slack conversation: @
Action Performed:
Pre-requisite: user must be logged in, must have sent the invite message link to any workspace to any conversation.
Expected Result:
Invite message page should be displayed.
Actual Result:
Workspace page is displayed
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
Bug6231498_1696920012998.Sgjo9409_1_.mp4
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: