[HOLD for payment 2024-07-22] [$500] Distance - Search list displays old searched results in offline mode

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.3.88-3
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Go to staging.new.expensify.com
2. Go offline
3. Click on FAB> Request money> Distance tab
4. Select Start point
5. Enter any letter into search field

Expected Result:

Search result should update according to entered letter

Actual Result:

Search list displays old searched results in offline mode

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• Windows: Chrome
• MacOS: Desktop

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

Windows: Chrome

Bug6245602_1697901225387.Recording__1286.mp4

MacOS: Desktop

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a86156cf627cbc84
• Upwork Job ID: 1715774289180717056
• Last Price Increase: 2024-06-28
• Automatic offers:
• cooldev900 | Contributor | 27794398

Issue Owner

Current Issue Owner: @eVoloshchak

[melvin-bot]

Triggered auto assignment to @mallenexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[cooldev900]

Proposal

from: @cooldev900

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

GooglePlacesAutocomplete shows old selected results in offline mode.

What is the root cause of that problem?

The route cause is that if the app turns to offline state, requestUrl.url becomes null and it couldn't send location filter request to google API.

App/src/components/AddressSearch/index.js

Line 400 in 6031f3e

url: props.network.isOffline ? null : ApiUtils.getCommandURL({command: 'Proxy_GooglePlaces&proxyUrl='}),

https://github.com/FaridSafi/react-native-google-places-autocomplete/blob/ef107387c6846924423b832a242eee83cb07eed4/GooglePlacesAutocomplete.js#L158

const [url, setUrl] = useState(getRequestUrl(props.requestUrl));

