-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[$250] Rules - Unable to clear default value and save value in Random report audit field #49742
Comments
Triggered auto assignment to @trjExpensify ( |
We think that this bug might be related to #wave-control |
@trjExpensify 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 |
Edited by proposal-police: This proposal was edited at 2024-09-25 20:28:42 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.The default value cannot be cleared by delete key. What is the root cause of that problem?When we press delete button => App/src/components/PercentageForm.tsx Lines 41 to 43 in a4a344b
What changes do you think we should make in order to solve the problem?We should remove this condition and will check valid when we submit the form here and if it's not valid we can show the error App/src/components/PercentageForm.tsx Lines 41 to 43 in a4a344b
What alternative solutions did you explore? (Optional)NA |
Edited by proposal-police: This proposal was edited at 2024-09-25 21:48:58 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.We are unable to clear the default value and save a new value in the "Random report audit" field. What is the root cause of that problem?When the delete button is pressed, the PercentageForm validates the new value using the The issue occurs because this function currently checks if the number is between 0 and 100 but only works with integers. It fails when the value is a decimal and returns App/src/libs/MoneyRequestUtils.ts Lines 55 to 59 in 5797fb1
What changes do you think we should make in order to solve the problem?
const defaultValue = Math.floor(policy?.autoApproval?.auditRate ?? CONST.POLICY.RANDOM_AUDIT_DEFAULT_PERCENTAGE);
/**
* Check if percentage is between 0 and 100
*/
function validatePercentage(amount: string): boolean {
const number = parseFloat(amount);
if (number == "") {
return true;
}
return number >= 0 && number <= 100 && (amount.split('.')[1]?.length ?? 0) <= 2;
} What alternative solutions did you explore? (Optional) |
ProposalPlease re-state the problem that we are trying to solve in this issue.Unable to clear Random report audit default value. What is the root cause of that problem?When we enable the App/src/libs/MoneyRequestUtils.ts Lines 55 to 59 in b5c47c5
App/src/components/PercentageForm.tsx Lines 41 to 43 in b5c47c5
So, when we press backspace, the input value becomes 0.0 and because it's invalid, the input is not updated. If we see the optimistic default value, the value is 5, not 0.05, so there is a difference between FE and BE. Line 2186 in b5c47c5
If we go to OD, we can't input decimals, so I believe the expected behavior here is to not allow decimals, which is what the page validation already did. The real problem I believe here is, that we need to convert the rate decimal representation to the percentage value by multiplying it by 100, and dividing it by 100 when saving it. Why? Because if we update the value with a number bigger than 1, the BE will return an invalid rate error. So, if we want to save 1%, we need to send it as 0.01. What changes do you think we should make in order to solve the problem?
We can create a PolicyUtils function for that. App/src/libs/actions/Policy/Policy.ts Lines 4160 to 4163 in b5c47c5
|
@marcaaron @JmillsExpensify @dylanexpensify putting this on your radar. I think we should display this as |
Job added to Upwork: https://www.upwork.com/jobs/~021839442575257050533 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @brunovjk ( |
This was basically a temporary side effect of needing to update the backend to accept the correct values. |
Of course you're on it!
…On Fri, Sep 27, 2024 at 12:37 AM Marc Glasser ***@***.***> wrote:
This was basically a temporary side effect of needing to update the
backend to accept the correct values.
—
Reply to this email directly, view it on GitHub
<#49742 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AD3246NJMNUA4KWSTUWNOI3ZYSLDBAVCNFSM6AAAAABO3JFFM6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNZYGEZDCNBZGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
*Tom Rhys Jones *
*Expensify*
|
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.40-0
Reproducible in staging?: Y
Reproducible in production?: Y
Issue was found when executing this PR: #49180
Email or phone of affected tester (no customers): applausetester+kh230901@applause.expensifail.com
Issue reported by: Applause Internal Team
Action Performed:
Expected Result:
The default value can be cleared by delete key and a new value can be saved.
Actual Result:
The default value cannot be cleared by delete key.
The default value can be cleared by CMD+A but the value reverts to the default value after saving a new value.
Workaround:
Unknown
Platforms:
Screenshots/Videos
Bug6615157_1727291073352.rules.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @brunovjkThe text was updated successfully, but these errors were encountered: