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

Water - Kayla and Mackenzie #15

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

Conversation

mvlofthus
Copy link

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? Postman was helpful for outlining what the parameters were called so we could pull the information we wanted (this will be helpful for future projects). We looked at some Slack API documentation as well, which gave us information about permissions and bots' needs.
Give a short summary of the request/response cycle. Where does your program fit into that scheme? Client sends a request to the API, the API responds to the client. Our programs is the client requesting information from and posting information to the Slack API.
How does your program check for and handle errors when using the Slack API? We raise argument errors when the response code or parsed message is not equal to 200 or is false. This ends the program when it cannot pull the information from the SlackAPI.
How did the design and organization of your project change over time? We ended up with more template methods in our parent class than we originally had. At first we were thinking our program structure, especially surrounding our selected_user, would rely more heavily on child classes, and ended up using the constructor class (workspace) more. We also found it was simpler (and possibly more space and time efficient) to have our slack.rb file address nil values for selected_user immediately and repeat the prompt to the user.
Did you use any of the inheritance idioms we've talked about in class? How? The recipient class was the template for the channel and user classes. We used polymorphism for the .list and details methods. We used template methods for .send message and .get.
How does VCR aid in testing a program that uses an API? It records the API response and stores that data, so we don't have to send a new API request each time we run our tests. This makes this process more efficient and prevents stack overflow, it also allows us to run these tests offline.

Kaylaj89 and others added 29 commits October 8, 2020 09:37
git push
…ecause if string is only returned and not 'putsed' it won't show up in slack.rb.
…could run, added comments on tests as necessary
Copy link

@beccaelenzil beccaelenzil 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. ✔️ See in-line comment for suggestions how to break out methods further.
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. Some of the tests have errors or fail because the cassettes were checked in, and thus my test data is different.
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

Excellent job overall. This code is well-written and well-tested. It is clear that the learning goals around understanding the request/response cycle, consuming an API, and implementing a design using inheritance from scratch were all met. I've left a few inline comments for you to review below. Keep up the hard work!

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

Comment on lines +16 to 51
until user_input == "7" || user_input == "quit"
puts options
user_input = gets.chomp.downcase

case user_input
when "1", "list users"
workspace.list_users
when "2", "list channels"
workspace.list_channels
when "3", "select user"
puts "Please enter User Name or ID"
user_identifier = gets.chomp.downcase
workspace.select_user(user_identifier)
when "4", "select channel"
puts "Please enter Chanel Name or ID"
channel_identifier = gets.chomp.downcase
workspace.select_channel(channel_identifier)
when "5", "details"
if workspace.selected_recipient == nil
puts "Please select a channel or user first to view details"
else
puts workspace.find_details
end
when "6", "send message"
if workspace.selected_recipient == nil
puts "Please select a channel or user first to send message"
else
puts "Send a message!"
message = gets.chomp
workspace.send_msg(message)
puts "\nMessage sent!"
end
end
end
puts "Thank you for using the Ada Slack CLI"
end

Choose a reason for hiding this comment

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

Consider wrapping this functionality into its own method.

Comment on lines -4 to +12
puts "Welcome to the Ada Slack CLI!"
puts "Welcome to the Ada Slack CLI!\n\n"
workspace = Workspace.new

# TODO project
puts "There are #{workspace.channels.count} channels in this workspace"
puts "There are #{workspace.users.count} users in this workspace."
puts
puts "*" * 40

Choose a reason for hiding this comment

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

Consider wrapping this set-up functionality into its own method.

return response
end

def self.send_msg(message, id)

Choose a reason for hiding this comment

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

Consider making send_msg an instance method that's called on an instance of a User or Channel. In this way, id does not need to be an argument.

}
)

unless response.code == 200 && response.parsed_response["ok"]

Choose a reason for hiding this comment

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

Good work error handling.

headers: { 'Content-Type' => 'application/x-www-form-urlencoded'}
)

unless response.code == 200 && response.parsed_response["ok"]

Choose a reason for hiding this comment

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

Nice work error handling with a custom exception.

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