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

System notifications (proof of concept) #12213

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

rubhanazeem
Copy link
Member

@rubhanazeem rubhanazeem commented Feb 16, 2022

This PR is just a proposal for adding system notifications to OBS using the Notifications API and ActionCable.

To test the notifications open the rails console and try to broadcast for the logged-in user.

NotificationsChannel.broadcast_to(User.first, {title: 'Title', body: 'Body'})

Be sure to replace User.first with a logged-in user.

@github-actions github-actions bot added the Frontend Things related to the OBS RoR app label Feb 16, 2022
@rubhanazeem rubhanazeem force-pushed the system-notifications branch 3 times, most recently from b99287a to f69cb7a Compare February 17, 2022 13:25
src/api/Gemfile.lock Outdated Show resolved Hide resolved
@danidoni
Copy link
Contributor

What about creating a rake task to send the test notification?

@rubhanazeem
Copy link
Member Author

rubhanazeem commented Feb 17, 2022

What about creating a rake task to send the test notification?

We don't need one atm. For now, we'll just go with rails c. Thanks for pointing that out, I've added the test instructions in the description.

@rubhanazeem rubhanazeem force-pushed the system-notifications branch 2 times, most recently from d00a5f6 to 02ae6d3 Compare February 17, 2022 14:46
Copy link
Contributor

@krauselukas krauselukas left a comment

Choose a reason for hiding this comment

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

@rubhanazeem nice :)

@github-actions github-actions bot added the Documentation 📖 Things regarding our documentation label Feb 21, 2022
@codecov
Copy link

codecov bot commented Feb 21, 2022

Codecov Report

Merging #12213 (39196c0) into master (29fe2bf) will increase coverage by 3.13%.
The diff coverage is 100.00%.

❗ Current head 39196c0 differs from pull request most recent head 52245d9. Consider uploading reports for the commit 52245d9 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12213      +/-   ##
==========================================
+ Coverage   88.21%   91.35%   +3.13%     
==========================================
  Files         702      660      -42     
  Lines       24091    23090    -1001     
==========================================
- Hits        21253    21093     -160     
+ Misses       2838     1997     -841     

@rubhanazeem rubhanazeem force-pushed the system-notifications branch 2 times, most recently from 4f3ed48 to 0c6c6ac Compare November 15, 2022 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation 📖 Things regarding our documentation Frontend Things related to the OBS RoR app
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants