Skip to content
This repository was archived by the owner on Aug 2, 2022. It is now read-only.

Handle case newaccount in Account Query DB - #9382

Merged
b1bart merged 8 commits into
release/2.0.xfrom
epe-255-account-querydb
Aug 26, 2020
Merged

Handle case newaccount in Account Query DB#9382
b1bart merged 8 commits into
release/2.0.xfrom
epe-255-account-querydb

Conversation

@nickjjzhao

@nickjjzhao nickjjzhao commented Aug 10, 2020 •
edited by heifner
Loading

Copy link
Copy Markdown
Contributor

Change Description

To fix issue #9278, adding code to handle case newaccount so new created accounts can be returned by calling API get_accounts_by_authorizers.

Change Type

Select ONE

  • Documentation
  • Stability bug fix
  • Other
  • Other - special case

API Changes

  • API Changes

@nickjjzhao
nickjjzhao requested a review from b1bart August 10, 2020 19:11
Platinumwrist
Platinumwrist approved these changes Aug 10, 2020
View reviewed changes

@Platinumwrist Platinumwrist left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

So this will help with permission and update to new user and owners that's great look great as well

jeffreyssmith2nd
jeffreyssmith2nd suggested changes Aug 10, 2020
View reviewed changes

@jeffreyssmith2nd jeffreyssmith2nd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This PR should be against develop not master

@jeffreyssmith2nd

jeffreyssmith2nd commented Aug 10, 2020

Copy link
Copy Markdown
Contributor

This PR should be against develop not master

And if its something we plan to backport, an additional PR should be opened against the release/2.0.x branch

@heifner

heifner commented Aug 10, 2020

Copy link
Copy Markdown
Contributor

Can we have some tests added to cover this case please.

@b1bart

b1bart commented Aug 10, 2020

Copy link
Copy Markdown
Contributor

This PR should be against develop not master

The bug is against 2.0.x so, we should push to develop and backport or commit to 2.0.x and upport eitherway

@b1bart

b1bart commented Aug 10, 2020

Copy link
Copy Markdown
Contributor

Can we have some tests added to cover this case please.

It looks like the original author omitted tests in general (hey, that was me). I agree we should add them

@b1bart
b1bart changed the base branch from master to release/2.0.x August 10, 2020 19:28
@b1bart

b1bart commented Aug 10, 2020

Copy link
Copy Markdown
Contributor

I went ahead and adjusted this PR's base to release/2.0.x

@nickjjzhao

nickjjzhao commented Aug 10, 2020

Copy link
Copy Markdown
Contributor Author

Can we have some tests added to cover this case please.

Okay, I will add some tests.

@nickjjzhao

nickjjzhao commented Aug 11, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

This PR should be against develop not master

Looks like feature "Account Query DB" has not been added to the develop branch yet as per EPE-106.

@b1bart b1bart mentioned this pull request Aug 11, 2020
Merged
1 of 7 tasks
jeffreyssmith2nd
jeffreyssmith2nd reviewed Aug 19, 2020
View reviewed changes

@jeffreyssmith2nd jeffreyssmith2nd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tests look good to me.

Comment thread plugins/chain_plugin/test/test_account_query_db.cpp Resolved
Comment thread plugins/chain_plugin/test/test_account_query_db.cpp Resolved
@b1bart
b1bart self-requested a review August 19, 2020 15:52
b1bart
b1bart suggested changes Aug 19, 2020
View reviewed changes
Comment thread plugins/chain_plugin/test/CMakeLists.txt Resolved
Comment thread plugins/chain_plugin/test/test_account_query_db.cpp Resolved
@b1bart
b1bart merged commit 091b528 into release/2.0.x Aug 26, 2020
@b1bart
b1bart deleted the epe-255-account-querydb branch August 26, 2020 15:20
@matthewdarwin matthewdarwin mentioned this pull request Aug 27, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

3 more reviewers
@jeffreyssmith2nd jeffreyssmith2nd jeffreyssmith2nd approved these changes
@Platinumwrist Platinumwrist Platinumwrist approved these changes
@b1bart b1bart b1bart approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@nickjjzhao @jeffreyssmith2nd @heifner @b1bart @Platinumwrist @arhag @larryk85