Skip to content

Conversation

@M-Burr
Copy link

@M-Burr M-Burr commented Sep 14, 2019

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? We carefully read the documentation on the provided links for users, channels, and message. This documentation was helpful and gave us a good idea of what to expect from an API response. We also used this documentation to identify what information we wanted (e.g., for our details method) and also what was required vs. optional in our query parameters. We would definitely recommend spending time in the beginning to understand API documentation.
Give a short summary of the request/response cycle. Where does your program fit into that scheme? 1) Request - sends an API request to the server when the program starts. 2) The server processes the request to determine if the request (e.g., get vs. post) is valid and/or if the query parameters are valid. 3)The server sends a response back. The response may have the information you want, or it may have an error response (e.g., 400 codes). Our program makes requests to the Slack API, and the Slack server responds our program.
How does your program check for and handle errors when using the Slack API? We raise an APIError unless the response has a 200 code.
Did you need to make any changes to the design work we did in class? If so, what were they? We were able to follow the class design. We originally made an instance method of .list, but realized that would be difficult to work with, & then restructured our code to create a class method for list (as was originally recommended in the class design). This made our whole program run more smoothly.
Did you use any of the inheritance idioms we've talked about in class? How? We used polymorphism in our .details and .send_message methods. This allowed us to treat different class objects (i.e., user vs. channel) the same by calling the same methods (i.e., .details, .send_message) on both classes.
How does VCR aid in testing a program that uses an API? It captures/"tapes" our API calls & then uses that recording to check against for our tests (rather than calling the API repeatedly during the testing process).

@beccaelenzil
Copy link

slack.rb

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene (no slack tokens) check - see comment
Comprehension questions check
Functionality
List users/channels check
Select user/channel check
Show details see comment
Send message check
Program runs without crashing check
Implementation
API errors are handled appropriately for post, yes. for get, no. -- see comment
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 -- may be able to make methods more flexible to DRY up your code.
Class and instance methods are used appropriately check
Tests written for User functionality see comment
Tests written for Channel Functionality check - see comment
Tests written for sending a message check
Overall Excellent job overall. It is clear to me that the learning goals around understanding the request/response cycle, consuming and API, and implementing a design using inheritance from scratch were all met. The main focus of my inline comments were on your user interface. In particular, do your make methods flexible to DRY up your code and be sure to deal appropriately (and test!) for invalid user input.

/tmp/

# Used by dotenv library to load environment variables.
# .env

Choose a reason for hiding this comment

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

You should uncomment this line to include it in your .gitignore file.


channel_details["slack_id"] = self.slack_id
channel_details["name"] = self.name
channel_details["topic"] = self.topic

Choose a reason for hiding this comment

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

the "topic" key provides this hash: {"value"=>"Non-work banter and water cooler conversation", "creator"=>"UMZNG8T8R", "last_set"=>1568061999} It seems that you just want "value" in this case.

end

def self.get(url, params)
return 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.

What is this isn't a success? Make sure you are prepared for something to go wrong and raise a SlackAPIError.

["List users", "List channels", "Quit"]
)

# path for list users

Choose a reason for hiding this comment

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

Consider using case/when for this control structure.

)

# user decides how they want to look up recipient
if search_choice == "1"

Choose a reason for hiding this comment

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

With these nested conditionals, make sure that users always have the option to quit.

@selected = nil
end

def select_user_slack_id(slack_id: nil)

Choose a reason for hiding this comment

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

The following 4 methods are all quite similar. Is there a way to DRY up this code?

end

puts "Thank you for using the Ada Slack CLI"
def print_user_list(list)

Choose a reason for hiding this comment

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

print_user_list and print_channel_list are quite similar. Is there a way to dry up this code?

end
end

it "selects user by slack_id" do

Choose a reason for hiding this comment

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

What if the recipient you try to select doesn't exist? What happens to a previously selected user if you try to select a bad user? Remember you always need to test for positive and negative results.

@workspace.select_channel_slack_id(slack_id: "HASFHAS")
end

expect{

Choose a reason for hiding this comment

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

It is not clear from the name of this test why this raises a standard error.

expect(@channel.details).must_be_kind_of Hash
end

it "has a slack_id" do

Choose a reason for hiding this comment

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

Include tests for the other details.

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