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

[Internal] Room - Room chat avatar is different for user who creates room & who has been invited to room #33470

Closed
2 of 6 tasks
lanitochka17 opened this issue Dec 22, 2023 · 103 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Hot Pick Ready for an engineer to pick up and run with Internal Requires API changes or must be handled by Expensify staff

Comments

@lanitochka17
Copy link

lanitochka17 commented Dec 22, 2023

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


Version Number: 1.4.15-4
Reproducible in staging?: Y
Reproducible in production?: Y
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:

  1. Launch app
  2. Tap fab--start chat
  3. Tap room
  4. Enter room name and select a workspace with custom avatar
  5. Tap create room
  6. Tap header
  7. Tap members---Invite
  8. Invite user X to room
    9 .Navigate to LHN and note avatar of room chat
  9. Go to https://staging.new.expensify.com/
  10. Login as user X
  11. Note avatar of room chat

Expected Result:

Room chat avatar must be same for user who creates room and for user who has been invited to room

Actual Result:

Room chat avatar is different for user who creates room and for user who has been invited to room

Workaround:

Unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6323221_1703194294854.full.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~013499a52b4634b0ed
  • Upwork Job ID: 1737991692071854080
  • Last Price Increase: 2023-12-29
Issue OwnerCurrent Issue Owner: @grgia
@lanitochka17 lanitochka17 added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 22, 2023
@melvin-bot melvin-bot bot changed the title Room - Room chat avatar is different for user who creates room & who has been invited to room [$500] Room - Room chat avatar is different for user who creates room & who has been invited to room Dec 22, 2023
Copy link

melvin-bot bot commented Dec 22, 2023

Job added to Upwork: https://www.upwork.com/jobs/~013499a52b4634b0ed

Copy link

melvin-bot bot commented Dec 22, 2023

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

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

melvin-bot bot commented Dec 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

Copy link

melvin-bot bot commented Dec 22, 2023

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

@unidev727
Copy link
Contributor

unidev727 commented Dec 22, 2023

Proposal

from: @unicorndev-727

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

Room chat avatar is different for user who creates room & who has been invited to room

What is the root cause of that problem?

The root cause is that the user who has been invited to room doesn't have the policy data about workspace in Onyx so that the default workspace avatar will be showed.
This issue only happens when we invite the user who isn't member of this workspace to the only room.

App/src/libs/ReportUtils.ts

Lines 1247 to 1249 in 8970aa3

const policyExpenseChatAvatarSource = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`]?.avatar
? allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`]?.avatar
: getDefaultWorkspaceAvatar(workspaceName);

It is a screen shot of the room owner.
image
It is a screen shot of the room invited user.
image

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

We can send the policy update request if ONYXKEYS.COLLECTION.POLICY}${report?.policyID} is null here.

What alternative solutions did you explore? (Optional)

Backend need to send the workspace data to the users who has been invited to the room and isn't the member of the workspace.

@dukenv0307
Copy link
Contributor

dukenv0307 commented Dec 22, 2023

Proposal

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

Room chat avatar is different for user who creates room and for user who has been invited to room

What is the root cause of that problem?

We're using the workspace avatar as the policy room avatar.

But when the user is invited to a policy room, but the user is not a member of that workspace, they don't have access to that information. That's expected because if user is not in the workspace, they shouldn't have access to the policy record.

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

We had the same issue before with the workspace name, and fixed it by adding the policyName as a field in the report of the room.

We should do the same here for the workspace avatar, we need to add a policyAvatar field in the report record, which will be synced with the avatar of the workspace, the back-end will also need to be updated to support this.

Then in front-end we'll use that field as a fallback in case the policy avatar is not found. If even that policyAvatar is not available, now we fallback to the default workspace avatar as we currently do.

This way, even though the user doesn't have any access to the workspace data, they can still access the policyAvatar (beside policyName) to display as room avatar.

What alternative solutions did you explore? (Optional)

NA

@alexpensify
Copy link
Contributor

@getusha - - when you get a chance, can you review if this proposal will fix this issue? Thanks!

Heads up, I will be offline from Friday, December 22, to Thursday, January 4, 2024. I will not be actively watching over this GitHub during that period. If anything urgent is needed here, please ask for help in the #expensify-open-source Slack Room-- thanks!

@getusha
Copy link
Contributor

getusha commented Dec 25, 2023

@unicorndev-727 do you mind adding more details to the solution you mentioned in your proposal?

@unidev727
Copy link
Contributor

unidev727 commented Dec 25, 2023

@unicorndev-727 do you mind adding more details to the solution you mentioned in your proposal?

In fact, this is the best place to call the backend API to allow workspace data to invited users who are not members of the workspace.

App/src/libs/actions/Report.ts

Lines 2165 to 2175 in 595bf40

type InviteToRoomParameters = {
reportID: string;
inviteeEmails: string[];
};
const parameters: InviteToRoomParameters = {
reportID,
inviteeEmails,
};
API.write('InviteToRoom', parameters, {optimisticData, successData, failureData});

Frontend is subscribted to policy data properly to get workspace icons here
const itemPolicy = policy[`${ONYXKEYS.COLLECTION.POLICY}${itemFullReport.policyID}`] || {};

