Skip to content

[HOLD for payment 2024-03-13] [$500] IOU - Destinations used for creating request in offline remains grayed out in online mode #36079

Closed

Description

@lanitochka17

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: 1.4.38-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers): applausetester+0507gm@applause.expensifail.com
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

  1. Navigate to staging.new.expensify.com as Employee
  2. Disable the internet connection
  3. Go to Workspace chat> +> Money request> Distance> Create request
  4. Go back online
  5. Go to Workspace chat> +> Money request> Distance
  6. Take a note of recent destination results

Expected Result:

All recent destination should not be grayed out

Actual Result:

Destinations used for creating request in offline remains grayed out in online 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
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6370914_1707338741603.Recording__2127.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01d576261e97da21a3
  • Upwork Job ID: 1755338770224349184
  • Last Price Increase: 2024-02-28
  • Automatic offers:
    • fedirjh | Reviewer | 0
    • barros001 | Contributor | 0

Activity

  1. added
    ExternalAdded to denote the issue can be worked on by a contributor
    BugSomething is broken. Auto assigns a BugZero manager.
    on Feb 7, 2024
  2. melvin-bot commented on Feb 7, 2024

    @melvin-bot
  3. changed the title [-] IOU - Destinations used for creating request in offline remains grayed out in online mode[/-] [+][$500] IOU - Destinations used for creating request in offline remains grayed out in online mode[/+] on Feb 7, 2024
  4. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Feb 7, 2024
  5. melvin-bot commented on Feb 7, 2024

    @melvin-bot

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

  6. melvin-bot commented on Feb 7, 2024

    @melvin-bot

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

  7. lanitochka17 commented on Feb 7, 2024

    @lanitochka17
    Author

    We think that this bug might be related to #wave5
    CC @dylanexpensify

  8. ZhenjaHorbach commented on Feb 7, 2024

    @ZhenjaHorbach
    Contributor

    Proposal

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

    IOU - Destinations used for creating request in offline remains grayed out in online mode

    What is the root cause of that problem?

    The main problem with issue is that destinations used for creating request in offline have only subtitle
    Which is always gray

    <View>
    {title && <Text style={[styles.googleSearchText]}>{title}</Text>}
    <Text style={[styles.textLabelSupporting]}>{subtitle}</Text>
    </View>

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

    I think the best way to fix this issue
    In case when we don't have a title we will use subtitle and vice versa

                                        {(title || subtitle) && <Text style={[styles.googleSearchText]}>{title || subtitle}</Text>}
                                        <Text style={[styles.textLabelSupporting]}>{subtitle || title}</Text>
    

    Or if we expect for destinations used for creating request in offline to have only one line
    So in this case we can use only title component in case when we have only one title or subtitle

                                if (title && subtitle) {
                                    return (
                                        <View>
                                            <Text style={[styles.googleSearchText]}>{title}</Text>
                                            <Text style={[styles.textLabelSupporting]}>{subtitle}</Text>
                                        </View>
                                    );
                                }
                                return (
                                    <View>
                                        <Text style={[styles.googleSearchText]}>{title || subtitle}</Text>
                                    </View>
                                );
    

    But this is more of a question for the designers

    What alternative solutions did you explore? (Optional)

    As alternative we can pass description for name when name is undefined to show always 2 strings when we select location (In offline mode name is undefined, and text from input is saved like description )

    onPress?.({
    address: autocompleteData.description ?? '',
    lat: details.geometry.location.lat ?? 0,
    lng: details.geometry.location.lng ?? 0,
    name: details.name,
    });

  9. barros001 commented on Feb 7, 2024

    @barros001
    Contributor

    Proposal

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

    When a Distance request is created while offline, the entered locations are saved and later displayed in the recent locations but they show "grayed out".

    What is the root cause of that problem?

    When the user searches for a waypoint in offline mode, the app will just take the input as is as it can't ask Google for a translation for that waypoint. Because of that the waypoint is saved with an address only, no name.

    if (isOffline && waypointValue) {
    const waypoint = {
    lat: null,
    lng: null,
    address: waypointValue,
    name: values.name || null,
    };
    saveWaypoint(waypoint);
    }

    Later on when the recent waypoint is displayed, we do the following:

    renderRow={(data) => {
    const title = data.isPredefinedPlace ? data.name : data.structured_formatting.main_text;
    const subtitle = data.isPredefinedPlace ? data.description : data.structured_formatting.secondary_text;
    return (
    <View>
    {title && <Text style={[styles.googleSearchText]}>{title}</Text>}
    <Text style={[styles.textLabelSupporting]}>{subtitle}</Text>
    </View>
    );
    }}

    This will render the bolded name first and then the description. Description is a copy of the address value as can be seen here:

    _.map(waypoints ? waypoints.slice(0, 5) : [], (waypoint) => ({
    name: waypoint.name,
    description: waypoint.address,
    geometry: {
    location: {
    lat: waypoint.lat,
    lng: waypoint.lng,
    },
    },
    })),

    Because some recent waypoints have been saved in offline mode, they do not have a name field so it will only show the description without the bolded name, giving the impression of being greyed out. This is not really grayed out in the sense that it's offline/not available, it's just that it doesn't display the bolded title because it doesn't have one.

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

    Assuming we want to display it bolded as the others, we update the following code:

    return (
    <View>
    {title && <Text style={[styles.googleSearchText]}>{title}</Text>}
    <Text style={[styles.textLabelSupporting]}>{subtitle}</Text>
    </View>
    );

    To be like:

        return (
            <View>
                {title && <Text style={[styles.googleSearchText]}>{title}</Text>}
                <Text style={[title ? styles.textLabelSupporting : styles.googleSearchText]}>{subtitle}</Text>
            </View>
        );

    This will display the description in bold if there is no title.

    What alternative solutions did you explore? (Optional)

    Another option is to simply do not list any "incomplete" (those created offline) waypoints on the recent waypoints list. As noted here, sometimes an waypoint created offline turns out to be invalid, and we likely don't way to show these in the list of recent waypoints.

    The way we identify these is by looking at lat and lng as these will be null for waypoints created offline (and they will remain null once back online if the waypoint is invalid). Here's where we would filter the waypoints:

    src/pages/iou/request/step/IOURequestStepWaypoint.js:

    waypoints.filter((w) => w.lat && w.lng) added to the _.map call.

            recentWaypoints: {
                key: ONYXKEYS.NVP_RECENT_WAYPOINTS,
    
                // Only grab the most recent 5 waypoints because that's all that is shown in the UI. This also puts them into the format of data
                // that the google autocomplete component expects for it's "predefined places" feature.
                selector: (waypoints) =>
                    _.map(waypoints ? waypoints.filter((w) => w.lat && w.lng).slice(0, 5) : [], (waypoint) => ({
                        name: waypoint.name,
                        description: waypoint.address,
                        geometry: {
                            location: {
                                lat: waypoint.lat,
                                lng: waypoint.lng,
                            },
                        },
                    })),
            },

    This would solve the issue of showing invalid waypoints in the recent list, but as noted here, some valid waypoints, created online, could still not have name, which would leave us with the original issue we're dealing with here ("grayed out" suggestion).

    The best solution here seems to be 1) filter invalid waypoints; 2) bold description when name is not available. This solves both issues. Or, if the team prefers, we hide anything without a name.

    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.

  10. fedirjh commented on Feb 12, 2024

    @fedirjh
    Contributor

    I don't think this is a bug. I think that current behaviour gives the user a hint that the selected address was saved in offline mode.

    @puneetlath What's your opinion on this? should we make any adjustments?

  11. 32 remaining items

  12. added
    Awaiting PaymentAuto-added when associated PR is deployed to production
    and removed on Mar 6, 2024
  13. changed the title [-][$500] IOU - Destinations used for creating request in offline remains grayed out in online mode[/-] [+][HOLD for payment 2024-03-13] [$500] IOU - Destinations used for creating request in offline remains grayed out in online mode[/+] on Mar 6, 2024
  14. melvin-bot commented on Mar 6, 2024

    @melvin-bot

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

  15. melvin-bot commented on Mar 6, 2024

    @melvin-bot

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

    If no regressions arise, payment will be issued on 2024-03-13. 🎊

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

  16. melvin-bot commented on Mar 6, 2024

    @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:

    • [@fedirjh] The PR that introduced the bug has been identified. Link to the PR:
    • [@fedirjh] 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:
    • [@fedirjh] 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:
    • [@fedirjh] Determine if we should create a regression test for this bug.
    • [@fedirjh] 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.
    • [@puneetlath] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:
  17. puneetlath commented on Mar 12, 2024

    @puneetlath
    Contributor

    @fedirjh friendly reminder about the checklist so that we can pay tomorrow.

  18. puneetlath commented on Mar 13, 2024

    @puneetlath
    Contributor

    @barros001 has been paid. @fedirjh still waiting on the checklist for you.

  19. fedirjh commented on Mar 13, 2024

    @fedirjh
    Contributor

    BugZero Checklist:

    • Link to the PR: This is not a regression.
    • Link to comment: N/A
    • Link to discussion: N/A
    • Determine if we should create a regression test for this bug: This is a minor UI improvement, I don't think we should have a regression test. In case we do, I would propose this test:

    Regression Test Proposal

    1. Open the app
    2. Disable the internet connection
    3. Click + > Request Money and select Distance
    4. Enter Start and End waypoints
    5. Click Next
    6. Select an Workspace
    7. Click Request
    8. Go back online
    9. Click + > Request Money and select Distance
    10. Click Start
    11. Verify that the previous used waypoints show in the Recent destinations in bold
    
    • Do we agree 👍 or 👎
  20. puneetlath commented on Mar 13, 2024

    @puneetlath
    Contributor

    Cool. All paid, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.DailyKSv2EngineeringExternalAdded to denote the issue can be worked on by a contributor

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions