Local Weekly Roundup by diegoreymendez · Pull Request #17066 · wordpress-mobile/WordPress-iOS

diegoreymendez · 2021-08-23T11:40:03Z

Implements the Weekly Roundup for iOS (with Local Notifications).

WPKit PR: wordpress-mobile/WordPressKit-iOS#438

Things to do in upcoming PRs:

Add configuration option to turn these notifications ON / OFF.

To test:

Test how the notifications look like and how they react to taps:

Check the debug menu at Me > App Setting > Debug > Weekly Roundup
Tap on schedule immediately (the other options can be used too but they don't guarantee a time of delivery). Keep in mind the static fallback / static notification is currently disabled so it will never show at this time.
Background the App, and see that the notifications come up.
Tap on any of them.

Test that the trigger time is right:

Place a breakpoint at BackgroundTasksCoordinator.swift:149.
Run the App.
When the BP is hit, po nextDate and make sure it's next Monday at 10 am (device time).
Try this also using another calendar locale.

Test tracking:

In your console, filter the messaged by "Tracked:".

Check the debug menu at Me > App Setting > Debug > Weekly Roundup
Tap "Schedule Immediately".
You should see up to a maximum of 5 messages that look like this:

🔵 Tracked: notification_scheduled <notification_type: weekly_roundup, site_id: 105284648>

Tap on one of the notifications, the App will open, and you should see a tracked notification like this:

🔵 Tracked: notification_tapped <notification_type: weekly_roundup, site_id: 105284648 >

Make sure you also see these events in the live analytics tool.

Regression Notes

Potential unintended areas of impact

None.

What I did to test those areas of impact (or what existing automated tests I relied on)
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 unit tests for my changes.
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 · 2021-08-23T11:40:08Z

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

peril-wordpress-mobile · 2021-08-23T11:40:08Z

You can trigger an installable build for these changes by visiting CircleCI here.

…is in the foreground.

…g saved forever.

Gio2018

Hey @diegoreymendez this worked good: Notification looked correct, tapping takes to the stats of the right site, nextDate looks correct and the tracks show correctly in both the console and the live view. I just left a few code comments that might be worth considering.

Gio2018 · 2021-08-31T18:36:17Z

        // 21-Oct-2017: We are only handling background URLSessions initiated by the share extension so there
        // is no need to inspect the identifier beyond the simple check here.
        if identifier.contains(WPAppGroupName) {
+            /*
            let manager = ShareExtensionSessionManager(appGroup: WPAppGroupName, backgroundSessionIdentifier: identifier)
            manager.backgroundSessionCompletionBlock = completionHandler
-            manager.startBackgroundSession()
+            manager.startBackgroundSession()*/
        }
    }


This method does not seem to do anything anymore, can we just remove it?

I actually had disabled this because I was having some trouble while testing, but it was unrelated. I've re-enabled this.

Gio2018 · 2021-08-31T21:48:54Z

Oh also it looks like some new files are missing the Jetpack target membership, can you please fix that?

peril-wordpress-mobile · 2021-09-01T13:58:14Z

	Warnings
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

diegoreymendez · 2021-09-01T15:00:09Z

@Gio2018 - Ready for another look.

Gio2018

diegoreymendez added 8 commits August 4, 2021 14:17

WIP: implementing local weekly roundup.

3337e33

Merge branch 'develop' into try/local-weekly-roundup

23757fb

Updates to the latest WeeklyRoundup code.

5b1d7be

Implementing the stats data retrieval for the Weekly Roundup.

ca4f3e4

Alpha implementation of local Weekly Roundup.

0b6d253

WIP: implementing Weekly Roundup.

f3de274

Improves some parts of the code.

8c2d3e2

WIP

356d2f9

diegoreymendez commented Aug 23, 2021

View reviewed changes

Comment thread WordPress/Classes/System/WordPressAppDelegate.swift Outdated

diegoreymendez commented Aug 23, 2021

View reviewed changes

Comment thread WordPress/Classes/ViewRelated/Stats/SiteStatsDashboardViewController.swift

diegoreymendez commented Aug 23, 2021

View reviewed changes

Comment thread Podfile Outdated

diegoreymendez added 10 commits August 23, 2021 14:06

Merges the latest from develop.

1585671

Fixed some minor issues in weekly roundup

0bffb25

Fixes the opening of the stats section of the App.

98d8ee5

Implements handling for the Weekly Roundup notification when the App …

f90bbf7

…is in the foreground.

We're now removing the last selected date from stats to avoid it bein…

980ddc7

…g saved forever.

Disables the static notification from weekly roundup.

b99828f

Changes the notifications to show on Monday instead of Sunday.

d055835

Removed an unnecessary variable.

d9e8e5e

Removes a commented line of code.

38d3223

Re-enabled some code that was disabled while testing.

366b01c

diegoreymendez mentioned this pull request Aug 26, 2021

Add likes to stats summary data wordpress-mobile/WordPressKit-iOS#438

Merged

diegoreymendez self-assigned this Aug 26, 2021

diegoreymendez added Notifications [Type] Enhancement labels Aug 26, 2021

diegoreymendez added this to the 18.2 milestone Aug 26, 2021

diegoreymendez added 2 commits August 31, 2021 11:05

Merges the latest from develop.

ca177ee

Updates WPKit.

cbb2e05

diegoreymendez requested a review from Gio2018 August 31, 2021 09:24

diegoreymendez marked this pull request as ready for review August 31, 2021 09:24

Removes some code that's not necessary.

cff3091

Gio2018 reviewed Aug 31, 2021

View reviewed changes

Adds some files that were missing from the Jetpack app target.

8545447

diegoreymendez added 5 commits September 1, 2021 15:59

Removes an unnecessary print directive.

990329d

Changes the syntax of some lines of code.

33585cb

Merge branch 'develop' into try/local-weekly-roundup

a2766c4

Removes an empty method.

078e949

Re-enables some code I had disabled.

982cf43

diegoreymendez requested a review from Gio2018 September 1, 2021 14:59

Gio2018 approved these changes Sep 1, 2021

View reviewed changes

diegoreymendez merged commit 7d22bce into develop Sep 1, 2021

diegoreymendez deleted the try/local-weekly-roundup branch September 1, 2021 16:27

This was referenced Dec 8, 2021

NSGenericException: Collection __NSCFSet was mutated while being enumerated #17639

Closed

Use derived context for WeeklyRoundup background task #17640

Closed

Use derived context for WeeklyRoundup background task #17642

Merged

Uh oh!

Conversation

diegoreymendez commented Aug 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Things to do in upcoming PRs:

To test:

Test how the notifications look like and how they react to taps:

Test that the trigger time is right:

Test tracking:

Regression Notes

Uh oh!

peril-wordpress-mobile Bot commented Aug 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-wordpress-mobile Bot commented Aug 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Gio2018 left a comment

Choose a reason for hiding this comment

Uh oh!

Gio2018 Aug 31, 2021

Choose a reason for hiding this comment

Uh oh!

diegoreymendez Sep 1, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Gio2018 commented Aug 31, 2021

Uh oh!

peril-wordpress-mobile Bot commented Sep 1, 2021

Uh oh!

diegoreymendez commented Sep 1, 2021

Uh oh!

Gio2018 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

diegoreymendez commented Aug 23, 2021 •

edited

Loading

peril-wordpress-mobile Bot commented Aug 23, 2021 •

edited

Loading

peril-wordpress-mobile Bot commented Aug 23, 2021 •

edited

Loading