Skip to content

Add Create Site notification#15059

Merged
ravishanker merged 16 commits intodevelopfrom
feature/create-site-notification
Jul 23, 2021
Merged

Add Create Site notification#15059
ravishanker merged 16 commits intodevelopfrom
feature/create-site-notification

Conversation

@renanferrari
Copy link
Copy Markdown
Contributor

@renanferrari renanferrari commented Jul 23, 2021

Fixes #14639

This PR builds on top of the work done in #14681 and #14691 to implement the remaining parts of the Create Site notification. I think the changes are pretty straightforward, but let me know if you have any questions.

A few improvements were made to make sure we stick to the following requirements:

  1. The notification is only scheduled when the user is logged in and doesn't have any sites.
  2. Once it's scheduled, two notifications are shown. The first one is shown a day after the user opened the app and the second one a week after the first.
  3. If the user creates a site or logs out after the notification has been scheduled, the notification isn't shown.

In the future, it would be good to combine the implementation here with the one we used for Blogging Reminders.

To test

Run the new unit tests. Additionally:

1. Notification is scheduled

  1. Clear app data.
  2. Login with an account that doesn't have any sites.
  3. Notice the main screen.
  4. On logcat, notice a message like this, indicating both notifications were scheduled:
WM-SystemJobScheduler: Scheduling work ID 1182934f-f3fc-41f1-954a-43cd9eb3f626 Job ID 1
WM-SystemJobScheduler: Scheduling work ID aa8ae790-4fc1-497e-b4db-62141fa94ddd Job ID 2
  1. Close the app and open it again.
  2. Make sure you don't see any new scheduling message on logcat.

2. Notification is not scheduled

  1. Clear app data.
  2. Login with an account that has at least one site.
  3. Notice the main screen.
  4. On logcat, make sure you don't see any scheduling message.

3. Notification is shown

To test if the notification is shown, I recommend changing the following line from DAYS to MINUTES.

  1. Clear app data and run the steps from test 1.
  2. Verify that both notifications appear after the initial delay.
  3. After the notifications are shown, click on them.
  4. Notice the site creation flow.
  5. On logcat, make sure you don't see any scheduling message.

4. Notification is not shown

  1. Clear app data and run the steps from test 1.
  2. Logout from the app or create a new site.
  3. Verify that the notifications don't appear after the initial delay.

Regression Notes

  1. Potential unintended areas of impact
    None.

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

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

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

peril-wordpress-mobile bot commented Jul 23, 2021

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

@peril-wordpress-mobile
Copy link
Copy Markdown

You can test the changes on this Pull Request by downloading the APKs:

@ravishanker ravishanker merged commit 1ffb585 into develop Jul 23, 2021
@ravishanker ravishanker deleted the feature/create-site-notification branch July 23, 2021 05:11
fun `cancel calls LocalNotificationScheduler with correct type`() {
createSiteNotificationScheduler.cancelCreateSiteNotification()
verify(localNotificationScheduler).cancelScheduledNotification(argThat { this == CREATE_SITE })
}
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.

Good job on writing test 👍

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

2 participants