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

[Android 13] Notifications - user manually disable notifications #15407

Merged
merged 6 commits into from
Nov 15, 2022

Conversation

sujitacharya2005
Copy link
Contributor

@sujitacharya2005 sujitacharya2005 commented Oct 11, 2022

Resolves brave/brave-browser#25042

When notification permission is not there and if any of the or both privacy or rewards enabled.
Show a warning dialog to enable notification permission dialog.

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

When both rewards and privacy is on and notification permission is off Screenshot_20221028_183841 only rewards is on and notification permission is off Screenshot_20221028_183956 only privacy is on and notification permission is off Screenshot_20221028_184209
both rewards and privacy is on and notification permission is off through brave settings Screenshot_20221101_231126 only rewards is on and notification permission is off through brave settings Screenshot_20221101_231004 only privacy is on and notification permission is off through brave settings Screenshot_20221101_231156
first_time_rewards_use.mp4

image

  1. When ever dialog opens the 1st button click
  • In android 13 case will open OS dialog 1st time otherwise it will redirect to OS notification setting page.
  • Before android-13 device it will always redirect to notification setting page.

Android - 13 Emulator

20221018_160024.mp4

@sujitacharya2005 sujitacharya2005 added CI/skip-linux CI/skip-macos-x64 Do not run CI builds for macOS x64 CI/skip-ios Do not run CI builds for iOS labels Oct 11, 2022
@sujitacharya2005 sujitacharya2005 added this to the 1.46.x - Nightly milestone Oct 11, 2022
@sujitacharya2005 sujitacharya2005 force-pushed the 25042_android_13_privacy_rewards branch 3 times, most recently from c2d4ec3 to 75a23a7 Compare October 12, 2022 12:24
Copy link

@Pavneet-Sing Pavneet-Sing left a comment

Choose a reason for hiding this comment

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

LGTM, though test plan is empty, preferably add a recording for demo. Before merging, Probably better to get an approval from someone from android rewards team for the flow implementation.

@sujitacharya2005 sujitacharya2005 force-pushed the 25042_android_13_privacy_rewards branch 3 times, most recently from 76ec5f2 to 47dcebc Compare October 18, 2022 20:27
@sujitacharya2005 sujitacharya2005 force-pushed the 25042_android_13_privacy_rewards branch 2 times, most recently from e5e69d5 to 1a8e4a3 Compare October 27, 2022 06:05
@sujitacharya2005 sujitacharya2005 force-pushed the 25042_android_13_privacy_rewards branch 3 times, most recently from f32e042 to 4bfc6a2 Compare November 1, 2022 17:47
@sujitacharya2005 sujitacharya2005 force-pushed the 25042_android_13_privacy_rewards branch 3 times, most recently from 46868ab to 81b248a Compare November 9, 2022 13:38
@sujitacharya2005 sujitacharya2005 force-pushed the 25042_android_13_privacy_rewards branch 2 times, most recently from a579b8f to a733e3e Compare November 9, 2022 19:13
Copy link
Contributor

@samartnik samartnik left a comment

Choose a reason for hiding this comment

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

lgtm

private int mLaunchedFrom;

public interface DismissListener {
void onDisMiss();
Copy link
Contributor

Choose a reason for hiding this comment

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

Dismiss is a whole word, I wonder why M capitalized

@@ -159,6 +160,10 @@ public void setNewOnboardingShown(boolean isShown) {
sharedPreferencesEditor.apply();
}

public boolean isBraveRewardsEnabled() {
return mSharedPreferences.getBoolean(BraveRewardsPreferences.PREF_ADS_SWITCH, false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be isBraveAdsEnabled as function name ? as we are checking f or ADS_SWITCH pref

@sujitacharya2005 sujitacharya2005 force-pushed the 25042_android_13_privacy_rewards branch 3 times, most recently from c365006 to 9f9accf Compare November 15, 2022 08:52
@sujitacharya2005 sujitacharya2005 merged commit 3bf5ecd into master Nov 15, 2022
@sujitacharya2005 sujitacharya2005 deleted the 25042_android_13_privacy_rewards branch November 15, 2022 16:49
@srirambv
Copy link
Contributor

Verification passed on Google Pixel 6 Emulator running 1.47.91 x64 build

  • Verified steps from PR
  • Verified not earning rewards notification is shown when rewards is enabled and show ads when Brave is not running enabled and OS notification is disabled
  • Verified not receiving privacy reports is shown when privacy report is enabled and OS notification is disabled
  • Verified rewards and privacy reports notification is shown when both rewards and privacy report is enabled but OS notification is disabled
  • Verified Enable notifications for Brave rewards is shown when enabling rewards and OS level notification is disabled
privacy privacyads rewards enablerewards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/skip-ios Do not run CI builds for iOS CI/skip-macos-x64 Do not run CI builds for macOS x64
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Android 13] Notifications - user manually disable notifications
5 participants