Sockets-Grace & Bita #23

Bitaman · 2019-03-23T02:00:38Z

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?	It was a good practice reading through the API documents and list our query parameters and writing code to make the get request and get data and then pars it for what we needed. The whole process of how to get data and how to use it made much more sense after doing this project.
Give a short summary of the request/response cycle. Where does your program fit into that scheme?	for example for printing the list of users with their name, ID and real name. we made a get request for the list user url which gave us a Json file as a response with a hash of data for each member. we then had to write methods to go through each hash and get the data that we needed.
How does your program check for and handle errors when using the Slack API?	for example we checked if for a channel we get error if trying to send a message to an invalid ID.
Did you need to make any changes to the design work we did in class? If so, what were they?	In addition to Recipient, user, channel and workspace, we add a new file to interact with the user. It calls slack.rb.
Did you use any of the inheritance idioms we've talked about in class? How?	both user and channel class are inheriting from the recipient. there are methods in recipient which never get implemented and we implemented them in user and channel separately.
How does VCR aid in testing a program that uses an API?	when a call get made to an API, VCR record the data and each time running a test instead of making a new call the test use the VCR which is like a recording of the actual call. this way we don't need to make a call for each test.

droberts-sea · 2019-03-29T23:40:37Z

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	No - There were a number of bugs that made the program crash, including one as soon as you run it. None of these are subtle - they're things like passing the wrong number of arguments to a method. This is sloppy work, and you two can do better.
Implementation
API errors are handled appropriately	some - You do a good job of defining and raising a custom exception, but your command line code does not rescue that exception, so the user still sees a stack trace instead of a polite error message.
Inheritance model matches in-class activity	yes
Inheritance idioms (abstract class, template methods, polymorphism) used appropriately	mostly - see inline
Methods are used to break work down into simpler tasks	yes
Class and instance methods are used appropriately	yes
Overall

In general this is a strong submission. You do a good job of implementing the design, and it is clear to me that the learning goals around the request/response cycle and consuming an API, as well as working with inheritance, were met. I do see room for growth around identifying test cases, and around general attention to detail. Please review my inline comments below, and keep up the hard work!

droberts-sea · 2019-03-29T23:19:48Z

lib/recipient.rb

+
+  CHANNEL_URL = "https://slack.com/api/channels.list"
+  USER_URL = "https://slack.com/api/users.list"
+  POST_URL = "https://slack.com/api/chat.postMessage"


Ideally the superclass would not need to know these details from the subclass. One of the benefits of inheritance is it allows you to add new subclasses without changing the parent class.

To do so, instead of using a constant for this, you could define a template method list_url which is overridden in the child classes to return the correct endpoint.

That would also allow you to get rid of the if statement in self.get below.

droberts-sea · 2019-03-29T23:21:02Z

lib/slack.rb

+    when "list users"
+      puts workspace.show_details("users")
+    when "list channels"
+      puts workspace.show_details("channels")


These should be calling print_details, not show_details

droberts-sea · 2019-03-29T23:22:33Z

lib/slack.rb

+      else
+        puts workspace.show_details
+      end
+    when "send message"


You have the wrong number of arguments to show_details here.

droberts-sea · 2019-03-29T23:22:54Z

lib/user.rb

+      name = member["name"]
+      real_name = member["real_name"]
+      new_member = User.new(slack_id, name, real_name)
+      members_list << new_member


You are missing two arguments in the call to User.new here

droberts-sea · 2019-03-29T23:23:27Z

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 does the same thing as .select.first.

droberts-sea · 2019-03-29T23:29:37Z

specs/workspace_spec.rb

+  it "returns channel details from show_details" do
+    VCR.use_cassette("slack_workspace") do
+      @workspace.select_channel("random")
+      expect(@workspace.show_details).must_be_kind_of String


What if you try to show details without having first selected something?

droberts-sea · 2019-03-29T23:30:29Z

specs/workspace_spec.rb

+  it "returns user details from show_details" do
+    VCR.use_cassette("slack_workspace") do
+      @workspace.select_user("grace.m.shea")
+      expect(@workspace.show_details).must_be_kind_of String


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

