js-algorand-sdk icon indicating copy to clipboard operation
js-algorand-sdk copied to clipboard

Remove function removeFalsyOrEmpty

Open barnjamin opened this issue 3 years ago • 6 comments

Subject of the issue

I'm trying to lookup holders of a given asset where they hold >0 tokens.

When I call client.lookupAssetBalances(asset_id).currencyGreaterThan(0).do() the resultant request has no currency-greater-than GET parameter, but setting it to 1 does produce the correct request URL.

I believe this is due to the step where it removes things that look falsy: https://github.com/algorand/js-algorand-sdk/blob/9afe012d48817978b3f49131dff86ccdb8c8d423/src/client/client.ts#L53-L61

I'm not sure what the correct fix is and I've got a temporary work around.

barnjamin avatar Aug 11 '21 13:08 barnjamin

@chaihoang Do you have updates on this?

Lumene98 avatar Sep 01 '21 12:09 Lumene98

@jasonpaulos

chaihoang avatar Sep 01 '21 15:09 chaihoang

This is definitely a bug that we'd like to fix, but in the meantime community members have noticed that a workaround is to pass 0 as a string, i.e. "0".

jasonpaulos avatar Sep 01 '21 15:09 jasonpaulos

Are there any end-points in the API where it makes sense to filter out a query parameter that has been set to 0?

If not, can we just exclude that in removeFalsyOrEmpty like so: if (!obj[key] || obj[key].length === 0) becomes if ((!obj[key] && obj[key] !== 0) || obj[key].length === 0) ?

achidlow avatar Apr 14 '22 04:04 achidlow

I hit this too - the '0' workaround does work but this should ideally be fixed, given how old this issue is.

If you think excluding other falsy values is going to be problematic then at least make an exception for Number(0) in that check? "Holders that have at least one micro-unit" is a fairly common use case that everyone trips over currently.

d13co avatar Jun 14 '23 16:06 d13co

It may be possible to remove removeFalsyOrEmpty. We need to fix errors like this after removing this function.

Scenario: LookupAccountCreatedApplications path # tests/cucumber/features/unit/v2indexerclient_paths.feature:270
    ✔ Given mock server recording request paths # tests/cucumber/steps/index.js:490
    ✔ When we make a LookupAccountCreatedApplications call with accountID "7ZUECA7HFLZTXENRV24SHLU4AVPUTMTTDUFUBNBD64C73F3UHRTHAIOF6Q" applicationID 0 includeAll "false" limit 0 next "" # tests/cucumber/steps/index.js:508
    ✖ Then expect the path used to be "/v2/accounts/7ZUECA7HFLZTXENRV24SHLU4AVPUTMTTDUFUBNBD64C73F3UHRTHAIOF6Q/created-applications" # tests/cucumber/steps/index.js:513
        AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
        + actual - expected

        + '/v2/accounts/7ZUECA7HFLZTXENRV24SHLU4AVPUTMTTDUFUBNBD64C73F3UHRTHAIOF6Q/created-applications?application-id=0&include-all=false&limit=0&next='
        - '/v2/accounts/7ZUECA7HFLZTXENRV24SHLU4AVPUTMTTDUFUBNBD64C73F3UHRTHAIOF6Q/created-applications'

shiqizng avatar Aug 10 '23 13:08 shiqizng