[$250] Wrong validation message when sending invoice

[m-natarajan]

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.35-7 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: @dubielzyk-expensify Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1726459401827979

Action Performed:

1. Click green (+) button
2. Send invoice
3. Enter $200
4. Choose recipient
5. Fill in company name (Dubby Co)
6. Fill in company website (www.dubbyco.com)

   Expected Result:

   Expected www.dubbyco.com to be a valid website address

   Actual Result:

   Gives error saying "Please enter a valid website using lowercase characters"

   Workaround:

   Unknown

   Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/0767a4d7-0299-4c3a-a5e9-ef6ad18e1e82

View all open jobs on GitHub

https://github.com/user-attachments/assets/4344065d-2b4c-4a91-9631-8b74a128f25a

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021835835067982464212
• Upwork Job ID: 1835835067982464212
• Last Price Increase: 2024-09-24

Issue Owner

Current Issue Owner: @ahmedGaber93

[melvin-bot[bot]]

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

[nkdengineer]

Edited by proposal-police: This proposal was edited at 2024-09-19 08:21:17 UTC.

Proposal

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

Wrong validation message when sending invoice

What is the root cause of that problem?

Here, we are considering that a valid website must include http/https/ftp in the URL, so a website like www.example.com would be an invalid website.

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

We should add the https:// prefix by using the Str.sanitizeURL function when validating the website and submitting the form here and here

      if (!ValidationUtils.isValidWebsite(Str.sanitizeURL(values.companyWebsite))) {
          errors.companyWebsite = translate('bankAccount.error.website');
      }

https://github.com/Expensify/App/blob/37bf6b2fef128786dfcd83257a56f56c0cd1eb3d/src/pages/iou/request/step/IOURequestStepCompanyInfo.tsx#L52

      IOU.sendInvoice(currentUserPersonalDetails.accountID, transaction, report, undefined, policy, policyTags, policyCategories, values.companyName, Str.sanitizeURL(values.companyWebsite));

https://github.com/Expensify/App/blob/37bf6b2fef128786dfcd83257a56f56c0cd1eb3d/src/pages/iou/request/step/IOURequestStepCompanyInfo.tsx#L72

OPTIONAL: We should also do the same in several places within the app that use the isValidWebsite function like here and here

What alternative solutions did you explore? (Optional)

[github-actions[bot]]

Jon Øvrebø Dubielzyk Your proposal will be dismissed because you did not follow the proposal template.

[Krishna2323]

Edited by proposal-police: This proposal was edited at 2024-09-19 15:55:37 UTC.

Proposal


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

Wrong validation message for website when sending invoice.

What is the root cause of that problem?

We are using isValidWebsite for validation which requires http/https/ftp URL scheme.

https://github.com/Expensify/App/blob/6c8211166e3b570a8947f6ff07a35973ecbcffb8/src/libs/ValidationUtils.ts#L236-L243 https://github.com/Expensify/App/blob/12779f1f3cc940b94cd0fd6de53775265217e6cd/src/pages/iou/request/step/IOURequestStepCompanyInfo.tsx#L52

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


• If we want a default value, we can get it using getDefaultCompanyWebsite and pass it to defaultValue prop.

What alternative solutions did you explore? (Optional)

• We can update yourCompanyWebsiteNote to also include the website example https://www.expensify.com.
• We can also update error.website to also include the website example https://www.expensify.com.
• We can use prefixCharacter prop to add https:// prefix.

What alternative solutions did you explore? (Optional 2)

• We should create a new util function similar to isValidWebsite but in that function we will use Url.URL_REGEX.
• Then we can use the new util function in IOURequestStepCompanyInfo and also in WorkspaceInvoicingDetailsWebsite if required.

  Result

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[bikramiter]

https://github.com/Expensify/App/blob/3c6526d79ddefc71b18c07139ce02f02d3543b8d/src/libs/ValidationUtils.ts#L236-L243

Note: As mentioned in the comment, this might need the corresponding scheme handling on the backend as well.

[ahmedGaber93]

this might need the corresponding scheme handling on the backend as well.

I think it should be done in FE side, not BE. by adding the URL scheme if not found before sending to BE

[ahmedGaber93]

Based on the issue Slack conversation, We have three possible options to fix this issue:

1. allow user to type URL without https:// and add it in behalf of the user before sending to BE
2. keep the current validation and just displaying the correct error message e.g. url must start with https://
3. add a default value https:// prefix on the input like this

Just to confirm, the expected result is option one? @kadiealexander @dubielzyk-expensify

[dubielzyk-expensify]

My personal take is 1 and 3 should be done here, but I dunno the technical or security implications of 1. I'd at least say 3 should be done.

Keen to hear what @trjExpensify @JmillsExpensify @Expensify/design thinks

[shawnborton]

Yeah that makes sense to me. Or even just do 2 and 3 might be easy without needing back end changes?

[dubielzyk-expensify]

Yeah that also works. Feels a bit inelegant that we don't validate it properly though, but perhaps it's the right priority at this time 👍

[ahmedGaber93]

I think We can use option one without any backend change, we will allow the user to type www.dubbyco.com and send the complete format https://www.dubbyco.com to the backend.

We already have a function for adding the missing https:// protocol in frontend sanitizeURL

[shawnborton]

Oh nice, that's great then!

[nkdengineer]

updated proposal following this comment here

[Krishna2323]

@ahmedGaber93, I believe we used prefix in the app somewhere in the past, maybe on the business details page, and adding it also helps the user understand that only https:// is accepted. It provides a visual reminder that only https:// is allowed, offering instant feedback while maintaining control over the protocol.

WDYT @shawnborton @dubielzyk-expensify?

[dubielzyk-expensify]

Sure. Let's go with option 1 and 3 then unless @shawnborton thinks otherwise. If it's all front-end stuff then chuck the prefix in and validate it to be a URL 👍

[ahmedGaber93]

@dubielzyk-expensify the option 3 is already used on company website on bank account steps, but it has two cases

A. if user email is not on private domain, the default value will be prefix https://
B. if user email is on private domain, the default value will be generated website url https://www.privatedomain.com

Should we use the same UX behavior here for consistency or just use case A? And if we will use the both cases, should we also add default value for company name like case B (if user email is on private domain, company name default value will be privatedomain)?

[shawnborton]

I'm down with a combo of 1 and 3. As for the latest question... I feel like the default value of https:// would be enough?

[ahmedGaber93]

Thanks, that's great.

[trjExpensify]

I'm not sure I'm following the 1 and 3 combo really. If we do 3, https:// is prefixed in the field and can't be removed, so wouldn't doing 3 only be enough?

If @ahmedGaber93 is saying in the bank account flow we populate your private domain email address into the field already, that kinda' makes sense to copy/paste over here in the invoice flow.

[Krishna2323]

Proposal Updated

• Updated main solution

[ahmedGaber93]

If we do 3, https:// is prefixed in the field and can't be removed, so wouldn't doing 3 only be enough?

@trjExpensify https:// can be removed by the user. It's a default value for the input. and this is the way we used in bank account flow

[Krishna2323]

@ahmedGaber93 https:// can't be removed if we use prefixCharacter prop.

[trjExpensify]

@trjExpensify https:// can be removed by the user. It's a default value for the input. and this is the way we used in bank account flow

Oh right, that seems silly doesn't it if we require it?

[ahmedGaber93]

Yeah, I am not sure if it required or not. But maybe we need to allow the user to remove it and type other scheme like http://

[ahmedGaber93]

@Krishna2323 Yes, prefixCharacter="https://" also can be another possible option

[dubielzyk-expensify]

I'm okay with only doing 3 👍

[shawnborton]

Works for me too!

[ahmedGaber93]

Ok, but the https:// prefix will be removable or not? It is removable in current bank account flow

[dubielzyk-expensify]

I think we should still technically make it removable if we also allow http://. Will also make it easier to copy a link if users prefer to do that.

[ahmedGaber93]

Ok, I will summarize the expected behavior from discussion to be clear for contributors

1. Add a removable https:// prefix link
2. If user email is on a private domain, populate a default value from this domain link paragraph 2

This behavior is exactly like the bank account flow.

[joekaufmanexpensify]

Noting I came across this same issue and reported it here. But we're not proceeding as I see we're fixing it here!

[ahmedGaber93]

@Krishna2323 Your proposal looks to have what we need to move forward, but can you update your proposal to clean the unnecessary points and make it match this expected behavior https://github.com/Expensify/App/issues/49286#issuecomment-2367859204

[Krishna2323]

@ahmedGaber93, main solution updated.

[melvin-bot[bot]]

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

[ahmedGaber93]

@Krishna2323 's proposal LGTM!

🎀 👀 🎀 C+ reviewed.

[melvin-bot[bot]]

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

[marcaaron]

allow user to type URL without https:// and add it in behalf of the user before sending to BE

How would we know that the website uses https?

I'm not really following why we require https:// at all? It doesn't seem like it would be necessary.

[ahmedGaber93]

allow user to type URL without https:// and add it in behalf of the user before sending to BE How would we know that the website uses https?

@marcaaron This was one of the possible options, but we didn't decide to use it. We have chosen the option 3 "add a default value https:// prefix on the input like this"

This default value https:// is removable, so user able to remove it and type http://. We already have the same behavior on bank account flow.