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

Avoid from overriding the Dtag accidentally #622

Closed
dadamu opened this issue Sep 15, 2021 · 3 comments · Fixed by #651
Closed

Avoid from overriding the Dtag accidentally #622

dadamu opened this issue Sep 15, 2021 · 3 comments · Fixed by #651
Assignees
Labels
kind/enhancement Enhance an already existing feature; no "New feature" to add x/profiles Module that allows to create and manage decentralized social profiles
Milestone

Comments

@dadamu
Copy link
Contributor

dadamu commented Sep 15, 2021

Feature description

Currently, updating the Dtag does not confirm the user's willing once. It might let users override it accidentally.
Adding a flag IsUpdateDtag to confirm if the user want to update the dtag maybe useful to avoid the situation mentioned above.

Implementation proposal

Add is_update_dtag field into MsgSaveProfile proto struct.

message MsgSaveProfile {
  string dtag = 1;

  string nickname = 2;

  string bio = 3];

  string profile_picture = 4;

  string cover_picture = 5;

  string creator = 6;

  bool is_update_dtag = 7;
}
@dadamu dadamu added the kind/new-feature Propose the addition of a new feature that does not yet exist label Sep 15, 2021
@RiccardoM RiccardoM added kind/enhancement Enhance an already existing feature; no "New feature" to add x/profiles Module that allows to create and manage decentralized social profiles and removed kind/new-feature Propose the addition of a new feature that does not yet exist labels Sep 30, 2021
@RiccardoM
Copy link
Contributor

Personally, I think that instead of adding a new field, we can change how the MsgSaveProfile works by making the DTag optional and then:

  1. if the user does not yet have a profile and the DTag is not specified, an error is returned
  2. if the user does have a profile and the DTag is not specified, the old DTag is preserved
  3. if the user does have a profile and the DTag is specified, the new DTag will replace the old one

This would also require the CLI command to be updated by having 0 required arguments and make the DTag an optional flag instead.

Please @bragaz let me know what you think as well

@RiccardoM RiccardoM added this to the v2.1.0 milestone Sep 30, 2021
@leobragaz
Copy link
Contributor

leobragaz commented Oct 4, 2021

@RiccardoM I would prepend for this solution instead adding an extra field to the msg.
The implementation should be easy to handle since it's pretty much similar to how we handle the other CLI command fields.
I think we should write an ADR anyway to track the changes. If you want, I can proceed to draft it and later, to implement it if passed.

@RiccardoM
Copy link
Contributor

@bragaz Ok, you can go ahead with the ADR

@leobragaz leobragaz self-assigned this Oct 7, 2021
mergify bot pushed a commit that referenced this issue Oct 13, 2021
## Description

This PR introduce the ADR-009 about the preservation of DTags from accidentals overrides.
Discussed here: #622.

---

### 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...

- [x] 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
- [x] targeted the correct branch (see [PR Targeting](https://github.com/desmos-labs/desmos/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] 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
- [x] 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)
RiccardoM pushed a commit that referenced this issue Oct 18, 2021
## Description

This PR implements what's described inside [ADR-009](#645).
Closes: #622 .

---

### 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...

- [x] 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
- [x] 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...

- [x] 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
- [x] 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Enhance an already existing feature; no "New feature" to add x/profiles Module that allows to create and manage decentralized social profiles
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants