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

Sockets - Tati Hana #20

Open
wants to merge 75 commits into
base: master
Choose a base branch
from
Open

Sockets - Tati Hana #20

wants to merge 75 commits into from

Conversation

hanalways
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? We first tackled the documentation through Slack for each of the three different URIs. After thoroughly understanding the requirements and parameters for each of the APIs, we explored POST by establishing our recipient class (post_message method) before GET (self.get method). Postman also aided in our understanding of APIs and helped cement the difference between URI, tokens, query parameters, body and header. Reading the documentation provided will definitely be something that will be carried over for every future project concerning APIs.
Give a short summary of the request/response cycle. Where does your program fit into that scheme? The request/response cycle starts with a user taking an action where the client sends the request. The server processes this request and sends a response where the client displays that response to the user. Our User and Channel classes inherit the Recipient class where they send a GET and POST request which includes payloads. The API returns a response as a JSON with the requested parameters.
How does your program check for and handle errors when using the Slack API? Our program responds to any errors by raising a custom SlackError message that inherits from StandardError class. We raise this error anytime we do not get a 200 OK response code, or if we return a JSON value false for “ok”, such as having a bad URI or token. We also raise this error if user invokes details when there is no channel or user previously selected, or when sending a message that’s nil or blank.
Did you need to make any changes to the design work we did in class? If so, what were they? We changed the parameters that were passed for certain methods such as self.get and send_message.
Did you use any of the inheritance idioms we've talked about in class? How? Our Recipient class is an abstract class as it only serves as a parent class of User and Channel (concrete classes). Our workspace class is also an example of concrete classes as it is instantiated in our main driver code. Our self.get and send_message methods are examples of template methods as they are overriden in User and Channel. We’ve used polymorphism when invoking send_message and show_details methods of our workspace on either User or Channel, thus making our Workspace an interface.
How does VCR aid in testing a program that uses an API? VCR aids in testing a program by recording each API response as a cassette and can replay them when you need to test your methods, thus improving stability and performance.

@droberts-sea
Copy link

slack.rb

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene (no slack tokens) yes
Comprehension questions yes
Functionality
List users/channels yes
Select user/channel yes
Show details yes
Send message yes
Program runs without crashing yes
Implementation
API errors are handled appropriately yes
Inheritance model matches in-class activity yes
Inheritance idioms (abstract class, template methods, polymorphism) used appropriately yes
Methods are used to break work down into simpler tasks yes
Class and instance methods are used appropriately yes
Overall Great job overall! This code is clean, well-organized and mostly well-tested, and it's clear to me that the learning goals around understanding the request/response cycle, interacting with an API and implementing a design from scratch using inheritance were met. I've left a few inline comments, but in general I like what I see. Keep up the hard work!


def self.list_url
raise NotImplementedError, "TODO: implement me in a child class"
end

Choose a reason for hiding this comment

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

This is a great use of a template method.

# :nocov:
def details
raise NotImplementedError "TODO: implement me in a child class"
end

Choose a reason for hiding this comment

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

Instead of excluding this from coverage, why not have a test that Recipient by itself without a subclass will raise this exception when this method is called?

workspace.send_message(message)
# rescue SlackCli::SlackError
puts "No user or channel selected, try again"
# end

Choose a reason for hiding this comment

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

This error message always prints out, even if the send is successful. Instead, you should check the result and give the user positive or negative feedback.

def select_user(name)
@selected = @users.find do |user|
user.name == name || user.slack_id == name
end

Choose a reason for hiding this comment

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

Good use of the .find enumerable here.

if @selected == nil
puts "That user does not exist"
else
return @selected

Choose a reason for hiding this comment

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

Since this code is here to interact with the user, I would probably consider it view code and move it to the main method.

describe "details" do
it "lists details for an instance of Channel" do
channel = channel_list[1]

Choose a reason for hiding this comment

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

What happens if you create a Channel manually with bogus data and then call .details on it?

channel = SlackCli::Channel.new('bad_id', 'channel_dne', '', 0)
channel.details

Presumably this would work just fine, but it makes an interesting test case.

it "returns nil if no channel selected" do
assert_nil(workspace.select_channel(""))
assert_nil(workspace.select_channel(nil))
end

Choose a reason for hiding this comment

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

I like that you're testing these failure cases. What happens to a previously selected user/channel in this case?

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