-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Water - Marj & Sophia #26
base: master
Are you sure you want to change the base?
Changes from all commits
357833e
56499a7
8aea6f9
ddc3a7f
fe91dea
14c2410
5e23a2d
e217619
874389d
dc5bc9a
70de03d
60784a0
c831b5b
079a887
4f15584
e332dd9
82d978e
09d048f
b66cd02
f4b031c
30556a7
875aa2c
8e5080d
2a08a46
3dd6feb
34e3e7d
baa3662
7e4ca44
a2a130d
ceb0bd1
cfc9255
1764779
5e78123
62a6fd4
9b3a944
cb7eb51
fb62659
4d2988b
39b0ea0
21daf11
ed81649
70d9cd4
b843781
91f9d05
c467a49
6c1f594
800eedf
ec7e876
08db616
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,9 @@ | ||
/coverage/ | ||
|
||
.DS_Store | ||
.idea/ | ||
|
||
# Ignore environemnt variables | ||
.env | ||
.byebug_history | ||
coverage |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
# frozen_string_literal: true | ||
|
||
source "https://rubygems.org" | ||
|
||
git_source(:github) {|repo_name| "https://github.com/#{repo_name}" } | ||
|
||
# gem "rails" | ||
gem 'dotenv' | ||
gem 'httparty' |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
GEM | ||
remote: https://rubygems.org/ | ||
specs: | ||
dotenv (2.7.6) | ||
httparty (0.18.1) | ||
mime-types (~> 3.0) | ||
multi_xml (>= 0.5.2) | ||
mime-types (3.3.1) | ||
mime-types-data (~> 3.2015) | ||
mime-types-data (3.2020.0512) | ||
multi_xml (0.6.0) | ||
|
||
PLATFORMS | ||
ruby | ||
|
||
DEPENDENCIES | ||
dotenv | ||
httparty | ||
|
||
BUNDLED WITH | ||
1.17.2 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
require 'httparty' | ||
require_relative 'recipient' | ||
|
||
CHANNEL_URL = "https://slack.com/api/conversations.list" | ||
|
||
|
||
class Channel < Recipient | ||
attr_reader :topic, :member_count | ||
|
||
def initialize(slack_id, name, topic, member_count) | ||
super(slack_id, name) | ||
@topic = topic | ||
@member_count = member_count | ||
end | ||
|
||
def self.list_all | ||
all_channels = self.get(CHANNEL_URL, PARAMS)["channels"] | ||
|
||
all_channels.map do |channel_hash| | ||
new( | ||
channel_hash["id"], | ||
channel_hash["name"], | ||
channel_hash["topic"], | ||
channel_hash["num_members"] | ||
) | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
require 'dotenv' | ||
require 'httparty' | ||
Dotenv.load | ||
|
||
# BASE_URL = 'https://slack.com/api/' | ||
|
||
BOT_API_KEY = ENV['BOT_TOKEN'] | ||
PARAMS = {token: ENV['SLACK_TOKEN']} | ||
|
||
class Recipient | ||
attr_reader :slack_id, :name | ||
|
||
def initialize(slack_id, name) | ||
@slack_id = slack_id | ||
@name = name | ||
end | ||
|
||
def send_message(message) | ||
response = HTTParty.post( | ||
"https://slack.com/api/chat.postMessage", | ||
body: { | ||
token: BOT_API_KEY, | ||
text: message, | ||
channel: @slack_id | ||
}, | ||
headers: { 'Content-Type' => 'application/x-www-form-urlencoded' } | ||
) | ||
return response.code == 200 && response.parsed_response["ok"] | ||
end | ||
|
||
def self.get(url, params) | ||
response = HTTParty.get(url, query: params) | ||
return response | ||
end | ||
|
||
private | ||
|
||
def details | ||
raise NotImplementedError, 'Implement me in a child class!' | ||
end | ||
|
||
def self.list_all | ||
raise NotImplementedError, 'Implement me in a child class!' | ||
end | ||
Comment on lines
+38
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well done on making these template methods!! |
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
require 'httparty' | ||
require_relative 'recipient' | ||
|
||
USER_URL = "https://slack.com/api/users.list" | ||
|
||
|
||
class User < Recipient | ||
attr_reader :real_name | ||
|
||
def initialize(slack_id, name, real_name) | ||
super(slack_id, name) | ||
@real_name = real_name | ||
end | ||
|
||
def self.list_all | ||
all_members = self.get(USER_URL, PARAMS)["members"] | ||
|
||
all_members.map do |user_hash| | ||
Comment on lines
+16
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Highlighting something that I wrote in the feedback table-- If It might be worth anticipating this failure and testing for it! |
||
new( | ||
user_hash["id"], | ||
user_hash["name"], | ||
user_hash["real_name"] | ||
) | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
require_relative 'user' | ||
require_relative 'channel' | ||
require 'httparty' | ||
|
||
class Workspace | ||
attr_reader :channels, :users, :selected | ||
|
||
def initialize(selected: nil) | ||
@channels = Channel.list_all | ||
@users = User.list_all | ||
@selected | ||
end | ||
|
||
def select_channel(user_input) | ||
user_input | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line technically doesn't do anything! Feel free to delete it |
||
|
||
if user_input | ||
selected = @channels.find { |hash| hash.name == user_input } | ||
|
||
if selected.nil? | ||
selected = @channels.find {|hash| hash.slack_id == user_input } | ||
end | ||
|
||
results = selected.nil? ? "This is not a valid Channel" : selected.name | ||
puts "Result: #{results} " | ||
Comment on lines
+24
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Following some other comments-- it might be interesting to move these lines back into |
||
end | ||
|
||
@selected = selected | ||
return selected | ||
end | ||
|
||
def select_user(user_input) | ||
user_input | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to above-- this line of code doesn't do anything! Feel free to delete it |
||
|
||
if user_input | ||
selected = @users.find { |hash| hash.name == user_input } | ||
|
||
if selected.nil? | ||
selected = @users.find {|hash| hash.slack_id == user_input } | ||
end | ||
|
||
results = selected.nil? ? "This is not a valid User" : selected.name | ||
puts "Result: #{results} " | ||
end | ||
Comment on lines
+35
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at this nested |
||
|
||
@selected = selected | ||
return selected | ||
end | ||
|
||
def show_details | ||
if @selected.is_a?(User) | ||
pp [@selected.slack_id, @selected.name, @selected.real_name] | ||
elsif @selected.is_a?(Channel) | ||
pp [@selected.name, @selected.topic, @selected.member_count, @selected.slack_id] | ||
end | ||
end | ||
|
||
def send_message | ||
# store the user's message | ||
# send message to the @selected recipient | ||
if @selected.nil? | ||
puts "Please select a user or channel." | ||
else | ||
puts "Enter your message here:" | ||
message = gets.chomp | ||
http_post_response = @selected.send_message(message) | ||
p http_post_response | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You probably don't need this line for the project submission!
Comment on lines
+66
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here, you name the variable It might be good to reconsider the variable name here. I might put |
||
end | ||
|
||
return !!http_post_response | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
AKA
Comment on lines
+58
to
+70
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be worth considering moving this a lot of logic into I bring this up because this method has a lot of logic around getting input and printing feedback. In the Consider something like this: def send_message(message)
is_successful = @selected.send_message(message)
return is_successful
end |
||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
PARAMS
constant gets defined inrecipient.rb
, but never used here. Instead, it gets used in the child classesChannel
andUser
only.Putting the
PARAMS
constant here isn't bad, actually, it's quite common! And it takes advantage of inheritance! This is awesome, well done :D But I also want to highlight that it makes the code a little less readable-- as I read the other classes, I will think, "where does this constant get defined?!" and eventually (quickly) realize it's in Recipient. Again, I don't think you made the wrong decision, I thinkPARAMS
has a good reason to live inrecipient.rb
, but in case you hadn't thought about it, it might be even better to repeat yourself and redefinePARAMS
in bothChannel
andUser
if that makes sense to you.