https://github.com/FaridSafi/react-native-google-places-autocomplete/blob/ef107387c6846924423b832a242eee83cb07eed4/GooglePlacesAutocomplete.js#L508-L511

 const _request = (text) => {
    _abortRequests();
    if (!url) {
      return;

https://github.com/FaridSafi/react-native-google-places-autocomplete/blob/ef107387c6846924423b832a242eee83cb07eed4/GooglePlacesAutocomplete.js#L88-L125

const [tempSearchedValue, setTempsearchedValue] = useState([])

  const buildRowsFromResults = useCallback(
    (results, text) => {
      let res = [];
      const shouldDisplayPredefinedPlaces = text
        ? results.length === 0 && text.length === 0
        : results.length === 0;
      if (
        shouldDisplayPredefinedPlaces ||
        props.predefinedPlacesAlwaysVisible === true
      ) {
        res = [
          ...props.predefinedPlaces.filter(
            (place) => place?.description.length,
          ),
        ];

        if (props.currentLocation === true && hasNavigator()) {
          res.unshift({
            description: props.currentLocationLabel,
            isCurrentLocation: true,
          });
        }
      }

      res = res.map((place) => ({
        ...place,
        isPredefinedPlace: true,
      }));

      return [...res, ...results];
    },
    [
      props.currentLocation,
      props.currentLocationLabel,
      props.predefinedPlaces,
      props.predefinedPlacesAlwaysVisible,
    ],
  );

And then even if we change the input value, the request won't will be run in request function so dataSource state value doesn't change.
As a result, GooglePlaceAutocomplete compoment shows old search result.

And if we don't search anything and it turns to offline, the predefined places shows in GooglePlaceAutocomplete.

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

Regarding discussion, we can filter recent destination by user's input in offline and send it as a props to GooglePlacesAutocomplete.

App/src/components/AddressSearch/index.js

Line 345 in 6031f3e

return (

const filteredPredefinedPlaces = useMemo(() => {
        console.log({predefinedPlaces: props.predefinedPlaces})
        if (!props.network.isOffline || !searchValue) return props.predefinedPlaces.length < 5 ? props.predefinedPlaces : props.predefinedPlaces.slice(0, 5);
        const filterRecentPlaces = props.predefinedPlaces.filter(value => !value?.description ? false :searchValue.split(" ").some((searchTerm) => value?.description.toLocaleLowerCase.includes(searchTerm.toLocaleLowerCase())))
        return filterRecentPlaces.length < 5 ? filterRecentPlaces : filterRecentPlaces.slice(0, 5);
    }, [
        props.network.isOffline, props.predefinedPlaces, searchValue
    ])

    return (
        /*
         * The GooglePlacesAutocomplete component uses a VirtualizedList internally,
         * and VirtualizedLists cannot be directly nested within other VirtualizedLists of the same orientation.
         * To work around this, we wrap the GooglePlacesAutocomplete component with a horizontal ScrollView
         * that has scrolling disabled and would otherwise not be needed
         */
        <>
            <ScrollView
                horizontal
                contentContainerStyle={styles.flex1}
                scrollEnabled={false}
                // keyboardShouldPersistTaps="always" is required for Android native,
                // otherwise tapping on a result doesn't do anything. More information
                // here: https://github.com/FaridSafi/react-native-google-places-autocomplete#use-inside-a-scrollview-or-flatlist
                keyboardShouldPersistTaps="always"
            >
                <View
                    style={styles.w100}
                    ref={containerRef}
                >
                    <GooglePlacesAutocomplete
                        disableScroll
                        fetchDetails
                        suppressDefaultStyles
                        enablePoweredByContainer={false}
                        predefinedPlaces={filteredPredefinedPlaces}      

App/src/pages/iou/WaypointEditor.js

Line 266 in 6031f3e

_.map(waypoints ? waypoints.slice(0, 5) : [], (waypoint) => ({

_.map(waypoints ? waypoints.slice(0, 20) : [], (waypoint) => ({

App/src/libs/actions/Transaction.ts

Line 100 in 6031f3e

Onyx.merge(ONYXKEYS.NVP_RECENT_WAYPOINTS, clonedWaypoints.slice(0, 5));

Onyx.merge(ONYXKEYS.NVP_RECENT_WAYPOINTS, clonedWaypoints.slice(0, 20));

What alternative solutions did you explore? (Optional)

N/A

Result

screen-capture.2.webm

[shubham1206agra]

@cooldev900 This has not been merged yet
#29045

Can you send the video with your solution?

[shubham1206agra]

If this is an upstream bug, @FaridSafi might help you with this

[cooldev900]

I will upload the video.

[cooldev900]

ice_video_20231022-084232.webm

const filteredPredefinedPlaces = useMemo(() => {
        if (!props.network.isOffline || !searchValue) return props.predefinedPlaces
        return props.predefinedPlaces.filter(value => value?.description.toLocaleLowerCase().includes(searchValue.toLocaleLowerCase()))
    }, [
        props.network.isOffline, props.predefinedPlaces, searchValue
    ])

    return (
        /*
         * The GooglePlacesAutocomplete component uses a VirtualizedList internally,
         * and VirtualizedLists cannot be directly nested within other VirtualizedLists of the same orientation.
         * To work around this, we wrap the GooglePlacesAutocomplete component with a horizontal ScrollView
         * that has scrolling disabled and would otherwise not be needed
         */
        <>
            <ScrollView
                horizontal
                contentContainerStyle={styles.flex1}
                scrollEnabled={false}
                // keyboardShouldPersistTaps="always" is required for Android native,
                // otherwise tapping on a result doesn't do anything. More information
                // here: https://github.com/FaridSafi/react-native-google-places-autocomplete#use-inside-a-scrollview-or-flatlist
                keyboardShouldPersistTaps="always"
            >
                <View
                    style={styles.w100}
                    ref={containerRef}
                >
                    <GooglePlacesAutocomplete
                        disableScroll
                        fetchDetails
                        suppressDefaultStyles
                        enablePoweredByContainer={false}
                        predefinedPlaces={filteredPredefinedPlaces}

@shubham1206agra @FaridSafi
As a alternative, I showed only how to search predefined places in SearchAddress.js component in offline mode.
We can fix it successfully with @FaridSafi by modifying react-native-google-places-autocomplete.

[melvin-bot]

@eVoloshchak, @mallenexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[mallenexpensify]

@eVoloshchak , can you please review the above proposal?

[melvin-bot]

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

[eVoloshchak]

@cooldev900, thank you for the proposal!

There are two ways we could go with this:

1. Implement the approach @cooldev900 is proposing, which is to save the search results we got when online and use them while offline
2. Disable the ability to create a Distance money request while offline

The problem with the first approach is that we'll be saving only a fraction of places (only those that the user has searched for earlier), so while offline this won't be very useful. Additionally, there will be no clear indication that the search is "restricted" while offline, it will essentially half-work, which is worse than not working at all imo

@mallenexpensify, what do you think?

[mallenexpensify]

I'm unsure, can you create a post in #expensify-open-source (since #expensify-bugs is being deprecated for External) and tag me in it? I'll cross-post in the internal distance room. Thx

[melvin-bot]

@eVoloshchak, @mallenexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[mallenexpensify]

@eVoloshchak can you create the #expensify-open-source post then drop the link in here? Thx

[melvin-bot]

@eVoloshchak @mallenexpensify 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]

Upwork job price has been updated to $500

[melvin-bot]

@eVoloshchak, @mallenexpensify, @neil-marcellini, @pac-guerreiro, @dubielzyk-expensify Huh... This is 4 days overdue. Who can take care of this?

[neil-marcellini]

@mallenexpensify the PR was merged so this is awaiting payment now. Looks like the automation failed, would you please handle it manually?

[mallenexpensify]

Contributor+: @eVoloshchak due $500 via NewDot

With many changes for offline distance bugs, do we want a new test case for this? I'm unsure

cc @paultsimura since it's an offline distance issue.

[melvin-bot]

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

• Add offline search functionality for addresses #35045
• Reapply - Add offline search functionality for addresses #43011

If no regressions arise, payment will be issued on 2024-07-22. 🎊

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

• @eVoloshchak requires payment through NewDot Manual Requests
• @pac-guerreiro does not require payment (Contractor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@eVoloshchak] The PR that introduced the bug has been identified. Link to the PR:
• [@eVoloshchak] 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:
• [@eVoloshchak] A discussion in #expensify-bugs 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:
• [@eVoloshchak] Determine if we should create a regression test for this bug.
• [@eVoloshchak] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@mallenexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

Payment Summary

Upwork Job

• Reviewer: @eVoloshchak owed $250 via NewDot
• Contributor: @pac-guerreiro is from an agency-contributor and not due payment

BugZero Checklist (@mallenexpensify)

• 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/1715774289180717056/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[melvin-bot]

@eVoloshchak, @mallenexpensify, @neil-marcellini, @pac-guerreiro, @dubielzyk-expensify Eep! 4 days overdue now. Issues have feelings too...

[mallenexpensify]

@eVoloshchak plz complete the BZ checklist above.

[eVoloshchak]

• The PR that introduced the bug has been identified. Link to the PR: N/A, this functionality wasn't implemented initially
• 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: N/A
• A discussion in #expensify-bugs 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: this was already discussed in https://expensify.slack.com/archives/C01GTK53T8Q/p1716990791744559

Regression Test Proposal

1. Click on FAB> Submit Expense > Distance tab
2. Select different locations several times to populate the predefined places list
3. Turn off the internet connection
4. Select Start point
5. Start typing into the search field, verify the list is updated according to the search query

Do we agree 👍 or 👎

[mallenexpensify]

Contributor+: @eVoloshchak due $500 via NewDot

Test case

• https://github.com/Expensify/Expensify/issues/415742

Thanks all!

[JmillsExpensify]

$500 approved for @eVoloshchak