Update workspace chats naming to be: [users displayName]'s expenses

danielrvidal commented 2 weeks ago

This is a draft for now as we're still discussing what this would look like in conjunction with displaying showing the workspace name somewhere. Slack here: https://expensify.slack.com/archives/C07HPDRELLD/p1737752494145689

Proposal: Update workspace chats naming to be: [users displayName]'s expenses

Problem: Users are not comprehending the subtleties of our workspace chat naming conventions. For the submitter the room name is the workspace name, which submitters are not sure what that means, and for the admins the room is the employees display name, which looks like a DM. We often hear users are not understanding how to use their LHN, which leads to confusion and low conversion.

Solution: Let’s update workspace chats name to default to: [users displayName]'s expenses for both the employee and the admin.

I think this would make it more clear for everyone involved what that room is meant to do. I also think we can stop calling them workspace chats, which no one understands imo, and start calling them the users expense room, which feels much more understandable. Each employee gets an expense room where we put all their expenses submitted from the workspace.

I'm adding a designer to work with on this. Let's discuss in the thread. cc @anmurali

melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @dannymcclain (Design), see these Stack Overflow questions for more details.

samranahm commented 2 weeks ago

Proposal

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

Update workspace chats naming to be: [users displayName]'s expenses

What is the root cause of that problem?

Currently we display PolicyName for the reportOwner if report is PolicyExpenseChat https://github.com/Expensify/App/blob/b59f23b37d82a7b3de21a1a75e4c7479e37211f1/src/libs/ReportUtils.ts#L3118-L3128

For submitters the policy name is report header name that make it bit confusing that what type of room currently I am in
For Admin reportOwnerDisplayName show in LHN so it can make some users confuse about report type
What changes do you think we should make in order to solve the problem?

we should get the Display Name of PolicyExpenseChat user and append 's expense and display this formatted name for both user and policy admin

    const reportOwnerDisplayName = getDisplayNameForParticipant(ownerAccountID) || login || report?.reportName;
    const formattedName = reportOwnerDisplayName ? `${reportOwnerDisplayName}'s expenses` : '';
    const isAdmin = isPolicyAdmin(report?.policyID, {[`${ONYXKEYS.COLLECTION.POLICY}${policy?.id}`]: policy});
    // If the policy expense chat is owned by this user or if the current user is policy admin, use the name of the formattedName as the report name.
    if (report?.isOwnPolicyExpenseChat || isAdmin) {
        return formattedName;
    }

This way for both user and admin required format [users displayName]'s expenses will be display eliminating all confusions

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

N/A

What alternative solutions did you explore? (Optional)

As Admin

https://github.com/user-attachments/assets/33faa9f8-f3a6-47d4-ba20-a5c48bd7cd1a

As employee

github-actions[bot] commented 2 weeks ago

⚠️ @samranahm Thanks for your proposal. Please update it to follow the proposal template, as proposals are only reviewed if they follow that format (note the mandatory sections).

samranahm commented 2 weeks ago

🚨 Edited by proposal-police: This proposal was edited at 2025-01-25 09:41:35 UTC.

Proposal

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

Update workspace chats naming to be: [users displayName]'s expenses

What is the root cause of that problem?

Currently we display PolicyName for the reportOwner if report is PolicyExpenseChat https://github.com/Expensify/App/blob/b59f23b37d82a7b3de21a1a75e4c7479e37211f1/src/libs/ReportUtils.ts#L3118-L3128

For submitters the policy name is report header name that make it bit confusing that what type of room currently I am in
For Admin reportOwnerDisplayName show in LHN so it can make some users confuse about report type
What changes do you think we should make in order to solve the problem?

we should get the Display Name of PolicyExpenseChat user and append 's expense and display this formatted name for both user and policy admin

    const reportOwnerDisplayName = getDisplayNameForParticipant(ownerAccountID) || login || report?.reportName;
    const formattedName = reportOwnerDisplayName ? `${reportOwnerDisplayName}${translateLocal('common.expensesSuffix')}` : '';
    const isAdmin = isPolicyAdmin(report?.policyID, {[`${ONYXKEYS.COLLECTION.POLICY}${policy?.id}`]: policy});
    // If the policy expense chat is owned by this user or if the current user is policy admin, use the name of the formattedName as the report name.
    if (report?.isOwnPolicyExpenseChat || isAdmin) {
        return formattedName;
    }

This way for both user and admin required format [users displayName]'s expenses will be display eliminating all confusions

Edit: If displayName is user's email (ONBOARDING_CHOICES.MANAGE_TEAM) case, so instead of displaying the whole email we can keep only userame part then it will be formatted as userame's expenses

In getPolicyExpenseChatName function when calling getDisplayNameForParticipant we should pass another argument shouldUseShortForm = true const reportOwnerDisplayName = getDisplayNameForParticipant(ownerAccountID, true) || login || report?.reportName;

then

In getDisplayNameForParticipant function

we should explicitly check

if (shouldUseShortForm && longName.includes('@')) {
   return longName.replace(/@.*$/, '');
}

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

N/A

What alternative solutions did you explore? (Optional)

We should implement translate in DisplayNamePage function and a new Key value in en & es for's expenses by user preferred locale

expensesSuffix: '\'s expenses',

I didn’t saw this happening in ReportUtils, so let’s discuss how we should proceed.

POC

As Admin

https://github.com/user-attachments/assets/33faa9f8-f3a6-47d4-ba20-a5c48bd7cd1a

As employee

dannymcclain commented 2 weeks ago

Not overdue Melvin. Cmon. I just got the issue. Anyways, I'll work on these within a couple days and post something to the thread for us to look at!

dannymcclain commented 2 weeks ago

Oops, sorry. I posted some mocks to the thread, and then Shawn posted a double-decker riff, and the thread has continued to spin wildly 😅

You thought this was going to be a simple change, but now we're talking about all sorts of ways to reorganize the LHN! 😆

samranahm commented 2 weeks ago

Oops, sorry. I posted some mocks to the thread, and then Shawn posted a double-decker riff, and the thread has continued to spin wildly 😅

You thought this was going to be a simple change, but now we're talking about all sorts of ways to reorganize the LHN! 😆

That'd be amazing 🙌🏻

samranahm commented 1 week ago

Proposal updated

Updates

Resolve email displayName case
Match the mocks from #56123

melvin-bot[bot] commented 1 week ago

@dannymcclain Huh... This is 4 days overdue. Who can take care of this?

danielrvidal commented 1 week ago

Closing this in lieu of https://github.com/Expensify/App/issues/56123

Expensify / App