Sockets - Jansen & Rosalyn by ranuseh · Pull Request #22 · Ada-C11/slack-cli

ranuseh · 2019-03-23T00:38:16Z

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 looked over the documentation and tried a different post and get request with various inputs like 40,000 chars to see the result and the type and/ or errors we were receiving. We got a lot of practice using VCR, postman, and pry. Pry was used to manipulate the data that we got back through HTTParty. The different types of tokens available and what could be done with them in the future.
Give a short summary of the request/response cycle. Where does your program fit into that scheme?	The cycle beings with the client sending a request, the server processes the request and the server sends a response and the client displays the response. Our program fits into the client part of the response cycle.
How does your program check for and handle errors when using the Slack API?	The send message method inside recipient there is an unless statement that checks for the correct response code and that it is okay. If those conditions are not met then we raise a SlackApiError that we defined in its own class.
Did you need to make any changes to the design work we did in class? If so, what were they?	We created a method called user get and channel gets in their respective classes which used different parameters depending on whether we were getting information about users or channels. We did it that way because the parameters changed depending on whether we were working with users or channels. in hindsight, the .get method in recipient would have worked just as well.
Did you use any of the inheritance idioms we've talked about in class? How?	We used an abstract class recipient and used template methods inside of recipient that was not intended to be evoked directly but rather through its children. User objects and channel objects both have a details method that treats them in the same way.
How does VCR aid in testing a program that uses an API?	It prevents you from sending multiple requests or post to the server which has costs associated with it. It also prevents the over-reliance on the external API in the case that it goes down and you can do offline testing.

…nd revise 'initialize' method for Recipient

…ient. Revise Recipient class tests. All tests passing

… up commented code in channel.rb and user.rb

…ss; Fix errors; All tests passing

…orkspace

…d 'select_user'

…or 'select_channel' and 'select_user' for clarity

… find channel based on slack id OR name

…t user based on username OR slack id

…ecting the appropriate recipient

…methods for clarity

…t the user know if no channel/user has the supplied name or ID

droberts-sea · 2019-03-30T01:52:08Z

slack.rb

What We're Looking For

Feature	Feedback
Core Requirements
Git hygiene (no slack tokens)	mostly - you've included coverage files!
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	some - see inline
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	Good job overall! It is clear to me that the learning goals around understanding the request/response cycle, consuming an API, and implementing a design from scratch using inheritance have been met. There is some room for improvement around code organization and test coverage, but in general I'm happy with what I see. Keep up the hard work!

droberts-sea · 2019-03-30T01:08:52Z

lib/channel.rb

+    def self.channels_get
+      query_params = { token: ENV["SLACK_API_TOKEN"] }
+      return Slack::Channel.get(CHANNEL_URL, query_params)
+    end


Since you're already inside of Slack::Channel, you can just say return get(...) on line 33.

droberts-sea · 2019-03-30T01:10:41Z

lib/recipient.rb

+
+    def self.get(url, params)
+      return HTTParty.get(url, query: params)
+    end


I think the params argument for this method is always the same: a hash containing the slack token. Could you supply that here and remove the extra argument?

Also, you should be doing things like error handling here.

In general, every time you add another layer to a piece of functionality, you should take care of some part of the puzzle in that layer. In this case, Channel.list is in charge of turning a response into an array of Channel objects, Channel.channels_get knows the URL and the query parameters, but this method doesn't add anything new.

droberts-sea · 2019-03-30T01:13:53Z

lib/slack_main.rb

+Dotenv.load
+
+CHANNEL_URL = "https://slack.com/api/channels.list"
+USER_URL = "https://slack.com/api/users.list"


You have these constants defined a few places. In general, each constant should be defined once. This makes your code easier to change, and avoids the problem of what to do if they don't have the same value.

droberts-sea · 2019-03-30T01:15:07Z

lib/slack_main.rb

+
+def clear_selected(workspace)
+  workspace.selected = nil
+end


I would say this is a demeter violation. Instead of saying to the Workspace "your selected is now nil", it would be cleaner to call a method to ask it to clear itself.

It might be that you could move this method completely into Workspace.

droberts-sea · 2019-03-30T01:46:11Z

lib/recipient.rb

+
+      unless response.code == 200 && response["ok"]
+        raise SlackApiError, "#{response["error"]}"
+      end


Good error checking here.

droberts-sea · 2019-03-30T01:47:43Z

lib/workspace.rb

+    def show_details
+      details = @selected.details
+
+      rows = []


Good use of polymorphism to keep this code generic, not specific to users or channels.

droberts-sea · 2019-03-30T01:48:40Z

specs/coverage/.last_run.json

