Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.56k stars 2.9k forks source link

[$250] Expense-Creating expense with specific merchant shows unexpected error #49079

Closed izarutskaya closed 1 month ago

izarutskaya commented 2 months ago

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.33-1 Reproducible in staging?: Y Reproducible in production?: Y Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

  1. Launch app
  2. Tap on workspace chat
  3. Tap plus icon -- submit expense
  4. Enter an amount and tap next
  5. Paste the text in merchant field - उद्यद्भानु-सहस्र-कोटि-सदृशां केयूर-हारोज्ज्वलां विम्बोष्ठीं स्मित-दन्तपङ्क्ति-रुचिरां पीताम्बरा-लङ्कृताम् । विष्णुब्रह्म-सुरेन्द्र-सेवितपदां तत्त्वस्वरूपां शिवां मीनाक्षीं प्रणतोऽस्मि सन्ततमहं कारुण्यवारांनिधिम् ॥१॥

मुक्ताहार-लसत्किरीट-रुचिरां पूर्णेन्दु-वक्त्रप्रभां शिञ्जन्नूपुर-किङ्किणी-मणिधरां पद्म-प्रभा-भासुराम् । सर्वाभीष्टफलप्रदां गिरिसुतां वाणी-रमा-सेवितां । मीनाक्षीं प्रणतोऽस्मि सन्ततमहं कारुण्यवारांनिधिम् ॥२॥

श्रीविद्यां शिव-वाम-भाग-निलयां ह्रीङ्कार-मन्त्रोज्ज्वलां श्रीचक्राङ्कित-बिन्दुमध्य-वसतिं श्रीमत्सभा-नायिकाम् । श्रीमत्षण्मुख-विघ्न-राजजननीं श्रीमज्जगन्मोहिनीं । मीनाक्षीं प्रणतोऽस्मि सन्ततमहं कारुण्यवारांनिधिम् ॥३॥

श्रीमत्सुन्दर-नायकिम् भयहरां ज्ञानप्रदां निर्मलां श्यामाभां कमला-सनार् चितपदां नारा-यणस्या-नुजाम् । वीणावेणु-मृदङ्गवाद्य-रसिकां नाना-विधा-मम्बिकां

  1. Save it
  2. Tap submit
  3. Note unexpected error is displayed
  4. Close the error
  5. Create a new expense with merchant "ye"
  6. Open the expense created
  7. Replace the "ye" with step 5 mentioned text and save it
  8. Note now expense is created

Expected Result:

Creating expense with specific merchant must not show error.

Actual Result:

Creating expense with specific merchant shows unexpected error. But after creating expense, if we save that specific merchant no error is shown.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

https://github.com/user-attachments/assets/94ea63cb-eac9-4bdc-b428-68d6f7ed0100

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021834373840732425560
  • Upwork Job ID: 1834373840732425560
  • Last Price Increase: 2024-09-19
melvin-bot[bot] commented 2 months ago

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.

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

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

klajdipaja commented 2 months ago

Edited by proposal-police: This proposal was edited at 2024-09-13 10:07:28 UTC.

Proposal

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

When using really long texts in Arabic, Hebrew, Cyrillic, or any of the East Asian languages in the merchant field the report has an unexpected error and actually it is not created at all. The backend returns a bad request error: 402 Maximum Size Exceeded merchant

What is the root cause of that problem?

The root cause of this issue is the max length we have set on the input field for the merchant in CONST.MERCHANT_NAME_MAX_LENGTH which does not allow for a text of longer than 255 characters to be inserted. My assumption is that the backend has reported a 255 bytes limit on the field which for languages with special characters does not mean the same numbers of characters. Some language like Sanskrit (which I think the text in the issue is in) take up to 3 bytes per characters.

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

We should set the merchant field to a third of the actual limit that we have from the database if we want to support this language. I tried with 75 characters from that text and did not get an error.

CONST.MERCHANT_NAME_MAX_LENGTH=125 I assume the merchant field would not require longer texts than that, otherwise it should have been a textarea not a simple input.

If we don't want to limit the size for all the languages we can add a validation to the input field before sending it to the backend, something like this:

const textEncoder = new TextEncoder();

const lengthInBytes=textEncoder.encode(inputValue);
if(lengthInBytes>CONST.MERCHANT_NAME_MAX_LENGTH){
//show user error
}

