Fire - Alice & Madeline #25

made-line · 2020-10-10T07:39:59Z

Assignment Submission: Slack CLI

Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.

Reflection

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?
Give a short summary of the request/response cycle. Where does your program fit into that scheme?
How does your program check for and handle errors when using the Slack API?
How did the design and organization of your project change over time?
Did you use any of the inheritance idioms we've talked about in class? How?
How does VCR aid in testing a program that uses an API?

codeandmorecode · 2020-10-10T23:55:49Z

SORRY FOR WRITING THIS HERE, WE SUBMITED WITHOUT ANSWERING THE QUESTIONS, SO WE WILL ANSWER THEM RIGHT HERE:

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 explored the documentation pages of the slack API more than anything, we also watched all the videos on Learn to understand how to use the information from the documentation pages.
Give a short summary of the request/response cycle. Where does your program fit into that scheme?	The request is sent from the client to the Service. The service processes the request, issues a response which travels from the service back to the client (our computer). Our program is in charge of sending the request to the service, and interpreting the response from the service.
How does your program check for and handle errors when using the Slack API?	We have a test file that checks if messages have been sent or not, users have been selected or not, etc.
How did the design and organization of your project change over time?	The most significant change was that initially our list and select methods gave back hashes with the information for the users and channels. We then changed them so that they return instances of users and channels classes, and therefore had to create those classes.
Did you use any of the inheritance idioms we've talked about in class? How?	We initially wanted to have a recipient class that would serve as a parent class but upon receiving help, we understood that it was not a requirement because the way our program was structured worked without the inheritance relationship. However, if we were to refactor, we could have a recipient class which would serve as an extension of the polymorphism our program already displays.
How does VCR aid in testing a program that uses an API?	VCR aid in testing a program because using VCR means that your program creates a cassette which serves as a testing tool to keep the program from making api calls, and allows the program's testing to run even if your wifi is down, thereby lowering the cost of your testing because api calls aren't free ;)

CheezItMan

Slack CLI

Major Learning Goals/Code Review

Criteria	yes/no, and optionally any details/lines of code to reference
Practices best practices working with APIs. The `.env` is not checked into git, and no API token was directly used in the Ruby code without `ENV`.	✔️
Practices error handling with APIs. For all pieces of code that make an API call, it handles API requests that come back with errors/error status codes appropriately.	✔️
Implements inheritance and inheritance idioms. There is a `Recipient` class. `User` and `Channel` inherit from `Recipient`. In `Recipient`, there are appropriate methods defined that are used in both `User` and `Channel`. Some may be implemented. Some may be template methods.	⚠️ , No inheritance implemented.
Practices clean code. `lib/slack.rb` only interacts with `Workspace` to show a separation of responsibilities. Complex logic is broken into smaller helper methods.	⚠️, You have some repetitive, code in in your Workspace class regarding selection and getting users/channels.
Practices instance methods vs. class methods appropriately. The methods to list all Channels or Users is likely a class method within those respective classes.	✔️, However like I noted inline your methods to list users and channels would make an excellent factory method (class method).
Practices best practices for testing. The project has and uses VCR mocking when running tests, and can run offline.	✔️
Practices writing tests. The `User`, `Channel`, and `Workspace` classes have unit tests.	⚠️, you only have tests for workspace
Practices writing tests. There are tests for sending messages (the location of these tests may differ, but is likely in `Recipient`)	⚠️, you have one test for sending messages.
Practices git with at least 15 small commits and meaningful commit messages	⚠️, only 13 commits and a lot of your commit messages focus on waves. Instead make commit messages describing functionality.

Functional Requirements

Functional Requirement	yes/no
As a user of the CLI program, I can list users and channels	✔️?
As a user of the CLI program, I can select users and channels	✔️?
As a user of the CLI program, I can show the details of a selected user or channel	✔️?
As a user of the CLI program, when I input something inappropriately, the program runs without crashing	✔️?

Overall Feedback

Overall Feedback	Criteria	yes/no
Yellow (Approaches Standards)	6+ in Code Review && 2+ in Functional Requirements

Code Style Bonus Awards

Was the code particularly impressive in code style for any of these reasons (or more...?)

Quality	Yes?
Perfect Indentation	✅
Descriptive/Readable	✅
Concise	✅

Summary

Your design has some duplication of functionality, you do make some unnecesary API calls and your testing is minimal at best. That said it works and hits most of the learning goals here.

