-
Notifications
You must be signed in to change notification settings - Fork 26
Leaves - Natalie & Katie #14
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: master
Are you sure you want to change the base?
Conversation
… Slack Token works.
…all to the slack channels api. Test is passing.
…ned by the command list_channels
…his error in the parent class Recipient for the method list when called from this class.
…e the method select is called.
… is raised when this happens and we have a test for it\!
slack.rbWhat We're Looking For
|
|
|
||
| def self.list | ||
| response = HTTParty.get("https://slack.com/api/channels.list?token=#{ENV['SLACK_TOKEN']}") | ||
| unless response["ok"] |
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.
Nice work for raising an exception for an unsuccessful API call.
| end | ||
|
|
||
| def self.list | ||
| raise SlackApiError.new("Call this method in child class") |
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.
There's actually a special exception for this case called a NotImplementedError.
| end | ||
|
|
||
| # TODO project | ||
| def menu_method |
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.
Nice work encapsulating functionality into methods.
|
|
||
| Dotenv.load | ||
| @workspace = SlackCLI::Workspace.new | ||
| def main |
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.
Your CLI flow insures that a user/channel will always be selected before a message is attempted to be sent. However, you may consider giving your user all the menu options rather than forcing them to what you see as the logical next choice after they have say, selected a user. They may want to select a different user! :)
| end | ||
|
|
||
| def self.list | ||
| response = HTTParty.get("https://slack.com/api/users.list?token=#{ENV['SLACK_TOKEN']}") |
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.
Consider moving the pull request to the class method self.get(url, params) in Recipient to get all your money's worth out of your parent class Recipient
|
|
||
|
|
||
|
|
||
| #Unsure if we can actually test this |
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.
Nice call! We don't actually have the tools to test this right now, but it's good that you raise an exception in your code if the request is unsuccessful.
| end | ||
| end | ||
|
|
||
| # it "should be a channel" do |
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.
These comments would help this test be more thorough, or you could just add this as additional assertions in a single test.
| end | ||
| # | ||
|
|
||
| #Move Test to Child Classes |
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.
Remove commented code.
| require_relative 'test_helper' | ||
|
|
||
|
|
||
| describe "SlackCLI::User" do |
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.
Consider also checking that instantiating a new User works correctly.
|
|
||
| module SlackCLI | ||
| class Recipient | ||
| def initialize |
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.
To fully use your parent class, include @name and @slack_id in Recipient since these are attributes of a User and Channel
slack.rb
Congratulations! You're submitting your assignment!
You and your partner should collaborate on the answers to these questions.
Comprehension Questions