and here
const policyExpenseChatAvatarSource = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`]?.avatar
.
But it doesn't work because we don't have the workspace data in ONYXKEYS.COLLECTION.POLICY.
So we need to update inviteToRoom API to share workspace data with the invited users who aren't member of the workspace by setting role to guest in policy so that the guest can only read policy data like name, avatar and so on, I think.

type InviteToRoomParameters = {
        reportID: string;
        inviteeEmails: string[];
        policyId: string;
        role: string;
    };

    const parameters: InviteToRoomParameters = {
        reportID,
        inviteeEmails,
        policyId: report.policyId,
        role: 'guest'
    };

    API.write('InviteToRoom', parameters, {optimisticData, successData, failureData});

image
image
This prevents inconsistencies in policy updates and also ensures security.

@dukenv0307
Copy link
Contributor

@getusha what do you think about my proposal? We already do the same with the policyName, and with that we don't need new authentication system for "guest" to access the workspace data as mentioned above.

Copy link

melvin-bot bot commented Dec 29, 2023

@alexpensify, @getusha Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label Dec 29, 2023
Copy link

melvin-bot bot commented Dec 29, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

Copy link

melvin-bot bot commented Jan 1, 2024

@alexpensify, @getusha Still overdue 6 days?! Let's take care of this!

@getusha
Copy link
Contributor

getusha commented Jan 1, 2024

Reviewing proposals.

@melvin-bot melvin-bot bot removed the Overdue label Jan 1, 2024
@alexpensify
Copy link
Contributor

@getusha - I'm back online again. Any update regarding the proposals?

@melvin-bot melvin-bot bot added the Overdue label Jan 4, 2024
@getusha
Copy link
Contributor

getusha commented Jan 4, 2024

🎀 👀 🎀
Pulling up internal engineer to confirm if we can return workspace data (avatar) to a non-member user that joined a room and fix this on the backend initially.

@melvin-bot melvin-bot bot removed the Overdue label Jan 4, 2024
Copy link

melvin-bot bot commented Jan 4, 2024

Triggered auto assignment to @grgia, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@grgia
Copy link
Contributor

grgia commented Jan 4, 2024

@getusha looking into this

@melvin-bot melvin-bot bot removed the Overdue label Aug 26, 2024
@alexpensify
Copy link
Contributor

Next Steps

@isabelastisser the discussion is ongoing here. I reassigned the Bug label to keep this one moving forward while I'm OOO. Thanks!

Heads up, I will be offline until Tuesday, September 3, 2024, and will not actively watch over this GitHub during that period.

If anything urgent is needed here, please ask for help in the #expensify-open-source Slack Room-- thanks!

@melvin-bot melvin-bot bot added the Overdue label Aug 28, 2024
@isabelastisser
Copy link
Contributor

Not overdue.

@techievivek
Copy link
Contributor

Not overdue, @dukenv0307 @getusha let's not forget this #37003 (comment), it is still broken on mweb?

@melvin-bot melvin-bot bot removed the Overdue label Sep 2, 2024
@isabelastisser
Copy link
Contributor

Bump @dukenv0307 @getusha

@alexpensify
Copy link
Contributor

Thanks, @isabelastisser, for your help! I'm online and taking this GH back.

@dukenv0307
Copy link
Contributor

@techievivek I checked again and the bug is fixed on mweb on the latest main. Can the QA team check again? Correct me if I missed something.

ios-mweb-resize.mp4
android-mweb-resize.mp4

@melvin-bot melvin-bot bot added the Overdue label Sep 6, 2024
@alexpensify
Copy link
Contributor

@techievivek - we need your follow-up here. Thanks!

@melvin-bot melvin-bot bot removed the Overdue label Sep 6, 2024
@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)

@melvin-bot melvin-bot bot added the Overdue label Sep 9, 2024
@techievivek
Copy link
Contributor

techievivek commented Sep 9, 2024

I checked again and the bug is fixed on mweb on the latest main. Can the QA team check again? Correct me if I missed something.

QA team just confirmed that this is fixed so we can move forward with the payment here. @alexpensify

@melvin-bot melvin-bot bot removed the Overdue label Sep 9, 2024
@alexpensify
Copy link
Contributor

alexpensify commented Sep 9, 2024

Payouts due: 2024-09-09

Upwork job is here.

@alexpensify
Copy link
Contributor

alexpensify commented Sep 9, 2024

@dukenv0307 - I sent an offer in Upwork. Please accept and I can complete the payment process.

Thanks!

@dukenv0307
Copy link
Contributor

Please accept

@alexpensify Done, thank you

@getusha
Copy link
Contributor

getusha commented Sep 10, 2024

@alexpensify i should get paid on Upwork as well since the issue was created before my ND payment eligibility date.

@alexpensify
Copy link
Contributor

@getusha - thanks for flagging, I sent over an offer in Upwork.

@alexpensify
Copy link
Contributor

@dukenv0307 - thanks, I completed the payment process in Upwork!

@getusha
Copy link
Contributor

getusha commented Sep 11, 2024

@alexpensify accepted the offer. ty!

@alexpensify
Copy link
Contributor

Awesome, everyone has been paid via Upwork.

#33470 (comment)

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. Daily KSv2 Hot Pick Ready for an engineer to pick up and run with Internal Requires API changes or must be handled by Expensify staff
Projects
No open projects
Status: No status
Development

No branches or pull requests

10 participants