Leaves - Natalie & Katie by NatalieTapias · Pull Request #14 · Ada-C12/slack-cli

NatalieTapias · 2019-09-13T23:30:44Z

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.

… Slack Token works.

…all to the slack channels api. Test is passing.

…ack Workspace.

…ned by the command list_channels

…his error in the parent class Recipient for the method list when called from this class.

…e the method select is called.

… is raised when this happens and we have a test for it\!

…age is high.

beccaelenzil · 2019-09-19T21:45:03Z

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!

beccaelenzil · 2019-09-19T21:32:37Z

lib/channel.rb

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


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

beccaelenzil · 2019-09-19T21:33:30Z

lib/recipient.rb

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


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

beccaelenzil · 2019-09-19T21:34:15Z

lib/slack.rb

+end

-  # TODO project
+def menu_method


Nice work encapsulating functionality into methods.

beccaelenzil · 2019-09-19T21:38:12Z

lib/slack.rb


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


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! :)

beccaelenzil · 2019-09-19T21:40:12Z

lib/user.rb

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


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

beccaelenzil · 2019-09-19T21:41:43Z

test/channel_test.rb

+
+
+
+#Unsure if we can actually test this


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.

beccaelenzil · 2019-09-19T21:42:13Z

test/channel_test.rb

+    end
+  end
+
+  # it "should be a channel" do


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

beccaelenzil · 2019-09-19T21:42:26Z

test/recipient_test.rb

+  end
+  # 
+
+  #Move Test to Child Classes


Remove commented code.

beccaelenzil · 2019-09-19T21:43:19Z

test/user_test.rb

+require_relative 'test_helper'
+
+
+describe "SlackCLI::User" do


Consider also checking that instantiating a new User works correctly.

beccaelenzil · 2019-09-19T21:52:21Z

lib/recipient.rb

+
+module SlackCLI
+  class Recipient
+    def initialize


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

NatalieTapias added 19 commits September 9, 2019 16:28

Added a .gitignore and .env to that file.

03a849a

Added files for Recipient class and Recipient Test. Also verified our…

cc0fefd

… Slack Token works.

Added tests for initializing SlackCLI::Recipient

4fc6986

We have troubleshooted VCR. We have written one test that makes one c…

9565f89

…all to the slack channels api. Test is passing.

We have a method within Module User to access a list of users in a Sl…

b54a8b1

…ack Workspace.

Found a way to accurately display channel information.

a7a45cd

Major breakthrough, now instantiating Channel objects, that are retur…

646a8e0

…ned by the command list_channels

Updated list method for Recipient, User, and Channel.

8fe71de

Added a SlackApiError class inheriting from StandardError. We raise t…

d7dc431

…his error in the parent class Recipient for the method list when called from this class.

Slack.rb draft

d212b8c

We have been ading to workspace test, testing the select user method

f384131

We wrote a test for method Workspace.select_channel

f70f803

Making progress and resettign @selected in workspace to @nil each tim…

ec53974

…e the method select is called.

Added channel workspace test to select a bogus channel. SlackAPIError…

928e058

… is raised when this happens and we have a test for it\!

Troubleshooted functionality on slack.rb

0b67fb5

Added Show Details Menu Option

6258d09

Workspace now has a functioning message method\!\!

d667167

Workspace Test is up and running

ce44cda

Finished some final touches on the main file. Tests passing and cover…

16b95e1

…age is high.

beccaelenzil reviewed Sep 19, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Leaves - Natalie & Katie#14

Leaves - Natalie & Katie#14
NatalieTapias wants to merge 19 commits intoAda-C12:masterfrom
NatalieTapias:master

NatalieTapias commented Sep 13, 2019

Uh oh!

beccaelenzil commented Sep 19, 2019 •

edited

Loading

Uh oh!

beccaelenzil Sep 19, 2019

Uh oh!

beccaelenzil Sep 19, 2019

Uh oh!

beccaelenzil Sep 19, 2019

Uh oh!

beccaelenzil Sep 19, 2019

Uh oh!

beccaelenzil Sep 19, 2019

Uh oh!

beccaelenzil Sep 19, 2019

Uh oh!

beccaelenzil Sep 19, 2019

Uh oh!

beccaelenzil Sep 19, 2019

Uh oh!

beccaelenzil Sep 19, 2019

Uh oh!

beccaelenzil Sep 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		require_relative 'test_helper'


		describe "SlackCLI::User" do

Conversation

NatalieTapias commented Sep 13, 2019

slack.rb

Comprehension Questions

Uh oh!

beccaelenzil commented Sep 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

slack.rb

What We're Looking For

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

beccaelenzil commented Sep 19, 2019 •

edited

Loading