[$1000] Request Money – The address input lacks a red outline or error message when an incomplete IOU is created offline #32405

kbecciv · 2023-12-02T15:35:31Z

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.7.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):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

Go to https://staging.new.expensify.com/
Log in
Global Create > Request Money
Select Distance option
Disable the internet connection
Enter a vague start location like "SF"
Enter a vague finish location like "LA"
Finish the request creation flow
Enable the internet connection
Verify there's a red dot on the request report in the LHN, on the report preview in the workspace chat, and on the request
Navigate to the request with the error
Click on the request
Click on the start waypoint

Expected Result:

The address input has a red outline and below it is an error message like “Unable to find the location for this address. Please edit the address and try again

Actual Result:

The address input lacks a red outline or error message when an incomplete IOU is created offline

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6298369_1701504006546.Address_input_has_no_red_outline.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01bc96448abc5e7e68
Upwork Job ID: 1730973988318359552
Last Price Increase: 2024-02-05
Automatic offers:

shubham1206agra | Contributor | 28064604

melvin-bot · 2023-12-02T15:35:35Z

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

melvin-bot · 2023-12-02T15:35:35Z

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

melvin-bot · 2023-12-02T15:35:37Z

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 · 2023-12-02T15:35:41Z

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

chiItepin · 2023-12-03T01:27:20Z

Proposal

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

The user can't tell if the address finder is offline due to a lack of a proper hint and inline validation, thus, causing the user to select an invalid address while being offline and being able to submit.

What is the root cause of that problem?

There is no proper validation to tackle the lack of network connection in WaypointEditor.js, the form's validation is currently not validating offline users.

