Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Add the ability to reset Brave Rewards #1642

Closed
anthonypkeane opened this issue Oct 14, 2019 · 3 comments
Closed

Add the ability to reset Brave Rewards #1642

anthonypkeane opened this issue Oct 14, 2019 · 3 comments

Comments

@anthonypkeane
Copy link

@anthonypkeane commented on Thu Sep 05 2019

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)


@tmancey commented on Fri Sep 06 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 commented on Fri Sep 06 2019

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


@srirambv commented on Fri Sep 06 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 commented on Fri Sep 06 2019

@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 commented on Fri Sep 06 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 commented on Fri Sep 06 2019

@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 commented on Fri Sep 06 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 commented on Fri Sep 06 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 commented on Fri Sep 06 2019

@srirambv is this reset for QA or end users?


@tmancey commented on Fri Sep 06 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 commented on Fri Sep 06 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 commented on Fri Sep 06 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 commented on Fri Sep 06 2019

@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 commented on Fri Sep 06 2019

  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 commented on Fri Sep 06 2019

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


@anthonypkeane commented on Fri Sep 06 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

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 commented on Fri Sep 06 2019

@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 commented on Fri Sep 06 2019

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


@kylehickinson commented on Mon Sep 23 2019

@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 commented on Mon 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 commented on Mon Sep 23 2019

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 commented on Thu Oct 10 2019

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

@kylehickinson kylehickinson added Epic: Rewards blocked If a ticket is blocked for some reason, if not using a sub-block label, please provide info in issue release-notes/exclude enhancement and removed Brave Rewards labels Oct 14, 2019
@NejcZdovc
Copy link

We will be margin brave/brave-browser#10064 really soon, so this will be implementable now

@NejcZdovc
Copy link

we should also add link to this reset button in clear data (#1658)

@kylehickinson
Copy link
Collaborator

Closed as this feature is no longer supported in new Rewards on Brave iOS

@kylehickinson kylehickinson added closed/wontfix and removed Epic: Rewards blocked If a ticket is blocked for some reason, if not using a sub-block label, please provide info in issue enhancement release-notes/exclude labels Jan 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants