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

Move Site Announcements off Stream #977

Open
wants to merge 6 commits into
base: the-future
Choose a base branch
from

Conversation

NuckChorris
Copy link
Member

What

  • Create a SiteAnnouncementView table joining User with SiteAnnouncement and tracking read+seen status
    • The separate read+seen will allow us to show a different unread announcement each visit, by showing the least-recently-seen one!
  • Expose the site announcement view through graphql

The next step will be rolling out a feature flag for client-side switching of the site announcement view to be backed by this

Why

  • Stream costs a lot of money, we should reserve it for actual feeds
  • Marking announcements as seen is broken for some reason
  • It's just all kinds of janky to use feeds for this

Checklist

  • All files pass Rubocop
  • Any complex logic is commented
  • Any new systems have thorough documentation
  • Any user-facing changes are behind a feature flag (or: explain why they can't be)
  • All the tests pass
  • Tests have been added to cover the new code

@NuckChorris NuckChorris force-pushed the nuck/site-announcements-off-stream branch from 4838163 to aa15fb2 Compare March 16, 2021 04:41
@NuckChorris NuckChorris force-pushed the nuck/site-announcements-off-stream branch from aa15fb2 to 6459a31 Compare March 16, 2021 05:18
@codeclimate
Copy link

codeclimate bot commented Mar 16, 2021

Code Climate has analyzed commit 6459a31 and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 2

The test coverage on the diff in this pull request is 90.1% (80% is the threshold).

This pull request will bring the total coverage in the repository to 73.3%.

View more on Code Climate.

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.

1 participant