Skip to content
This repository has been archived by the owner on Oct 15, 2019. It is now read-only.

Add the ability to reset Brave Rewards #204

Closed
anthonypkeane opened this issue Sep 5, 2019 · 23 comments
Closed

Add the ability to reset Brave Rewards #204

anthonypkeane opened this issue Sep 5, 2019 · 23 comments
Assignees

Comments

@anthonypkeane
Copy link

Add the ability for a user to Reset Brave Rewards

As on Android, resetting will return the users device back to "new user" state

In Settings, under OTHER SETTINGS, Sync, add a section called Brave Rewards

Here add the following

Reset Brave Rewards

Tapping it pops up a modal with

Title:
Reset Brave Rewards
This will delete your Brave wallet and reset all of your Brave Rewards data

Cancel | Confirm

If a user hasn't joined rewards, hide the option.

(Pls don't work on implementing this yet)

@anthonypkeane anthonypkeane self-assigned this Sep 5, 2019
@tmancey tmancey added this to the Rewards and Ads on iOS milestone Sep 6, 2019
@tmancey
Copy link

tmancey commented Sep 6, 2019

To shutdown Brave ads execute AdsServiceImpl::ShutdownBatAds. @NejcZdovc Could you please let iOS team know how to safely reset state for Ledger, thanks

@NejcZdovc
Copy link

@tmancey there is no mechanism in place to do that currently

@srirambv
Copy link

srirambv commented Sep 6, 2019

We also need to provide an option to hide the rewards button from URL bar if its not enabled.

Here's my suggestion for implementation

Settings
|_ Other Settings
    |_ Sync
    |_ Rewards
       |_ Hide rewards button (Disabled state if rewards is enabled)
       |_ Reset Rewards

@kylehickinson
Copy link
Collaborator

@anthonypkeane @srirambv

  1. Are these just for QA
  2. Are they MVP?
  3. We don't have other settings/rewards settings in our brave-ios side, is this supposed to be added to the brave-ios client side where other client settings are like shields etc or added to the Brave Rewards settings page
  4. As far as I know the only way to "reset" rewards is to dump all the data… This is not exactly a super safe thing to do, since there's no Shutdown for ledger

@srirambv
Copy link

srirambv commented Sep 6, 2019

Reset rewards should be MVP. For 4, Android does a force relaunch to remove any files relating to rewards once its turned off.

@kylehickinson
Copy link
Collaborator

@srirambv So you click "reset rewards" and it kills the app? Or does it show a message saying "Rewards will be reset when the app is closed". Neither of these sound like a good solution/user experience

@srirambv
Copy link

srirambv commented Sep 6, 2019

@kylehickinson this is what Android shows when you reset rewards.
image
Confirm shows the following message
image
Selecting Relaunch restarts the app with Rewards disabled(wallet/ac/tips/ads everything removed).
If something goes wrong then this shows up when trying to reset
image

cc: @SergeyZhukovsky @samartnik to add more details on the background working of this

@kylehickinson
Copy link
Collaborator

kylehickinson commented Sep 6, 2019

Just an FYI iOS cannot force a relaunch. We can 1. crash the app on purpose (not good), 2. Basically tell the user to bring up the multitasking tray and swipe up to force-quit it

@NejcZdovc
Copy link

@srirambv is this reset for QA or end users?

@tmancey
Copy link

tmancey commented Sep 6, 2019

@srirambv @anthonypkeane Android does not shutdown/cleanup properly (on my last check) as this was never added as part of rewards but was added, just in Android. We need a ticket for brave-core where we can implement a solution for Rewards like Ads which has a ShutdownBatAds function for this.

@srirambv
Copy link

srirambv commented Sep 6, 2019

@NejcZdovc this is not specific for QA. This is an end user feature that was implemented. @brave/android folks can give more info on what's being done in Android

@tmancey
Copy link

tmancey commented Sep 6, 2019

@NejcZdovc was used mainly to allow users to switch between staging/production environments. @anthonypkeane @srirambv Can QA/dev right now not just delete the app as Rewards needs the ability to shutdown its service for this to work correctly?

@NejcZdovc
Copy link

@brave/android can you please explain how this works? Is just file deleted? Asking as we don't have support for this on desktop

@anthonypkeane
Copy link
Author

  1. Are these just for QA
    No. Support also use it if a users wallet is messed up. This option will prevent users from having to delete and re-install the app
  1. Are they MVP?
    If it can be done easily yes, if you say it is a tonne of work, let's discuss.
  1. We don't have other settings/rewards settings in our brave-ios side, is this supposed to be added to the brave-ios client side where other client settings are like shields etc or added to the
    Yes, as above, we need to create a Brave Rewards section is settings.
    -> I do need to chat @jamesmudgett on this too however as we may need also use this setting as a means to navigate to Rewards (people may not know what the BAT logo does)

Brave Rewards settings page

  1. As far as I know the only way to "reset" rewards is to dump all the data… This is not exactly a super safe thing to do, since there's no Shutdown for ledger
    ok, please link in with the rewards team as above ^^

@kylehickinson
Copy link
Collaborator

@anthonypkeane you can't get to Rewards settings page if rewards is disabled

@anthonypkeane
Copy link
Author

Just an FYI iOS cannot force a relaunch. We can 1. crash the app on purpose (not good), 2. Basically tell the user to bring up the multitasking tray and swipe up to force-quit it

This was the approach on Android as the OS required it. You might decide on a OS specific way here.
For clarify, I'm not saying we need to close the app, please LMK how it can be done on for iOS

@anthonypkeane
Copy link
Author

@anthonypkeane you can't get to Rewards settings page if rewards is disabled

Ok, this ticket has exploded. I will provide a design for the simple UI to clear things up on this side. cc @jamesmudgett

@kylehickinson
Copy link
Collaborator

There is no OS specific way to do this on iOS. Like I and others mentioned before there's no shutdown for ledger, so it's unsafe to just delete all the files to reset it. We can nil everything out and hope it cleans up properly, and then delete files but that's about as good as it gets. It wouldn't require the app to be closed, just that next time you open the panel it'd be back to the join rewards page. Just unsure whether or not there'll be side effects

@anthonypkeane anthonypkeane self-assigned this Sep 6, 2019
@kylehickinson
Copy link
Collaborator

@anthonypkeane This will be available in the debug menu (#166), is that enough for MVP until a more stable solution is available in the actual native libs?

@tmancey
Copy link

tmancey commented Sep 23, 2019

@anthonypkeane @NejcZdovc @kylehickinson For this to be implemented we need a mechanism to shutdown Ledger so that files, databases etc. are not no longer changed. Then in the service layer for iOS we can safely remove files, however at this point if we remove files, they maybe rewritten by the native library. @NejcZdovc your thoughts please?

@anthonypkeane
Copy link
Author

Not a requirement for V1 MVP, but if there is time, please add this back in @tmancey
However, this will be needed at some point soon so please work on the backend components as needed.

@kylehickinson kylehickinson removed this from the Rewards and Ads on iOS milestone Sep 25, 2019
@kylehickinson
Copy link
Collaborator

Won't have for MVP as it requires a lot of work on ledger side to ensure a proper reset

@anthonypkeane
Copy link
Author

Issue moved to brave/brave-ios #1642 via ZenHub

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants