[HOLD for payment 2024-10-25] [$250] Chat - User is able to send message with max character limit

[IuliiaHerets]

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: 9.0.31-3
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4943661
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

1. Navigate to any 1:1 conversation
2. Enter a text that is over 10k characters but less than 15K characters - You can use Lorem Ipsum generator for this
3. After the compose box displays an error that the maximum amount of characters is 10k, add attachment
4. Send the message with the attachment

Expected Result:

User should not be able to send messages over 10k characters

Actual Result:

User is able to send messages over 10k characters if there is added attachment.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6597905_1725908762232.Screen_Recording_20240909_214223_Chrome.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021838751947628833125
• Upwork Job ID: 1838751947628833125
• Last Price Increase: 2024-10-02
• Automatic offers:
• nyomanjyotisa | Contributor | 104327662

Issue Owner

Current Issue Owner: @anmurali

[melvin-bot]

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

[IuliiaHerets]

@anmurali I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[nyomanjyotisa]

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

Proposal

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

Chat - User is able to send message with max character limit

What is the root cause of that problem?

We don't disable the Send button on Send Attachment modal if the message exceed the max char on the composer

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

Add new props here called shouldDisableSendButton default to false

and disabled the send button if it true here

isDisabled={isConfirmButtonDisabled || shouldDisableSendButton}

Then here pass the hasExceededMaxCommentLength as shouldDisableSendButton

<AttachmentModal
    headerTitle={translate('reportActionCompose.sendAttachment')}
    onConfirm={addAttachment}
    onModalShow={() => setIsAttachmentPreviewActive(true)}
    onModalHide={onAttachmentPreviewClose}
    shouldDisableSendButton={hasExceededMaxCommentLength}
>

Or we can assign the shouldDisableSendButton as default value here

const [isConfirmButtonDisabled, setIsConfirmButtonDisabled] = useState(shouldDisableSendButton);

and don't change this

Result

What alternative solutions did you explore? (Optional)

We can disable the attachment item on the popovermenu if hasExceededMaxCommentLength

Add new props here shouldDisableAttachmentItem default to false

Add disabled using the shouldDisableAttachmentItem value here

{
    icon: Expensicons.Paperclip,
    text: translate('reportActionCompose.addAttachment'),
    onSelected: () => {
        if (Browser.isSafari()) {
            return;
        }
        triggerAttachmentPicker();
    },
    shouldCallAfterModalHide: true,
    disabled: shouldDisableAttachmentItem,
},

Then here pass the hasExceededMaxCommentLength as shouldDisableAttachmentItem

<AttachmentPickerWithMenuItems
    ...
    shouldDisableAttachmentItem={hasExceededMaxCommentLength}
/>

Result

[dominictb]

Proposal

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

• User is able to send messages over 10k characters if there is added attachment.

What is the root cause of that problem?

• In here, we don't have a logic to throw an error modal if the comment length is greater than 10K characters.

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

• We should pass down the hasExceededMaxCommentLength from here to here.

• Then, in AttachmentModal, use the hasExceededMaxCommentLength in here:

        if (hasExceededMaxCommentLength) {
            setIsModalOpen(false);
            setIsAttachmentInvalid(true);
            setAttachmentInvalidReasonTitle('attachmentPicker.maxLengthExceed');
            setAttachmentInvalidReason('attachmentPicker.maxLengthExceedSubtitle');
            return false;
        }

• With the above change, when the comment length is greater than 10K and we press on "Send" button in attachment modal, the error modal will be shown.

• Optional, we can display the danger modal instead of success modal by adding prop danger here if hasExceededMaxCommentLength is true.

What alternative solutions did you explore? (Optional)

Result

Screen.Recording.2024-09-11.at.19.09.51.mov

[melvin-bot]

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

[melvin-bot]

@anmurali 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[melvin-bot]

@anmurali 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

[melvin-bot]

@anmurali 12 days overdue. Walking. Toward. The. Light...

[melvin-bot]

@anmurali 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!

[melvin-bot]

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

[melvin-bot]

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

[mananjadhav]

Proposal

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

• We can send a message > 10,000 characters if sent via the attachment.

What is the root cause of that problem?

• There is no validation for onConfirm call in the AttachmentModal that verifies the character length.

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

• I think we should also solve this from the backend if we do want to enforce the character limit.
• Second we need to pass hasExceededMaxCommentLength to AttachmentModal as a new prop isSendDisabled

App/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx

Line 182 in 43c2409

const {hasExceededMaxCommentLength, validateCommentMaxLength} = useHandleExceedMaxCommentLength();

• Then in AttachmentModal we can disable the Button using isSendDisabled || isConfirmButtonDisabled condition and also reuse <ExceededCommentLength /> to show up just above the send button.

What alternative solutions did you explore? (Optional)

• We disable the + actions menu when the ExceededCommentLength flag is true.

[melvin-bot]

@anmurali, @Pujan92 Huh... This is 4 days overdue. Who can take care of this?

[Pujan92]

I will review this tomorrow.

[melvin-bot]

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

[melvin-bot]

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

[anmurali]

@Pujan92 - This still needs your review, can you pls prioritize this?

[jasperhuangg]

@Pujan92 I think we should actually go with both @nyomanjyotisa's primary and alternative solutions of disabling the "Add attachment" and "Send" buttons. It seems safer to disable them in both places in the UI.

[melvin-bot]

📣 @nyomanjyotisa 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

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

[melvin-bot]

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

• Fix: Chat - User is able to send message with max character limit #50487

If no regressions arise, payment will be issued on 2024-10-25. 🎊

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

• @Pujan92 requires payment through NewDot Manual Requests
• @nyomanjyotisa requires payment automatic offer (Contributor)

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

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

[Pujan92]

Regression Test Steps

1. Open any report
2. Enter a text with more than 10k characters
3. Once the composer shows an error of length exceeded, click the + button to add an attachment
4. Verify that the add attachment option is disabled / send button of attachment modal is disabled

[melvin-bot]

@anmurali, @Pujan92, @jasperhuangg, @nyomanjyotisa Whoops! This issue is 2 days overdue. Let's get this updated quick!

[anmurali]

Payment Summary

• @Pujan92 approved $250 via New Dot
• @nyomanjyotisa approved $250 via Upwork

[melvin-bot]

@anmurali @jasperhuangg Be sure to fill out the Contact List!

[garrettmknight]

$250 approved for @Pujan92