Skip to content

Conversation

@NatalieTapias
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 initially started exploring the Slack API during the class exercises and then reinforced that understanding through analyzing data via postman. By practicing using an API, we both feel more comfortable using the documentation and test information exchange using postman.
Give a short summary of the request/response cycle. Where does your program fit into that scheme? The request/response cycle is made up of three parts: the request, thinking about the request, and sending a response. Our program can take a request, verify that it can complete the request and how, and then send the response. Each time that our program initializes an HTTParty, this constitutes a request, and the resulting object is the response.
How does your program check for and handle errors when using the Slack API? Our program uses SlackAPI errors to alert for errors and our methods use begin/rescue to keep the program from breaking.
Did you need to make any changes to the design work we did in class? If so, what were they? For this project, we started with the project using the scaffolding provided in class. We added a slack class to control the CLI. This helped to keep our menu methods in the workspace.rb and out of the CLI.
Did you use any of the inheritance idioms we've talked about in class? How? We have an abstract class called recipient that is the parent class to channel and user. The parent declares the list method as a template method.
How does VCR aid in testing a program that uses an API? VCR works record a particular API call, and replay a recording of this call for the purpose of testing. As long as we were working with the same data and referencing variables with consistent name, there was no need for us to make another API call.

…all to the slack channels api. Test is passing.
…his error in the parent class Recipient for the method list when called from this class.
… is raised when this happens and we have a test for it\!
@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 check
Inheritance model matches in-class activity somewhat - see comments about Recipient constructor and self.get
Inheritance idioms (abstract class, template methods, polymorphism) used appropriately check
Methods are used to break work down into simpler tasks check
Class and instance methods are used appropriately check
Tests written for User functionality check
Tests written for Channel Functionality check
Tests written for sending a message check
Overall Excellent job overall. This code is well-written and well-tested. It is clear to me that the learning goals around understanding the request/response cycle, consuming an API, and implementing a design using inheritance from scratch were all met. There is some room for improvement in the user experience for the CLI and how you deal with invalid user input. I've left a few inline comments for you to review below, but in general I'm quite impressed by what I see here. Keep up the hard work!


def self.list
response = HTTParty.get("https://slack.com/api/channels.list?token=#{ENV['SLACK_TOKEN']}")
unless response["ok"]

Choose a reason for hiding this comment

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

Nice work for raising an exception for an unsuccessful API call.

end

def self.list
raise SlackApiError.new("Call this method in child class")

Choose a reason for hiding this comment

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

There's actually a special exception for this case called a NotImplementedError.

end

# TODO project
def menu_method

Choose a reason for hiding this comment

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

Nice work encapsulating functionality into methods.


Dotenv.load
@workspace = SlackCLI::Workspace.new
def main

Choose a reason for hiding this comment

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

Your CLI flow insures that a user/channel will always be selected before a message is attempted to be sent. However, you may consider giving your user all the menu options rather than forcing them to what you see as the logical next choice after they have say, selected a user. They may want to select a different user! :)

end

def self.list
response = HTTParty.get("https://slack.com/api/users.list?token=#{ENV['SLACK_TOKEN']}")

Choose a reason for hiding this comment

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

Consider moving the pull request to the class method self.get(url, params) in Recipient to get all your money's worth out of your parent class Recipient




#Unsure if we can actually test this

Choose a reason for hiding this comment

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

Nice call! We don't actually have the tools to test this right now, but it's good that you raise an exception in your code if the request is unsuccessful.

end
end

# it "should be a channel" do

Choose a reason for hiding this comment

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

These comments would help this test be more thorough, or you could just add this as additional assertions in a single test.

end
#

#Move Test to Child Classes

Choose a reason for hiding this comment

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

Remove commented code.

require_relative 'test_helper'


describe "SlackCLI::User" do

Choose a reason for hiding this comment

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

Consider also checking that instantiating a new User works correctly.


module SlackCLI
class Recipient
def initialize

Choose a reason for hiding this comment

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

To fully use your parent class, include @name and @slack_id in Recipient since these are attributes of a User and Channel

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