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.

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

feat: deletion of accounts from nodes #205

Merged
merged 1 commit into from Aug 13, 2019

Conversation

@bstrie
Copy link
Collaborator

commented Aug 12, 2019

Adds a new HTTP endpoint that allows the admin of a node to delete accounts on that node.

Part of #196

@bstrie bstrie requested a review from emschwartz Aug 12, 2019

@emschwartz
Copy link
Collaborator

left a comment

This PR should also include tests for the deletion behavior in the Redis store tests (those are written as integration tests now because they require talking to an actual Redis instance rather than mocks).

crates/interledger-store-redis/src/store.rs Outdated Show resolved Hide resolved
@bstrie

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 13, 2019

@emschwartz test added, let me know if it looks good to you and I'll squash and merge.

let old_len = accounts.len();
store.remove_account(accounts[0].id()).and_then(move |_| {
store.get_all_accounts().and_then(move |accounts| {
assert_eq!(old_len, accounts.len() + 1);

This comment has been minimized.

Copy link
@emschwartz

emschwartz Aug 13, 2019

Collaborator

I would check to make sure the specific account you tried to remove is no longer there

This comment has been minimized.

Copy link
@bstrie

bstrie Aug 13, 2019

Author Collaborator

I wanted to ask, are all of these tests racing with each other on the same instance of redis? Because if so, then it's going to be difficult to test for this at all until #186 allows us to create accounts with specific IDs.

@bstrie bstrie force-pushed the bstrie:deleteaccount branch from 26bb30d to 958684b Aug 13, 2019

@bstrie

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 13, 2019

Changes made, commits squashed and rebased.

@bstrie bstrie merged commit 6c732fd into interledger-rs:master Aug 13, 2019

1 check passed

ci/circleci: build Your tests passed on CircleCI!
Details

dora-gt added a commit to dora-gt/interledger-rs that referenced this pull request Aug 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.