Skip to content

Conversation

@CloudyLopez
Copy link

slack.rb

Congratulations! You're submitting your assignment!

You and your partner should collaborate on the answers to these questions.

Comprehension Questions

Question Answer
How did you go about exploring the Slack API? Did you learn anything that would be useful for your next project involving an API? using postman helped in exploring it.
Give a short summary of the request/response cycle. Where does your program fit into that scheme? client(program) sent a request to a server at slack, in turn, sent us a response.
How does your program check for and handle errors when using the Slack API? by checking the value of the responses "ok" key and raising an error if false.
Did you need to make any changes to the design work we did in class? If so, what were they? yes but mostly because we hadn't fleshed things out.
Did you use any of the inheritance idioms we've talked about in class? How? yes, we used a template method for self.list method in recipient and a polymorphism for the send message method.
How does VCR aid in testing a program that uses an API? you dont have to make and get post requests, it saves you time (and potentially) money.

Kimberly-Fasbender and others added 27 commits March 19, 2019 13:43
@CheezItMan
Copy link

CheezItMan commented Mar 31, 2019

slack.rb

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene (no slack tokens) Good number of commits and good commit messages
Comprehension questions Check
Functionality
List users/channels Check
Select user/channel Check
Show details Check
Send message Check
Program runs without crashing Check
Implementation
API errors are handled appropriately Check
Inheritance model matches in-class activity Check
Inheritance idioms (abstract class, template methods, polymorphism) used appropriately Check
Methods are used to break work down into simpler tasks Check
Class and instance methods are used appropriately Check
Overall Small note, don't name your API key key. Make the name a bit more descriptive. Nice work with the table view and colored output. It looks lovely. You did a great job on the project and hit all the learning goals. Nice work. See my inline comments and let me know if there are any questions. Well done!

end

def details
super.push("topic", "member_count")

Choose a reason for hiding this comment

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

Very clever 👍

end

def self.get(url, params)
user_data = HTTParty.get(url, query: params)

Choose a reason for hiding this comment

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

It's not always user data, so this variable name could use some improvement.

end

def self.list
raise NotImplementedError, "Implement me in a child class!"

Choose a reason for hiding this comment

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

🥇

end

def select_user(user_descriptor)
users.find do |user|

Choose a reason for hiding this comment

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

When using find it's intended to be used like this:

@selected = users.find do |user|
  user_descriptor == user.name || user_descriptor == user.slack_id
end

end
end

if @selected == nil

Choose a reason for hiding this comment

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

There's a subtle error in the way you have it written. Consider:

  1. I select a valid user
  2. Then I try to select an invalid user name
  3. Since @selected is not nil we won't get the returned error message.

end
end

def select_channel(channel_descriptor)

Choose a reason for hiding this comment

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

See notes for select_user

Also if you notice the methods are identical... you could have one method that does both!

end

describe "#send_message" do
it "raises an Error if incorrect parameters are given" do

Choose a reason for hiding this comment

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

So maybe verify that it must_raise

@@ -0,0 +1,156 @@
require_relative "test_helper"

Choose a reason for hiding this comment

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

Nice testing here!

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