[$250] Chat - In offline, parent conversation is not highlighted.

[IuliiaHerets]

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.0.78-1
Reproducible in staging?: Yes
Reproducible in production?: Yes
Issue reported by: Applause Internal Team
Device used: Redmi note 10s Android 13
App Component: Chat Report View

Action Performed:

1. Go to https://staging.new.expensify.com/home
2. Open a report
3. Send a message and create a thread message
4. Tap on header from link and navigate to parent message
5. Note conversation is highlighted
6. Go offline
7. Repeat step 3 & step 4
8. Note conversation is not highlighted

Expected Result:

In offline, parent conversation must be highlighted.

Actual Result:

In offline, parent conversation is not highlighted.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6701658_1735019559394.Screenrecorder-2024-12-24-11-11-44-33.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021874048824719107982
• Upwork Job ID: 1874048824719107982
• Last Price Increase: 2025-01-07
• Automatic offers:
• daledah | Contributor | 105615170

Issue Owner

Current Issue Owner: @allroundexperts

[melvin-bot]

Triggered auto assignment to @sonialiap (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.

[daledah]

Edited by proposal-police: This proposal was edited at 2024-12-24 15:44:07 UTC.

Proposal

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

In offline, parent conversation is not highlighted.

What is the root cause of that problem?

1. We're not allow navigating to parent within reportID

App/src/components/ParentNavigationSubtitle.tsx

Lines 50 to 52 in 126d92a

	if (isVisibleAction && !isOffline) {
	// Pop the chat report screen before navigating to the linked report action.
	Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(parentReportID, parentReportActionID), true);

2. If we remove isOffline. In case users login to the App by deeplink, then go offline, the pages will be empty (has not been loaded yet)

App/src/hooks/usePaginatedReportActions.ts

Line 22 in d5cdd55

const [reportActionPages] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_PAGES}${reportIDWithDefault}`);

so the data is empty

App/src/libs/PaginationUtils.ts

Lines 175 to 177 in d5cdd55

	if (pages.length === 0) {
	return {data: id ? [] : sortedItems, hasNextPage: false, hasPreviousPage: false};
	}

that shows the loading screen even though the data is loaded.

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

Solution 1: Disable navigating to parent if isOffline and pages is empty

We should update this condition in here and here

But this solution is not a best idea since even the page is not loaded, we still can show the parent message.

Solution 2:

Remove isOffline in 2 these places

Then update getContinuousChain logic to return sortedItems if page is empty

if (pages.length === 0) {
        const item = sortedItems.find((item) => getID(item) === id);
        return {data: id && !item ? [] : sortedItems, hasNextPage: false, hasPreviousPage: false};
    }

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

We should update getContinuousChain test to cover this case

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[FitseTLT]

Proposal

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

Chat - In offline, parent conversation is not highlighted.

What is the root cause of that problem?

We are not linking to the parent report action id in offline mode here

App/src/components/ParentNavigationSubtitle.tsx

Lines 50 to 52 in 126d92a

	if (isVisibleAction && !isOffline) {
	// Pop the chat report screen before navigating to the linked report action.
	Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(parentReportID, parentReportActionID), true);

When comment linking to parent report action was implemented a regression occurred where if you create threads offline and try to comment link back to parent actions it was showing skeleton loading although the report actions where available in onyx so we disabled comment linking to parent actions in here by adding conditions in all three places here here and here
But the reason it was showing loading indicator was because usePaginatedReportActions was returning empty data (reportActions list) because when linking to report actions we return empty if page is empty here

App/src/libs/PaginationUtils.ts

Lines 175 to 176 in 126d92a

	if (pages.length === 0) {
	return {data: id ? [] : sortedItems, hasNextPage: false, hasPreviousPage: false};

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

Remove the isOffline checks in all three places here here and here

To avoid the skeleton loading problem we should only return empty if the linked report action doesn't exist in sortedItems list

App/src/libs/PaginationUtils.ts

Lines 175 to 176 in 126d92a

	if (pages.length === 0) {
	return {data: id ? [] : sortedItems, hasNextPage: false, hasPreviousPage: false};

 if (pages.length === 0) {
        return {data: id && !sortedItems.some((action) => getID(action) === id) ? [] : sortedItems, hasNextPage: false, hasPreviousPage: false};
    }

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

We can create a test for getContinuousChain by passing empty pages and proper id param to assert that it doesn't return empty if the action exists in sortedItems

What alternative solutions did you explore? (Optional)

[daledah]

@FitseTLT Your new proposal is same as mine 🙏. You deleted the previous one after reading my proposal.

You just added another place that can be easy to figure out (by searching if (isVisibleAction && !isOffline) in our code). I think we can create the util function to prevent duplicated code.

[FitseTLT]

@FitseTLT Your new proposal is same as mine 🙏. You deleted the previous one after reading my proposal.

You just added another place that can be easy to figure out (by searching if (isVisibleAction && !isOffline) in our code). I think we can create the util function to prevent duplicated code.

I reposted after digging into the previous changes to identify the correct problem we were trying to solve with the check and proposed a full solution based on that which includes all places (you are missing) that we need to remove the isOffline check. It has nothing to do with your solution. And suggesting to creat util function doesn't proof that you could easily find the missed case. Let's just wait for the reviewer

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[daledah]

@allroundexperts What do you think about my proposal? Thanks

[melvin-bot]

@sonialiap, @allroundexperts Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

@sonialiap @allroundexperts this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

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

[melvin-bot]

@sonialiap, @allroundexperts Still overdue 6 days?! Let's take care of this!

[allroundexperts]

Thanks for the proposals everyone.

I don't see any huge difference in @FitseTLT's proposal, when compared to what @daledah proposed earlier. As such, I think we should go with @daledah's proposal.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