Daniela & Cloudy - Leaves by dnsanche · Pull Request #22 · Ada-C12/slack-cli

dnsanche · 2019-09-14T05:02:34Z

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?	I learned that there are get and post requests and each one can have different parameters. Each API has its own naming, so they will change depending the API.
Give a short summary of the request/response cycle. Where does your program fit into that scheme?	User selects an option, then program runs and sends a get or post request to the server depending user input, server checks the request and sends back a response.
How does your program check for and handle errors when using the Slack API?	the program checks that the query is valid and sends code different than 200 and ok if it is invalid.
Did you need to make any changes to the design work we did in class? If so, what were they?	I did few changes. I didn't implement the abstract method in recipient.
Did you use any of the inheritance idioms we've talked about in class? How?	Yes, I created a parent class called recipient, so user class and channel class inherit name and id from it.
How does VCR aid in testing a program that uses an API?	It records the first time the program sends a request to the server in testing and then everytime, you run same test, it runs same recorded response preventing a new call to the server.

…lso, set up test helper with all require_realtives.

…rinted on the screen

…e kinks

dHelmgren · 2019-09-23T20:41:03Z

slack.rb

What We're Looking For

Feature	Feedback
Core Requirements
Git hygiene (no slack tokens)	Great!
Comprehension questions	good
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	yes
Inheritance model matches in-class activity	no
Inheritance idioms (abstract class, template methods, polymorphism) used appropriately	no, you've diverged pretty heavily in this case. You've completely removed template methods, meaning that your work for something like `list_channels` is duplicated.
Methods are used to break work down into simpler tasks	mostly, see comment.
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	Good work overall! You've got a few bumps in terms of how you chose to deviate from the design we gave you, and I hope my comments help you understand why we laid the project out the way we did! Keep up the hard work.

dHelmgren · 2019-09-23T20:22:52Z

lib/channel.rb

@@ -0,0 +1,34 @@
+#lib/channel.rb
+require 'httparty'
+require 'awesome_print'


and here! Doing this sort of double inclusion causes warnings like this:

/Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/colorize-0.8.1/lib/colorize/class_methods.rb:97: warning: method redefined; discarding old red /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/awesome_print-1.8.0/lib/awesome_print/core_ext/string.rb:19: warning: previous definition of red was here /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/colorize-0.8.1/lib/colorize/class_methods.rb:97: warning: method redefined; discarding old green /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/awesome_print-1.8.0/lib/awesome_print/core_ext/string.rb:19: warning: previous definition of green was here /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/colorize-0.8.1/lib/colorize/class_methods.rb:97: warning: method redefined; discarding old yellow /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/awesome_print-1.8.0/lib/awesome_print/core_ext/string.rb:19: warning: previous definition of yellow was here /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/colorize-0.8.1/lib/colorize/class_methods.rb:97: warning: method redefined; discarding old blue /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/awesome_print-1.8.0/lib/awesome_print/core_ext/string.rb:19: warning: previous definition of blue was here /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/colorize-0.8.1/lib/colorize/class_methods.rb:97: warning: method redefined; discarding old cyan /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/awesome_print-1.8.0/lib/awesome_print/core_ext/string.rb:19: warning: previous definition of cyan was here /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/colorize-0.8.1/lib/colorize/class_methods.rb:97: warning: method redefined; discarding old white /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/awesome_print-1.8.0/lib/awesome_print/core_ext/string.rb:19: warning: previous definition of white was here /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/table_print-1.5.6/lib/table_print/column.rb:13: warning: method redefined; discarding old name= /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/table_print-1.5.6/lib/table_print/row_group.rb:224: warning: shadowing outer local variable - value /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/table_print-1.5.6/lib/table_print/returnable.rb:27: warning: mismatched indentations at 'end' with 'def' at 25 /Users/devin/Documents/Ada/c12/slack-cli/test/message_test.rb:20: warning: assigned but unused variable - response

Which are messy and make it hard to debug, but also often hint at bigger problems with code.

dHelmgren · 2019-09-23T20:22:58Z

lib/slack.rb

+require "httparty"
+require "awesome_print"
+require "colorize"
+require "table_print"


You've included this here...

dHelmgren · 2019-09-23T20:26:22Z

lib/slack.rb

+  @workspace = Slack::Workspace.new()
+  while true
+    puts "Welcome to the Ada Slack CLI!".colorize(:color => :orange, :mode => :bold)
+    puts "\nPlease Choose from the following options:


These two lines print every time you come back to the top of the loop, but it would make more sense for them to only print when the CLI starts.

dHelmgren · 2019-09-23T20:26:48Z

