Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add outstanding token airdrops to REST API #9286

Merged
merged 11 commits into from
Sep 16, 2024
Merged

Add outstanding token airdrops to REST API #9286

merged 11 commits into from
Sep 16, 2024

Conversation

edwin-greene
Copy link
Contributor

@edwin-greene edwin-greene commented Sep 6, 2024 •
edited
Loading

Description:
Adds the outstanding token airdrops endpoint to the REST API.

Related issue(s):

Fixes #8276

Notes for reviewer:
Currently only supports fungible token airdrops.

Checklist

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

@edwin-greene
Add outstanding token airdrops to rest api
6a31732 
Signed-off-by: Edwin Greene <edwin@swirldslabs.com>
@edwin-greene edwin-greene added enhancement Type: New feature rest-java Area: Java REST API labels Sep 6, 2024
@edwin-greene edwin-greene added this to the 0.114.0 milestone Sep 6, 2024
@edwin-greene edwin-greene self-assigned this Sep 6, 2024
@edwin-greene edwin-greene linked an issue Sep 6, 2024 that may be closed by this pull request
HIP-904 Add Outstanding Token Airdrops endpoint to REST API #8276
Closed
@codecov Codecov
Copy link

codecov bot commented Sep 6, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 93.49593% with 8 lines in your changes missing coverage. Please review.

Project coverage is 92.55%. Comparing base (307c002) to head (8c0abbc).
Report is 22 commits behind head on main.

Files with missing lines Patch % Lines
...era/mirror/restjava/repository/JooqRepository.java 91.66% 1 Missing and 3 partials ⚠️
...edera/mirror/restjava/mapper/CollectionMapper.java 71.42% 1 Missing and 1 partial ⚠️
...a/repository/TokenAirdropRepositoryCustomImpl.java 90.90% 0 Missing and 2 partials ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #9286      +/-   ##
============================================
- Coverage     93.15%   92.55%   -0.61%     
- Complexity     1092     7039    +5947     
============================================
  Files           363      912     +549     
  Lines         11285    29760   +18475     
  Branches       1642     3760    +2118     
============================================
+ Hits          10513    27543   +17030     
- Misses          633     1446     +813     
- Partials        139      771     +632

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@edwin-greene
Fix code smell
0812194 
Signed-off-by: Edwin Greene <edwin@swirldslabs.com>
@edwin-greene edwin-greene marked this pull request as ready for review September 6, 2024 21:01
@edwin-greene edwin-greene requested a review from a team September 6, 2024 21:01
@edwin-greene
Cleanup unneeded comments
db31f89 
Signed-off-by: Edwin Greene <edwin@swirldslabs.com>
steven-sheehy
steven-sheehy requested changes Sep 11, 2024
View reviewed changes
...-rest-java/src/main/java/com/hedera/mirror/restjava/repository/AbstractCustomRepository.java Outdated Show resolved Hide resolved
...-mirror-rest-java/src/main/java/com/hedera/mirror/restjava/common/IntegerRangeParameter.java Outdated Show resolved Hide resolved
...-mirror-rest-java/src/main/java/com/hedera/mirror/restjava/common/IntegerRangeParameter.java Outdated Show resolved Hide resolved
...r-rest-java/src/main/java/com/hedera/mirror/restjava/controller/TokenAirdropsController.java Outdated Show resolved Hide resolved
...r-rest-java/src/main/java/com/hedera/mirror/restjava/dto/OutstandingTokenAirdropRequest.java Outdated Show resolved Hide resolved
hedera-mirror-rest/api/v1/openapi.yml Outdated Show resolved Hide resolved
...rror-rest-java/src/test/java/com/hedera/mirror/restjava/service/TokenAirdropServiceTest.java Outdated Show resolved Hide resolved
...rror-rest-java/src/test/java/com/hedera/mirror/restjava/service/TokenAirdropServiceTest.java Outdated Show resolved Hide resolved
...va/src/main/java/com/hedera/mirror/restjava/repository/NftAllowanceRepositoryCustomImpl.java Show resolved Hide resolved
...va/src/main/java/com/hedera/mirror/restjava/repository/TokenAirdropRepositoryCustomImpl.java Outdated Show resolved Hide resolved
@edwin-greene
Updates per feedback
154aacb 
Signed-off-by: Edwin Greene <edwin@swirldslabs.com>
@edwin-greene
Remove serial number parameter for now
a404dd8 
Signed-off-by: Edwin Greene <edwin@swirldslabs.com>
@xin-hedera
Copy link
Collaborator

xin-hedera commented Sep 11, 2024 •
edited
Loading

Pagination by serialnumber may be added later.

Can this even work?

Serial number should be in the order part so as to limit the entries in the response. If we don't support seiral number pagination, we won't have it in the next url, then wouldn't the endpoint provide incomplete response? Or duplicate info? Or even stuck and never get to the next page?

steven-sheehy
steven-sheehy requested changes Sep 12, 2024
View reviewed changes
...rror-rest-java/src/test/java/com/hedera/mirror/restjava/common/NumberRangeParameterTest.java Outdated Show resolved Hide resolved
hedera-mirror-rest/api/v1/openapi.yml Outdated Show resolved Hide resolved
...ra-mirror-rest-java/src/main/java/com/hedera/mirror/restjava/mapper/TokenAirdropsMapper.java Outdated Show resolved Hide resolved
...rror-rest-java/src/main/java/com/hedera/mirror/restjava/service/TokenAirdropServiceImpl.java Outdated Show resolved Hide resolved
...st-java/src/test/java/com/hedera/mirror/restjava/controller/TokenAirdropsControllerTest.java Outdated Show resolved Hide resolved
...va/src/main/java/com/hedera/mirror/restjava/repository/TokenAirdropRepositoryCustomImpl.java Outdated Show resolved Hide resolved
@edwin-greene
Updates per feedback. Repository class changes
bc82e6c 
Signed-off-by: Edwin Greene <edwin@swirldslabs.com>
@edwin-greene
Reformat line breaks for easier diff review. Rename mapper
a6459ab 
Signed-off-by: Edwin Greene <edwin@swirldslabs.com>
@edwin-greene
Update jooqRepository and simplify conditionals
3f72fd3 
Signed-off-by: Edwin Greene <edwin@swirldslabs.com>
@edwin-greene
Merge branch 'main' into 8276-hip-904-add-outstanding-token-airdrops-…
3eaaa4a 
…endpoint-to-rest-api
@edwin-greene
Fix code smells
e131e22 
Signed-off-by: Edwin Greene <edwin@swirldslabs.com>
@edwin-greene
Update openapi, tests and query
8c0abbc 
Signed-off-by: Edwin Greene <edwin@swirldslabs.com>
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Sep 16, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

steven-sheehy
steven-sheehy approved these changes Sep 16, 2024
View reviewed changes
Copy link
Member

@steven-sheehy steven-sheehy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I think we're missing corner case test coverage in the repository layer similar to what NftAllowanceRepositoryTest has. But probably we can figure out a way to have a common test framework for multi-column pagination in a follow up.

@edwin-greene edwin-greene mentioned this pull request Sep 16, 2024
Closed
jascks
jascks approved these changes Sep 16, 2024
View reviewed changes
Copy link
Contributor

@jascks jascks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

xin-hedera
xin-hedera approved these changes Sep 16, 2024
View reviewed changes
Copy link
Collaborator

@xin-hedera xin-hedera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

jnels124
jnels124 approved these changes Sep 16, 2024
View reviewed changes
Copy link
Contributor

@jnels124 jnels124 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@edwin-greene edwin-greene merged commit b2349db into main Sep 16, 2024
29 checks passed
@edwin-greene edwin-greene deleted the 8276-hip-904-add-outstanding-token-airdrops-endpoint-to-rest-api branch September 16, 2024 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steven-sheehy steven-sheehy steven-sheehy approved these changes

@xin-hedera xin-hedera xin-hedera approved these changes

@jnels124 jnels124 jnels124 approved these changes

@jascks jascks jascks approved these changes

Assignees

@edwin-greene edwin-greene

Labels
enhancement Type: New feature rest-java Area: Java REST API
Projects
None yet
Milestone
0.114.0
Development

Successfully merging this pull request may close these issues.

HIP-904 Add Outstanding Token Airdrops endpoint to REST API
5 participants
@edwin-greene @xin-hedera @jascks @jnels124 @steven-sheehy