Branches -- Angele & Paige by minipaige02 · Pull Request #17 · Ada-C12/slack-cli

minipaige02 · 2019-09-13T23:53:07Z

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 used the documentation and Postman to test out the Slack API. We learned that it's useful to look through the documentation thoroughly -- we found additional endpoints to use that weren't in the project instructions.
Give a short summary of the request/response cycle. Where does your program fit into that scheme?	The user sends a request to the API with a key and relevant parameters, then the API sends back a response containing the requested information or an error message. Our program constructs the request and then it interprets the response from the API. It then formats the response for the user to understand.
How does your program check for and handle errors when using the Slack API?	It checks for and handles errors in the same methods that call the API. It interprets the JSON file when there is an error and sends back a message to the user. We also stop the user from entering information that might cause an error in sending a request to the API.
Did you need to make any changes to the design work we did in class? If so, what were they?	Our initial design included a message class, which we removed because the message was not significant enough in this program.
Did you use any of the inheritance idioms we've talked about in class? How?	We used polymorphism in the child classes of Recipient in self.get. The code was defined in Recipient and the only different between the implementations in User and Channel was that they passed in different URLs. Details was a template method defined in the abstract class, Recipient, but in User and Channel the code was different because they dealt with different pieces of information.
How does VCR aid in testing a program that uses an API?	It aids in that it allows the tests to be run several times without actually calling the API, which could take time, money, and could have a rate limit.

…nt self.get

… a test

…or id

…ponse, added test with cassette created with bad token

…onality to workspace and tests.

…workspace_test to use VCR for show_details test.

kaidamasaki · 2019-09-20T23:20:10Z

slack.rb

What We're Looking For

Feature	Feedback
Core Requirements
Git hygiene (no slack tokens)	Yes.
Comprehension questions	Yes.
Functionality
List users/channels	Yes.
Select user/channel	Yes.
Show details	Yes.
Send message	Yes.
Program runs without crashing	Yes.
Implementation
API errors are handled appropriately	Mostly. (Didn't check HTTP status.)
Inheritance model matches in-class activity	Yes.
Inheritance idioms (abstract class, template methods, polymorphism) used appropriately	Yes.
Methods are used to break work down into simpler tasks	Yes.
Class and instance methods are used appropriately	Yes.
Tests written for User functionality	Yes.
Tests written for Channel Functionality	Yes.
Tests written for sending a message	Yes.
Overall	Well done! There were a few little things I called out inline but overall things looked good!

kaidamasaki · 2019-09-20T22:45:11Z

.gitignore

 /specs/cassettes/
+
+# Ignore .DS_Store
+.DS_Store


You can ignore these globally:

https://coderwall.com/p/vz6ymw/how-to-globally-gitignore-ds_store

kaidamasaki · 2019-09-20T22:50:54Z

lib/channel.rb

+      key = ENV["API_TOKEN"]
+      response = HTTParty.get("https://slack.com/api/channels.info", query: {token: key , channel: @slack_id})
+      if response.keys.include? "error"  || response["ok"] == false
+        raise SlackCLI::SlackApiError


It would be nice if you'd included what the error was in the message here.

Suggested change

raise SlackCLI::SlackApiError

raise SlackCLI::SlackApiError.new(response["error"])

kaidamasaki · 2019-09-20T22:59:16Z

lib/recipient.rb

+      if response.keys.include? "error"  || response["ok"] == false
+        raise SlackCLI::SlackApiError.new("Message not sent due to error: #{response["error"]}")
+      else
+        return true


I appreciate what you're doing here but since this method raises the only thing it can ever return is true so I probably wouldn't bother with returning something.

kaidamasaki · 2019-09-20T22:59:33Z

lib/recipient.rb

+    end
+
+    def self.list
+      raise NotImplementedError.new("Self.list should be implemented in child class")


Abstract methods! 🎉

kaidamasaki · 2019-09-20T23:02:02Z

lib/slack.rb

+      puts "Select Channel"
+      print "Please enter a Channel Name or Slack ID: "
+      query = gets.chomp.downcase
+      query_result = workspace.select_channel(query)


I'd have liked to have seen the error here handled with an exception and then rescued down below. That way the message could be constructed inside of select_channel and you wouldn't need the extra check below.

Plus, it could give you to opportunity to DRY up your code by putting all of your error handling in a single begin/rescue within your until wrapping your case.

kaidamasaki · 2019-09-20T23:12:49Z

lib/recipient.rb

+          }
+        )
+
+      if response.keys.include? "error"  || response["ok"] == false


You should be checking response.code here.

kaidamasaki · 2019-09-20T23:13:24Z

lib/user.rb

+
+    def details
+      key = ENV["API_TOKEN"]
+      response = HTTParty.get("https://slack.com/api/users.info", query: {token: key , user: @slack_id})


If you made self.get accept parameters then you could have avoided duplicating error handling.

minipaige02 and others added 30 commits September 10, 2019 13:56

Added recipient test.

1bf4bfb

updated Recipient and its test

c7d314f

added file for User and test

2be7169

Added remaining classes and initialize tests.

3aedac1

created self.list in User and test

cc78eca

added self.get to Recipient and refactored User self.list to use pare…

fede551

…nt self.get

Delete list_users.yml

0875424

Hid the API token and other small edits.

c89c942

Added self.list method and tests.

09d3f01

update list_users.yml

a16cf80

updated Workspace to initialize with User and Channel .list and added…

d727d4b

… a test

Added test for workspace initialize.

19d7b70

small changes to require and require_relative

62270c4

retested VCR

ef7d608

created CLI and menu_options method

84b14fd

Added list channels method to slack.rb

1a3a95c

Updated self.list for channels and corresponding cassetes.

6c5aeca

Added test for find user method.

e2d73e7

added find_instance method and added tests for finding users by name …

b13fa09

…or id

Added tests to find a channel with the find_recipient method.

df4857e

Added methods and tests to select user and select channel.

f9cff44

Began writing API error handling.

c1f0c8a

updated Recipient self.gets to raise SlackApiError on errored API res…

bf8a8ad

…ponse, added test with cassette created with bad token

added send_message to Recipient and added tests

c317d0d

Added details methods for user and channel. Added show details functi…

504008e

…onality to workspace and tests.

added select_user, select_channel, and send_message to slack.rb

51c83c4

Added details functionality to slack.rb

3588db0

Merged menu changes.

2fb2f5a

formatting updates

43098a4

moved send_message from slack.rb to Workspace

66ce941

geli-gel and others added 4 commits September 13, 2019 14:26

moved methods around in slack.rb, added comment in workspace

f5af80e

updated User details method to pull in more information, and updated …

c85040b

…workspace_test to use VCR for show_details test.

updated Channel details method to pull more info and added tests

2b77254

Refactored workspace send message method.

1842a8d

kaidamasaki reviewed Sep 20, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Branches -- Angele & Paige#17

Branches -- Angele & Paige#17
minipaige02 wants to merge 34 commits intoAda-C12:masterfrom
geli-gel:master

minipaige02 commented Sep 13, 2019

Uh oh!

kaidamasaki commented Sep 20, 2019

Uh oh!

kaidamasaki Sep 20, 2019

Uh oh!

kaidamasaki Sep 20, 2019

Uh oh!

kaidamasaki Sep 20, 2019

Uh oh!

kaidamasaki Sep 20, 2019

Uh oh!

kaidamasaki Sep 20, 2019

Uh oh!

kaidamasaki Sep 20, 2019

Uh oh!

kaidamasaki Sep 20, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	raise SlackCLI::SlackApiError
	raise SlackCLI::SlackApiError.new(response["error"])

Conversation

minipaige02 commented Sep 13, 2019

slack.rb

Comprehension Questions

Uh oh!

kaidamasaki commented Sep 20, 2019

slack.rb

What We're Looking For

Uh oh!

kaidamasaki Sep 20, 2019

Choose a reason for hiding this comment

Uh oh!

kaidamasaki Sep 20, 2019

Choose a reason for hiding this comment

Uh oh!

kaidamasaki Sep 20, 2019

Choose a reason for hiding this comment

Uh oh!

kaidamasaki Sep 20, 2019

Choose a reason for hiding this comment

Uh oh!

kaidamasaki Sep 20, 2019

Choose a reason for hiding this comment

Uh oh!

kaidamasaki Sep 20, 2019

Choose a reason for hiding this comment

Uh oh!

kaidamasaki Sep 20, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants