[$250] Add support for viewing full screen Group Chat custom avatars

[marcaaron]

Problem

When we added support for custom Group Chat avatars there are some places left where we cannot yet view the full screen avatar.

Solution

Navigate to the correct page like we do here when user taps on the avatar: https://github.com/Expensify/App/blob/ddb83f2493c3d08b3b3febcfd9dc93729f38e3c2/src/components/RoomHeaderAvatars.tsx#L19-L28

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c10e84b6d429f37d
• Upwork Job ID: 1785082381325901824
• Last Price Increase: 2024-04-29
• Automatic offers:
• s77rt | Reviewer | 0
• tsa321 | Contributor | 103805311

Issue Owner

Current Issue Owner: @

[s77rt]

No progress yet. Focused on QBO

[dannymcclain]

I think ideally we'd follow the same pattern that we use for custom workspace avatars.

cc @Expensify/design - I actually think the order of these should be Upload, View, Remove. But that's not a super pressing concern.

[marcaaron]

Hey @s77rt, let us know if you have started working on this. I can make it external, but would love if you could be the C+ still.

[shawnborton]

I agree with the proposed solution from Danny.

[s77rt]

@marcaaron Making this External sounds good to me. I'd love to be the C+ still

[melvin-bot[bot]]

Triggered auto assignment to @zanyrenney (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

[melvin-bot[bot]]

:warning: It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time :warning:

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~01c10e84b6d429f37d

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Triggered auto assignment to Design team member for new feature review - @dubielzyk-expensify (NewFeature)

[nexarvo]

Proposal

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

Add support for viewing full screen Group Chat custom avatars

What is the root cause of that problem?

New Feature

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

Right now we don't have component which exactly take report and displays it's avatar. We can update ReportAvatar component to show report avatar like this:

-    const policyName = ReportUtils.getPolicyName(report, false, policy);
-    const avatarURL = ReportUtils.getWorkspaceAvatar(report);

+    const policyName = policy ? ReportUtils.getPolicyName(report, false, policy): report?.reportName;
+    const avatarURL = policy ? ReportUtils.getWorkspaceAvatar(report): report?.avatarUrl;

We will also need to add below code:

onViewPhotoPress={() => Navigation.navigate(ROUTES.REPORT_AVATAR.getRoute(report.reportID))}

here: https://github.com/Expensify/App/blob/6ff8c828f1d97f1b97e037d651cb30cac2e3c20b/src/pages/ReportDetailsPage.tsx#L233 and also in navigateToAvatarPage in RoomHeaderAvatars function

We will also need to remove shouldDisableViewPhoto prop to show the view photo menu item here: https://github.com/Expensify/App/blob/6ff8c828f1d97f1b97e037d651cb30cac2e3c20b/src/pages/ReportDetailsPage.tsx#L238

What alternative solutions did you explore? (Optional)

Alternatively we can pass isGroupChat prop in ReportAvatar component and check for that instead of checking for policy in above suggested code like this:

-    const policyName = ReportUtils.getPolicyName(report, false, policy);
-    const avatarURL = ReportUtils.getWorkspaceAvatar(report);

+    const policyName = !isGroupChat ? ReportUtils.getPolicyName(report, false, policy): report?.reportName;
+    const avatarURL = !isGroupChat ? ReportUtils.getWorkspaceAvatar(report): report?.avatarUrl;

[!NOTE] We will also need to handle default avatar condition in ReportAvatar if report?.avatarUrl is not available and report is using default avatar

Result

https://github.com/Expensify/App/assets/53095363/74913bd5-8c60-4da3-a5c7-6a612c65167d

[s77rt]

@nexarvo Thanks for the proposal. Your RCA is correct. The solution looks overall good to me.

🎀 👀 🎀 C+ reviewed Link to proposal

[melvin-bot[bot]]

Current assignee @marcaaron is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[marcaaron]

Thanks everyone 🚀

[melvin-bot[bot]]

📣 @s77rt 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

[melvin-bot[bot]]

📣 @nexarvo You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot[bot]]

