[HOLD for payment 2024-08-01] [$250] Subscription size - Decimal point can be inserted and not removed on the confirmation page

@miljakljajic

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.84-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: N/A
Issue reported by: Applause - Internal Team

Issue found when executing PR #43328

Action Performed:

Go to staging.new.expensify.com/settings/subscription/subscription-size
Enter a digit and a decimal point
Click Next

Expected Result:

Decimal point will be removed on the next page

Actual Result:

Decimal point can be inserted and not removed on the confirmation page

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6514089_1718444363848.bandicam_2024-06-15_17-37-25-460.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01d8c409a169a06c82
Upwork Job ID: 1803448780086879493
Last Price Increase: 2024-06-19
Automatic offers:

fedirjh | Reviewer | 102862371
Krishna2323 | Contributor | 102862372

Issue Owner

Current Issue Owner: @miljakljajic

@miljakljajic

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

@miljakljajic

@miljakljajic 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

Proposal

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

Subscription size - Decimal point can be inserted and not removed on the confirmation page

What is the root cause of that problem?

We haven't added a check for decimal point.

App/src/libs/ValidationUtils.ts

Lines 485 to 488 in 75c104b

    
           function isValidSubscriptionSize(subscriptionSize: string): boolean { 
        
               const parsedSubscriptionSize = Number(subscriptionSize); 
        
               return !Number.isNaN(parsedSubscriptionSize) && parsedSubscriptionSize > 0 && parsedSubscriptionSize <= CONST.SUBSCRIPTION_SIZE_LIMIT && Number.isInteger(parsedSubscriptionSize); 
        
           }

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

Check if the number contains ..

function isValidSubscriptionSize(subscriptionSize: string): boolean {
    const parsedSubscriptionSize = Number(subscriptionSize);
    if (subscriptionSize.includes('.')) {
        return false;
    }
    return !Number.isNaN(parsedSubscriptionSize) && parsedSubscriptionSize > 0 && parsedSubscriptionSize <= CONST.SUBSCRIPTION_SIZE_LIMIT && Number.isInteger(parsedSubscriptionSize);
}

What alternative solutions did you explore? (Optional)

We can use NUMBER regex.

function isValidSubscriptionSize(subscriptionSize: string): boolean {
    const parsedSubscriptionSize = Number(subscriptionSize);
    return (
        !Number.isNaN(parsedSubscriptionSize) &&
        parsedSubscriptionSize > 0 &&
        parsedSubscriptionSize <= CONST.SUBSCRIPTION_SIZE_LIMIT &&
        Number.isInteger(parsedSubscriptionSize) &&
        CONST.REGEX.NUMBER.test(subscriptionSize)
    );
}

What alternative solutions did you explore? (Optional 2)

If we don't want to show error when entering 0 after decimal then we need to trim the decimal points on confirmation page.

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

Line 44 in 75c104b

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

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

Additionally we should also clear the draft value when saving the subscription size.

App/src/pages/settings/Subscription/SubscriptionSize/index.tsx

Lines 29 to 32 in 75c104b

    
           const onFinished = () => { 
        
               Subscription.updateSubscriptionSize(subscriptionSizeFormDraft ? Number(subscriptionSizeFormDraft[INPUT_IDS.SUBSCRIPTION_SIZE]) : 0, privateSubscription?.userCount ?? 0); 
        
               Navigation.goBack(); 
        
           };

To:

    const onFinished = () => {
        FormActions.clearDraftValues(ONYXKEYS.FORMS.SUBSCRIPTION_SIZE_FORM);
        Subscription.updateSubscriptionSize(subscriptionSizeFormDraft ? Number(subscriptionSizeFormDraft[INPUT_IDS.SUBSCRIPTION_SIZE]) : 0, privateSubscription?.userCount ?? 0);
        Navigation.goBack();
    };

Proposal Updated

Added alternative 1 and 2

@miljakljajic

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

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

@fedirjh

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

@fedirjh

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

fix: Subscription size - Decimal point can be inserted and not removed on the confirmation page. #44507

If no regressions arise, payment will be issued on 2024-08-01. 🎊

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

@fedirjh requires payment automatic offer (Reviewer)
@Krishna2323 requires payment automatic offer (Contributor)

@fedirjh

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:

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

@Krishna2323

@Krishna2323 you've been paid in upwork, thank you!

@fedirjh your original offer expired as you didn't accept it in upwork in time. Please accept the new offer.

@fedirjh

actually @fedirjh do we pay you in NewDot? I noticed you're listed for manual payments. Let me know and I'll provide a payment summary instead.

@miljakljajic

do we pay you in NewDot?

@miljakljajic Yes, that’s correct. Can you provide a summary instead? Thank you

BugZero Checklist:

Link to the PR: N/A This could have been caught in feat: Subscription size backend integration #43484
Link to comment: N/A
Link to discussion: N/A
Determine if we should create a regression test for this bug: N/A, this is a minor edge-case bug, no need to add regression tests

@fedirjh

Of course, apologies.

@fedirjh is owed 250 for their work reviewing this issue.

@fedirjh

$250 approved for @fedirjh

Navigation Menu

Repository navigation

[HOLD for payment 2024-08-01] [$250] Subscription size - Decimal point can be inserted and not removed on the confirmation page #43796

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

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)

What alternative solutions did you explore? (Optional 2)

Proposal Updated

21 remaining items

BugZero Checklist:

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	function isValidSubscriptionSize(subscriptionSize: string): boolean {
	const parsedSubscriptionSize = Number(subscriptionSize);
	return !Number.isNaN(parsedSubscriptionSize) && parsedSubscriptionSize > 0 && parsedSubscriptionSize <= CONST.SUBSCRIPTION_SIZE_LIMIT && Number.isInteger(parsedSubscriptionSize);
	}

	const onFinished = () => {
	Subscription.updateSubscriptionSize(subscriptionSizeFormDraft ? Number(subscriptionSizeFormDraft[INPUT_IDS.SUBSCRIPTION_SIZE]) : 0, privateSubscription?.userCount ?? 0);
	Navigation.goBack();
	};

[HOLD for payment 2024-08-01] [$250] Subscription size - Decimal point can be inserted and not removed on the confirmation page #43796

Description

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Activity

melvin-bot commented on Jun 15, 2024

lanitochka17 commented on Jun 15, 2024

Krishna2323 commented on Jun 15, 2024

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)

What alternative solutions did you explore? (Optional 2)

Krishna2323 commented on Jun 15, 2024

Proposal Updated

melvin-bot commented on Jun 18, 2024

melvin-bot commented on Jun 19, 2024

melvin-bot commented on Jun 19, 2024

21 remaining items

melvin-bot commented on Jul 25, 2024

melvin-bot commented on Jul 25, 2024

miljakljajic commented on Aug 1, 2024

miljakljajic commented on Aug 1, 2024

fedirjh commented on Aug 2, 2024

fedirjh commented on Aug 2, 2024

BugZero Checklist:

miljakljajic commented on Aug 5, 2024

JmillsExpensify commented on Aug 5, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions