[HOLD for payment 2024-06-28] [HOLD for payment 2024-06-24] [$250] Subscription size value not trim able to add spaces before the value

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

---

Issue found when validating the PR #42683
Version Number: 1.4.79-2
Reproducible in staging?: y
Reproducible in production?: New feature
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:

1. Go to /settings/subscription/subscription-size
2. Enter spaces and then a values click next
3. Notice spaces showing on next page

Expected Result:

Spaces should not displayed with value

Actual Result:

Subscription size value not trim able to add spaces before the value

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

Bug6501964_1717527964130.2024-06-04_23-59-06.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0110dce96b3114bcdc
• Upwork Job ID: 1798096545497235456
• Last Price Increase: 2024-06-04
• Automatic offers:
• bernhardoj | Contributor | 102646806

Issue Owner

Current Issue Owner: @

Issue Owner

Current Issue Owner: @Christinadobrzyn

[melvin-bot]

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

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[m-natarajan]

@Christinadobrzyn FYI 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

[melvin-bot]

Triggered auto assignment to @tylerkaraszewski (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot]

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

[melvin-bot]

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

[neonbhai]

Proposal

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

Subscription size value not trim able to add spaces before the value

What is the root cause of that problem?

We display the original data when showing the entered subscription size here:

App/src/pages/settings/Subscription/SubscriptionSize/substeps/Confirmation.tsx

Line 39 in 70d36c6

title={translate('subscription.subscriptionSize.activeMembers', {size: subscriptionSizeFormDraft ? subscriptionSizeFormDraft[INPUT_IDS.SUBSCRIPTION_SIZE] : 0})}

We should trim the data before we display it.

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

Change this line

App/src/pages/settings/Subscription/SubscriptionSize/substeps/Confirmation.tsx

Line 39 in 70d36c6

title={translate('subscription.subscriptionSize.activeMembers', {size: subscriptionSizeFormDraft ? subscriptionSizeFormDraft[INPUT_IDS.SUBSCRIPTION_SIZE] : 0})}

to

title={translate('subscription.subscriptionSize.activeMembers', {size: subscriptionSizeFormDraft ? subscriptionSizeFormDraft[INPUT_IDS.SUBSCRIPTION_SIZE].trim() : 0})}

Alternatively

We may save the size value in a variable to make code simpler.

[amyevans]

This doesn't need to block deploy since it's a new feature and cannot be accessed yet except via direct link. @tylerkaraszewski I'm happy to take it over if you want, just lmk!

[Krishna2323]

Proposal

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

Subscription size value not trim able to add spaces before the value

What is the root cause of that problem?

We are not trimming the value before moving to next step.

App/src/pages/settings/Subscription/SubscriptionSize/substeps/Size.tsx

Lines 27 to 33 in 2339cbb

	<FormProvider
	formID={ONYXKEYS.FORMS.SUBSCRIPTION_SIZE_FORM}
	submitButtonText={translate('common.next')}
	onSubmit={onNext}
	validate={validate}
	style={[styles.mh5, styles.flexGrow1]}
	>

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

We can introduce a new prop (shouldTrimEmptySpaces) inside FormProvider and when passed, we will trim the values before validating, submitting and saving the value to draft , we already have a prop shouldTrimValues inside FormProvider but that only removes invisible characters from a string.

We will trim the value if shouldTrimEmptySpaces is true. We need to trim in all places below.

App/src/components/Form/FormProvider.tsx

Line 105 in 9b179ce

const trimmedStringValues = shouldTrimValues ? ValidationUtils.prepareValues(values) : values;

App/src/components/Form/FormProvider.tsx

Line 173 in 9b179ce

const trimmedStringValues = shouldTrimValues ? ValidationUtils.prepareValues(inputValues) : inputValues;

App/src/components/Form/FormProvider.tsx

Line 198 in 9b179ce

const trimmedStringValues = shouldTrimValues ? ValidationUtils.prepareValues(inputValues) : inputValues;

And also update the code below:

App/src/components/Form/FormProvider.tsx

Lines 353 to 355 in 9b179ce

	if (inputProps.shouldSaveDraft && !formID.includes('Draft')) {
	FormActions.setDraftValues(formID as OnyxFormKey, {[inputKey]: value});
	}

To:

if (inputProps.shouldSaveDraft && !formID.includes('Draft')) {
    let trimmedValue = value;
    if (typeof value === 'string' && shouldTrimEmptySpaces) {
        trimmedValue = value.trim();
    }
    FormActions.setDraftValues(formID as OnyxFormKey, {[inputKey]: trimmedValue});
}

What alternative solutions did you explore? (Optional)

We can trim spaces by default and use the same prop shouldTrimValues for disabling trimming.

If we need, we can only trim the draft value instead of all values.

[Krishna2323]

Proposal Updated

• Updated main solution and added an alternative

[cretadn22]

Proposal

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

Subscription size value not trim

What is the root cause of that problem?

We lack the logic to trim the subscription size value

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

In the FormProvider, we've incorporated a property called shouldTrimValues. If shouldTrimValues is set to true, we'll invoke this function.

App/src/libs/ValidationUtils.ts

Line 453 in 3c04b02

function prepareValues(values: ValuesType): ValuesType {

However, currently, this function does not include logic to trim values. I propose that we integrate logic to trim values within this function, if the input value is a string.

code change:

if (typeof inputValue === 'string') {
            trimmedStringValues[inputID] = StringUtils.removeInvisibleCharacters(inputValue).trim();
}

Additionally, before saving the draft, we should also trim the value. We need to verify whether the value is a string; if it is, we should apply a trim function.

App/src/components/Form/FormProvider.tsx

Line 354 in 3c04b02

FormActions.setDraftValues(formID as OnyxFormKey, {[inputKey]: value});

code change:

FormActions.setDraftValues(formID as OnyxFormKey, {[inputKey]: typeof value === 'string' ? value.trim() : value});

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.

[melvin-bot]

📣 @bernhardoj 🎉 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 📖

[bernhardoj]

PR is ready

cc: @thesahindia

[Christinadobrzyn]

Just a heads up - I'm going to be ooo until June 24th. I'm not going to assign anyone new but if you need a new BZ teammate for any reason please feel free to ask for one to be assigned here.

[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 1.4.84-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix subscription size value not trim able to add spaces before the value #43328

If no regressions arise, payment will be issued on 2024-06-24. 🎊

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

• @bernhardoj requires payment automatic offer (Contributor)
• @thesahindia requires payment through NewDot Manual Requests

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

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

[melvin-bot]

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

• Fix subscription size value not trim able to add spaces before the value #43328

If no regressions arise, payment will be issued on 2024-06-28. 🎊

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

• @bernhardoj requires payment automatic offer (Contributor)
• @thesahindia requires payment through NewDot Manual Requests

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

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

[Christinadobrzyn]

Payouts due:

• Contributor: $250 @bernhardoj (paid in Upwork - https://www.upwork.com/nx/wm/offer/102646806)
• Contributor+: $250 @thesahindia (through NewDot)

Upwork job is here.

@thesahindia let us know about a regression test!

[Christinadobrzyn]

still have a few days till payment!

[thesahindia]

This bug was present since the implementation of this page. It was a minor issue that got missed in #42683. I don't think we need a test case for this.

[Christinadobrzyn]

Sounds good - no regression test for this. Thanks @thesahindia!

[Christinadobrzyn]

Awesome! Payment summary is here - #43069 (comment)

This is good to close. Feel free to reach out if I missed anything.

[JmillsExpensify]

$250 approved for @thesahindia