Fire - Roshni and Pauline - Slack CLI #6

ghostfruitleaf · 2020-10-09T04:57:14Z

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?	We spent time reading through the Slack API documentation and testing things out with Postman. We learned various things, such as needing to use User ID to send to both channels and users with chat.postMessage or being able to customize the bot's username and emoji.
Give a short summary of the request/response cycle. Where does your program fit into that scheme?	The client (e.g. our program) makes a request to the server (e.g. Slack API). The server processes the request and returns a response to the client. The client can then look at the response and determine what to do next and they may choose to send another request and continue with the request/response cycle.
How does your program check for and handle errors when using the Slack API?	In our program, we raise a SlackAPIError if the user enters a message that is over 4000 characters, receives a "false" ok response, or a non-200 code response. We also raise an ArgumentError if the user enters an invalid emoji.
How did the design and organization of your project change over time?	Although we knew from the beginning that we wanted to include a Recipient, User, Channel, and Workspace class, as we progressed through the project and began working on the optional requirements, we decided to add a Bot class that would inherit from the User class to address customizing the bot's emoji and username.
Did you use any of the inheritance idioms we've talked about in class? How?	We used inheritance idioms in the form of abstract classes and template methods. Recipient was an abstract class that was the parent of the User and Channel classes and it had template methods (details and list all methods). Bot in turn inherited from User.
How does VCR aid in testing a program that uses an API?	VCR is helpful when testing a program that uses an API because it records HTTP interactions (“cassettes”) and allows you to replay them whenever you need to without having to make another API call.

…est server

…l.rb, output should be hash rather than returnable

…ga .ga .ga .ga .

…clarification line in slack.rb to reprint menu after every action, added sleep to get in recipient in case of infinite loop

… channel tests

…workspace select_channel test

…ersion...

…ackApiError calls in send_message

…ests that have thankfully passed

…share in details, completed adjusting user_test, started adjusting workspace_test

…ntly working

…m/PaulineChane/slack-cli

…to current_bot field

…ithub.com/PaulineChane/slack-cli

…omized message

…d send_message to account for new parameters

kaidamasaki

Excellent job! There were some small things I called out in the comments but overall your solution was really strong.

You did an extremely good job of turning the UML diagrams we gave you into a coherent set of classes that all work well together and have clearly defined responsibilities. 😃

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.	✔️
Practices clean code. `lib/slack.rb` only interacts with `Workspace` to show a separation of responsibilities. Complex logic is broken into smaller helper methods.	✔️
Practices instance methods vs. class methods appropriately. The methods to list all Channels or Users is likely a class method within those respective classes.	✔️
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.	✔️
Practices writing tests. There are tests for sending messages (the location of these tests may differ, but is likely in `Recipient`)	✔️
Practices git with at least 15 small commits and meaningful commit messages	✔️

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
Green (Meets/Exceeds Standards)	7+ in Code Review && 3+ in Functional Requirements	✔️
Yellow (Approaches Standards)	6+ in Code Review && 2+ in Functional Requirements
Red (Not at Standard)	0-5 in Code Review or 0,1 in Functional Reqs, or assignment is breaking/doesn’t run with less than 5 minutes of debugging

Code Style Bonus Awards

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

Quality	Yes?
Perfect Indentation	✅
Elegant/Clever	✅
Descriptive/Readable	✅
Concise	✅
Logical/Organized	✅

kaidamasaki · 2020-10-14T23:27:59Z

lib/recipient.rb

+      # we don't want to break the program here because the reason
+      # could be anything
+      raise SlackApiError, "Error: #{response.parsed_response['error']}. Please try again"
+      return false


You don't need to return false here. The raise will make sure this line never runs.

kaidamasaki · 2020-10-14T23:31:36Z

lib/bot.rb

+    url = 'https://slack.com/api/auth.test'
+    query = {token: Bot.token}
+    sleep(0.5)
+    response = HTTParty.get(url, query: query)['user_id']


You should check the response code of this request.

kaidamasaki · 2020-10-14T23:38:55Z

lib/bot.rb

+
+  def self.list_all
+    bots = super
+    bots = bots.filter{ |user| user.is_bot || user.slack_id == "USLACKBOT"}


Nice touch filtering out the bot users.

kaidamasaki · 2020-10-14T23:39:56Z

lib/channel.rb

+  end
+
+  def details
+    tp self, :slack_id, :name, :topic, :member_count


You should leave the actual printing of things up to slack.rb. (This is why your unit tests print out tables in the middle.)

kaidamasaki · 2020-10-14T23:40:29Z

lib/channel.rb

+    url = "https://slack.com/api/conversations.list"
+    param = {token: Channel.token}
+    raw_channels = Channel.get(url, param)['channels']
+    all_channels = raw_channels.map do |channel|


Nice use of map!

kaidamasaki · 2020-10-15T23:23:21Z

lib/user.rb

+
+  attr_reader :real_name, :time_zone, :is_bot
+
+  def initialize(slack_id:, name:, real_name:, time_zone: "Pacific Daylight Time", is_bot: false)


Configurable timezones! 🎉

kaidamasaki · 2020-10-15T23:24:03Z

lib/user.rb

+    raw_users = User.get(url, param)['members']
+    all_users = [] # to skip over deleted users
+    raw_users.each do |member|
+      next if member['deleted']


I really like your attention to detail with skipping bad entries like this.

kaidamasaki · 2020-10-15T23:25:22Z

lib/workspace.rb

+        end
+      end
+    end
+    return nil # returns nil to indicate user not found


I might raise a UserNotFoundError instead of returning nil.

You can forget to check for a nil return value but you notice right away if you got an exception.

kaidamasaki · 2020-10-15T23:29:38Z

test/channel_test.rb