Which would add more flexibility to the input, if the user types in latin letter they would have 255 chars but if in Sanskrit only 70, but at the same time it would add confusion for the end-user if they type different languages.

What alternative solutions did you explore? (Optional)

The backend can change the definition of the field on their end and increase it to 750 or what they see fit.

klajdipaja commented 2 months ago

Proposal

Updated

melvin-bot[bot] commented 2 months ago

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

sakluger commented 2 months ago

Next step is for @eVoloshchak to review the latest proposal.

melvin-bot[bot] commented 2 months ago

@sakluger, @eVoloshchak Eep! 4 days overdue now. Issues have feelings too...

eVoloshchak commented 2 months ago

The root cause for this issue is effectively the same as for https://github.com/Expensify/App/issues/17619, where we decided to make changes on backend so the backend would calculate the true number of characters I think we should do the same here (which would mean this has to be internal) @sakluger, what do you think?

melvin-bot[bot] commented 2 months ago

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

sakluger commented 2 months ago

Ah, I think I understand. The backend merchant field limit is based on bytes rather than characters, and the characters in some languages take up more memory than others. I agree that we should handle this in the BE by changing the limit to measure the number of characters rather than bytes.

sakluger commented 2 months ago

I'll make this a first pick since it seems like a fairly simple change.

In https://github.com/Expensify/Web-Expensify/pull/37253, we started checking the lengths of input strings in characters rather than bytes for "Personal Details" fields. For this issue, we need to do the same for the merchant field.

sakluger commented 1 month ago

@tylerkaraszewski is this something you would want to help with since you created the similar PR for https://github.com/Expensify/Web-Expensify/pull/37253?

sakluger commented 1 month ago

Okay time to recruit. I'll post in Slack.

melvin-bot[bot] commented 1 month ago

@sakluger @eVoloshchak 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!

sakluger commented 1 month ago

Actually, I'm going to demote the priority of this one to weekly since it's a bit of an edge case issue.

sakluger commented 1 month ago

Adding to wave-collect project since it has to do with Workspace chats.

trjExpensify commented 1 month ago

Is it actually worth it? Can someone tell me if there is a legit merchant name (i.e the name of a business you get a receipt from) that is ever going to be as long as the below?

उद्यद्भानु-सहस्र-कोटि-सदृशां केयूर-हारोज्ज्वलां
विम्बोष्ठीं स्मित-दन्तपङ्क्ति-रुचिरां पीताम्बरा-लङ्कृताम् ।
विष्णुब्रह्म-सुरेन्द्र-सेवितपदां तत्त्वस्वरूपां शिवां
मीनाक्षीं प्रणतोऽस्मि सन्ततमहं कारुण्यवारांनिधिम् ॥१॥
मुक्ताहार-लसत्किरीट-रुचिरां पूर्णेन्दु-वक्त्रप्रभां
शिञ्जन्नूपुर-किङ्किणी-मणिधरां पद्म-प्रभा-भासुराम् ।
सर्वाभीष्टफलप्रदां गिरिसुतां वाणी-रमा-सेवितां ।
मीनाक्षीं प्रणतोऽस्मि सन्ततमहं कारुण्यवारांनिधिम् ॥२॥

श्रीविद्यां शिव-वाम-भाग-निलयां ह्रीङ्कार-मन्त्रोज्ज्वलां
श्रीचक्राङ्कित-बिन्दुमध्य-वसतिं श्रीमत्सभा-नायिकाम् ।
श्रीमत्षण्मुख-विघ्न-राजजननीं श्रीमज्जगन्मोहिनीं ।
मीनाक्षीं प्रणतोऽस्मि सन्ततमहं कारुण्यवारांनिधिम् ॥३॥

श्रीमत्सुन्दर-नायकिम् भयहरां ज्ञानप्रदां निर्मलां
श्यामाभां कमला-सनार् चितपदां नारा-यणस्या-नुजाम् ।
वीणावेणु-मृदङ्गवाद्य-रसिकां नाना-विधा-मम्बिकां
tylerkaraszewski commented 1 month ago

Sorry, I just got to this now. I can help if you need me, but I doubt that's required.

trjExpensify commented 1 month ago

Is it actually worth it? Can someone tell me if there is a legit merchant name (i.e the name of a business you get a receipt from) that is ever going to be as long as the below?

Didn't hear back on this, so closing. It can be reopened if a merchant name is conceivably that long.