Skip to content
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-07-26] [$1000] Dev: Clicking on task header sometimes opens assignee page #22498

Closed
1 of 6 tasks
kbecciv opened this issue Jul 8, 2023 · 28 comments
Closed
1 of 6 tasks
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2

Comments

@kbecciv
Copy link

kbecciv commented Jul 8, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Action Performed:

  1. Open a chat
  2. Go to the "Assign task" section
  3. Enter a title and create task
  4. After creating click on task header somewhere

Expected Result:

Clicking on task header should not open assignee page

Actual Result:

Clicking on task header sometime open assignee page

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: Dev 1.3.38-3
Reproducible in staging?: no
Reproducible in production?: no
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
Notes/Photos/Videos: Any additional supporting documentation

Recording.3517.mp4

Expensify/Expensify Issue URL:
Issue reported by: @ayazhussain79
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1688771023484909

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0188e528004d0d6c31
  • Upwork Job ID: 1678461651273142272
  • Last Price Increase: 2023-07-10
@kbecciv kbecciv added DeployBlockerCash This issue or pull request should block deployment Needs Reproduction Reproducible steps needed labels Jul 8, 2023
@OSBotify
Copy link
Contributor

OSBotify commented Jul 8, 2023

👋 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:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@melvin-bot
Copy link

melvin-bot bot commented Jul 8, 2023