+        # NOTE: We didn't write a test to check whether the token is valid because that has already been tested in the
+        # .get method in Recipient.rb from which it inherits, meaning it should work here too for channel.rb


Nice documentation! 😃

kaidamasaki · 2020-10-15T23:51:42Z

test/recipient_test.rb

+        message = ""
+        4005.times do
+          message += "A"
+        end


Ruby actually has built in syntax for this (for some reason):

Suggested change

message = ""

4005.times do

message += "A"

end

message = "A" * 4005

roshni-patel and others added 30 commits October 6, 2020 16:41

Worked through design exercise and setup files

2b6c2ca

Moved class files to lib folder

56401fe

First draft of channel.rb

660e263

v0 of user.rb User class, added recipient get method

d35a786

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

1ba5769

Edited channel.rb list_all and details methods

f03138d

added empty test files and SlackApiError custom exception

ba97b42

oops, committed from wrong directory, see last commit's comments

9bf9be8

removed quick script to test token, reorganized files to work with rake

e52353d

debugging User and pushing for collaborators to test issues with

c49ccc8

Finished list_all and details methods

0ce5a32

Wrote basic CLI program for wave 1

dad996c

added tests for recipient.rb

9723559

working nominal test for self.get method in recipient

7ccc2a1

Edited CLI program, workspace.rb, working and able to get info from t…

14cc009

…est server

fixed initialize param in recipient to keywords

03cd82b

forgot to delete the two tests files we don't really need, oops (again)

0509517

First draft of channel.rb tests

7a5c126

First draft of channel.rb tests

12b6c6e

added more to list_all Channel test

e044a86

Updated channel test file details method and details method in channe…

18fb075

…l.rb, output should be hash rather than returnable

completed tests for user.rb

1f40e2f

Added some more assertions to channel.rb test

53c5c90

added workspace.rb test for constructor; wave 1 has been finishedga .…

80b886b

…ga .ga .ga .ga .

added and tested select_user and send_message in workspace.rb, added …

a67a28f

…clarification line in slack.rb to reprint menu after every action, added sleep to get in recipient in case of infinite loop

wrote a quick test for SlackApiError

b797ec7

Updated CLI for wave 2, wrote select channel method, and wrote select…

9320a03

… channel tests

added send_message method to workspace and recipient, minor edits to …

6fc0370

…workspace select_channel test

Added error handling for details in case statement

6728752

laid out tests for send_message, added extra space to url recipient v…

e153bc8

…ersion...

roshni-patel and others added 29 commits October 7, 2020 20:35

Added send message option to CLI

48e553c

removed reset test and reset funciton in workspace.show_details

3a4daf0

adding SLACK_TOKEN class constant to recipient

20cc803

added clear_selection method to workspace

2e1da0b

fixed a lot of tests and added token method to recipient.rb, fixed Sl…

548752e

…ackApiError calls in send_message

added working tests for workspace.rb send_message method

a53e54a

Updated slack CLI with clear selection method

d2610b5

modified send_message in recipient to return parsed response, added t…

6280336

…ests that have thankfully passed

created bot class for optional, modified user to have more fields to …

113ac54

…share in details, completed adjusting user_test, started adjusting workspace_test

clean up tests and user after field modifications

45b9c78

added save message history and get message history methods, not curre…

07dba27

…ntly working

completed v0 of bot class for optional

5275735

adding slack.rb updates to merge branch 'master' of https://github.co…

d1acf8b

…m/PaulineChane/slack-cli

fixed current_bot method for bot.rb

6b0c9db

removed redundant methods in workspace that can be directly called on…

74d7cb9

…to current_bot field

Started writing bot tests

564a4d1

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

be1cf5c

fixed some bot functions, one test is not working still

e38f46c

mostly working CLI, will need to change a few things

69a6be4

minor fixes to worksapce set_user and bot tests

a7528ad

adding more slack functionalities; Merge branch 'master' of https://g…

fdf0980

…ithub.com/PaulineChane/slack-cli

fixed select_user method to account for current bot to send with cust…

5d639de

…omized message

updated cli to combine emoji and username

1584681

caught error in implementation of sending customized message; modifie…

8fac170

…d send_message to account for new parameters

fixed parameters for send_message in workspace

39da3ab

fixed test for bot to send with customized emoji

f95cefb

very minor fixes to slack.rb, should be working now :D

d6c315a

fixed emoji check regex and bug from opening empty message history file

8be0ff4

fixed failing test on send_message methods

aa96ec5

kaidamasaki reviewed Oct 16, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fire - Roshni and Pauline - Slack CLI #6

Fire - Roshni and Pauline - Slack CLI #6

ghostfruitleaf commented Oct 9, 2020

kaidamasaki left a comment

kaidamasaki Oct 14, 2020

kaidamasaki Oct 14, 2020

kaidamasaki Oct 14, 2020

kaidamasaki Oct 14, 2020

kaidamasaki Oct 14, 2020

kaidamasaki Oct 15, 2020

kaidamasaki Oct 15, 2020

kaidamasaki Oct 15, 2020

kaidamasaki Oct 15, 2020

kaidamasaki Oct 15, 2020


		attr_reader :real_name, :time_zone, :is_bot

		def initialize(slack_id:, name:, real_name:, time_zone: "Pacific Daylight Time", is_bot: false)

		# NOTE: We didn't write a test to check whether the token is valid because that has already been tested in the
		# .get method in Recipient.rb from which it inherits, meaning it should work here too for channel.rb

Fire - Roshni and Pauline - Slack CLI #6

Are you sure you want to change the base?

Fire - Roshni and Pauline - Slack CLI #6

Conversation

ghostfruitleaf commented Oct 9, 2020

Assignment Submission: Slack CLI

Reflection

kaidamasaki 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

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