hedera-mirror-node icon indicating copy to clipboard operation
hedera-mirror-node copied to clipboard

Nft allowances Range additions

Open mgoelswirlds opened this issue 1 year ago • 2 comments

Description: This PR adds the support for greater and lower bound conditions for query parameters in the request.

This PR modifies

  • The service layer and the repository layer to handle greater and lower bound conditions for query parameters.
  • Adds extra parameter validations as per the openapi spec
  • Shortcuts invalid ranges

Related issue(s):

Fixes #8176

Notes for reviewer:

  • This PR will have changes to the next link depending on the Next link PR
  • This PR does not add all validations to Controller tests since they are validated in services tests and these take longer to execute.

Checklist

  • [ ] Documented (Code comments, README, etc.)
  • [x] Tested (unit, integration, etc.)

mgoelswirlds avatar May 09 '24 15:05 mgoelswirlds

Codecov Report

Attention: Patch coverage is 97.97980% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 92.46%. Comparing base (4b9bf04) to head (9b22015). Report is 22 commits behind head on main.

Files Patch % Lines
...java/com/hedera/mirror/restjava/service/Bound.java 96.36% 1 Missing and 1 partial :warning:
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8261      +/-   ##
============================================
+ Coverage     92.30%   92.46%   +0.15%     
+ Complexity     7292     7239      -53     
============================================
  Files           899      883      -16     
  Lines         29366    29199     -167     
  Branches       3585     3580       -5     
============================================
- Hits          27107    26998     -109     
+ Misses         1439     1385      -54     
+ Partials        820      816       -4     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar May 09 '24 19:05 codecov[bot]

How are we going to merge this without the next link fix?

Given the http request from the user, e.g., /api/v1/200/allowances/nfts?account.id=gt:200&account.id=lt:800&order=asc, the response may have a next link like /api/v1/200/allowances/nfts?account.id=gt:200&account.id=lt:800&order=asc&account.id=gt:202, which violates max=2 constraint.

Do we ship it and tell users that's a known bug?

Response: This is getting fixed in another PR which will ship in the same release.

xin-hedera avatar May 13 '24 17:05 xin-hedera