Sockets - Pauline & Niv by paulentine · Pull Request #17 · Ada-C11/slack-cli

paulentine · 2019-03-22T23:08:23Z

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 read the documentations, learned how to post request and how to conceal our token with VCR.
Give a short summary of the request/response cycle. Where does your program fit into that scheme?	The request/response cycle explains how an API retrieves information from the web. First, a request is made using HTTParty. Then, a response is sent back and we parse it for the information we want. Our program initiates this cycle when it retrieves the list of users and list of channels from slack and sends a message.
How does your program check for and handle errors when using the Slack API?	We tests for error messages by posting bad requests, which should then raise a SlackApiError.
Did you need to make any changes to the design work we did in class? If so, what were they?	We chose to load our user and channels in self.load in each of these methods instead of using self.get(url, params). We loaded the lists into workspace(users, channels:) so that workspace would be less coupled to the class User or class Channel.
Did you use any of the inheritance idioms we've talked about in class? How?	We set up template methods (self.load, details, self.list) and polymorphism (send_message) in our abstract class (Recipient)
How does VCR aid in testing a program that uses an API?	VCR records an API request and its response so that when a test is called, it does not make a new call, but instead plays back the recorded response. This saves us from having to make multiple calls which can be costly and is useful if Slack's server is down.

kaidamasaki · 2019-04-01T21:11:35Z

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	Mostly. (You forgot in a couple of places.)
Inheritance model matches in-class activity	Mostly. (Changes were clearly outlined and well thought out.)
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.
Overall	Great job! Even though you missed a few edge cases your solution was really clear and well thought out.

kaidamasaki · 2019-04-01T19:44:32Z

lib/channel.rb

+        token: TOKEN,
+      }
+      response = HTTParty.get(URL, query: query_parameters)['channels']
+      response.each do |channel|


Forgot to validate response.code and response.parsed_response["ok"].

kaidamasaki · 2019-04-01T19:47:14Z

lib/recipient.rb

+        raise SlackApiError, "Error when posting #{text} to #{name}, error: #{response.parsed_response["error"]}"
+      end
+
+      return response.code == 200 && response.parsed_response["ok"]


I would consider just returning true here since you've already done the check above.

kaidamasaki · 2019-04-01T19:51:33Z

lib/channel.rb

+    end
+
+    def self.list
+      return @@all


While it doesn't cause a problem here generally class variables should be your tool of last resort. I probably would have only stored them in a field in Workspace instead of here and there.

kaidamasaki · 2019-04-01T19:53:36Z

lib/user.rb

+    def self.load
+      query_parameters = { token: TOKEN }
+      response = HTTParty.get(BASE_URL << USERS_LIST_PATH, query: query_parameters)
+      response["members"].length.times do |i|


Forgot to validate response.code and response.parsed_response["ok"].

kaidamasaki · 2019-04-01T20:18:48Z

lib/workspace.rb

+      @selected = selected
+    end
+
+    def select_channel(id_or_name:)


Tiny issue: it's a little odd to use a keyword argument here since you only have a single argument.

kaidamasaki · 2019-04-01T20:49:03Z

lib/workspace.rb

+          @selected = user
+        end
+      end
+      return @selected


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

kaidamasaki · 2019-04-01T21:03:48Z

specs/recipient_spec.rb

+  describe "self.list" do
+
+  end
+end


I'd also like to see a test that verifies that self.load raises NotImplementedError.

kaidamasaki · 2019-04-01T21:04:45Z

specs/user_spec.rb

+describe "User" do
+  before do 
+    VCR.use_cassette("load_users") do
+      @response = SlackAPI::User.load


I'd like to see some edge case testing on User.load.

kaidamasaki · 2019-04-01T21:05:58Z

specs/user_spec.rb

+    end
+  end
+
+  describe "send message" do


I'd also like to see some edge case tests on sending messages.

kaidamasaki · 2019-04-01T21:06:59Z

specs/workspace_spec.rb

+     end
+  end
+
+  describe "select_user" do


I'd like a test on what happens if the user doesn't exist. Also, do you lose your selection if you attempt to select an invalid user? Should you?

kaidamasaki · 2019-04-01T23:39:04Z

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	`select user` works; `select channel` crashes. It looks like you added an argument to `get_user_input` and forgot to pass it in.
Show details	👍🏼
Send message	👍🏼
Program runs without crashing	No, see above.
Implementation
API errors are handled appropriately	Yes.
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.
Overall	Great job! Aside from the `ArgumentError` mentioned above everything looks good. Your code is clear, concise and well factored.

paulentine and others added 30 commits March 19, 2019 11:35

Added #list_channels, setup Slack auth

46d2104

Updated URLs

5557af1

Setup spec_helper with VCR

f54ce2d

created skeleton

e2c60a5

created skeleton

fc2f66e

created skeleton

5348b32

Fixed merge conflict

b1c5b02

Wrote skeleton for tests

d02f2be

Fixed another merge conflict

91a050a

added self.list method

1cc5b1a

Moved Slack#list_channel to Channel#self.list

01193f9

Merge branch 'master' of https://github.com/paulentine/slack-cli

8685784

working on self.list and test

5db8e83

Added Channel#details & #self.list

042cfc9

Standardized indentations

32f0573

self.list method

5a55aa0

Fixed merge conflict

832c4be

Merge branch 'master' of https://github.com/paulentine/slack-cli

e91ecd4

passed tests for self.list

e5a9dec

Wrote tests for Channel#self.list

2695c6f

Added table_print, test for array length

dd4ad23

added self.load

86d6b0e

Merge branch 'master' of https://github.com/paulentine/slack-cli

dd139ef

Renamed #list to #load, wrote #details & test

d8ed86e

Merge branch 'master' of https://github.com/paulentine/slack-cli

c7da16c

Renamed #list to #load, wrote #details & test

499b62b

Started writing driver code

6d0150a

Fixed VCR error

45e8404

Wrote conditionals for invalid input

febd4e4

Made .load private

da8db5f

paulentine and others added 27 commits March 21, 2019 15:25

Updated driver code for select user & channel

803b051

Refactored tests, fixed formatting

e2b178c

created send_message method and test

f639218

Merge branch 'master' of https://github.com/paulentine/slack-cli

0c1c1ee

Wrote Channel#send_message

50eac82

Added driver code for details & selected + error checking

ebb8293

Established inheritance, moved #send_message

891b75b

add inheritance

8fb6865

Removed "recipient" param from send_message

e521cd0

Updated test

f21bd3e

Added edge case tests

5c4c034

added slackapi error test

1889248

#details now return string, updated tests

22b432a

Merge branch 'master' of https://github.com/paulentine/slack-cli

4437b76

add tests for sending message

3b8288a

Merge branch 'master' of https://github.com/paulentine/slack-cli

937be60

workspace spec

2e4edef

Merge branch 'master' of https://github.com/paulentine/slack-cli

b937f4a

Updated driver code to check for empty message

372e031

Merge branch 'master' of https://github.com/paulentine/slack-cli

958fa10

completed test for sending empty text

8af3f77

passing tests

43c947c

Merge branch 'master' of https://github.com/paulentine/slack-cli

4955e3b

renamed list to all

373a3b5

clean up

ac38d87

Cleaned up formatting, comments, alphabetized require

2c0c328

added tests for workspace

ba7559b

kaidamasaki reviewed Apr 1, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sockets - Pauline & Niv#17

Sockets - Pauline & Niv#17
paulentine wants to merge 69 commits intoAda-C11:masterfrom
paulentine:master

paulentine 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 commented Apr 1, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

paulentine 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!

kaidamasaki commented Apr 1, 2019

slack.rb

What We're Looking For

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants