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

[$250] [HOLD for payment 2024-04-09] [CRITICAL] [Groups] Frontend (Part 1): Create Group Chats + Splits. Deprecate Group DM creation. Support viewing both Group DMs and Group Chats. #32317

Closed
3 of 4 tasks
marcaaron opened this issue Nov 30, 2023 · 59 comments
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 Reviewing Has a PR in review Weekly KSv2

Comments

@marcaaron
Copy link
Contributor

marcaaron commented Nov 30, 2023

Holding on:

Group Chats Design Doc

Changes described in detail here.

Goals

  • Update the the Group Chat creation flow UI to have a new "confirm" screen

Current UI:

Image

New UI (with differences noted):

Image

  • The creator will be shown as the "admin"

API call:

  • Set the chatType to group when calling OpenReport so the backend will know we are creating a "Group Chat" and add a constant for this.
  • For now, default avatar is fine - no need to have the upload yet.
  • Group name should also be non-interactive for now (we will add this later).
    • It should automatically take the standard name: %firstName%, %firstName%, …. one for each member, with the first member alphabetically appearing first (leftmost)
    • If a Group chat contains only one member because everyone else has left, then the default name will be %firstName%’s group chat
  • We should send a new param called groupChatAdminLogins. For now, it will include the creator of the chat only.

Migration notes:

  • We are moving from one kind of "Group DM" towards a new format of "Group Chat"
  • So, we must make sure that when merging this PR both Group DMs and Group Chats are still supported in all App versions. Though this new version should not create Group DMs and only create Group Chat. This means any existing Group DMs should be visible by all users. And old app versions can safely create Group DMs.
  • Splits should also be creating Group Chats instead of Group DMs (see this doc section)
  • Add the default Group chat avatar assets. Default to one based on reportID (as this won't ever change) similar to how the default avatars are generated.

Image

This will all need to be performed in one App PR. We will hard deprecate older versions after it is deployed to production by updating the minimum app version so there is a hard dependency on this PR getting approved and merged:

Other things to note:

  • New Group Chats (like DMs) should have a notificationPreference of 'hidden' until a comment is left. We must ensure this behavior is preserved.

Implementation notes:

Our existing screen for this flow is: NewChatSelectorPage, but the Chat tab section is the NewChatPage.
Its Modal stack navigator is here.

We need to:
Modify the “Create group” button so that it says “Next”.
Add an additional page to the NewChatSelectorPage stack navigator.
Create a new page which we will call NewChatConfrmPage
Add the appropriate routes for this page at /new/chat/confirm

Switching the button to “Next” here when we have tapped the “Add to group” option.
Navigating to the new page

We need to add logic to navigate the user to the confirm page when they have selected “Add to group” here and have some staging participants.

We’ll also need some way for this new page to access those participants. The selectedOptions are currently stored here and house the members being added to the group. We will move them to a new Onyx key called newGroupChat that will be exclusively used to hold the state for a Group chat that is in the process of being created. This will allow the user to refresh the page and pick up where they left off (which is not possible today) and also allow us to share this state between all pages in the flow.

Start group button
By this point, the newGroupChat Onyx state should have the following required data for the OpenReport API call:
optimisticReportID
optimistic accountIDs for any participants
groupChatAdminLogins - should only contain the creator
reportName - empty string (for now)

onPress we will navigate to the report just like we do already for Group DMs.

To do this, we will modify the navigateAndOpenReport() method here to handle the:
memberRoles (if any)
reportName

It already handles the logins. So, we need to add arguments for roles and reportName to that method. We must also modify the logic here when we have the group chat case since we won’t ever need to get an existing chat and will always be creating one.

The optimisticReport returned from ReportUtils.buildOptimisticChatReport() must be modified to update the participants field (see refactor plan here) with the corresponding role for each participant. Finally, the optimistic report is passed here to be created in the server. We must also pass the memberRoles as it will contain the link between any optimistic accountID and the login/email.

Group chat avatars will also need to display correctly in the Search function, Report header, details page, etc. We must modify the methods that get the avatars from personal details or workspace and first check for the report.avatarUrl. We can modify the existing ReportUtils.getIcons() method to first check the report.avatarUrl when we have a “Group chat” and fallback to a default avatar based off of the reportID.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0174ced808f29e748c
  • Upwork Job ID: 1777806130629287936
  • Last Price Increase: 2024-04-09
@Expensify Expensify locked and limited conversation to collaborators Nov 30, 2023
@marcaaron marcaaron self-assigned this Nov 30, 2023
@melvin-bot melvin-bot bot added the Monthly KSv2 label Dec 4, 2023
@melvin-bot melvin-bot bot added the Overdue label Jan 5, 2024
@marcaaron marcaaron added Planning Changes still in the thought process and removed Monthly KSv2 Overdue labels Jan 10, 2024
@marcaaron
Copy link
Contributor Author

Thanks Melv but we're good. This is still in planning.

@melvin-bot melvin-bot bot added the Monthly KSv2 label Jan 10, 2024
@marcaaron marcaaron removed their assignment Jan 19, 2024
@marcaaron marcaaron removed the Planning Changes still in the thought process label Jan 19, 2024
@marcaaron marcaaron changed the title [HOLD] Frontend: Create Group Chats + Splits. Deprecate Group DM creation. Support viewing both Group DMs and Group Chats. [HOLD] [CRITICAL] Frontend: Create Group Chats + Splits. Deprecate Group DM creation. Support viewing both Group DMs and Group Chats. Jan 19, 2024
@marcaaron marcaaron added Weekly KSv2 and removed Monthly KSv2 labels Jan 19, 2024
@marcaaron marcaaron self-assigned this Jan 19, 2024
@melvin-bot melvin-bot bot added the Overdue label Jan 29, 2024
@marcaaron
Copy link
Contributor Author

almost ready to come off hold. I have been focusing on Wave work the past week.

We are also going to be incorporating some of the components from the Simplified Collect project so waiting to see where that stuff lands so we don't end up building the new invite flow more than once.

@melvin-bot melvin-bot bot removed the Overdue label Jan 30, 2024
@melvin-bot melvin-bot bot added the Overdue label Feb 7, 2024
@marcaaron
Copy link
Contributor Author

Not really much blocking this one at this point so I'm going to remove the HOLD. The backend parts for creating a Group Chat should all be on production now. I think we can probably start engaging with somebody external to help implement this.

@melvin-bot melvin-bot bot removed the Overdue label Feb 7, 2024
@marcaaron marcaaron changed the title [HOLD] [CRITICAL] Frontend: Create Group Chats + Splits. Deprecate Group DM creation. Support viewing both Group DMs and Group Chats. [CRITICAL] Frontend: Create Group Chats + Splits. Deprecate Group DM creation. Support viewing both Group DMs and Group Chats. Feb 7, 2024
@marcaaron marcaaron changed the title [CRITICAL] Frontend: Create Group Chats + Splits. Deprecate Group DM creation. Support viewing both Group DMs and Group Chats. [CRITICAL] Frontend (Part 1): Create Group Chats + Splits. Deprecate Group DM creation. Support viewing both Group DMs and Group Chats. Feb 13, 2024
@arielgreen
Copy link
Contributor

@waterim is taking this one on https://expensify.slack.com/archives/C03UK30EA1Z/p1707785405223319

@Expensify Expensify unlocked this conversation Feb 14, 2024
@arielgreen arielgreen changed the title [CRITICAL] Frontend (Part 1): Create Group Chats + Splits. Deprecate Group DM creation. Support viewing both Group DMs and Group Chats. [CRITICAL] [Groups] Frontend (Part 1): Create Group Chats + Splits. Deprecate Group DM creation. Support viewing both Group DMs and Group Chats. Feb 15, 2024
@waterim
Copy link
Contributor

waterim commented Feb 16, 2024

Hello, Im Artem from Callstack and already working on this!

@marcaaron
Copy link
Contributor Author

Done! Thanks for your help on this one.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Apr 8, 2024
Copy link

melvin-bot bot commented Apr 9, 2024

Issue is ready for payment but no BZ is assigned. @joekaufmanexpensify you are the lucky winner! Please verify the payment summary looks correct and complete the checklist. Thanks!

Copy link

melvin-bot bot commented Apr 9, 2024

Payment Summary

Upwork Job

  • ROLE: @s77rt paid $(AMOUNT) via Upwork (LINK)
  • Contributor: @waterim is from an agency-contributor and not due payment

BugZero Checklist (@joekaufmanexpensify)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@joekaufmanexpensify joekaufmanexpensify added the External Added to denote the issue can be worked on by a contributor label Apr 9, 2024
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2024-04-09] [CRITICAL] [Groups] Frontend (Part 1): Create Group Chats + Splits. Deprecate Group DM creation. Support viewing both Group DMs and Group Chats. [$250] [HOLD for payment 2024-04-09] [CRITICAL] [Groups] Frontend (Part 1): Create Group Chats + Splits. Deprecate Group DM creation. Support viewing both Group DMs and Group Chats. Apr 9, 2024
Copy link

melvin-bot bot commented Apr 9, 2024

Job added to Upwork: https://www.upwork.com/jobs/~0174ced808f29e748c

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 9, 2024
Copy link

melvin-bot bot commented Apr 9, 2024

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

@joekaufmanexpensify joekaufmanexpensify removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 9, 2024
@joekaufmanexpensify
Copy link
Contributor

@marcaaron @s77rt just to confirm, this regression was valid, correct?

@s77rt
Copy link
Contributor

s77rt commented Apr 9, 2024

Yes that's correct

@joekaufmanexpensify

This comment was marked as outdated.

@joekaufmanexpensify
Copy link
Contributor

@s77rt offer for $250 sent via upwork!

@s77rt
Copy link
Contributor

s77rt commented Apr 9, 2024

Actually there were many regressions / missed cases:
#39247
#39295
#39254
#39323
#39560

@joekaufmanexpensify
Copy link
Contributor

Interesting. Curious if @marcaaron you agree that all of these were regressions?

Reason I ask is each regression typically reduces bounty by 50%. So if there were 6 regressions here, the bounty would be reduced from $500 > $8. So want to be super sure that each of them was in fact a regression.

@joekaufmanexpensify
Copy link
Contributor

Noting that @marcaaron and I discussed this 1:1 and decided that there should be no regression penalties here. Marc flagged that @s77rt has gone above and beyond to do a great job working on this project, and any missed cases are not indicative of mistakes or anything like that.

So we're going to pay out the full $500 to @s77rt for this one via Upwork.

@s77rt
Copy link
Contributor

s77rt commented Apr 11, 2024

Thank you! I have accepted the Upwork offer

@joekaufmanexpensify
Copy link
Contributor

@s77rt $500 sent and contract ended!

@joekaufmanexpensify
Copy link
Contributor

Upwork job closed.

@joekaufmanexpensify
Copy link
Contributor

All set. TY!

Copy link

melvin-bot bot commented Aug 11, 2024

⚠️ 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.

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 Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

8 participants