const validate = (values) => {
...
        if (!isOffline && waypointValue !== '' && waypointAddress !== waypointValue) {
            ErrorUtils.addErrorMessage(errors, `waypoint${waypointIndex}`, 'distance.errors.selectSuggestedAddress');
        }

This condition is basically excluding the addition of the error message for users that are offline, quite an issue since it may introduce "invalid" addresses.

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

Ideally, a proper validation message should be shown to the user to let them know they are offline at the moment if they are offline and there are no autocomplete items as shown below:

to do this, the validate function would need an addition such as:

        if (isOffline && hasNoRecentWaypoints) {
            ErrorUtils.addErrorMessage(errors, `waypoint${waypointIndex}`, 'common.youAppearToBeOffline');
        }

Additionally, the !isOffline check from the following current condition should be removed such as:

      // before
        if (!isOffline && waypointValue !== '' && waypointAddress !== waypointValue) {
            ErrorUtils.addErrorMessage(errors, `waypoint${waypointIndex}`, 'distance.errors.selectSuggestedAddress');
        }
      // after
         if (waypointValue !== '' && waypointAddress !== waypointValue) {
            ErrorUtils.addErrorMessage(errors, `waypoint${waypointIndex}`, 'distance.errors.selectSuggestedAddress');
        }

Please select a suggested address or use current location

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.

tienifr · 2023-12-04T09:35:04Z

Proposal

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

Request Money – The address input lacks a red outline or error message when an incomplete IOU is created offline

What is the root cause of that problem?

We have 2 problems here:

Error message is not shown in form

To show the error messages, we have to call validate function, but this function is just called when users press Save button or blur the input

App/src/pages/iou/WaypointEditor.js

Lines 216 to 217 in d2056af

    
           validate={validate} 
        
           onSubmit={submit}

When users go offline, we allow them to write any addresses, so BE will validate them then return the correct latitude and longitude. If the addresses are invalid, BE will return the error -> validate function is not called -> there's no error is shown

red not is not shown in menu item

In

App/src/components/DistanceRequest/index.js

Lines 198 to 199 in d2056af

    
           renderItem={({item, drag, isActive, getIndex}) => ( 
        
               <DistanceRequestRenderItem

we did not pass brickRoadIndicator to DistanceRequestRenderItem and just show the error above Save button here

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

We need to define the new property in waypoint like isChecking to indicate that this waypoint is verified or not,

In saveWaypoint function, we will set isChecking to true if it's new waypoint without lat and lng

Onyx.merge(`${isDraft ? ONYXKEYS.COLLECTION.TRANSACTION : ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {
        comment: {
            waypoints: {
                [`waypoint${index}`]: {
                    ...waypoint,
                    isChecking: (_.isUndefined(waypoint?.isChecking) && !waypoint?.lat && !waypoint?.lng)? true : false
                },
            },
        },

then set isChecking to false for all waypoints in successData of GetRoute API (for now these waypoints are verifed)

In FormProvider.js we create the new props named validateOnMounted, and run it function right after component did mount

    useEffect(()=>{
        if(!validateOnMounted){
            return;
        }
        const errors = validateOnMounted()
        setErrors(errors)
    },[])

In WaypointEditor define validateOnMounted function, then pass it to FormProvider. We can early return if the waypoint.isChecking is true (wait for BE result), or waypointValue is empty (users don't set the waypoint). If the lat or lng is empty, that means BE can't verify that waypoint so we add the error message for it

    const validateOnMounted = ()=>{
        const errors = {}

        const waypointValue = transaction.comment.waypoints[`waypoint${waypointIndex}`];

        if(_.isEmpty(waypointValue) || waypointValue.isChecking || (waypointValue.lat && waypointValue.lng)){
            return errors;
        }
        ErrorUtils.addErrorMessage(errors, `waypoint${waypointIndex}`, 'distance.errors.selectSuggestedAddress');
        return errors
    }

...

<FormProvider
...
validateOnMounted={validateOnMounted}
..

We can check if the current waypoint has error in

App/src/components/DistanceRequest/index.js

Line 198 in d2056af

renderItem={({item, drag, isActive, getIndex}) => (

const waypointValue = transaction.comment?.waypoints?.[`waypoint${getIndex()}`];
const hasError = !waypointValue.isChecking && !_.isEmpty(waypointValue) && (!waypointValue.lat || !waypointValue.lng)
return ( <DistanceRequestRenderItem
...
brickRoadIndicator={hasError && 'error'}

Result

Screen.Recording.2023-12-21.at.11.40.36.mp4

What alternative solutions did you explore? (Optional)

conorpendergrast · 2023-12-05T16:27:54Z

Reproduced on iOS (SafarI). @ntdiary two proposals for your review above!

shubham1206agra · 2023-12-05T16:59:02Z

Taking over as per @ntdiary's request

chiItepin · 2023-12-05T17:13:54Z

I'd suggest avoid submitting these with the "help wanted" label then

ntdiary · 2023-12-05T17:26:59Z

I'd suggest avoid submitting these with the "help wanted" label then

@chiItepin, ah, is there any misunderstanding here? 😂 As my bandwidth is not sufficient, @shubham1206agra will take over as C+ to review all the proposals, to avoid delays in the review process. 😄

shubham1206agra · 2023-12-05T17:29:35Z

@conorpendergrast Do we also need to show a red brick indicator in the Distance field on the request details page?

situchan · 2023-12-05T17:31:42Z

@conorpendergrast Do we also need to show a red brick indicator in the Distance field on the request details page?

Being handled in #29783

shubham1206agra · 2023-12-05T17:49:32Z

@tienifr Can you point to the code where pendingFields was added?

situchan · 2023-12-05T17:52:47Z

TBH, I am not sure this is worth fixing after #29783 is done, as we will show red dot already on 2 pages

shubham1206agra · 2023-12-05T17:55:04Z

@chiItepin Your RCA is not right. We do not disable the request flow if the user is offline

shubham1206agra · 2023-12-05T17:56:32Z

TBH, I am not sure this is worth fixing after #29783 is done, as we will show red dot already on 2 pages

I think we should have some indicator of which address has a problem. So that we can fix the address.

chiItepin · 2023-12-05T17:56:58Z

@shubham1206agra I see, but how is the user able to look for an address if the user is offline and there are no autocomplete items (recently added addresses)?

shubham1206agra · 2023-12-05T17:58:10Z

@shubham1206agra I see, but how is the user able to look for an address if the user is offline and there are no autocomplete items (recently added addresses)?

By manually entering the address and proceed with that

chiItepin · 2023-12-05T17:59:46Z

Hmm, I thought the app needed to sanitize the address, without being online and without autocomplete items, the user could literally enter anything thus, the map is not recognizing the address if this is the case which is what the reporter described though, map not being picked up due to invalid address, no?

tienifr · 2023-12-07T02:52:14Z

@tienifr Can you point to the code where pendingFields was added?

@shubham1206agra We update pendingFields here when users change the waypoints

I also updated it in my proposal. Thanks

After users select the new address we update the pendingFields and call GetRoute API. In theory we have to set pendingFields to null in successData and failureData but we didn't do that

shubham1206agra · 2023-12-07T04:24:07Z

We clear pendingFields when editing distance requests, too.

App/src/libs/actions/IOU.js

Line 760 in 3827a9c

pendingFields: clearedPendingFields,

shubham1206agra · 2024-01-25T11:51:26Z

@conorpendergrast I am OOO right now. I will review this on 30th

conorpendergrast · 2024-01-25T13:56:53Z

Sounds good, thanks!

conorpendergrast · 2024-01-29T11:13:56Z

It's not the 30th yet! That's tomorrow

melvin-bot · 2024-01-29T16:00:43Z

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

conorpendergrast · 2024-01-29T17:02:02Z

Also double-checking which wave this should be part of

shubham1206agra · 2024-01-30T14:04:05Z

@conorpendergrast I was able to repro your steps behavior but not the original behavior. I think if the backend is giving an error code of failure while making a request, then it should not be allowed at all. Can you confirm the ideal offline behavior with vague waypoints internally? As BE may have removed support of the same.

conorpendergrast · 2024-01-31T15:09:08Z

@shubham1206agra can do, thanks for moving this along!

conorpendergrast · 2024-01-31T15:37:00Z

Checking via internal channel now

conorpendergrast · 2024-02-01T14:40:14Z

Ok, well, good news! Yes, it is expected that you can create a request like this. Quoting @neil-marcellini :

There’s no reasonable way to validate addresses while offline, but we want users to be able to enter addresses manually, or from their recent waypoints, and create distance requests while offline.

However, we don’t handle errors for this flow well. It involves a mini project, described by this issue. [DISTANCE] LOW: Handle errors with geocoding for offline distance requests. I’m seeking someone to take it over

So, we'll close this out in favour of #22719. That covers this scenario well!

Thank you for your time everyone!

shubham1206agra · 2024-02-02T03:39:09Z

@conorpendergrast Can I get partial compensation in this case? As I have tried to move this along for a long time by doing quite back and forth with a proposal.

conorpendergrast · 2024-02-02T09:38:31Z

@shubham1206agra I can put that forward! What do you feel is a fair partial compensation?

shubham1206agra · 2024-02-02T09:39:55Z

250 is fine for me.

conorpendergrast · 2024-02-05T10:10:35Z

Ok, let me reopen to discuss that internally!

conorpendergrast · 2024-02-05T10:17:53Z

Posted internally

melvin-bot · 2024-02-05T16:00:29Z

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

conorpendergrast · 2024-02-06T13:10:38Z

Still discussing internally

conorpendergrast · 2024-02-06T20:36:15Z

@shubham1206agra we've agreed with the 25% based on the work here. I've sent you the offer now

shubham1206agra · 2024-02-07T02:12:33Z

@conorpendergrast Accepted

conorpendergrast · 2024-02-07T09:54:17Z

And paid! Thanks @shubham1206agra

conorpendergrast · 2024-02-07T09:54:55Z

Payouts due:

Contributor+: $250 @shubham1206agra - Contract

Upwork job is here.

kbecciv added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 2, 2023

melvin-bot bot assigned conorpendergrast Dec 2, 2023

melvin-bot bot changed the title ~~Request Money – The address input lacks a red outline or error message when an incomplete IOU is created offline~~ [$500] Request Money – The address input lacks a red outline or error message when an incomplete IOU is created offline Dec 2, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 2, 2023

melvin-bot bot assigned ntdiary Dec 2, 2023

melvin-bot bot added the Overdue label Dec 5, 2023

melvin-bot bot removed the Overdue label Dec 5, 2023

melvin-bot bot added the Overdue label Jan 29, 2024

melvin-bot bot removed the Overdue label Jan 29, 2024

conorpendergrast added this to [#whatsnext] Wave 05 - Deprecate Free Jan 29, 2024

conorpendergrast moved this to Release 5: Best in Class in [#whatsnext] Wave 05 - Deprecate Free Jan 29, 2024

conorpendergrast closed this as completed Feb 1, 2024

github-project-automation bot moved this from Release 5: Best in Class to Done in [#whatsnext] Wave 05 - Deprecate Free Feb 1, 2024

shubham1206agra mentioned this issue Feb 5, 2024

Load map when image is not loaded in Money Request view for distance request #34133

Merged

49 tasks

conorpendergrast reopened this Feb 5, 2024

conorpendergrast closed this as completed Feb 7, 2024

Tony-MK mentioned this issue Feb 7, 2024

[$500] IOU - User have no access to Distance request chat after getting Online #36085

Closed

6 tasks

[$1000] Request Money – The address input lacks a red outline or error message when an incomplete IOU is created offline #32405

[$1000] Request Money – The address input lacks a red outline or error message when an incomplete IOU is created offline #32405

Comments

kbecciv commented Dec 2, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Dec 2, 2023

melvin-bot bot commented Dec 2, 2023

melvin-bot bot commented Dec 2, 2023 • edited by conorpendergrast Loading

Bug0 Triage Checklist (Main S/O)

melvin-bot bot commented Dec 2, 2023

chiItepin commented Dec 3, 2023 • edited Loading

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 alternative solutions did you explore? (Optional)

tienifr commented Dec 4, 2023 • edited Loading

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?

Result

What alternative solutions did you explore? (Optional)

conorpendergrast commented Dec 5, 2023

shubham1206agra commented Dec 5, 2023 • edited Loading

chiItepin commented Dec 5, 2023 • edited Loading

ntdiary commented Dec 5, 2023

shubham1206agra commented Dec 5, 2023

situchan commented Dec 5, 2023

shubham1206agra commented Dec 5, 2023

situchan commented Dec 5, 2023

shubham1206agra commented Dec 5, 2023

shubham1206agra commented Dec 5, 2023

chiItepin commented Dec 5, 2023

shubham1206agra commented Dec 5, 2023

chiItepin commented Dec 5, 2023

tienifr commented Dec 7, 2023 • edited Loading

shubham1206agra commented Dec 7, 2023

shubham1206agra commented Jan 25, 2024

conorpendergrast commented Jan 25, 2024

conorpendergrast commented Jan 29, 2024

melvin-bot bot commented Jan 29, 2024

conorpendergrast commented Jan 29, 2024

shubham1206agra commented Jan 30, 2024

conorpendergrast commented Jan 31, 2024

conorpendergrast commented Jan 31, 2024

conorpendergrast commented Feb 1, 2024

shubham1206agra commented Feb 2, 2024

conorpendergrast commented Feb 2, 2024

shubham1206agra commented Feb 2, 2024

conorpendergrast commented Feb 5, 2024

conorpendergrast commented Feb 5, 2024

melvin-bot bot commented Feb 5, 2024

conorpendergrast commented Feb 6, 2024

conorpendergrast commented Feb 6, 2024

shubham1206agra commented Feb 7, 2024

conorpendergrast commented Feb 7, 2024

conorpendergrast commented Feb 7, 2024

kbecciv commented Dec 2, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Dec 2, 2023 •

edited by conorpendergrast

Loading

chiItepin commented Dec 3, 2023 •

edited

Loading

tienifr commented Dec 4, 2023 •

edited

Loading

shubham1206agra commented Dec 5, 2023 •

edited

Loading

chiItepin commented Dec 5, 2023 •

edited

Loading

tienifr commented Dec 7, 2023 •

edited

Loading