-
Notifications
You must be signed in to change notification settings - Fork 46
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
fix: fixed how profiles-related data are deleted #784
Conversation
Codecov Report
@@ Coverage Diff @@
## master #784 +/- ##
==========================================
- Coverage 81.30% 81.20% -0.11%
==========================================
Files 76 76
Lines 6495 6460 -35
==========================================
- Hits 5281 5246 -35
- Misses 970 972 +2
+ Partials 244 242 -2
Continue to review full report at Codecov.
|
…-links-fixes # Conflicts: # x/profiles/legacy/v4/store_test.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
}, | ||
check: func(ctx sdk.Context) { | ||
kvStore := ctx.KVStore(keys[types.StoreKey]) | ||
require.False(t, kvStore.Has(profilestypes.ApplicationLinkClientIDKey("client_id"))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
require.False(t, kvStore.Has(profilestypes.ApplicationLinkClientIDKey("client_id"))) | |
require.False(t, kvStore.Has(v4.ApplicationLinkClientIDKey("client_id"))) |
The client id key of profilesv4 is not the same as the new one. We should check old key is deleted properly or not, instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, thanks for spotting this! 🙏
…app-chain-links-fixes � Conflicts: � x/profiles/legacy/v4/store_test.go
## Description This PR adds the hooks to the `x/profiles` keeper, allowing other keepers to react to events such as profiles saving, deletion and others. Depends-On: #784 --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/desmos-labs/desmos/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://docs.cosmos.network/v0.44/building-modules/intro.html) - [ ] included the necessary unit and integration [tests](https://github.com/desmos-labs/desmos/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable)
Description
This PR fixes how DTag transfer requests, app links and chain links are deleted. Particularly, there was a bug inside the
DeleteAllUserAppLinks
that did not remove all the keys (theApplicationLinkClientIDKey
was left there).I have replaced the individual key removal from within that method with a call to the properly implemented
DeleteApplicationLink
instead.I have also improved
DeleteApplicationLink
andDeleteChainLink
methods to require anApplicationLink
and aChainLink
respectively. Now they do not return any error if the link is not found, and that responsibility (existence check) is moved to the called.Depends-On: #787
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change