droberts-sea · 2019-03-29T23:31:05Z

specs/workspace_spec.rb

+  it "raises an error for invalid channel" do
+    VCR.use_cassette("slack-posts") do
+      @workspace.select_channel("everyone")
+      @workspace.selected.slack_id = "whatever"


I like these edge cases!

droberts-sea · 2019-03-29T23:32:35Z

specs/user_spec.rb

+        user = response.first
+
+        expect(response).wont_be_nil
+        expect(response.first.name).must_equal user.name


The expectation on line 11 is not useful, you've essentially said expect(response.first.name).must_equal response.first.name, which will always be true.

droberts-sea · 2019-03-29T23:34:05Z

specs/user_spec.rb

+describe "User class" do
+  describe "self.list" do
+    it "can return all users" do
+      VCR.use_cassette("slack_user") do


You should include tests for User's entire public interface: User.list, the constructor, User#details, and User#send_message.

Bitaman and others added 30 commits March 19, 2019 13:57

set up the files

4b2d502

created classes and shells for methods in each file

f502ab6

added base_urls and tokens to recipient, user, and channel

8c83fc0

added httparty. refined planned tests for user_spec

3f9786f

initial menu drafted for slack.rb

ff7f7a2

added require relative to specs, channel params added to channel.rb

145d312

channel self.list somewhat working! returns not_authed

e9f005c

auth issue overcome by moving .env from specs to lib

af7056c

use_casette tests missing parentheses, added

7bb84a6

get the list of users and wrote the VCR for that, it's passing

6569872

made changes to user

604fd81

tried to merge

24c7fd2

channel list functioning

b8d8051

merging

9258eb5

trying to get changed files

4e04cc4

channel tests added, debugging

2742dd9

refining channel_list output, created workspace files

390fedb

workspace tests evolving

093b250

resolved HTTPS error

fe50b30

list methods tests/methods refined. passing those tests.

8f059c8

added the detail method to user

1a3732a

had to make changes to user and recipient to make it work

a72b53a

user spec finished

32350f2

slack menu completed

d6de455

adding tests to workspace spec, resolving errors

21ee33b

trying to merge

eef74a4

user_spec edited, passes test

5cc917a

tests refined

d54ba41

initialized workspace in each describe block

8b4e25b

error corrected

0304ab5

gracemshea and others added 26 commits March 22, 2019 10:50

debugging update

855498a

debugging joy

47933a2

syntax error fixed

4dd4e61

checked the user and channel class

55e891f

working through workspace_spec issues

2cc177f

marge attempt

017c3b1

push

18a0e8a

merging

a420ef7

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

04f1abd

select_user and select_channel by name functional

a2c704d

merging

1d870f8

print_details passes test

9e05aaf

show_details method passing tests

9cb2224

Merge

b454049

slack menu debugged

282dd5c

fixing the slack

d35ea14

merging

478bc3b

send message test passing

2281f75

fixing send message

577f10c

for sharing

501080d

merge attempt

a78220e

reversed a merge error

f2bd699

raises errors for invalid channel and user

0db833f

passing to bita

cfce3ba

removing slac spec and recipient spec

0151a13

added status text and imoji to user

791d92a

droberts-sea reviewed Mar 29, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sockets-Grace & Bita #23

Sockets-Grace & Bita #23

Bitaman commented Mar 23, 2019

droberts-sea commented Mar 29, 2019

droberts-sea Mar 29, 2019

droberts-sea Mar 29, 2019

droberts-sea Mar 29, 2019

droberts-sea Mar 29, 2019

droberts-sea Mar 29, 2019

droberts-sea Mar 29, 2019

droberts-sea Mar 29, 2019

droberts-sea Mar 29, 2019

droberts-sea Mar 29, 2019

droberts-sea Mar 29, 2019

Sockets-Grace & Bita #23

Are you sure you want to change the base?

Sockets-Grace & Bita #23

Conversation

Bitaman commented Mar 23, 2019

slack.rb

Comprehension Questions

droberts-sea commented Mar 29, 2019

slack.rb

What We're Looking For

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment