Report fields - List value disappears after deleting offline

[izarutskaya]

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.6-0 Reproducible in staging?: Y Reproducible in production?: Y Found when validating PR : https://github.com/Expensify/App/pull/44730 Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

1. Navigate to staging.new.expensify.com
2. Create a workspace
3. Go to workspace editor > more features > enable report fields
4. Add a new List type report field
5. Add a list value
6. Go offline
7. Delete the list value

Expected Result:

While offline the deleted list value should be crossed

Actual Result:

The deleted list value disappears even if app is while offline

Workaround:

Unknown

Platforms:

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

• [x] Android: Native
• [x] Android: mWeb Chrome
• [ ] iOS: Native
• [x] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

https://github.com/Expensify/App/assets/115492554/140f2f47-78ca-443e-90a6-7609336fc9e2

View all open jobs on GitHub

[melvin-bot[bot]]

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

[izarutskaya]

We think this issue might be related to the #wave-control

[devguest07]

Proposal

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

Report fields - List value disappears after deleting offline

What is the root cause of that problem?

The condition to show the empty state exclude the fields where there's a pending action equal to delete. This means this reportField (and any others with pendingAction: "delete") would be filtered out. If all items in filteredPolicyFieldList have pendingAction: "delete", then after filtering, the resulting array would have a length of 0.

Therefore, shouldShowEmptyState would be true if:

1. All items in filteredPolicyFieldList have pendingAction: "delete", causing the filtered array to have a length of 0.
2. AND isLoading is false.

https://github.com/Expensify/App/blob/89782561069f121e6f64dc337dcf81da1f538b2d/src/pages/workspace/reportFields/WorkspaceReportFieldsPage.tsx#L130

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

If we want to show the empty state only when there are no fields at all, regardless of their pendingAction:

const shouldShowEmptyState = Object.values(filteredPolicyFieldList).length === 0 && !isLoading;

What alternative solutions did you explore?

We can consider using hasVisibleCategories as we do for the other lists. Then adjust the WorkspaceEmptyStateSection display condition

https://github.com/Expensify/App/blob/7bed11398c29e5fd88464b0c17fd52f2df9cd522/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx#L260

[mountiny]

@waterim @rezkiy37 who can take this one?

[sakluger]

Bump @waterim @rezkiy37

[sakluger]

Posted in Slack asking for a volunteer: https://expensify.slack.com/archives/C06ML6X0W9L/p1721356132987569?thread_ts=1721148004.979399&cid=C06ML6X0W9L

[waterim]

Hello, Im Artem from Callstack and would like to take this issue

[waterim]

This issue will be fixed in follow-up PR here

[waterim]

Update: This one will not be fixed in a mentioned PR, it will be fixed separately

[mountiny]

Thanks for the update

[sakluger]

@mountiny it seems like there's nobody ready to work on this quite yet. Do you think we should keep this as a daily or is it okay to switch to weekly?

[mountiny]

@waterim should be able to pick this up in the next days from what I heard but can make it weekly

[waterim]

Working on this one now, got some capacity

[melvin-bot[bot]]

@sakluger @rushatgabhane @mountiny @waterim @shubham1206agra this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[waterim]

After spending quite some time on this Im not sure how to fix it properly without any workarounds because we are storing values for List as an array, not as a collection and Im unable to add OfflineFeedback exclusively to the value of the list, only to the reportField itself(this doesn't help, cause it will cross out all values of course)

Thats why we didn't have delete pending action for the list from the beginning as data will be messed up Of course there is a way to make values as objects from frontend side and handle+convert data which comes from the backend to be the same

One more workaround which came to my mind is to store previous values and compare in offline mode these values with newest one from optimistic onyx data and cross out missing one, but thats looks like a super bad workaround for this kind of situation

@mountiny Please correct me if Im wrong And if you know a way how we can add an offline feedback to the string[] it will really help a lot

Will continue tomorrow, hopefully will find some good and clean solution to this

[mountiny]

I am not sure from top of my head, asked in slack here

[rezkiy37]

Hi, I’m Michael (Mykhailo) from Callstack and I would like to work on this issue.

[waterim]

@mountiny I had a talk with Michael, he will proceed with this issue as he has more context with this part as he coded it

[rezkiy37]

So the feature was implemented here - https://github.com/Expensify/App/pull/44730. There was a discussion regarding pending actions for list values.

We don't have many options since we are operating on the array. We can apply pendingAction for the entire report field.

Screenshot

I noted I will use only ADD and UPDATE. Otherwise, REMOVE marks all list values as deleted and crossed out.

The user should confirm removing in a modal to delete a list value. So this action is obvious. Right after that, the app removes selected list values and grays out other ones.

On the other hand, if the app does not remove them and grays out all list values, it is not understandable which ones were removed. So it confuses the user.

WDYT, @sakluger @mountiny @izarutskaya

[rezkiy37]

Just a friendly reminder - https://github.com/Expensify/App/issues/45166#issuecomment-2258524272.

cc @sakluger @mountiny @izarutskaya @shubham1206agra

[sakluger]

Discussing next steps in Slack: https://expensify.slack.com/archives/C06ML6X0W9L/p1722973693600849?thread_ts=1722924772.797079&cid=C06ML6X0W9L

[rezkiy37]

There was a discussion in Slack. In short, we decided to use the Offline Pattern A (optimistic without feedback).

I am working on the draft PR - https://github.com/Expensify/App/pull/46961.

[shubham1206agra]

@rezkiy37 How does this help us solve the given issue?

[rezkiy37]

It changes the offline behavior for the values list. We will use the Pattern A.

Used when…

• the user should be given instant feedback and
• the user does not need to know when the change is done on the server in the background How to implement: Use API.write() to implement this pattern. For this pattern, we should only put optimisticData in the options. We don't need successData or failureData as we don't care what response comes back at all.

So we are not expecting "While offline the deleted list value should be crossed" anymore.

The currently expected result is "While offline the deleted list value disappears immediately".

[rezkiy37]

I've opened the PR (https://github.com/Expensify/App/pull/46961) for review.

[shubham1206agra]

@rezkiy37 This approach will create a problem. When deleting a report field, it will show not found page for a brief moment.

[rezkiy37]

@shubham1206agra we are applying the new pattern for list values only, but not for a report field itself. Please correct me if I am wrong.

[shubham1206agra]

@rezkiy37 Oh, you are right.

[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.

[shubham1206agra]

False flag here ^.

[shubham1206agra]

@mountiny Please close this issue.