Branches - Amal and Julia

[juliabouv]

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 used the documentation to figure out how to make correct requests. We used Postman to test our queries and requests to find the correct input.
Give a short summary of the request/response cycle. Where does your program fit into that scheme?	The client makes a request to the server, and the server processes the request and sends a response back to the client. In our program, we are the client making a request to the server, which is the Slack API.
How does your program check for and handle errors when using the Slack API?	Our program looks for keywords in the JSON file that indicate an error, such as if a response["error"] is truthy. If there is an error, it will raise a SlackApiError.
Did you need to make any changes to the design work we did in class? If so, what were they?	We decided to change the design of the send_message methods within Recipient and Workspace. We could use either of them individually to send a message, but using both methods in one design seemed redundant. We decided to use the send_message in Workspace and delete the Recipient send_message method because the message is sent from the Workspace to a recipient so it seemed to belong there.
Did you use any of the inheritance idioms we've talked about in class? How?	Recipient was an abstract class because it was never instantiated, but used for Channel and User to inherit from. We used template methods within Recipient such as #details and list that were implemented in subclasses, and would raise an error if attempted to implement in the parent class. Polymorphism was also used because Channel and User were both able to implement the same methods within Workspace.
How does VCR aid in testing a program that uses an API?	VCR records the response of the API for specific requests in your test cases. The cassettes are a reliable recording of the response that aren't effected by the API going down. It also reduces the cost of using the API because the request isn't going to the server, it's going to the cassette.

[kaidamasaki]

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	Almost. You checked for response["error"] but not to see if the status was something other than 200.
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.
Tests written for User functionality	Yes.
Tests written for Channel Functionality	Yes.
Tests written for sending a message	Yes.
Overall	Well done! There were a few small issues here and there but overall this was a really good submission. Everything was implemented and worked well and your program was well factored.

[kaidamasaki]

Triple quotes aren't needed in Ruby like they are in some other languages. Ruby does multi-line strings by default.

Suggested change

	details = """
	details = "

(In fact Ruby actually reads this as a empty string followed by a multi-line string followed by another empty string. By default in Ruby strings get joined together if they are next to each other. greeting = "Hello," " world!" is the same as greeting = "Hello, world!".)

[kaidamasaki]

Abstract method! 🎉

[kaidamasaki]

Running workspace.select_channel(input: input) twice here is redundant. (The user would already be selected if it worked the first time.)

[kaidamasaki]

I would have liked to have seen this as an error raised in select_channel and then rescued here. You could even put your entire case inside of the begin/rescue to DRY up your code.

until search == "quit"
  begin
    case search 
    when "list channels", "channels"
      # ...
    when "select channel", "channel"
      puts "Please enter a channel name or slack_id for the channel you would like to select:"
      print prompt
      input = gets.chomp

      workspace.select_channel(input: input)

      puts "
      #{input} is now selected
      "
    when "select user", "user"
      puts "Please enter a username or slack_id for the user you would like to select:"
      print prompt
      input = gets.chomp

      workspace.select_user(input: input)

      puts "
      #{input} is now selected
      "
    # other menu options...
  rescue SlackWorkspaceError => error
    puts e.message
  end
end

[kaidamasaki]

A keyword argument is probably overkill here. Positional parameters can be optional too.

Suggested change

	def select_channel(input: nil)
	def select_channel(input = nil)

Though I'm not sure it makes sense to select a channel without giving a name.

[kaidamasaki]

I'd like to see this raise instead of just returning nil.

[kaidamasaki]

Good use of map and find!

[kaidamasaki]

You don't need to VCR.use_cassette here since Channel.new doesn't make any API calls.

[kaidamasaki]

Good use of must_include! This is way more robust then checking the exact string.

[kaidamasaki]

You should also check response.code here.

Suggested change

	if response["error"]
	if response["error"] || response.code != 200