Triggered auto assignment to @yuwenmemon (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@hungvu193
Copy link
Contributor

hungvu193 commented Jul 10, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

Dev: Clicking on task header sometimes opens assignee page

What is the root cause of that problem?

We're wrapping our subTitle inside a Pressable that filled the width of the header here:

<PressableWithoutFeedback
onPress={() => {
Navigation.navigate(ROUTES.getReportRoute(props.report.parentReportID));
}}
accessibilityLabel={parentNavigationSubtitle}
accessibilityRole={CONST.ACCESSIBILITY_ROLE.LINK}
>
<Text
style={[styles.optionAlternateText, styles.textLabelSupporting, styles.link]}
numberOfLines={1}
>
{parentNavigationSubtitle}
</Text>
</PressableWithoutFeedback>

Screenshot 2023-07-10 at 13 29 52

Because of that, when user click on header, sometimes it will trigger the onPress function of the subTitle, which will open the
assignee page.

What changes do you think we should make in order to solve the problem?

Instead of making it fill the width of the header, we should only make the subTitle clickable in its actual width.

What alternative solutions did you explore? (Optional)

N/A

@melvin-bot melvin-bot bot added Daily KSv2 and removed Hourly KSv2 labels Jul 10, 2023
@thienlnam thienlnam removed the DeployBlockerCash This issue or pull request should block deployment label Jul 10, 2023
@thienlnam
Copy link
Contributor

Only reproducible on dev so removing blocker

@thienlnam
Copy link
Contributor

Actually, with the latest task redesign I don't think this bug exists anymore. If by assignee page you mean the DM - that is expected

@ayazhussain79
Copy link
Contributor

@thienlnam this bug is on after new task redesign you can check that that is regression of this PR

@thienlnam
Copy link
Contributor

@ayazhussain79 From the video in the issue post it looks like clicking the task header opens the chat with the assignee. That's expected behavior. Is there another issue you're trying to point out here that I'm missing?

@ayazhussain79
Copy link
Contributor

@thienlnam please check that again you can assign task by clicking on header

screen-recording-2023-07-10-at-104019-pm_pOvdPfRc.mp4

@thienlnam
Copy link
Contributor

Ohhh I see - yes that does look like a bug - thanks for clarifying

@thienlnam thienlnam reopened this Jul 10, 2023
@thienlnam thienlnam added External Added to denote the issue can be worked on by a contributor and removed Needs Reproduction Reproducible steps needed labels Jul 10, 2023
@melvin-bot melvin-bot bot changed the title Dev: Clicking on task header sometimes opens assignee page [$1000] Dev: Clicking on task header sometimes opens assignee page Jul 10, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 10, 2023

Job added to Upwork: https://www.upwork.com/jobs/~0188e528004d0d6c31

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 10, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 10, 2023

Triggered auto assignment to @tjferriss (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Jul 10, 2023

Triggered auto assignment to Contributor-plus team member for initial proposal review - @sobitneupane (External)

@Pujan92
Copy link
Contributor

Pujan92 commented Jul 10, 2023

@thienlnam isn't it should open the assignee page for taskreport as looks correct as per the code?

onPress={() => (isTaskReport ? Navigation.navigate(ROUTES.getTaskReportAssigneeRoute(props.report.reportID)) : ReportUtils.navigateToDetailsPage(props.report))}

@thienlnam
Copy link
Contributor

cc @jasperhuangg Was it intended to make this change? I believe the only spot we want to allow changing the assignee is when clicked below - thoughts?

@Nikhil-Vats
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

Dev: Clicking on header area opens assignee page.

What is the root cause of that problem?

Here, we are opening the assignee page if the report is a task report. This is a regression from this PR.

<PressableWithoutFeedback
onPress={() => (isTaskReport ? Navigation.navigate(ROUTES.getTaskReportAssigneeRoute(props.report.reportID)) : ReportUtils.navigateToDetailsPage(props.report))}
style={[styles.flexRow, styles.alignItemsCenter, styles.flex1]}
disabled={isTaskReport && !ReportUtils.isOpenTaskReport(props.report)}

What changes do you think we should make in order to solve the problem?

If we want to open the details page when we click on the header area (except parent chat) then we can change line 165 to -

onPress={() => ReportUtils.navigateToDetailsPage(props.report)}

Also, we should remove the disabled prop in line 167 as we won't open assignee page in any case now.

What alternative solutions did you explore? (Optional)

NA

@Nikhil-Vats
Copy link
Contributor

@thienlnam in the previous design, user was not able to open assignee page when clicking around header but user was able to open assignee page when clicked on the assignee shown after "To" (in old header design). But I agree, in the new design the header should not open assignee page.

@Nodebrute
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

Dev: Clicking on task header sometimes opens assignee page

What is the root cause of that problem?

I think this is intentional. Still it's very confusing for a user. we should only navigate to assignee page by clicking assignee. Navigating to details page like @Nikhil-Vats mentioned will not work. This the result of @Nikhil-Vats solution

Screen.Recording.2023-07-11.at.1.56.21.AM.mov

What changes do you think we should make in order to solve the problem?

There are 3 possible solutions to this

  1. Pressing here should not do anything.
  2. We can show profile details of Task Assigner
  3. We can show dedicated details page as we show for IOU reports like this
Screenshot 2023-07-11 at 1 54 12 AM

What alternative solutions did you explore? (Optional)

@jasperhuangg
Copy link
Contributor

cc @jasperhuangg Was it intended to make this change? I believe the only spot we want to allow changing the assignee is when clicked below - thoughts?

@thienlnam The header previously was used to open the assignee view. The design never specified that that wasn't supposed to happen so I just left things as is. I can update this though–I don't think it makes sense to have them both open the assignee modal.

@jasperhuangg
Copy link
Contributor

Thanks for the proposals everyone, but I'm going to be tackling this internally since it's directly related to a PR I just merged!

@melvin-bot melvin-bot bot added the Reviewing Has a PR in review label Jul 10, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 11, 2023

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@melvin-bot
Copy link

melvin-bot bot commented Jul 11, 2023

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@tjferriss tjferriss assigned jasperhuangg and unassigned yuwenmemon Jul 12, 2023
@tjferriss
Copy link
Contributor

Assigning @jasperhuangg and un-assigning @yuwenmemon as Jasper is working on the PR.

@jasperhuangg
Copy link
Contributor

The PR is on prod cc @tjferriss

Let's get @ayazhussain79 paid out for reporting the bug and we should be good to close this out

@tjferriss
Copy link
Contributor

@ayazhussain79 offer has been sent via Upworks

@ayazhussain79
Copy link
Contributor

@tjferriss offer accepted, Thank you

@tjferriss
Copy link
Contributor

Payment has been sent.

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Jul 19, 2023
@melvin-bot melvin-bot bot changed the title [$1000] Dev: Clicking on task header sometimes opens assignee page [HOLD for payment 2023-07-26] [$1000] Dev: Clicking on task header sometimes opens assignee page Jul 19, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jul 19, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 19, 2023

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot
Copy link

melvin-bot bot commented Jul 19, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.42-26 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-07-26. 🎊

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.

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2
Projects
None yet
Development

No branches or pull requests