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

WP Stories integration - implement StoryNotificationTrackerProvider interface #12068

Conversation

mzorz
Copy link
Contributor

@mzorz mzorz commented Jun 1, 2020

This PR builds on top of #12059

This PR implements the new StoryNotificationTrackerProvider interface, which allows the host app to be signaled of when a Notification is being produced by the StorySaveService in the Stories library. Then, it can properly channel the event through to its own SystemNotificationsTracker.

Also in this PR we're checking ARG_NOTIFICATION_TYPE for when an error notification is tapped and properly tracking it down in StoryComposerActivity (see onCreate() in that class).

To test:

  1. inject an error when saving an image in PhotoEditor (for example introduce a line that throws an Exception in saveImageFromPhotoEditorViewAsLoopFrameFile().
  2. run the WP app, create a new story with an image background frame and tap PUBLISH
  3. observe the error notification is shown, and also the trackShownNotification method implemented in the WordPress class is executed. (placing a breakpoint there should be enough).
  4. Finally, tap on the error notification and verify the trackTappedNotification method is executed as well.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • 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.

@mzorz mzorz added this to the 15.2 milestone Jun 1, 2020
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Jun 1, 2020

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

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Jun 1, 2020

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

Base automatically changed from feature/wp-stories-part92-notif-delete-intent to feature/wp-stories-part8-upload-service June 21, 2020 00:44
@@ -995,4 +1005,32 @@ public void onTrimMemory(final int level) {
}
}
}

private class StoryNotificationTrackerProvider implements NotificationTrackerProvider {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @mzorz what do you think about putting this class into another file? Just checking because this class is getting a bit big. Let me know though because it's also a really small class 🙏

Copy link
Contributor

@jd-alexander jd-alexander left a comment

Choose a reason for hiding this comment

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

Thanks for these changes @mzorz I followed the testing instructions and I achieved the expected outcome. I really like this implementation 😉 LGTM 🚢

@jd-alexander jd-alexander merged commit b6ba7eb into feature/wp-stories-part8-upload-service Jun 21, 2020
@jd-alexander jd-alexander deleted the feature/wp-stories-part10-tracks-notifications branch June 21, 2020 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants