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

Reader Post: Add receivesCommentNotifications property #17322

Merged
merged 7 commits into from
Oct 15, 2021

Conversation

dvdchr
Copy link
Contributor

@dvdchr dvdchr commented Oct 14, 2021

Depends on: wordpress-mobile/WordPressKit-iOS#452 👈 Please review this first 🙂

Adds receivesCommentNotifications to ReaderPost. This will be used by the upcoming PRs to enable post subscribers to receive push notifications. The default value of the receivesCommentNotifications is set to false because the push notification feature is opt-out by default, so this checks out.

To Test

  • Go to the Reader section, and verify that all the posts are loaded correctly.
  • Tap one of the posts, verify that the post is displayed correctly.

Regression Notes

  1. Potential unintended areas of impact
    Potential crash when parsing the new property?

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Ensure that all the screens depending on ReaderPost objects work fine.

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

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.

@dvdchr dvdchr added this to the 18.5 milestone Oct 14, 2021
@dvdchr dvdchr requested review from aerych and ScoutHarris October 14, 2021 16:06
@dvdchr dvdchr self-assigned this Oct 14, 2021
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Oct 14, 2021

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

Generated by 🚫 dangerJS

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Oct 14, 2021

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

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Oct 14, 2021

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

Copy link
Member

@aerych aerych left a comment

Choose a reason for hiding this comment

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

Howdy @dvdchr 👋
Looks great! I did a smoke test of the reader and everything seemed to work as expected. Changes to the data model look good to me.
:shipit: !

@dvdchr
Copy link
Contributor Author

dvdchr commented Oct 15, 2021

Thank you @aerych !

@dvdchr dvdchr enabled auto-merge October 15, 2021 11:15
@dvdchr dvdchr merged commit 46f5e4c into develop Oct 15, 2021
@dvdchr dvdchr deleted the feature/reader-post-add-receivesNotification-property branch October 15, 2021 12:49
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.

2 participants