fix: claim delay field validation

[joanna-pagepro]

Description

Claim delay field validation updated so that a user has to add a number between 0.0001 and 99999.

Testing

• Step 1. Open action sidebar and select Payment builder action type
• Step 2. Edit Claim delay field inside the table with payments and check if the validation works as it should.

Diffs

New stuff ✨

• ClaimDelayField component - uses cleave-zen inside

Changes 🏗

• delay field validation updated
• claim delay column value formatting updated for completed action

Deletions ⚰️

TODO

Resolves #2368

[davecreaser]

All looks good to me. Tested and the validation is working as expected for both min and max.

[marcocolony]

Hey @joanna-pagepro, thanks for picking this one up!

• Minimum and maximum values validation is working ✅
• Only up to 4 decimal points can be entered ✅
• I'm able to proceed with the payment creation when a number bigger than one is entered ✅
• Values over 99999 can be entered ❌
• The error message that was previously displayed whenever a decimal number was entered is no longer there, but I'm unable to proceed and an error is thrown in the console ❌

[arrenv]

@joanna-pagepro
If possible, it would be good to solve the live validation lag issue when we have a lot of rows.

If not possible, then it would be good to limit the field length to 5 digits with 0.0001 and 99999.

If that is not possible due to the decimal place, then it would still be good to limit the length to 5 digits with 0.001 and 99999

[joanna-pagepro]

@marcocolony can you review it again?

[marcocolony]

Hey @joanna-pagepro

I don't think it's quite fixed just yet, please see the examples I tested below

[melyndav]

Hey @joanna-pagepro great start on this one.

When attempting to test this, I get default validation.

Can we remove step="0.001" to avoid the default validation?

[joanna-pagepro]

@marcocolony @melyndav can you check again?

[marcocolony]

@joanna-pagepro Thanks for looking into this again

I'm afraid I'm still able to enter as many digits as I want here.

But the decimal values look great, always limited to 4 🙌

[joanna-pagepro]

@marcocolony as discussed during the last daily - you will see an error on submit, it's not validated live. You should be able to enter as many digits as you want, but you shouldn't be able to submit the form if the value is bigger than 99999 or smaller than 0.0001

[marcocolony]

Thank's @joanna-pagepro, everything looks good!

[jakubcolony]

Nice start @joanna-pagepro, please be aware that incoming claim delay (fetched from the DB) can be up to 2^256 so we need to do all the maths operations using BigNumber.

On a side note, allowing setting the claim delay as fractional number exposes us to rounding issues such as this one. It's not a code issue, as it converted 0.1111 hr to 400 mins, then 400 mins / 3600 = 0.11111111111 seconds. @arrenv something to be aware of I suppose.

Screen.Recording.2024-05-28.at.23.20.23.mov

[jakubcolony]

This can't be easily tested since there's no saga to set the global claim delay, however I spotted some issues:

The problem with toNumber here is that it's possible for expendituresGlobalClaimDelayHours to be > 2^53 which will cause it to throw an error (and it wouldn't be caught). All claim delay calculations should use BigNumbers (great shout on already switching it here) and strings.

This made me realise the expendituresGlobalClaimDelayHours will not be set correctly because of line 29 where we check if global claim delay's type is number, which it will never be.

And, if dataRef.current[row.index]?.delay is a floating point number, BigNumber will throw an error.

Feel free to ping me on Discord and I'll provide you with instructions on how to set the global claim delay for testing.

[jakubcolony]

Should use BigNumber

[jakubcolony]

Should use BigNumber

[arrenv]

On a side note, allowing setting the claim delay as fractional number exposes us to rounding issues such as this one. It's not a code issue, as it converted 0.1111 hr to 400 mins, then 400 mins / 3600 = 0.11111111111 seconds. @arrenv something to be aware of I suppose.

@jakubcolony I am not understanding the rounding to 400 mins here? Could we not just convert it to seconds in all cases? i.e. 0.1111 * 3600 then as it comes back / 3600?

[jakubcolony]

@arrenv Apologies, I meant seconds!

0.1111 hours * 3600 = 399.96 ≈ 400 seconds (what we send and store on chain)

Then back in the UI:

400 seconds / 3600 = 0.11111111111 hours

The solution could simply be to round to 4 decimal places when we display the delay too.

[melyndav]

Hey @joanna-pagepro just confirming if you're still seeing the validation as I am?

[joanna-pagepro]

@melyndav no, I'm not seeing this. I've disabled the default form validation, but maybe it doesn't work the same on every browser. I will check it on Monday.

[arrenv]

@arrenv Apologies, I meant seconds!

0.1111 hours * 3600 = 399.96 ≈ 400 seconds (what we send and store on chain)

Then back in the UI:

400 seconds / 3600 = 0.11111111111 hours

The solution could simply be to round to 4 decimal places when we display the delay too.

Oh, I made an error in mine as well, haha. Makes sense the rounding of the seconds.

@joanna-pagepro Can we please round to 4 decimal places on the completed view in the UI as @jakubcolony has suggested.

[joanna-pagepro]

@arrenv @jakubcolony can you review again?

[jakubcolony]

Codewise looking good, however there's an issue with claim delays starting with multiple zeros:

Screen.Recording.2024-06-09.at.22.11.56.mov

[joanna-pagepro]

@arrenv @jakubcolony can you review this one?

[jakubcolony]

Not sure what happened since last review but I started to see the native browser validation:

Screen.Recording.2024-06-12.at.21.56.20.mov

[joanna-pagepro]

@jakubcolony it should be ok now

[bassgeta]

Nice one, works for minimum and maximum checks 👌

LGTM!

[jakubcolony]

I'm unable to set the claim delay to 0 since the validation requires it to be at least 0.0001. I'm not sure this is correct, you should not be able to set it to something such as 0.00000001 but it should still be possible to create a payment with no claim delay.