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

refactor: get balance via fspconfigid of registration AB#32518 #6358

Merged
merged 6 commits into from
Jan 10, 2025
Merged

refactor: get balance via fspconfigid of registration AB#32518 #6358

merged 6 commits into from
Jan 10, 2025

Conversation

jannisvisser
Copy link
Contributor

@jannisvisser jannisvisser commented Jan 8, 2025 •
edited
Loading

AB#32518

Describe your changes

3 commits build on each other

  1. first got fspConfigId via registration (also in both bulk methods)
  2. then optimized by not repeating queries in bulk methods unnecessarily
  3. then realized, getting fspConfigId via registration may be incorrect if FSP changed along the way, so instead got via transaction.

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have added tests wherever relevant
  • I have made sure that all automated checks pass before requesting a review
  • I do not need any deviation from our PR guidelines

Portalicious preview deployment

This PR does not have any preview deployments yet.

@jannisvisser jannisvisser added the chore Something that does not affect the end user label Jan 8, 2025
@jannisvisser jannisvisser force-pushed the refactor.get-balance-better-fix branch from 7ba0c0c to 1121333 Compare January 8, 2025 09:35
@jannisvisser jannisvisser requested a review from RubenGeo January 8, 2025 11:48
@jannisvisser jannisvisser requested a review from RubenGeo January 8, 2025 14:13
@jannisvisser
Copy link
Contributor Author

@RubenGeo I see you approved this PR, so we'll leave be the potential changes we discussed on wednesday I interpret then?

@jannisvisser jannisvisser force-pushed the refactor.get-balance-better-fix branch from f0b85d0 to 737dc93 Compare January 10, 2025 09:12
@jannisvisser jannisvisser enabled auto-merge (squash) January 10, 2025 09:13
@jannisvisser
Copy link
Contributor Author

@RubenGeo solved the rebase conflict locally and enabled automerge

@jannisvisser jannisvisser merged commit c1ff2bb into main Jan 10, 2025
7 checks passed
@jannisvisser jannisvisser deleted the refactor.get-balance-better-fix branch January 10, 2025 09:24
Piotrk39 pushed a commit that referenced this pull request Jan 10, 2025
@jannisvisser @Piotrk39
refactor: get balance via fspconfigid of registration AB#32518 (#6358)
2efb7ca 
* refactor: get balance via fspconfigid of registration AB#32518

* refactor: improve performance of bulk calls by not repeating queries AB#32518

* refactor: get via transaction AB#32518

* test: add API test get-balance + small cleanup AB#32517

* fix: improve query AB#32518

* chore: add note
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RubenGeo RubenGeo RubenGeo approved these changes

Assignees
No one assigned
Labels
chore Something that does not affect the end user
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jannisvisser @RubenGeo