@@ -0,0 +1,5 @@
+{
+  "result": {
+    "covered_percent": 94.12


Make sure that your coverage files aren't included in git!

droberts-sea · 2019-03-30T01:49:42Z

specs/workspace_spec.rb

+    it "returns nil if given invalid identifier" do
+      @workspace.select_channel("INVALID_IDENTIFIER")
+      expect(@workspace.selected).must_equal nil
+    end


Nice failure case. What happens to a previously selected channel in this case?

droberts-sea · 2019-03-30T01:50:01Z

specs/workspace_spec.rb

+      it "returns a table for a selected channel" do
+        @workspace.select_channel("general")
+        expect(@workspace.show_details).must_be_kind_of Terminal::Table
+      end


What happens if you call show_details without first selecting a user or channel?

ranuseh and others added 30 commits March 19, 2019 13:52

Added API token

8073e93

Add VCR configurations to test_helper

f55599f

Added files and recipient class and method outlines

f241dc8

Add recipient_spec.rb and a test for instantiation. Add attr_reader a…

dc4523d

…nd revise 'initialize' method for Recipient

added method to get recipients in slack_main.rb

545af26

Add base URLs to channel.rb and user.rb. Add self.get method to Recip…

928543c

…ient. Revise Recipient class tests. All tests passing

Added methods outline to workspace, user and channel.

b5e8c5e

Add instantiation of Workspace in slack_main.rb

4f53578

Implemented self.list method in channels

27e2850

Revise self.list method, add tests

f244829

Add 'table_print' gem

1a228f3

Write details method and test for Channel

447ed65

Added helper method to channels

4b0069e

Add self.send_message method and test. All tests passing

3b72fc4

added arg error and test to channel class

923fc92

Add messages to main driver code

f4e0c2c

Add menu and 'list channels' functionality

632e595

Added list method to users and tests to pass

578c529

Added send msg method to user

9e5b393

user send message test all passing

1f2fe55

List users implemented in slack_main

e343c31

Outline of methods in user and channel to select user or channel

3f83f98

added comments to new select methods

21fe12f

Moved send message and error message to recipient class

306d877

Move exception conditionals to 'send_message' inside Recipient; Clean…

96f5c24

… up commented code in channel.rb and user.rb

Added super slack id and name to channel and user class

2f9c7f2

Make User inherit 'name' and 'slack_id' attributes from Recipient cla…

9e40151

…ss; Fix errors; All tests passing

Added implement me in child class in details method in recipient class:

0c35596

Add workspace_spec; All tests passing

62fdb9a

Added test to workspace for channel class

ed448bc

JansenMartin and others added 28 commits March 21, 2019 18:14

Add select-channel and select-user functionalities

0d7e212

Add show-details functionality to main; Fill in 'details' method in W…

6f810c8

…orkspace

Add 'describe' blocks to tests

aa7f35e

Add empty describe block for 'show_details' method

18a295d

Clean up comments

f576965

Replace curly-brace syntax with do...end block in 'select_channel' an…

0895c5d

…d 'select_user'

Change 'slack_id' variable to 'identifier'; change wording in tests f…

0cfe2ad

…or 'select_channel' and 'select_user' for clarity

Add test for 'select_channel'; Revise 'select_channel' so that it can…

9822a15

… find channel based on slack id OR name

Add test for 'select_user'; Revise 'select_user' so that it can selec…

6c74475

…t user based on username OR slack id

Add test to ensure 'select_channel' and 'select_user' methods are sel…

46629ca

…ecting the appropriate recipient

Change variable name in tests for 'select_user' and 'select_channel' …

9ee02fd

…methods for clarity

Make more specific prompts for selecting users and channels

3b1c82c

Clean up comments

e6f4a0e

Move 'selected' attribute to attr_accessor in Workspace; Make main le…

f5f1bdf

…t the user know if no channel/user has the supplied name or ID

formatted list user and channel to print to a table.

23f1e05

Added details to show to channel and user classes

6945ab0

Added message to details selection if no user or channel was selected

e8da559

User and Channel details formatted to display as a table in slack main

c6b126f

update user list in main

5cb81ee

Add details to list-users and list-channels functionalities

431fce8

Added send message option to menu

d858007

Filled in the method for show details and test

8937fb4

Wrote test for send message in both user and channel

29b409a

Small formatting changes

8ce2e94

Fill out 'send_message' method inside Workspace; Add test

3050c10

Add edge case test for sending messages to users

a686a9e

Add headers to users and channels tables

9022bf6

Fix typos in user_spec; Add tests for edge cases in workspace_spec

ee7f654

droberts-sea reviewed Mar 30, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sockets - Jansen & Rosalyn#22

Sockets - Jansen & Rosalyn#22
ranuseh wants to merge 65 commits intoAda-C11:masterfrom
ranuseh:master

ranuseh commented Mar 23, 2019

Uh oh!

droberts-sea commented Mar 30, 2019

Uh oh!

droberts-sea Mar 30, 2019

Uh oh!

droberts-sea Mar 30, 2019

Uh oh!

droberts-sea Mar 30, 2019

Uh oh!

droberts-sea Mar 30, 2019

Uh oh!

droberts-sea Mar 30, 2019

Uh oh!

droberts-sea Mar 30, 2019

Uh oh!

droberts-sea Mar 30, 2019

Uh oh!

droberts-sea Mar 30, 2019

Uh oh!

droberts-sea Mar 30, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ranuseh commented Mar 23, 2019

slack.rb

Comprehension Questions

Uh oh!

droberts-sea commented Mar 30, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants