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 E/E #311912] [$500] Assignee does not show on Confirm task page #25680

Closed
6 tasks done
kavimuru opened this issue Aug 22, 2023 · 45 comments
Closed
6 tasks done

[HOLD E/E #311912] [$500] Assignee does not show on Confirm task page #25680

kavimuru opened this issue Aug 22, 2023 · 45 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@kavimuru
Copy link

kavimuru commented Aug 22, 2023

HELD for https://github.com/Expensify/Expensify/issues/311912

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. click on FAB icon and select new chat
  2. enter email and create a new chat
  3. click on FAB icon and select assign a task
  4. Enter title and click on Next button
  5. click on assignee and search for user with whom you created new chat
    6.select that user

Expected Result:

Assignee does not show on Confirm task page

Actual Result:

Assignee should show on Confirm task page

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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: 1.3.56-2
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
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-08-21.at.10.50.26.AM.mov
Recording.1491.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0182f226dc34b9a020
  • Upwork Job ID: 1694007582851416064
  • Last Price Increase: 2023-08-22
  • Automatic offers:
    • gadhiyamanan | Reporter | 26281429
@kavimuru kavimuru added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 22, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 22, 2023

Triggered auto assignment to @CortneyOfstad (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Aug 22, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Aug 22, 2023
@OSBotify
Copy link
Contributor

👋 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 Aug 22, 2023

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

@aimane-chnaif
Copy link
Contributor

This is not a blocker. Also happens on production.

Repro step:

  1. click on FAB icon and select new chat
  2. enter email and create a new chat
  3. Logout and re-login same account
  4. click on FAB icon and select assign a task
  5. Enter title and click on Next button
  6. click on assignee and search for user with whom you created new chat
  7. select that user
Screen.Recording.2023-08-22.at.3.22.22.PM.mov

@mountiny mountiny added External Added to denote the issue can be worked on by a contributor Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Aug 22, 2023
@melvin-bot melvin-bot bot changed the title Assignee does not show on Confirm task page [$1000] Assignee does not show on Confirm task page Aug 22, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 22, 2023

Job added to Upwork: https://www.upwork.com/jobs/~0182f226dc34b9a020

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

melvin-bot bot commented Aug 22, 2023

Current assignee @CortneyOfstad is eligible for the External assigner, not assigning anyone new.

@melvin-bot
Copy link

melvin-bot bot commented Aug 22, 2023

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

@mountiny mountiny changed the title [$1000] Assignee does not show on Confirm task page [$500] Assignee does not show on Confirm task page Aug 22, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 22, 2023

Upwork job price has been updated to $500

@b4s36t4
Copy link
Contributor

b4s36t4 commented Aug 22, 2023

Proposal

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

Assignee does not show on Confirm task page

What is the root cause of that problem?

This happened after #23773 in which we've updated the way we fetch the user details to display.

Here

function getAssignee(assigneeAccountID, personalDetails) {
const details = personalDetails[assigneeAccountID];
if (!details) {
we're checking in the personalDetails.

Although we've initiated a chat with the User A, but User B haven't responded which is resulting personalDetails for the User B as undefined.

And here

if (!details) {
we're returning an empty details which is causing the issue not to display anything in the Confirm task page.

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

Since we can't do anything about the user details not available we can return with the details we have.

We can do default details instead of returning empty details which is causing the issues.

if (!details) {
    return {
        icons: [{name: assignee, type: CONST.ICON_TYPE_AVATAR, source: UserUtils.getAvatar('', assigneeAccountID)}],
        displayName: assignee,
        subtitle: assignee,
    };
}

assignee is passed form NewTaskPage which is props.task.assignee

The PR which caused the issue (maybe not) is have an issue with showing title for user icons, so we can follow the same approach we're following here to get the icon.

// If user doesn't exist, use a default avatar
userToInvite.icons = [
{
source: UserUtils.getAvatar('', optimisticAccountID),
name: searchValue,
type: CONST.ICON_TYPE_AVATAR,
},
];
}

What alternative solutions did you explore? (Optional)

NA

Result

Kapture.2023-08-22.at.20.43.19.mp4

@DylanDylann
Copy link
Contributor

Proposal

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

Assignee does not show on Confirm task page

What is the root cause of that problem?

When assignong a task to an assignee, we will check if chatReport with this assignee is existed or not

if (!chatReport) {
chatReport = ReportUtils.buildOptimisticChatReport([assigneeAccountID]);
chatReport.isOptimisticReport = true;
// When assigning a task to a new user, by default we share the task in their DM
// However, the DM doesn't exist yet - and will be created optimistically once the task is created
// We don't want to show the new DM yet, because if you select an assignee and then change the assignee, the previous DM will still be shown
// So here, we create it optimistically to share it with the assignee, but we have to hide it until the task is created
chatReport.isHidden = true;
Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${chatReport.reportID}`, chatReport);
// If this is an optimistic report, we likely don't have their personal details yet so we set it here optimistically as well
const optimisticPersonalDetailsListAction = {
accountID: assigneeAccountID,
avatar: lodashGet(allPersonalDetails, [assigneeAccountID, 'avatar'], UserUtils.getDefaultAvatarURL(assigneeAccountID)),
displayName: lodashGet(allPersonalDetails, [assigneeAccountID, 'displayName'], assigneeEmail),
login: assigneeEmail,
};
Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[assigneeAccountID]: optimisticPersonalDetailsListAction});
}

If it does not exist, we will create an optimistic chat report and add this assignee to PERSONAL_DETAILS_LIST (on ONYX)

Then when displaying the assignee field we will get information from PERSONAL_DETAILS_LIST based on assigneeAccountID

But in this case, because we init the chat with this user before. So the condition check !chatReport is false

if (!chatReport) {

and this user will be not added to PERSONAL_DETAILS_LIST. So It displayed empty when displaying the assignee field

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

when assigning new assignee we also should check if chat report exist and this assignee is not in PERSONAL_DETAILS_LIST, we will add this user to PERSONAL_DETAILS_LIST like here

const optimisticPersonalDetailsListAction = {
accountID: assigneeAccountID,
avatar: lodashGet(allPersonalDetails, [assigneeAccountID, 'avatar'], UserUtils.getDefaultAvatarURL(assigneeAccountID)),
displayName: lodashGet(allPersonalDetails, [assigneeAccountID, 'displayName'], assigneeEmail),
login: assigneeEmail,
};
Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[assigneeAccountID]: optimisticPersonalDetailsListAction});

What alternative solutions did you explore? (Optional)

When create new chat with new user, we also add this user to PERSONAL_DETAILS_LIST like here

const optimisticPersonalDetailsListAction = {
accountID: assigneeAccountID,
avatar: lodashGet(allPersonalDetails, [assigneeAccountID, 'avatar'], UserUtils.getDefaultAvatarURL(assigneeAccountID)),
displayName: lodashGet(allPersonalDetails, [assigneeAccountID, 'displayName'], assigneeEmail),
login: assigneeEmail,
};
Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[assigneeAccountID]: optimisticPersonalDetailsListAction});

@parteekcoder
Copy link

parteekcoder commented Aug 22, 2023

Proposal

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

After selecting an assignee, it still shows Assignee on Confirm task page

What is the root cause of that problem?

Here in this Menu option, there is no check that if an assignee is already chosen then don't show the Assignee option

https://github.com/Expensify/App/blob/main/src/pages/tasks/NewTaskPage.js#L168-#L175

         <MenuItem
                            label={assignee.displayName ? props.translate('task.assignee') : ''}
                            title={assignee.displayName || ''}
                            description={assignee.displayName ? LocalePhoneNumber.formatPhoneNumber(assignee.subtitle) : props.translate('task.assignee')}
                            icon={assignee.icons}
                            onPress={() => Navigation.navigate(ROUTES.NEW_TASK_ASSIGNEE)}
                            shouldShowRightIcon
                        />

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

We can add a simple condition here that if we have already chosen assignee then don't show this assignee option. props.task stores the information about the task i.e title, desc, asignee. So we can use this object to check whether we have chosen any assignee to not

      !props.task.assignee &&   <MenuItem
                            label={assignee.displayName ? props.translate('task.assignee') : ''}
                            title={assignee.displayName || ''}
                            description={assignee.displayName ? LocalePhoneNumber.formatPhoneNumber(assignee.subtitle) : props.translate('task.assignee')}
                            icon={assignee.icons}
                            onPress={() => Navigation.navigate(ROUTES.NEW_TASK_ASSIGNEE)}
                            shouldShowRightIcon
                        />

What alternative solutions did you explore? (Optional)

Supporting videos

Before changes:

Screencast.from.23-08-23.01.17.32.AM.IST.mp4

After changes:

Screencast.from.23-08-23.01.16.24.AM.IST.mp4

@rushatgabhane
Copy link
Member

rushatgabhane commented Aug 23, 2023

@ all Please help me understand this issue.

Problem: Assignee isn't shown on confirm task page.
Root cause: The chat report exists, but the assignee doesn't exist in personalDetails.

Why is the assignee not being added to personalDetails when creating the chat report? Could you please link some code if possible, thanks!

@CortneyOfstad
Copy link
Contributor

Not overdue 👍

@melvin-bot melvin-bot bot removed the Overdue label Oct 12, 2023
@CortneyOfstad
Copy link
Contributor

Heading OoO until Oct. 25, so reassigning BZ to keep an eye on this while I'm gone 👍

@CortneyOfstad CortneyOfstad removed their assignment Oct 18, 2023
@CortneyOfstad CortneyOfstad added Bug Something is broken. Auto assigns a BugZero manager. and removed Bug Something is broken. Auto assigns a BugZero manager. labels Oct 18, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 18, 2023

Triggered auto assignment to @conorpendergrast (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot melvin-bot bot added the Daily KSv2 label Oct 18, 2023
@melvin-bot

This comment was marked as duplicate.

@melvin-bot melvin-bot bot removed the Weekly KSv2 label Oct 18, 2023
@CortneyOfstad CortneyOfstad self-assigned this Oct 18, 2023
@mountiny mountiny added Weekly KSv2 and removed Daily KSv2 labels Oct 18, 2023
@conorpendergrast
Copy link
Contributor

@CortneyOfstad Welcome back! Assigning back to you; no change

@conorpendergrast conorpendergrast removed their assignment Oct 25, 2023
@CortneyOfstad
Copy link
Contributor

Perfect — thanks @conorpendergrast!

@melvin-bot melvin-bot bot added the Overdue label Nov 3, 2023
@CortneyOfstad
Copy link
Contributor

@mountiny Any updates — TIA!

@melvin-bot melvin-bot bot removed the Overdue label Nov 6, 2023
@mountiny
Copy link
Contributor

mountiny commented Nov 7, 2023

No not really this is very low priority cc @thienlnam would you actually be interested in taking this one over as you got more context in the Tasks area so the fix might be quicker for you

@thienlnam
Copy link
Contributor

Didn't read the entire thread but do you have a summary of the problem? Something with the personal details not showing up for a new user?

@mountiny
Copy link
Contributor

mountiny commented Nov 8, 2023

It seems to be basically unable ot select the assignee in the new task flow when you just created a new chat with someone. I am double checking if we are even fixing these now

@thienlnam
Copy link
Contributor

Ah yeah, I've seen this problem before - I believe it lies in that when you create optimistic personal details for someone you don't actually store the email as part of it until they respond to you

So when you select this user in the new task flow, it generates a new optimistic personal details for this person since it doesn't link to the existing contact but then when the task is created it pulls the existing user.

Anyways, it's not that simple to fix and I'd say not worth our time right now so we should just close this - thoughts?

@aimane-chnaif
Copy link
Contributor

yes, correct.
Here's clear repro step: #25680 (comment)
So after logout and login, optimistic data is cleared and real data has only accountID and displayName, no login.

@mountiny
Copy link
Contributor

mountiny commented Nov 9, 2023

I have discussed this with Jason and I am going to create a project to collect these and close it, we are not going to focus on the task fixes right now

@mountiny
Copy link
Contributor

mountiny commented Nov 9, 2023

Added it to the project and we will come back to this once we are ready to refocus on this feature

@mountiny mountiny closed this as completed Nov 9, 2023
@DylanDylann
Copy link
Contributor

@mountiny @CortneyOfstad Are we eligible for compensation for our effort? Contributors are already assigned and started on the PR. It's just due to the decision changes and the solution becomes outdated.

Some other similar cases where compensation is made 100% thought PR is closed
#27108 (comment)
#26470 (comment)

@mountiny
Copy link
Contributor

mountiny commented Nov 9, 2023

I am sorry but in this case given the circumstances I would err on a side of no payout but rather just first dibs for you once we get back to this. We are going to fix this eventually but closing only to keep plate clener, otherwise we could put this on hold. Its added to a project in Todo category. This would require backend changes and external only work was not sufficient so I think full compensation at this point would not be appropriate either

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
No open projects
Development

No branches or pull requests