[Due for payment 2025-04-16] [$250] Chat - Hidden Attachment is shown when navigating through attachments

@kadiealexander

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

Version Number: 9.1.16-2
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/runs/view/33714&group_by=cases:section_id&group_order=asc&group_id=292475
Issue reported by: Applause - Internal Team

Action Performed:

As User A, open the app and navigate to User B's chat
Send two attachment to User B
As User B, flag one of the attachment as "Intimidation/Bullying"
As User A, navigate to User B's chat > Open the attachment that is not flagged
click on the arrow in the side to navigate to the flagged attachment.

Expected Result:

Hidden Attachment is not shown

Actual Result:

The hidden attachment is shown when navigating through attachments

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6778062_1742565265198.Screen_Recording_2025-03-21_at_4.51.27_in_the_afternoon.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021904422893400846619
Upwork Job ID: 1904422893400846619
Last Price Increase: 2025-04-01
Automatic offers:

FitseTLT | Contributor | 106778391

Issue Owner

Current Issue Owner: @kadiealexander

@kadiealexander

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

🚨 Edited by proposal-police: This proposal was edited at 2025-03-21 15:26:41 UTC.

Proposal

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

Open
Chat - Hidden Attachment is shown when navigating through attachments

What is the root cause of that problem?

We are currently trying to intialize hidden state via isAttachmentHidden from the context here

App/src/components/Attachments/AttachmentCarousel/CarouselItem.tsx

Line 37 in 67ea2f7

    
           const [isHidden, setIsHidden] = useState(() => (item.reportActionID ? isAttachmentHidden(item.reportActionID) : item.hasBeenFlagged));

but when the ReportAttachmentsContext context value is not initialized isAttachmentHidden will return undefined which is falsey and isHidden will be false.

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

Whenever the context value is no set we should fallback isHIdden to hasBeenFlagged


    const [isHidden, setIsHidden] = useState(() => (item.reportActionID && isAttachmentHidden(item.reportActionID)) ?? item.hasBeenFlagged);

this will result the attachment to be visible if the user has revealed the message from the report screen but if we want to always start flagged attachments as hidden in attachment modal we can initialize the state to item.hasBeenFlagged and in that case we will not need the context.

In addition to that, currently this use effect is running without a change in current report id so it is unnecessarily resetting the context when opening the attachment modal because currentReportID object is changing

App/src/pages/home/report/ReportAttachmentsContext.tsx

Line 23 in 67ea2f7

}, [currentReportID]);

so we need to change the dependency to currentReportID?.currentReportID

NOTE:
Currently we are only updating the ReportAttachmentsContext value only from PureReportActionItem here but we have got hide/show message buttons from attachment modal too so if we want to have hidden context state shared between PureReportActionItem and carousel item and get updated both sides case we need to do a couple of things.

Update the hiddenAttachments ref to a state so that it will re-render on change so update ReportAttachmentsProvider to

const [hiddenAttachments, setHiddenAttachments] = useState<Record<string, boolean>>({});

    useEffect(() => {
        // We only want to store the attachment visibility for the current report.
        // If the current report ID changes, clear the ref.
        setHiddenAttachments({});
    }, [currentReportID?.currentReportID]);

    const contextValue = useMemo(
        () => ({
            isAttachmentHidden: (reportActionID: string) => hiddenAttachments[reportActionID],
            updateHiddenAttachments: (reportActionID: string, value: boolean) => {
                setHiddenAttachments({
                    ...hiddenAttachments,
                    [reportActionID]: value,
                });
            },
        }),
        [hiddenAttachments],
    );

Then inside PureReportActionItem add an effect that updates isHidden state if the context value updates

 const isHiddenAttach = isAttachmentHidden(action.reportActionID);
    useEffect(() => {
        if (isHiddenAttach === undefined) {
            return;
        }
        setIsHidden(isHiddenAttach);
    }, [isHiddenAttach]);

We will not need the isHidden state in carousel item, we can directly use the context value so change it to

const {isAttachmentHidden, updateHiddenAttachments} = useContext(ReportAttachmentsContext);
    const isHidden = (item.reportActionID && isAttachmentHidden(item.reportActionID)) ?? item.hasBeenFlagged;

    const renderButton = (style: StyleProp<ViewStyle>) => (
        <Button
            small
            style={style}
            onPress={() => {
                updateHiddenAttachments(item.reportActionID, !isHidden);
            }}

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

We can render AttachmentModal with a flagged attachment and add a test ID to hide button here and assert that it has a text of reveal message.

What alternative solutions did you explore? (Optional)

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

@fedirjh

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

Reviewing label has been removed, please complete the "BugZero Checklist".

@fedirjh

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.1.24-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

Fix - Chat - Hidden Attachment is shown when navigating through attachments #59708

If no regressions arise, payment will be issued on 2025-04-16. 🎊

For reference, here are some details about the assignees on this issue:

@fedirjh requires payment through NewDot Manual Requests
@FitseTLT requires payment automatic offer (Contributor)

@fedirjh

@fedirjh @kadiealexander @fedirjh The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@FitseTLT

Payments needed:

- Contributor: @FitseTLT paid $250 via Upwork (offer)
- Reviewer: @fedirjh owed $250 via manual request

@fedirjh please complete the checklist!

@fedirjh

Adding myself as B0 to help as Kadie is OOO for the next week.

Bump @fedirjh for the checklist!

BugZero Checklist:

[Contributor] Classify the bug:

Bug classification

Source of bug:

1a. Result of the original design (eg. a case wasn't considered)
1b. Mistake during implementation
1c. Backend bug
1z. Other:

Where bug was reported:

2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
2b. Reported on staging (eg. found during regression or PR testing)
2d. Reported on a PR
2z. Other:

Who reported the bug:

[Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

Link to comment: [TS migration] Migrate AttachmentCarousel to typescript #36658 (comment)
[Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

Link to discussion: N/A
[Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

N/A , we have a regression test in the OP.

Regression Test Proposal Template

[BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

Link to issue:

Regression Test Proposal

Precondition:

Test:

Do we agree 👍 or 👎

@fedirjh

Awesome, cheers @fedirjh - here is the payment summary

Closing!

@fedirjh

$250 approved for @fedirjh

Navigation Menu

Repository navigation

[Due for payment 2025-04-16] [$250] Chat - Hidden Attachment is shown when navigating through attachments #58912

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Proposal

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

What is the root cause of that problem?

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

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

What alternative solutions did you explore? (Optional)

22 remaining items

BugZero Checklist:

Regression Test Proposal

Precondition:

Test:

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[Due for payment 2025-04-16] [$250] Chat - Hidden Attachment is shown when navigating through attachments #58912

Description

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Activity

melvin-bot commented on Mar 21, 2025

FitseTLT commented on Mar 21, 2025

Proposal

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

What is the root cause of that problem?

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

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

What alternative solutions did you explore? (Optional)

melvin-bot commented on Mar 25, 2025

melvin-bot commented on Mar 25, 2025

22 remaining items

melvin-bot commented on Apr 9, 2025

melvin-bot commented on Apr 9, 2025

melvin-bot commented on Apr 9, 2025

kadiealexander commented on Apr 16, 2025

jliexpensify commented on Apr 18, 2025

fedirjh commented on Apr 18, 2025

BugZero Checklist:

Regression Test Proposal

Precondition:

Test:

jliexpensify commented on Apr 20, 2025

garrettmknight commented on Apr 22, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions