-
Notifications
You must be signed in to change notification settings - Fork 358
Accept symbol, proc, and nil arguments for broadcasts_refreshes #748
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -210,10 +210,20 @@ def broadcasts_refreshes_to(stream) | |||||||||||||||||||||||||||||||||
| after_commit -> { broadcast_refresh_later_to(stream.try(:call, self) || send(stream)) } | ||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| # Same as <tt>#broadcasts_refreshes_to</tt>, but the designated stream for page refreshes is automatically set to | ||||||||||||||||||||||||||||||||||
| # the current model, for creates - to the model plural name, which can be overriden by passing <tt>stream</tt>. | ||||||||||||||||||||||||||||||||||
| # Same as <tt>#broadcasts_refreshes_to</tt>, but the designated stream for page refreshes is | ||||||||||||||||||||||||||||||||||
| # automatically set to the current model. For creates, the stream defaults to the model's plural | ||||||||||||||||||||||||||||||||||
| # name, which can be overridden by passing <tt>stream</tt>. | ||||||||||||||||||||||||||||||||||
| # If <tt>stream</tt> is a Symbol, a method with that name will be called to determine the stream. | ||||||||||||||||||||||||||||||||||
| # If <tt>stream</tt> is a Proc, it will be called with the model instance to determine the stream. | ||||||||||||||||||||||||||||||||||
| # If <tt>stream</tt> is explicitly set to nil, the after_create_commit callback will be skipped entirely. | ||||||||||||||||||||||||||||||||||
| def broadcasts_refreshes(stream = model_name.plural) | ||||||||||||||||||||||||||||||||||
| after_create_commit -> { broadcast_refresh_later_to(stream) } | ||||||||||||||||||||||||||||||||||
| after_create_commit do | ||||||||||||||||||||||||||||||||||
| case stream | ||||||||||||||||||||||||||||||||||
| when String then broadcast_refresh_later_to(stream) | ||||||||||||||||||||||||||||||||||
| when Symbol then broadcast_refresh_later_to(send(stream)) | ||||||||||||||||||||||||||||||||||
| else broadcast_refresh_later_to(stream.try(:call, self)) | ||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||
| end if stream | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+220
to
+226
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
What is the intended behavior of supporting explicitly Rather than suffixing the
Suggested change
|
||||||||||||||||||||||||||||||||||
| after_update_commit -> { broadcast_refresh_later } | ||||||||||||||||||||||||||||||||||
| after_destroy_commit -> { broadcast_refresh } | ||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,6 @@ | ||
| class Board < ApplicationRecord | ||
| broadcasts_refreshes | ||
| broadcasts_refreshes nil | ||
| broadcasts_refreshes :columns; def columns = [self, :columns] | ||
| broadcasts_refreshes ->(board) { [board, :cards] } | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this case statement aims to coerce the argument, but does not invoke different methods across the branches, what do you think about re-structuring it to extract out the method call?