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

Remove ens name from local database #10134

Closed
wants to merge 2 commits into from
Closed

Remove ens name from local database #10134

wants to merge 2 commits into from

Conversation

vkjr
Copy link
Contributor

@vkjr vkjr commented Mar 5, 2020

fixes #8497

Summary

Added option to remove ENS name from phone.

Design notes

Removing name doesn't require contract transactions, so existing figma designs not fully applicable.

Platforms

  • Android
  • iOS

Steps to test

  • Open Status
  • remove one of previously added ens names
  • make sure it is not present among available ones

status: ready

@vkjr vkjr requested a review from a team March 5, 2020 13:05
@vkjr vkjr self-assigned this Mar 5, 2020
@ghost
Copy link

ghost commented Mar 5, 2020

Pull Request Checklist

  • Docs: Updated the documentation, if affected
  • Docs: Added or updated inline comments explaining intention of the code
  • Tests: Ensured that all new UI elements have been assigned accessibility IDs
  • Tests: Signaled need for E2E tests with label, if applicable
  • Tests: Briefly described what was tested and what platforms were used
  • UI: In case of UI changes, ensured that UI matches Figma
  • UI: In case of UI changes, requested review from a Core UI designer
  • UI: In case of UI changes, included screenshots of implementation

@status-im-auto
Copy link
Member

status-im-auto commented Mar 5, 2020

Jenkins Builds

Commit #️⃣ Finished (UTC) Duration Platform Result
87241da #1 2020-03-05 13:15:11 ~9 min android-e2e 📄log
✔️ 87241da #1 2020-03-05 13:15:22 ~9 min ios 📦ipa 📲
87241da #1 2020-03-05 13:16:20 ~10 min android 📄log
✔️ 9749971 #2 2020-03-10 09:05:27 ~9 min ios 📦ipa 📲
9749971 #2 2020-03-10 09:05:29 ~9 min android 📄log
9749971 #2 2020-03-10 09:06:37 ~10 min android-e2e 📄log
9749971 #3 2020-05-12 16:16:29 ~2 min android 📄log
9749971 #3 2020-05-12 16:18:13 ~3 min android-e2e 📄log

@flexsurfer
Copy link
Member

flexsurfer commented Mar 5, 2020

@vkjr so it only partially fixes the issue? because the issue says unassociated the whisper key from relevant ENS name and Make sure tx state and errors are correctly handled.

@vkjr
Copy link
Contributor Author

vkjr commented Mar 5, 2020

@flexsurfer, correct. I just wrote a comment in the original issue to clarify if we need calling a transaction on registrar. As far as I understand there is no appropriate function in a contract that would stop name resolving without its releasing.

@vkjr vkjr mentioned this pull request Mar 5, 2020
(fx/defn remove-username
{:events [::remove-username]}
[{:keys [db] :as cofx} name]
(let [names (get-in db [:multiaccount :usernames] [])
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation is a bit weird here, it should be either one space after each binding form or all expressions should be aligned

@flexsurfer
Copy link
Member

hey @vkjr what's the status of this PR?

@flexsurfer
Copy link
Member

@andremedeiros @vkjr what we gonna do with this one? probably we need to finish it?

@jakubgs jakubgs closed this May 12, 2020
@jakubgs jakubgs deleted the ens/remove-name branch May 12, 2020 13:50
@flexsurfer
Copy link
Member

@jakubgs why?

@jakubgs
Copy link
Member

jakubgs commented May 12, 2020

Was caught in the repo branch cleanup. Can be reopened.

@jakubgs jakubgs restored the ens/remove-name branch May 12, 2020 16:13
@jakubgs jakubgs reopened this May 12, 2020
@jakubgs
Copy link
Member

jakubgs commented May 12, 2020

Thx for the ping.

@jakubgs
Copy link
Member

jakubgs commented Jun 29, 2020

What's up with this?

@oskarth
Copy link
Contributor

oskarth commented Jul 1, 2020

Small request - could you please rebase to latest master so this commit b5fda12 gets included? This means we aren't running E2E tests on eth.prod cluster anymore, and since this happens on every push, the impact is quite big. This ensures we have more accurate metrics going forward from Jul 1 onward, which would be awesome. See https://discuss.status.im/t/user-growth-and-retention/1782 for more

@flexsurfer
Copy link
Member

this one seems dead

@flexsurfer flexsurfer closed this Jul 1, 2020
@vkjr vkjr deleted the ens/remove-name branch February 2, 2021 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[ENS] Remove name flow
6 participants