Branches - Farah && Steph by Steph0088 · Pull Request #11 · Ada-C12/slack-cli

Steph0088 · 2019-09-13T22:26:52Z

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?	Slack API had good documentation that allowed us to understand how to get and post data. We definitely continued to use postman and it really helped us conceptualize how data is organized and returned by API's.
Give a short summary of the request/response cycle. Where does your program fit into that scheme?	In the slack.rb file the user selects and option and it calls on a method within one of our classes that requests information from the Slack API. That information is returned in the method within the class method and printed for the user.
How does your program check for and handle errors when using the Slack API?	We knew the names of our Channels and Users and so we used tests to ensure that it was correctly returning them.
Did you need to make any changes to the design work we did in class? If so, what were they?	We generally used the same format and classes that were provided.
Did you use any of the inheritance idioms we've talked about in class? How?	We did in our Recipient class where we used self.list, which is an abstract method that was implemented in the children classes.
How does VCR aid in testing a program that uses an API?	It helps ensure you can still "call" on the slack api even when there is no connection to the API.

tildeee · 2019-09-21T07:06:08Z

slack.rb

What We're Looking For

Feature	Feedback
Core Requirements
Git hygiene (no slack tokens)	x
Comprehension questions	x
Functionality
List users/channels	Yes, needs one small adjustment
Select user/channel	x
Show details	x
Send message
Program runs without crashing	x
Implementation
API errors are handled appropriately	Handled some user errors
Inheritance model matches in-class activity	x
Inheritance idioms (abstract class, template methods, polymorphism) used appropriately	x
Methods are used to break work down into simpler tasks	x
Class and instance methods are used appropriately	x
Tests written for User functionality	x
Tests written for Channel Functionality	x
Tests written for sending a message
Overall

Great work on this project, Farah and Steph! Your code is logical and clean, and your tests are well-constructed. You two did a great job getting the API functionality going and wrapping in tests, VCR, and design.

I have a few comments about the submission:

The tests look great, honestly! The tests are well-written and, overall, test details that are worth checking. Keep up this test-writing style!
The design that y'all went with did not use a Workspace class, but you all made it work by using class methods ✨, and you two used them correctly and well. Even though it doesn't look like some other projects, you two did a very smart way of still doing the same necessary functionality
Ultimately, this project is going to be marked as approaching standard because we didn't get to that sweet end of Wave 3
I'm adding just a few comments for suggestions on refactoring

Any other comment I have a minor one about handling API errors: we didn't go into handling API errors in depth during class, but I'd encourage you two to start thinking about: what happens if my response comes back with a status code that's not 200 OK? Your code currently doesn't handle that and would break in those cases! Food for thought.

Again good work on this overall!

tildeee · 2019-09-21T07:07:29Z

lib/.gitignore

@@ -0,0 +1,57 @@
+.env


we actually don't need a .gitignore file in the lib folder... just one in the project root directory!

tildeee · 2019-09-21T07:08:03Z

lib/channel.rb

+  def initialize(slack_id, name, topic, member_count)
+    super(slack_id, name)
+    @topic = topic
+    @member_count = member_count


Do @topic or @member_count get used? Could we delete these?

tildeee · 2019-09-21T07:08:21Z

lib/channel.rb

+    super(slack_id, name)
+    @topic = topic
+    @member_count = member_count
+    @@channels_list << self


I think that it's very clever to use the class variable @@channels_list and to shovel in self! However, does this variable ever get used beyond this? Do we need it?

tildeee · 2019-09-21T07:08:26Z

lib/channel.rb

+    @@channels_list << self
+  end
+
+  def self.list


Nice helper method!

tildeee · 2019-09-21T07:08:40Z

lib/slack.rb

+  puts"Welcome to the Ada Slack CLI!"
+  puts"Select an option by number:"
+  options_array=["List Channels","List Users","Select User","Select Channel","Quit"]
+  options_array.each_with_index{ |channel, index|


Nitpick: the syntax for this line means that every element in options_array when we iterate through it will be named channel... but options_array doesn't contain channels, it contains options! Would it make sense if this variable were named option instead of channel?

tildeee · 2019-09-21T07:08:54Z

lib/slack.rb

+
+  while user_answer != 5
+    if user_answer == 1
+      Channel.printed_channels_list


Forgot to puts this!

tildeee · 2019-09-21T07:09:06Z

lib/slack.rb

+    end
+
+    puts "Select an option by number:"
+    options_array.each_with_index { |channel,index|


Nitpick: same as above; instead of channel, is there a better variable name?

tildeee · 2019-09-21T07:09:27Z

test/channel_test.rb

+        channels_array = Channel.printed_channels_list
+        channels_array.each do |channel|
+          if channel["name"] == "general"
+            assert (channel), "Expected true"


To be honest, I'm just not sure what this line of code does-- what does it test?

tildeee · 2019-09-21T07:09:34Z

test/channel_test.rb

+      end
+    end
+
+    it "should return an array of channels" do


Looking at your implementation, the Channel.printed_channels_list method DOES NOT return an array of channels, and it returns an array of hashes! Your test name and test contents should reflect this

tildeee · 2019-09-21T07:09:52Z

test/channel_test.rb

+  end
+
+  describe "self.select_channel_details method" do
+    it "should return true four 'general' details" do


this is a great test! Good detail! Keep doing things like this when it's appropriate!

tildeee · 2019-09-21T07:10:00Z

test/channel_test.rb

+      end
+    end
+
+    it "should return 'Not valid' statement if the Channel does not exist" do


Good edge case test!

Steph0088 and others added 28 commits September 10, 2019 12:31

Copied ENV file to lib: Authenticated Token Works

07fa5f1

Created Recipient Class and test constructor

50dd9ac

added new user method list

ff25ea4

created list method and passed test

33ee029

added self.list method for User and passed nominal test

b91c159

Added tests for Channel and List method

eb1d375

Resolve merge conflict

49fa7e9

Formatted tabs

8fa86da

Attempted adding API component to user.list method

21f5bfa

Fixed indentation/do&end issues

fc314b6

resolved conflict

4a56f31

Resolved more merge conflicts

dccd549

deleted cassettes and added filter sensitive data for token

dd51b5c

Fixed list channel method and tests

9bac8ab

Completed tests for users and channel class

149b664

Created while loop for slack.rb

7b9089a

Created Channels list and Users list

2cc91bf

Finished Wave 1

a32b69a

pseudocode

2a5d88b

Added psuedocode for wave 2

9069a39

started writing code for wave 2, token issues

4ff86ed

Completed user details method

06908fe

Added select channel details method

ed6d503

Cleaned up

5d45c48

Started writing tests for user

08181a4

Created more tests for User class

466523c

Fixed user class and created more tests

74ef32f

Added tests for channel class

66f643d

tildeee reviewed Sep 21, 2019

View reviewed changes

lib/channel.rb

@@channels_list << self

end

def self.list

Copy link

tildeee Sep 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice helper method!

tildeee reviewed Sep 21, 2019

View reviewed changes

lib/slack.rb

while user_answer != 5

if user_answer == 1

Channel.printed_channels_list

Copy link

tildeee Sep 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to puts this!

tildeee reviewed Sep 21, 2019

View reviewed changes

test/channel_test.rb

end

end

it "should return 'Not valid' statement if the Channel does not exist" do

Copy link

tildeee Sep 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good edge case test!

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Branches - Farah && Steph#11

Branches - Farah && Steph#11
Steph0088 wants to merge 28 commits intoAda-C12:masterfrom
Steph0088:master

Steph0088 commented Sep 13, 2019

Uh oh!

tildeee commented Sep 21, 2019

Uh oh!

tildeee Sep 21, 2019

Uh oh!

tildeee Sep 21, 2019

Uh oh!

tildeee Sep 21, 2019

Uh oh!

tildeee Sep 21, 2019

Uh oh!

tildeee Sep 21, 2019

Uh oh!

tildeee Sep 21, 2019

Uh oh!

tildeee Sep 21, 2019

Uh oh!

tildeee Sep 21, 2019

Uh oh!

tildeee Sep 21, 2019

Uh oh!

tildeee Sep 21, 2019

Uh oh!

tildeee Sep 21, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Steph0088 commented Sep 13, 2019

slack.rb

Comprehension Questions

Uh oh!

tildeee commented Sep 21, 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