Skip to content

Conversation

@LVala
Copy link
Contributor

@LVala LVala commented Aug 2, 2024

It also renames /admin/player to /admin/panel.

@LVala LVala marked this pull request as ready for review August 2, 2024 14:20
@LVala LVala requested a review from mickel8 August 2, 2024 14:24
@LVala LVala mentioned this pull request Aug 2, 2024
63 tasks
socket =
socket
|> assign(:nickname, nickname)
|> assign(:id, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Better start with a random value

Copy link
Contributor Author

@LVala LVala Aug 2, 2024

Choose a reason for hiding this comment

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

Instead, I'll use a unique id to create the message id.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick:

Suggested change
|> assign(:id, 0)
|> assign(:msg_cnt, 0)

@LVala LVala requested a review from mickel8 August 2, 2024 16:14
socket =
socket
|> assign(:nickname, nickname)
|> assign(:id, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick:

Suggested change
|> assign(:id, 0)
|> assign(:msg_cnt, 0)

Comment on lines +48 to +52
scope "/admin" do
pipe_through :auth

delete "/chat/:id", PageController, :delete_chat_message
end
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can merge this with already exisiting scoep "admin", can't we?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The admin scope is piped through :browser, so we cannot use it for that. I could refactor the admin scope a bit, but I'm not sure if it's worth it.

@LVala LVala merged commit 17515c5 into master Aug 3, 2024
@LVala LVala deleted the stream-panel branch August 3, 2024 08:54
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.

3 participants