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: Kalki and Sandy SLACK CLI #20

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

Conversation

KayKay-git
Copy link

@KayKay-git KayKay-git commented Oct 9, 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 found learning to navigate the API documentation both challenging and helpful. We learned to focus on the important aspects of the documentation; what type of permissions, what type of arguments, what we can expect the document to look like etc. This will serve us in the future if/when we need to create programs that interact with live API servers.
-- ---
Give a short summary of the request/response cycle. Where does your program fit into that scheme? In summary, a client sends a message to a server, the server will process the request and send a response to the client. Our Ruby program is the client requesting information from the Slack API using HTTParty, the Slack Server API then responded with a json file which holds the information we requested.
-- ---
How does your program check for and handle errors when using the Slack API? We created a custom Slack API error and raised it whenever we received an unsuccessful response (something other than 200) from the API.
-- ---
How did the design and organization of your project change over time? Initially, we included error checking in the user and channel files along with some being in the workspace files but decided it was better to move most of it to the Slack CLI because it made writing unit tests less complicated.
-- ---
Did you use any of the inheritance idioms we've talked about in class? How? Yes, we used the Recipient class as our abstract class with template methods. It served as the parent class to user and channel.
How does VCR aid in testing a program that uses an API? The VCR aids in testing because it records the response from the first call to the API and saves it. Our program can then interact with those files to run unit tests rather than having to rely on calls made directly to the live API server. This is both time and cost efficient.

@KayKay-git KayKay-git changed the title FIRE: Kalki and Sandy FIRE: Kalki and Sandy SLACK CLI Oct 9, 2020
Copy link

@kaidamasaki kaidamasaki left a comment

Choose a reason for hiding this comment

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

Awesome job! Your code was clean and concise and your classes had really clear separations of responsibility.

Your architecture wound up really elegant and I appreciated how much of your error handling you managed to do via exceptions. 😃

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) ✔️
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

Code Style Bonus Awards

Was the code particularly impressive in code style for any of these reasons (or more...?)

Quality Yes?
Perfect Indentation
Elegant/Clever
Descriptive/Readable
Concise
Logical/Organized

sleep(1)

unless response['ok'] && response.code == 200
raise SlackAPIError, "Error: #{response['error']}, #{response.code}, #{response.message}"

Choose a reason for hiding this comment

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

I love how much information you included in this message!


def main
puts "Welcome to the Ada Slack CLI!"
puts "Welcome to the Ada Slack CLI!\n".yellow

Choose a reason for hiding this comment

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

I like the use of colors. 😃

Comment on lines +19 to +22
@selected = @users.find do |user|
valid_users = [user.name.downcase, user.slack_id.downcase, user.real_name.downcase]
valid_users.include?(user_name_or_id)
end

Choose a reason for hiding this comment

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

Nice use of find!

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