lib/slack.rb

+    when "select user", "3"
+      puts "Please enter username or Slack ID"
+      user_selection = gets.chomp
+      @workspace.select_user(user_selection)


I'd love to see some sort of confirmation message if things went right here.

dHelmgren · 2019-09-23T20:27:33Z

lib/slack.rb

+      elsif recipient.class == Slack::User
+        print "Please enter the message: "
+        message = gets.chomp
+        id = recipient.id


consider breaking these larger blocks out into helper methods so that this logic is easier to read!

dHelmgren · 2019-09-23T20:33:37Z

lib/message.rb

+class SlackApiError < StandardError ; end
+
+module Slack  
+  def self.send_msg(message, channel)


If you're going to go the route of making an API wrapper, move all of the messages into the wrapper!

dHelmgren · 2019-09-23T20:36:23Z

lib/channel.rb

+
+    def self.channels_list
+      channels = []
+      response = HTTParty.get("#{CHANNEL_URI}/channels.list", query: {token: CHANNEL_KEY}) 


The route I'd rather see you go here is to have a generic list method for both channels and users that you implement in recipient, which could call a method that actually parses the response appropriately, similar to what csv_record did in OO ride share

dHelmgren · 2019-09-23T20:37:14Z

lib/recipient.rb

+    attr_reader :name, :id
+    def initialize(name, id)
+      @name = name
+      @id = id


If you're writing a class this short, ask yourself 2 questions:

If it's this short, do I need it?

If it's needed, is someone else doing its job?

dHelmgren · 2019-09-23T20:39:05Z

test/message_test.rb

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


I'd call this file 'slackapi_wrapper_test

dHelmgren · 2019-09-23T20:39:42Z

lib/message.rb

@@ -0,0 +1,25 @@
+#lib/message.rb


This isn't really a message class, it's a slackapi_wrapper class. This holds a bunch of information about how to contact the slack API

ohcloud and others added 30 commits September 10, 2019 15:38

set up environment, added classes for recipient, user, and channel. A…

c96e696

…lso, set up test helper with all require_realtives.

ran into weird errors, pushing so dani can pull.

d6b04bd

Finished list of users and channels

0300de9

created the logic for user. created workspace.rb.

62092a6

Pre-Cloudy

bdaced2

Pre Cloudy

93cf8bb

working driver code. and command line stuff

3209db1

Updates to slack.rb and workspace.rb

a6f7fd4

working on main.

c3c746f

pushing and pulling

13a9df3

still working on printing all correct properties. almost there.

214cb9c

Same as Cloudy

83702a2

same as Cloudy

21e8578

Fixed driver code loop. Removed recursive loop

a6d48e8

Added option to enter numbers on the menu, and to slow down what is p…

3e442c7

…rinted on the screen

Finished driver code wave 2

5400aa6

Finished setting up sending a message to the channel

bf99630

created message to user

66654ed

Message working

da83a78

accepting all changes

099722c

updated all changes

6d125e8

started testing for users and channels. all tests pass

bb9ccde

added more tests. whoohooo,

a51f1f5

completing testing, incoming weird errors, were trying to work out th…

b0ffc4c

…e kinks

Added VCR and finshed some tests in workspace

30148ba

going to update token. have deleted it as its now disabled

5f33fb1

Added workspace again

30d5654

Added the cassetes we lost due token publishing

7876ed7

Finished tests for details

99c9d3b

Finished all tests

26ea133

Added additional test to improve coverage

f4097e6

dHelmgren reviewed Sep 23, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Daniela & Cloudy - Leaves#22

Daniela & Cloudy - Leaves#22
dnsanche wants to merge 31 commits intoAda-C12:masterfrom
OhCloud:master

dnsanche commented Sep 14, 2019 •

edited

Loading

Uh oh!

dHelmgren commented Sep 23, 2019

Uh oh!

dHelmgren Sep 23, 2019

Uh oh!

dHelmgren Sep 23, 2019

Uh oh!

dHelmgren Sep 23, 2019

Uh oh!

dHelmgren Sep 23, 2019

Uh oh!

dHelmgren Sep 23, 2019

Uh oh!

dHelmgren Sep 23, 2019

Uh oh!

dHelmgren Sep 23, 2019

Uh oh!

dHelmgren Sep 23, 2019

Uh oh!

dHelmgren Sep 23, 2019

Uh oh!

dHelmgren Sep 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

dnsanche commented Sep 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

slack.rb

Comprehension Questions

Uh oh!

dHelmgren commented Sep 23, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dnsanche commented Sep 14, 2019 •

edited

Loading