Also you didn't answer the comprehension questions.

CheezItMan · 2020-10-15T17:23:56Z

lib/workspace.rb

+  def get_users
+    user_response = HTTParty.get(GET_USER_PATH, query: { token: ENV["SLACK_TOKEN"] })
+
+    @users = user_response["members"].map do |member|
+      User.new(member["name"], member["real_name"], member["id"])
+    end
+
+    unless user_response.code == 200
+      raise SlackApiError, "Error: #{user_response.parsed_response["error"]}"
+    end
+
+    return @users
+  end


I just want to point out that listing users is a lot like listing channels. You could abstract this content out into a parent class.

You could also create a factory method in your classes like we did in Ride Share or Grocery Store.

CheezItMan · 2020-10-15T17:27:55Z

lib/workspace.rb

+  def select_user(username_or_id)
+    selected_user = self.get_users
+      .select { |user| user.username == username_or_id || user.id == username_or_id }
+      .first
+    return selected_user
+  end


This makes an api call every time you select a user, why not instead use your instance variable @users if it's not empty.

CheezItMan · 2020-10-15T17:28:30Z

lib/workspace.rb

+  def select_channel(name_or_id)
+    selected_channel = self.get_channels
+      .select { |channel| channel.name == name_or_id || channel.id == name_or_id }
+      .first
+    return selected_channel
+  end


Just note how similar this is to select_user could you absract this into a helper method?

CheezItMan · 2020-10-15T17:37:24Z

lib/user.rb

@@ -0,0 +1,13 @@
+class User 


Just noting the User and Channel classes are pretty small, you could make the method to retrieve all the users a method here inside User (class method), similar to how we did Ride share.

CheezItMan · 2020-10-15T17:38:19Z

test/workspace_test.rb

+    end
+  end
+
+  it "returns \"Message sent!\" when message is sent" do


What happens if you send a message with no channel or user selected?

CheezItMan · 2020-10-15T17:38:57Z

test/workspace_test.rb

+require_relative "test_helper"
+require_relative "../lib/workspace"
+
+describe "Workspace" do


I suggest using some sub-describes for the functionality here. Like one for each method allowing you to group tests a bit.

CheezItMan · 2020-10-15T17:39:25Z

test/workspace_test.rb

+    end
+  end
+
+  it "select channel" do


What about selecting a channel that doesn't exist?

CheezItMan · 2020-10-15T17:44:22Z

test/workspace_test.rb

+      expect(@workspace.send_message(recipient_id, message)).must_be_same_as true
+    end
+  end
+end


No tests for users or channels?

made-line and others added 12 commits October 7, 2020 18:15

progress 2

8148aa4

Wave 1 updated with tables using table_print

eaf73bf

WAVE 1 with tests - COMPLETE

318944d

WAVE 1 WITH TESTS COMPLETE

721fb5b

Wave 2 command skeleton added in

0906103

finished wave 2 methods and CLI logic. still needs tests.

d624e03

wave 3 progress

ce30e6d

Wave 3 progress

0d18458

Wave 3 complete and working, tests for wave 2 and 3 in progress

a5aadcb

some testing wave 2 & 3

7484d5d

finished wave 3 and tests

a7afc06

fixed wave 3 test and fixed username vs name for selecting user

097459b

fixed wave 3 test - again

32c105d

CheezItMan reviewed Oct 15, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fire - Alice & Madeline #25

Fire - Alice & Madeline #25

made-line commented Oct 10, 2020

codeandmorecode commented Oct 10, 2020 •

edited

Loading

CheezItMan left a comment

CheezItMan Oct 15, 2020

CheezItMan Oct 15, 2020

CheezItMan Oct 15, 2020

CheezItMan Oct 15, 2020

CheezItMan Oct 15, 2020

CheezItMan Oct 15, 2020

CheezItMan Oct 15, 2020

CheezItMan Oct 15, 2020

Fire - Alice & Madeline #25

Are you sure you want to change the base?

Fire - Alice & Madeline #25

Conversation

made-line commented Oct 10, 2020

Assignment Submission: Slack CLI

Reflection

codeandmorecode commented Oct 10, 2020 • edited Loading

CheezItMan left a comment

Choose a reason for hiding this comment

Slack CLI

Major Learning Goals/Code Review

Functional Requirements

Overall Feedback

Code Style Bonus Awards

Summary

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

codeandmorecode commented Oct 10, 2020 •

edited

Loading