Skip to content

Conversation

@krismosk
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 read through the documentation closely and it was pretty helpful because we were better informed about required vs optional arguments and how to handle anticipated errors. For our next project, we would be more comfortable reading through the documentation first and also understand that not all documentation is the same and customizing API calls might involve some online research.
Give a short summary of the request/response cycle. Where does your program fit into that scheme? Request: When the program starts, a request is made to the API. Processing: API generates all users and channels within Workspace. Then the User interacts with the CLI and an additional API call is made when they want send a message to a user or channel.
How does your program check for and handle errors when using the Slack API? We create a custom exception class called SlackAPIError that inherits from StandardError, if the response from the API has an error, we catch the error and output a readable message to the user indicating what went wrong.
Did you need to make any changes to the design work we did in class? If so, what were they? We removed the recipient class and we removed any inheritance relationship between recipient and user and channel. We put recipient's responsibility as a instance variable "selected" in the Workspace class. Also, Workspace does all of the heavy lifting/communication in our program, user and channel are light-weight classes that only know about themselves.
Did you use any of the inheritance idioms we've talked about in class? How? Besides our custom error inheritance, we have no other inheritance relationships in our program. However, channel and user are polymorphic classes and each implement the same types of methods.
How does VCR aid in testing a program that uses an API? It captures/records an API call and re-uses it for testing purposes. This allows you to conserve API call usage.

… posted message to channel. Passed corresponding tests.
…. Passed corresponding tests. Wave 3 complete.
@beccaelenzil
Copy link

beccaelenzil commented Sep 19, 2019

slack.rb

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene (no slack tokens) check
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 We do not have the tools yet to test a bad get request in this instance, but it is good to think about and build in functionality to deal with bad requests.
Inheritance model matches in-class activity no
Inheritance idioms (abstract class, template methods, polymorphism) used appropriately not used
Methods are used to break work down into simpler tasks consider breaking down CLI functionality into methods
Class and instance methods are used appropriately no class methods
Tests written for User functionality yes - in workspace_test
Tests written for Channel Functionality yes - in workspace_test
Tests written for sending a message yes
Overall Good work overall. This code is well-written and well-tested. The requirements around understanding the request/response cycle and consuming an API were met.

One of the key learning goals for this project was implementing a design using inheritance. I have left an inline comment demonstrating one place where using inheritance would have resulted in a less dependent design. Let's talk about the benefits of template methods and inheritance at our next one on one.


puts prompt

while command = gets.chomp.downcase

Choose a reason for hiding this comment

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

Consider using helper methods to encapsulate some of the functionality in this CLI.

return result
end

def search(data_source, query_term)

Choose a reason for hiding this comment

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

This code is very similar to select a user and select a channel. Could you DRY this up somehow?

return HTTParty.get(url, query: query_parameters)
end

def user_list

Choose a reason for hiding this comment

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

What you've done here works. Nice work! However, the recommendation to put this method in the user class was there to reduce dependencies between classes. When you instantiate a new User below, this means that Workspace needs to know everything about User. This is a perfect situation for a class method.

query_parameters = {
token: ENV['SLACK_TOKEN']
}
return HTTParty.get(url, query: query_parameters)

Choose a reason for hiding this comment

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

Make sure you handle bad requests appropriately.

@@ -0,0 +1,238 @@
require_relative 'test_helper'

describe "Workspace" do

Choose a reason for hiding this comment

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

Thorough tests. Nice work!

puts options
while selected_command = gets.chomp.downcase
case selected_command
when "main menu"

Choose a reason for hiding this comment

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

Consider adding the options "show details" and "send message" to the main menu so that user of the CLI can send a second message to an already selected recipient.

else
puts "Invalid input. Returning to main menu..."
puts prompt
break

Choose a reason for hiding this comment

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

Consider allowing users to re-enter their input rather than breaking the program when the user input is invalid.

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