Skip to content

Add "Create site" push notification#14681

Closed
planarvoid wants to merge 2 commits intodevelopfrom
feature/14639-create-site-push-notification
Closed

Add "Create site" push notification#14681
planarvoid wants to merge 2 commits intodevelopfrom
feature/14639-create-site-push-notification

Conversation

@planarvoid
Copy link
Copy Markdown
Contributor

@planarvoid planarvoid commented May 19, 2021

Fixes #14639

This draft contains partially completed "Create a site" notifications. The currently working version creates a notification in the main activity which is shown to the user without a site. What's missing is:

  • we need to set the delay when the notification is shown
  • we need to schedule multiple notifications after various delays with various copys
  • we need to update the texts of the notifications
  • Unit tests for all the classes that could be unit tested

I think it's enough to do most of the changes in the CreateSitePushScheduler. We also don't need to cancel the notifications since all of them are only shown if there are no sites connected to the account.

To test:

Regression Notes

  1. Potential unintended areas of impact

  2. What I did to test those areas of impact (or what existing automated tests I relied on)

  3. What automated tests I added (or what prevented me from doing so)

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile
Copy link
Copy Markdown

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

import org.wordpress.android.push.local.LocalPushScheduleWorker.Factory;
import org.wordpress.android.util.AppLog;
import org.wordpress.android.util.AppLog.T;
import org.wordpress.android.util.UploadWorker;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reporter: CheckStyle
Rule: com.puppycrawl.tools.checkstyle.checks.imports.UnusedImportsCheck
Severity: ERROR
File: /home/circleci/project/WordPress/src/debug/java/org/wordpress/android/WordPressDebug.java L17
Unused import - org.wordpress.android.util.UploadWorker.


import org.wordpress.android.modules.DaggerAppComponentDebug;
import org.wordpress.android.push.local.LocalPushScheduleWorker;
import org.wordpress.android.push.local.LocalPushScheduleWorker.Factory;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reporter: CheckStyle
Rule: com.puppycrawl.tools.checkstyle.checks.imports.UnusedImportsCheck
Severity: ERROR
File: /home/circleci/project/WordPress/src/debug/java/org/wordpress/android/WordPressDebug.java L14
Unused import - org.wordpress.android.push.local.LocalPushScheduleWorker.Factory.

import com.yarolegovich.wellsql.WellSql;

import org.wordpress.android.modules.DaggerAppComponentDebug;
import org.wordpress.android.push.local.LocalPushScheduleWorker;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reporter: CheckStyle
Rule: com.puppycrawl.tools.checkstyle.checks.imports.UnusedImportsCheck
Severity: ERROR
File: /home/circleci/project/WordPress/src/debug/java/org/wordpress/android/WordPressDebug.java L13
Unused import - org.wordpress.android.push.local.LocalPushScheduleWorker.

@peril-wordpress-mobile
Copy link
Copy Markdown

You can test the changes on this Pull Request by downloading the APK here.

@renanferrari
Copy link
Copy Markdown
Contributor

renanferrari commented May 19, 2021

@ravishanker Before you start working on the missing functionality mentioned by @planarvoid, could you please split this into another PR with everything that we might want to reuse while working on other notifications? I'm thinking mainly of all the changes related to WorkManager initialization, including the factories setup, etc. (also, great work on those, @planarvoid!)

@planarvoid planarvoid closed this May 28, 2021
@renanferrari renanferrari mentioned this pull request Jul 23, 2021
3 tasks
@jkmassel jkmassel deleted the feature/14639-create-site-push-notification branch October 17, 2024 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement "Create a site" push notification

4 participants