This issue has not been updated in over 15 days. @s77rt, @marcaaron, @nexarvo, @zanyrenney, @dubielzyk-expensify eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[s77rt]

Still working on the PR

[melvin-bot[bot]]

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

[s77rt]

@nexarvo The PR has been reverted. Can you raise a PR with fixes for the found regressions?

[s77rt]

@nexarvo Are you still able to work on this?

[s77rt]

@marcaaron Let's reopen this for proposals

[tsa321]

@s77rt How about merging additional code from my draft PR in deploy blocker issue into his code?

[s77rt]

@tsa321 I haven't read the code fully but let's not use base64 uri. Dealing with long strings can be slow. If you want to pick up this issue please use the proposal template addressing the regressions from the previous PR

[s77rt]

@marcaaron Can you readd Help Wanted label

[marcaaron]

summarizing the status of this one... seems like @nexarvo has bailed on this issue and needs to be unassigned here?

@tsa321 do you have some proposal for us?

[melvin-bot[bot]]

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

[tsa321]

@s77rt I have made a test branch. I will make a proposal later.

[s77rt]

@tsa321 Looking forward for the proposal. (code-diffs, branches, PRs, should be avoided)

[tsa321]

Edited by proposal-police: This proposal was edited at 2023-10-11T14:27:00Z.

Proposal

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

Add support for viewing the photo of a group chat.

What is the root cause of that problem?

This is a new feature.

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

We should implement the following changes:

1. Allow Display of Group Chat Photos:

   Remove the shouldDisableViewPhoto property from the code:

   https://github.com/Expensify/App/blob/b12a80e477c4563ceff522ab34b6ac6cbc2b4c49/src/pages/NewChatConfirmPage.tsx#L154

   and in ReportDetailsPage:
   

   https://github.com/Expensify/App/blob/ad7c1d5e394c8a9ee1ce7427233aa085350bb500/src/pages/ReportDetailsPage.tsx#L504

   Then, add the onViewPhotoPress property:

   onViewPhotoPress={() => Navigation.navigate(ROUTES.REPORT_AVATAR.getRoute(optimisticReportID.current))}
   

   in the AvatarWithImagePicker of ReportDetailsPage and NewChatConfirmPage

2. Add optimisticReportID to newGroupDraft Onyx:

   Add the optimisticReportID to newGroupDraft in Onyx to support viewing the photo of ReportAvatar on the NewChatConfirm page. Set this when the NewChatConfirm page mounts with the following code:

   useEffect(() => {
       Report.setGroupDraft({ optimisticReportID: optimisticReportID.current });
   }, []);
   

3. Update ReportAvatar Logic:

   In the ReportAvatar component, include logic to determine whether the avatar is from a newGroupChatDraft or a common report avatar. This requires newGroupDraft in Onyx. The updated code for ReportAvatar might look like this:

function ReportAvatar({report = {} as Report, route, policies, isLoadingApp = true, newGroupDraft}: ReportAvatarProps) {
    const reportIDFromRoute = String(route.params?.reportID || 0);
    const isNewGroupDraftAvatar = reportIDFromRoute === newGroupDraft?.optimisticReportID;

    const policyID = route.params.policyID ?? '-1';
    const policy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`];
    const policyName = ReportUtils.getPolicyName(report, true, policy);
    const isPolicyRelatedReport = !!policyName;
    const [shouldShowDefaultGroupAvatar, setShouldShowDefaultGroupAvatar] = useState(false);

    const attachmentProperty = useMemo(() => {
        if (isNewGroupDraftAvatar) {
            return {
                source : shouldShowDefaultGroupAvatar ? ReportUtils.getDefaultGroupAvatar(newGroupDraft?.optimisticReportID) : newGroupDraft?.avatarUri,
                headerTitle : newGroupDraft?.reportName ?? ReportUtils.getGroupChatName(newGroupDraft?.participants) ?? '',
                originalFileName : newGroupDraft?.avatarFileName,
            }
        }

        if (isPolicyRelatedReport) {
            return {
                source: UserUtils.getFullSizeAvatar(ReportUtils.getWorkspaceAvatar(report), 0), 
                headerTitle: policyName,
                originalFileName: policy?.originalFileName ?? policy?.id ?? report?.policyID ?? '',
            }
        }

        return {
            source:  UserUtils.getFullSizeAvatar(report?.avatarUrl ?? '', 0),
            headerTitle: ReportUtils.getReportName(report),
            originalFileName: report?.originalFileName ?? CONST.FAKE_IMAGE_FILENAME,
        };
    },[shouldShowDefaultGroupAvatar, newGroupDraft, report, policy])

    useEffect(() => {
        const onError = () => {
            setShouldShowDefaultGroupAvatar(true);
        }
        if (isNewGroupDraftAvatar) {
            FileUtils.readFileAsync(newGroupDraft?.avatarUri, newGroupDraft?.avatarFileName, () => {}, onError);
        }
    }, []);

    const onModalClose = useCallback(() => {
       if (isNewGroupDraftAvatar) {
            Navigation.closeAndNavigate(ROUTES.NEW_CHAT_CONFIRM);
        } else if (report?.reportID)  {
            Navigation.goBack(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(report?.reportID ?? '-1'))
        } else {
            Navigation.dismissModal();
        }
    }, [isNewGroupAvatar]);

    return (
        <AttachmentModal
            headerTitle={attachmentProperty.headerTitle}
            defaultOpen
            source={attachmentProperty.source}
            onModalClose={onModalClose}
            isWorkspaceAvatar={isPolicyRelatedReport && !isNewGroupDraftAvatar}
            maybeIcon
            // In the case of default workspace avatar, originalFileName prop takes policyID as value to get the color of the avatar
            originalFileName={attachmentProperty.originalFileName}
            shouldShowNotFoundPage={!isNewGroupDraftAvatar && !report?.reportID && !isLoadingApp}
            isLoading={(!report?.reportID || !policy?.id) && !!isLoadingApp}
        />
    );
}

We will displays the default avatar when the blob uri of group avatar isn't available

4. Check avaibility of newGroupDraft.avatarUri:

   We check the object URI of the avatarUri using FileUtils.readFileAsync. In onError callback, we set the shouldShowDefaultAvatar to true.

A test branch has been created to demonstrate the code changes.

---

Regarding regression from previous PR :

@s77rt

1. Regarding the regression issue with the new group avatar modal infinitely loading/spinning after a refresh:

We could display the default avatar after a user refreshes the report avatar page. This is achieved by checking whether the object URI of newGroupDraft.avatarUri exists, using FileUtils.readFileAsync. In the onError parameter, we set shouldShowDefaultGroupAvatar to true. The complete code can be found in the test branch or in the JavaScript code provided above. To get the default group avatar we use ReportUtils.getDefaultGroupAvatar(optimisticReportID).

2. Regarding the issue of the ReportAvatar of a deleted workspace being blank:

   I believe this is due to the following code:

   https://github.com/nexarvo/Expensify-App/blob/1c3e7db84ff296c9a62fd48530de4c2306027994/src/pages/ReportAvatar.tsx#L44-L46

   The avatarUrl and fileName depend on the policy. However, if the policy is deleted, it becomes undefined, which results in incorrect values for these two properties.

   We might be able to check the policy report based on report.policyID or based on report data, and we need to ensure compatibility with the logic used to retrieve the policy name, which determines whether it is a policy-related report:

   https://github.com/Expensify/App/blob/156f0a2bd0e94558ad621caa701bd749dd124bfe/src/libs/ReportUtils.ts#L739-L757