Skip to content

Conversation

@teesloane
Copy link
Contributor

Describe your changes

Up for discussion: adding clarification copy about remaining slots to sign up for on the dispatcher side. Originally suggested up by @chadmohr in this slack thread.

Screenshot

CleanShot.2024-06-06.at.19.10.31.mp4

Checklist before requesting a review

  • I have performed a self-review of my code
  • If it is a core feature, I have added tests.
  • Are there other PRs or Issues that I should link to here?
  • Will this be part of a product update? If yes, please write one phrase
    about this update in the description above.

@teesloane teesloane requested a review from mveytsman June 6, 2024 23:17
Comment on lines +39 to +55
@broadcasted_infos [
:task_created,
:task_deleted,
:task_updated,
:campaign_rider_created,
:campaign_rider_deleted
]

@impl Phoenix.LiveView
def handle_info({event, entity}, socket) when event in @broadcasted_infos do
if entity_in_campaigns?(socket.assigns.campaigns, entity.campaign_id) do
{:noreply, refetch_and_assign_data(socket)}
else
{:noreply, socket}
end
end

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adding this!

two feedbacks:

  1. I think this should go after the handle_events
  2. It needs a
@impl Phoenix.LiveView
  @doc "silently ignore other kinds of messages"
  def handle_info(_, socket), do: {:noreply, socket}

after it to silently ignore messages we don't care about (or else the LV crashes :()

Copy link
Member

@mveytsman mveytsman left a comment

Choose a reason for hiding this comment

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

Looks good except small bit of feedback!

@teesloane
Copy link
Contributor Author

Ready for re-review @mveytsman !

@teesloane teesloane requested a review from mveytsman July 29, 2024 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants