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

Use bot name in help handler #89

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

Conversation

tielur
Copy link
Member

@tielur tielur commented Apr 23, 2018

Currently when using the help handler it has the bot name hard coded to @alice. If you don't name your bot alice then the help context doesn't make sense.

This PR adds bot_name/1 that takes the conn and will returns the bot's slack name.
general help
handle helper

@@ -4,7 +4,7 @@ defmodule Alice.Handlers.HelpTest do
alias Alice.Handlers.TestHandler

test "help_for_handler generates the correct output" do
assert Help.help_for_handler(TestHandler) ==
assert Help.help_for_handler(TestHandler, %{slack: %{me: %{name: "alice"}}}) ==
Copy link
Member Author

Choose a reason for hiding this comment

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

@adamzaninovich not a fan of this hard-coded conn. wasn't sure if there was a cleaner way to do this in tests?

Copy link
Member

Choose a reason for hiding this comment

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

maybe just use Conn.new and pass in the slack component

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea I like, that at least hides the use of the slack key to the internals until that gets changed with the whole different backend stuff.

@adamzaninovich
Copy link
Member

Re: help.ex, it seems a bit dirty to pass the conn all the way down like that. I would at least suggest just passing the name down instead, but maybe there is an even better way? Not sure.

@doc """
Returns the name of the bot
"""
def bot_name(conn), do: conn.slack.me.name
Copy link
Member

Choose a reason for hiding this comment

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

prefer this format (like the other functions in here):

def bot_name(conn = %Conn{}) do
  conn.slack.me.name
end

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 fixed

@@ -4,7 +4,7 @@ defmodule Alice.Handlers.HelpTest do
alias Alice.Handlers.TestHandler

test "help_for_handler generates the correct output" do
assert Help.help_for_handler(TestHandler) ==
assert Help.help_for_handler(TestHandler, %{slack: %{me: %{name: "alice"}}}) ==
Copy link
Member

Choose a reason for hiding this comment

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

maybe just use Conn.new and pass in the slack component

assert Help.help_for_handler(TestHandler, conn) == test_help_message_output(conn)
end

defp test_help_message_output(%Alice.Conn{slack: %{me: %{name: bot_name}}}) do
Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm maybe this should just use the Conn.bot_name on the passed in Conn, instead of deconstructing using the slack key again?

@adamzaninovich
Copy link
Member

🍐

@adamzaninovich adamzaninovich marked this pull request as draft April 11, 2020 19:01
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.

2 participants