Sockets - Kirsten & Nara by bonara · Pull Request #6 · Ada-C11/slack-cli

bonara · 2019-03-22T21:32:22Z

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 have read the slack API documentation, tested with postman and googled some of the optional enhancements. We have learned that starting from the documentation is a good idea when working with APIs.
Give a short summary of the request/response cycle. Where does your program fit into that scheme?	Request is when we use HTTParty to get data or send payload through the API. In response to our request, we will get a code indicating whether the request was successful or not and the data if it was a get request. We request data from slack and process it in our program. Users are also allowed to send messages by making a post request to slack.
How does your program check for and handle errors when using the Slack API?	We check response.code and the ok key in the response. If response ok is false or response.code is not equal to 200, we raise SlackAPIError.
Did you need to make any changes to the design work we did in class? If so, what were they?	We haven't made changes to the design work we did in class. It worked well for our program.
Did you use any of the inheritance idioms we've talked about in class? How?	We have used an abstract class and template methods. Our recipient class was an abstract class and details and self.list were the template methods.
How does VCR aid in testing a program that uses an API?	It reduces the number of API calls you need to make by recording the response. It eliminates the external dependencies for your testing if you have no internet connection or slack is down.

kaidamasaki · 2019-04-01T17:33:53Z

slack.rb

What We're Looking For

Feature	Feedback
Core Requirements
Git hygiene (no slack tokens)	👍🏼
Comprehension questions	👍🏼
Functionality
List users/channels	👍🏼
Select user/channel	👍🏼
Show details	👍🏼
Send message	👍🏼
Program runs without crashing	👍🏼
Implementation
API errors are handled appropriately	In some places; in others it looks like you forgot to check.
Inheritance model matches in-class activity	Mostly.
Inheritance idioms (abstract class, template methods, polymorphism) used appropriately	👍🏼
Methods are used to break work down into simpler tasks	👍🏼 `slack.rb` could have been cleaned up a little but overall it looks good!
Class and instance methods are used appropriately	👍🏼
Overall	Great job! There are a few rough edges (I feel like maybe you were pressed for time) but overall it's a really solid solution with good, clean code.

kaidamasaki · 2019-04-01T16:18:51Z

specs/recipient_spec.rb

@@ -0,0 +1 @@
+require "test_helper"


This should be require_relative, not require. Using require here breaks rake (but not guard).

Suggested change

require "test_helper"

require_relative "test_helper"

Also, while it's hard to test an abstract class it would be good to at a minimum make sure that your template methods raise NotImplementedError.

kaidamasaki · 2019-04-01T16:31:53Z

lib/recipient.rb

+      url = CHANNEL_URL
+    end
+
+    response = HTTParty.get(url, query: params)


Forgot to check for errors.

kaidamasaki · 2019-04-01T16:33:36Z

lib/recipient.rb

+      "icon_emoji" => settings["icon_emoji"],
+    }
+    if recipient.class == User
+      params["as_user"] = true


as_user isn't for if the target is a user, it lets you send things as though you are the user whose token you are making the call with.

kaidamasaki · 2019-04-01T16:34:41Z

lib/user.rb

+  def self.list
+    raw_data = self.get("user")
+
+    unless raw_data.code == 200


Forgot to check for the parsed out ok.

kaidamasaki · 2019-04-01T16:38:15Z

lib/workspace.rb

+    selected = channels.select do |channel|
+      channel.name == user_input || channel.slack_id == user_input
+    end
+    @selected = selected.first


Ruby's .find enumerable method might be helpful to clarify this code.

kaidamasaki · 2019-04-01T16:59:18Z

specs/channel_spec.rb

@@ -0,0 +1,15 @@
+require_relative "test_helper"
+
+describe "Channel class" do


I would have liked to see some edge case tests on Channel. What happens if you try to get a channel that doesn't exist, etc?

kaidamasaki · 2019-04-01T17:21:14Z

lib/workspace.rb

+  end
+
+  def print_details(recipients)
+    if recipients == "users"


I'd like to see recipients just be an array of objects, but if you want it to be something you switch on like this you should use :users instead of "users" since generally we want to use atoms when we match on things like this.

kaidamasaki · 2019-04-01T17:26:52Z

lib/workspace.rb

+    return return_array
+  end
+
+  def update_settings(name, emoji)


I like that your configuration system is persistent and uses JSON. 😃

kaidamasaki · 2019-04-01T17:30:54Z

specs/workspace_spec.rb

+      end
+    end
+
+    it "returns a user object from select_user" do


What if the user you try to select doesn't exist? What happens to a previously selected user if you try to select a bad user?

kaidamasaki · 2019-04-01T17:31:27Z

specs/workspace_spec.rb

+      end
+    end
+
+    it "returns a channel object from select_channel" do


Same for select_channel.

kanderson38 and others added 25 commits March 19, 2019 11:56

update test_helper for VCR

c4dc49a

Created lib and spac files.

5152b71

successful call to users api

9098d75

Added channel class and self.list method

6f06b65

create list of users

a9ca644

Added self.list method for channel.rb

ca2213c

Added tests to channel_spec.rb

ab56d8a

added user_spec

c8ae6e9

handled !200 errors

2cf0c05

Updated slack.rb to get user input

8baa484

list_user_details

a431470

added channels print_details method

6d54995

added workspace init tests

f8fc318

select_user method

9f3bb6d

Added select_channel method and test

eac545e

Added show details, select user and channel methods and tests.

fd0bb90

updated slack.rb to include new menu items

9f1b726

Added send message to recipient and worskpace.

1fc9558

updated CLI and fixed print_details in workspace

7136fec

Added a test to handle empty messages.

d076c81

Added error message if invalid user or channel selected

c687709

Corrected a typo in the test

abb531d

started bot-settings code

bfc5486

set new bot-settings and write to json file

c496d3a

added conditions to check if recipient is user when posting.

f1f47a5

kaidamasaki reviewed Apr 1, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sockets - Kirsten & Nara#6

Sockets - Kirsten & Nara#6
bonara wants to merge 25 commits intoAda-C11:masterfrom
bonara:master

bonara commented Mar 22, 2019

Uh oh!

kaidamasaki commented Apr 1, 2019

Uh oh!

kaidamasaki Apr 1, 2019

Uh oh!

kaidamasaki Apr 1, 2019

Uh oh!

kaidamasaki Apr 1, 2019

Uh oh!

kaidamasaki Apr 1, 2019

Uh oh!

kaidamasaki Apr 1, 2019

Uh oh!

kaidamasaki Apr 1, 2019

Uh oh!

kaidamasaki Apr 1, 2019

Uh oh!

kaidamasaki Apr 1, 2019

Uh oh!

kaidamasaki Apr 1, 2019

Uh oh!

kaidamasaki Apr 1, 2019

Uh oh!

kaidamasaki Apr 1, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -0,0 +1,15 @@
		require_relative "test_helper"

		describe "Channel class" do

Conversation

bonara commented Mar 22, 2019

slack.rb

Comprehension Questions

Uh oh!

kaidamasaki commented Apr 1, 2019

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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants