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

Fire: Ayesha/ Blaine - SLACK API Project Submission #23

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

Conversation

ayaseef
Copy link

@ayaseef ayaseef commented Oct 10, 2020

Assignment Submission: Slack CLI

Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.

Reflection

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 opened a new workspace. Used postman and referenced the seven wonders assignment to understand the API content. This was a 100% new concept for both of us. Will probably utilize VHS & SimpleCov in the future projects.
Give a short summary of the request/response cycle. Where does your program fit into that scheme? Our Slack CLI program was a client that would make a request to the API server using the HTTParty. The API server would receive our request, process our request, and send a response back. With the help of Postman app, we deciphered the Server response and utilized the JSON data to interpret, manipulate, and display it to the user in our Slack CLI program.
How does your program check for and handle errors when using the Slack API? Our program throws a SlackError and rescues it in order to keep the CLI loop going. It also gives the end-user reminders to take steps in order.
How did the design and organization of your project change over time? We began with the CLI slack.rb and a workspace.rb. From there we expanded and created classes for User and Channel.
Did you use any of the inheritance idioms we've talked about in class? How? Recipient was our abstract class. It served as the parent class to our Channel class and User class. It wasn't instantiated, but used by the child classes
How does VCR aid in testing a program that uses an API? We used VCR to "RECORD" screenshots of the API calls that were requested. Hence, if we need to process the CLI program when the API server is down or the data changes, we have the DATA requested when the call was originally made and successful. It also allows for quicker and more thorough testing practices.

Copy link

@dHelmgren dHelmgren left a comment

Choose a reason for hiding this comment

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

Slack CLI

Major Learning Goals/Code Review

Criteria yes/no, and optionally any details/lines of code to reference
Practices best practices working with APIs. The .env is not checked into git, and no API token was directly used in the Ruby code without ENV. ✔️
Practices error handling with APIs. For all pieces of code that make an API call, it handles API requests that come back with errors/error status codes appropriately. ✔️
Implements inheritance and inheritance idioms. There is a Recipient class. User and Channel inherit from Recipient. In Recipient, there are appropriate methods defined that are used in both User and Channel. Some may be implemented. Some may be template methods. ✔️
Practices clean code. lib/slack.rb only interacts with Workspace to show a separation of responsibilities. Complex logic is broken into smaller helper methods. ✔️
Practices instance methods vs. class methods appropriately. The methods to list all Channels or Users is likely a class method within those respective classes. ✔️
Practices best practices for testing. The project has and uses VCR mocking when running tests, and can run offline. ✔️
Practices writing tests. The User, Channel, and Workspace classes have unit tests. ✔️
Practices writing tests. There are tests for sending messages (the location of these tests may differ, but is likely in Recipient) couldn't find send tests
Practices git with at least 15 small commits and meaningful commit messages ✔️

Functional Requirements

Functional Requirement yes/no
As a user of the CLI program, I can list users and channels ✔️
As a user of the CLI program, I can select users and channels ✔️
As a user of the CLI program, I can show the details of a selected user or channel ✔️
As a user of the CLI program, when I input something inappropriately, the program runs without crashing ✔️

Overall Feedback

Overall Feedback Criteria yes/no
Green (Meets/Exceeds Standards) 7+ in Code Review && 3+ in Functional Requirements ✔️
Yellow (Approaches Standards) 6+ in Code Review && 2+ in Functional Requirements
Red (Not at Standard) 0-5 in Code Review or 0,1 in Functional Reqs, or assignment is breaking/doesn’t run with less than 5 minutes of debugging

Comment on lines +93 to +97
selected.post_message(end_user_message)
puts "Your message has been sent!"
rescue
puts "You need to select a User or Channel."
puts "Start again."

Choose a reason for hiding this comment

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

This piece of code never seems to send my messages, always telling me that I have to select a user or channel, and I suspect that this is because you've made an error with how you're using the keyword rescue. Here's an example how how to use recuse completely in this situation:

Suggested change
selected.post_message(end_user_message)
puts "Your message has been sent!"
rescue
puts "You need to select a User or Channel."
puts "Start again."
begin
selected.post_message(end_user_message)
rescue
puts "You need to select a User or Channel."
puts "Start again."
else
puts "Your message has been sent!"
end

Comment on lines +44 to +46
def get_details
@selected.get_details
end

Choose a reason for hiding this comment

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

I'm not sure what this is doing? Should this be a template method? @selected doesn't exist elsewhere in this class.

@@ -0,0 +1,34 @@
require_relative 'recipient'
require 'prettyprint'

Choose a reason for hiding this comment

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

Since you required prettyprint in recipient, you don't need to require it 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.

2 participants