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

fix(AccountsAssets): historicApi for AccountsAssets, bug fixes, error handling, e2e-tests #721

Merged
merged 14 commits into from
Oct 26, 2021
Merged

fix(AccountsAssets): historicApi for AccountsAssets, bug fixes, error handling, e2e-tests #721

merged 14 commits into from
Oct 26, 2021

Conversation

TarikGul
Copy link
Member

@TarikGul TarikGul commented Oct 18, 2021 •
edited
Loading

rel: #698

This PR does a few things.

  1. Add a historicApi to account assets. This fixes a bug that exists in querying assets as well, because it didn't use the .at key.
  2. Add historicApi to the tests.
  3. Add statemine to the e2e tests.

Todo:

  • Add historicApi and adjust tests
  • Update error handling
  • Fix historic runtime bug
  • Add AssetBalance e2e-tests
  • Add AssetApproval e2e-tests (This is wish washy, as there are no approvals I am able to find)
  • Update e2e-script to use statemine

@TarikGul TarikGul mentioned this pull request Oct 18, 2021
Closed
10 tasks
@TarikGul TarikGul marked this pull request as ready for review October 19, 2021 14:55
insipx
insipx approved these changes Oct 22, 2021
View reviewed changes
Copy link
Contributor

@insipx insipx left a comment

Choose a reason for hiding this comment

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

lgtm

niklasad1
niklasad1 approved these changes Oct 26, 2021
View reviewed changes
Copy link
Member

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

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

LGTM

dvdplm
dvdplm reviewed Oct 26, 2021
View reviewed changes
scripts/config.ts Show resolved Hide resolved
src/services/accounts/AccountsAssetsService.spec.ts Outdated Show resolved Hide resolved
src/services/accounts/AccountsAssetsService.ts Outdated Show resolved Hide resolved
src/services/accounts/AccountsAssetsService.ts Outdated Show resolved Hide resolved
@TarikGul TarikGul merged commit 583936d into master Oct 26, 2021
@TarikGul TarikGul deleted the tarik-accounts-assets branch October 26, 2021 15:40
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dvdplm dvdplm dvdplm approved these changes

@insipx insipx insipx approved these changes

@niklasad1 niklasad1 niklasad1 approved these changes

@jsdw jsdw Awaiting requested review from jsdw

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@TarikGul @dvdplm @insipx @niklasad1