From 2cd2e1fcd39e95efc519e9f1839005c2c19c740c Mon Sep 17 00:00:00 2001 From: Kenneth Bogner Date: Mon, 3 Jun 2024 23:59:33 -0400 Subject: [PATCH] Matches for group (#72) * Move RetrieveSlackUserInfo to minitest * Add method to HistoricalMatch for getting profiled members * Add historical match filter --- .gitignore | 2 + app/controllers/profile_controller.rb | 2 +- app/controllers/recent_matches_controller.rb | 12 ++++- app/models/historical_match.rb | 14 ++++++ .../collects_recent_matches_for_user.rb | 4 +- .../use_email_to_deliver_notification.rb | 4 +- ...er_info.rb => retrieve_slack_user_info.rb} | 2 +- .../recent_matches/_historical_match.html.erb | 24 ++++++++++ app/views/recent_matches/filter.html.erb | 13 +++++ config/routes.rb | 1 + .../collects_recent_matches_for_user_spec.rb | 10 ++-- .../slack/retrieves_slack_user_info_spec.rb | 42 ----------------- test/models/historical_match_test.rb | 16 +++++++ .../mailer/build_group_mailer_message_test.rb | 2 +- .../use_email_to_deliver_notification_test.rb | 8 ++-- .../slack/retrieve_slack_user_info_test.rb | 47 +++++++++++++++++++ 16 files changed, 144 insertions(+), 59 deletions(-) rename app/services/slack/{retrieves_slack_user_info.rb => retrieve_slack_user_info.rb} (94%) create mode 100644 app/views/recent_matches/_historical_match.html.erb create mode 100644 app/views/recent_matches/filter.html.erb delete mode 100644 spec/lib/slack/retrieves_slack_user_info_spec.rb create mode 100644 test/services/slack/retrieve_slack_user_info_test.rb diff --git a/.gitignore b/.gitignore index abced15..aea16bd 100644 --- a/.gitignore +++ b/.gitignore @@ -28,3 +28,5 @@ !/app/assets/builds/.keep .irb_history + +.DS_Store diff --git a/app/controllers/profile_controller.rb b/app/controllers/profile_controller.rb index b35d1f0..9395e53 100644 --- a/app/controllers/profile_controller.rb +++ b/app/controllers/profile_controller.rb @@ -11,7 +11,7 @@ def load_current_user_profile(slack_user_id) @user_profile = SlackUserProfile.find_by(slack_user_id: slack_user_id) return if @user_profile.present? - Slack::RetrievesSlackUserInfo.new.call(user: slack_user_id) + Slack::RetrieveSlackUserInfo.new.call(user: slack_user_id) @user_profile = SlackUserProfile.find_by(slack_user_id: slack_user_id) end diff --git a/app/controllers/recent_matches_controller.rb b/app/controllers/recent_matches_controller.rb index 1b828ef..e838670 100644 --- a/app/controllers/recent_matches_controller.rb +++ b/app/controllers/recent_matches_controller.rb @@ -5,14 +5,24 @@ def index @recent_matches = CollectsRecentMatchesForUser.new.call(user: @current_user) end + def filter + load_current_user_profile + + @historical_matches = HistoricalMatch.where(filter_params).order(matched_on: :desc) + end + private def load_current_user_profile @user_profile = SlackUserProfile.find_by(slack_user_id: @current_user.slack_user_id) return if @user_profile.present? - Slack::RetrievesSlackUserInfo.new.call(user: @current_user.slack_user_id) + Slack::RetrieveSlackUserInfo.new.call(user: @current_user.slack_user_id) @user_profile = SlackUserProfile.find_by(slack_user_id: @current_user.slack_user_id) end + + def filter_params + params.permit(:grouping) + end end diff --git a/app/models/historical_match.rb b/app/models/historical_match.rb index f681710..2650408 100644 --- a/app/models/historical_match.rb +++ b/app/models/historical_match.rb @@ -33,6 +33,20 @@ def self.protracted_in(grouping) SQL end + def profiled_members + members.map do |member| + user_profile = SlackUserProfile.find_by(slack_user_id: member) + + if user_profile.nil? + Slack::RetrieveSlackUserInfo.new.call(user: member) + + user_profile = SlackUserProfile.find_by(slack_user_id: member) + end + + user_profile + end + end + private def at_least_two_members diff --git a/app/services/collects_recent_matches_for_user.rb b/app/services/collects_recent_matches_for_user.rb index 16a0039..c246e07 100644 --- a/app/services/collects_recent_matches_for_user.rb +++ b/app/services/collects_recent_matches_for_user.rb @@ -1,6 +1,6 @@ class CollectsRecentMatchesForUser def initialize - @retrieves_slack_user_info = Slack::RetrievesSlackUserInfo.new + @retrieve_slack_user_info = Slack::RetrieveSlackUserInfo.new end def call(user:) @@ -26,7 +26,7 @@ def fetch_user_profile(slack_user_id) user_profile = SlackUserProfile.find_by(slack_user_id: slack_user_id) if user_profile.nil? - @retrieves_slack_user_info.call(user: slack_user_id) + @retrieve_slack_user_info.call(user: slack_user_id) user_profile = SlackUserProfile.find_by(slack_user_id: slack_user_id) end diff --git a/app/services/notify/use_email_to_deliver_notification.rb b/app/services/notify/use_email_to_deliver_notification.rb index ff6772a..565d09e 100644 --- a/app/services/notify/use_email_to_deliver_notification.rb +++ b/app/services/notify/use_email_to_deliver_notification.rb @@ -1,7 +1,7 @@ module Notify class UseEmailToDeliverNotification def initialize - @retrieves_slack_user_info = Slack::RetrievesSlackUserInfo.new + @retrieve_slack_user_info = Slack::RetrieveSlackUserInfo.new @build_group_mailer_message = Mailer::BuildGroupMailerMessage.new end @@ -26,7 +26,7 @@ def call(notification, group) private def convert_to_match_member(member_id) - slack_user = @retrieves_slack_user_info.call(user: member_id) + slack_user = @retrieve_slack_user_info.call(user: member_id) Mailer::MatchMember.from_slack_user(slack_user) end end diff --git a/app/services/slack/retrieves_slack_user_info.rb b/app/services/slack/retrieve_slack_user_info.rb similarity index 94% rename from app/services/slack/retrieves_slack_user_info.rb rename to app/services/slack/retrieve_slack_user_info.rb index 0ab6667..ace62e2 100644 --- a/app/services/slack/retrieves_slack_user_info.rb +++ b/app/services/slack/retrieve_slack_user_info.rb @@ -1,5 +1,5 @@ module Slack - class RetrievesSlackUserInfo + class RetrieveSlackUserInfo include RateLimitRetryable def call(user:) diff --git a/app/views/recent_matches/_historical_match.html.erb b/app/views/recent_matches/_historical_match.html.erb new file mode 100644 index 0000000..36fc647 --- /dev/null +++ b/app/views/recent_matches/_historical_match.html.erb @@ -0,0 +1,24 @@ +
+
+
+ <% historical_match.profiled_members.each do |member| %> + <%= link_to profile_path(member.slack_user_id) do %> +
+ + <%= member.name %> +
+ <% end %> + <% end %> +
+
+
+ <%= historical_match.grouping.titleize %> +
+
+
+ <%= historical_match.matched_on.strftime("%b %d, %Y") %> +
+
+
+
+
\ No newline at end of file diff --git a/app/views/recent_matches/filter.html.erb b/app/views/recent_matches/filter.html.erb new file mode 100644 index 0000000..9343bb1 --- /dev/null +++ b/app/views/recent_matches/filter.html.erb @@ -0,0 +1,13 @@ +
+
+

Historical matches

+
+ +
+ <% if @historical_matches.empty? %> +

No matches made yet

+ <% else %> + <%= render partial: "historical_match", collection: @historical_matches %> + <% end %> +
+
\ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index ddbfac6..b9d9ab7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -20,6 +20,7 @@ root to: "root#index" get "/matches", to: "recent_matches#index", as: "recent_matches" + get "/matches/filter", to: "recent_matches#filter", as: "filter_recent_matches" get "/profile/:slack_user_id", to: "profile#show", as: "profile" resources :calendar_links, except: [:show] diff --git a/spec/lib/collects_recent_matches_for_user_spec.rb b/spec/lib/collects_recent_matches_for_user_spec.rb index fd19c08..ad6d152 100644 --- a/spec/lib/collects_recent_matches_for_user_spec.rb +++ b/spec/lib/collects_recent_matches_for_user_spec.rb @@ -4,9 +4,9 @@ let(:subject) { CollectsRecentMatchesForUser.new } before(:example) do - @retrieves_slack_user_info = double(Slack::RetrievesSlackUserInfo) + @retrieve_slack_user_info = double(Slack::RetrieveSlackUserInfo) - allow(Slack::RetrievesSlackUserInfo).to receive(:new) { @retrieves_slack_user_info } + allow(Slack::RetrieveSlackUserInfo).to receive(:new) { @retrieve_slack_user_info } end it "merges matches and slack profile data from the database for match members" do @@ -15,7 +15,7 @@ SlackUserProfile.create(name: "Leia", slack_user_id: "USER_ID_2", avatar_url: "https://example.com/x/512/512") match = HistoricalMatch.create(members: ["USER_ID_1", "USER_ID_2"], grouping: "test", matched_on: match_date) - expect(@retrieves_slack_user_info).to_not receive(:call) + expect(@retrieve_slack_user_info).to_not receive(:call) user_matches = subject.call(user: user) @@ -38,7 +38,7 @@ match1 = HistoricalMatch.create(members: ["USER_ID_1", "USER_ID_2"], grouping: "test", matched_on: match_date - 1.day) match2 = HistoricalMatch.create(members: ["USER_ID_1", "USER_ID_2"], grouping: "test", matched_on: match_date) - expect(@retrieves_slack_user_info).to_not receive(:call) + expect(@retrieve_slack_user_info).to_not receive(:call) user_matches = subject.call(user: user) @@ -67,7 +67,7 @@ match_date = Date.today HistoricalMatch.create(members: ["USER_ID_1", "USER_ID_2"], grouping: "test", matched_on: match_date) - expect(@retrieves_slack_user_info).to receive(:call).with(user: "USER_ID_2") + expect(@retrieve_slack_user_info).to receive(:call).with(user: "USER_ID_2") subject.call(user: user) end diff --git a/spec/lib/slack/retrieves_slack_user_info_spec.rb b/spec/lib/slack/retrieves_slack_user_info_spec.rb deleted file mode 100644 index e7b6a01..0000000 --- a/spec/lib/slack/retrieves_slack_user_info_spec.rb +++ /dev/null @@ -1,42 +0,0 @@ -require "rails_helper" - -RSpec.describe Slack::RetrievesSlackUserInfo do - let(:subject) { Slack::RetrievesSlackUserInfo.new } - - before(:example) do - @slack_client = double(Slack::Web::Client) - allow(Slack::ClientWrapper).to receive(:client) { @slack_client } - end - - it "loads a user's information from slack" do - slack_user = "USER_ID" - - expect(@slack_client).to receive(:users_info).with(user: "USER_ID") { - Slack::Messages::Message.new(ok: true, user: { - id: slack_user, - profile: Slack::Messages::Message.new(real_name: "Luke", image_512: "https://example.com/x/512/512") - }) - } - - user_info = subject.call(user: slack_user) - - expect(user_info.id).to eq(slack_user) - end - - it "stores some user profile information to the database" do - slack_user = "USER_ID" - - expect(@slack_client).to receive(:users_info).with(user: "USER_ID") { - Slack::Messages::Message.new(ok: true, user: { - id: slack_user, - profile: Slack::Messages::Message.new(real_name: "Luke", image_512: "https://example.com/x/512/512") - }) - } - - expect { - subject.call(user: slack_user) - }.to change { - SlackUserProfile.find_by(slack_user_id: slack_user) - }.from(NilClass) - end -end diff --git a/test/models/historical_match_test.rb b/test/models/historical_match_test.rb index f0a324a..cdfee5b 100644 --- a/test/models/historical_match_test.rb +++ b/test/models/historical_match_test.rb @@ -154,4 +154,20 @@ class HistoricalMatchTest < ActiveSupport::TestCase assert_equal matches, [match2] end + + test "#profiled_members returns the user profiles for each member" do + match = @subject.new(members: ["Frodo", "Sam"]) + frodo_profile = SlackUserProfile.create(slack_user_id: "Frodo") + sam_profile = SlackUserProfile.create(slack_user_id: "Sam") + + assert_equal match.profiled_members, [frodo_profile, sam_profile] + end + + test "#profiled_members creates a user profile if one does not exist" do + retrieves_user_info = Mocktail.of_next(Slack::RetrieveSlackUserInfo) + + @subject.new(members: ["Frodo"]).profiled_members + + verify { retrieves_user_info.call(user: "Frodo") } + end end diff --git a/test/services/mailer/build_group_mailer_message_test.rb b/test/services/mailer/build_group_mailer_message_test.rb index 84a372d..36c95d3 100644 --- a/test/services/mailer/build_group_mailer_message_test.rb +++ b/test/services/mailer/build_group_mailer_message_test.rb @@ -3,7 +3,7 @@ module Mailer class BuildGroupMailerMessageTest < ActiveSupport::TestCase setup do - @retrieves_slack_user_info = Mocktail.of_next(Slack::RetrievesSlackUserInfo) + @retrieve_slack_user_info = Mocktail.of_next(Slack::RetrieveSlackUserInfo) @subject = BuildGroupMailerMessage.new end diff --git a/test/services/notify/use_email_to_deliver_notification_test.rb b/test/services/notify/use_email_to_deliver_notification_test.rb index f760e48..879c178 100644 --- a/test/services/notify/use_email_to_deliver_notification_test.rb +++ b/test/services/notify/use_email_to_deliver_notification_test.rb @@ -6,7 +6,7 @@ class UseEmailToDeliverNotificationTest < ActiveSupport::TestCase # Mocktail wasn't working for a class like GroupingMailer. It was returning nil # and it wasn't clear why. I decided to use Minitest::Mock instead. @mailer = Minitest::Mock.new - @retrieves_slack_user_info = Mocktail.of_next(Slack::RetrievesSlackUserInfo) + @retrieve_slack_user_info = Mocktail.of_next(Slack::RetrieveSlackUserInfo) @build_group_mailer_message = Mocktail.of_next(Mailer::BuildGroupMailerMessage) @group = group_with(name: "test", slack_channel_name: "test-channel") @@ -22,10 +22,10 @@ class UseEmailToDeliverNotificationTest < ActiveSupport::TestCase pending_notifications: [notification] ) - stubs { @retrieves_slack_user_info.call(user: "USER_ID_1") }.with { + stubs { @retrieve_slack_user_info.call(user: "USER_ID_1") }.with { slack_user_message("USER_ID_1", "Luke", "luke@rebels.com") } - stubs { @retrieves_slack_user_info.call(user: "USER_ID_2") }.with { + stubs { @retrieve_slack_user_info.call(user: "USER_ID_2") }.with { slack_user_message("USER_ID_2", "Leia", "leia@rebels.com") } stubs { |m| @@ -63,7 +63,7 @@ class UseEmailToDeliverNotificationTest < ActiveSupport::TestCase pending_notifications: [notification] ) - stubs { |m| @retrieves_slack_user_info.call(user: m.any) } + stubs { |m| @retrieve_slack_user_info.call(user: m.any) } .with { raise "Should not be called" } stubs { |m| @build_group_mailer_message.render( diff --git a/test/services/slack/retrieve_slack_user_info_test.rb b/test/services/slack/retrieve_slack_user_info_test.rb new file mode 100644 index 0000000..11b9e3c --- /dev/null +++ b/test/services/slack/retrieve_slack_user_info_test.rb @@ -0,0 +1,47 @@ +require "test_helper" + +module Slack + class RetrieveSlackUserInfoTest < ActiveSupport::TestCase + setup do + @subject = RetrieveSlackUserInfo.new + end + + test "loads a user's information from slack" do + stub_slack_client do |slack_client| + @subject.call(user: "USER_ID") + + verify { slack_client.users_info(user: "USER_ID") } + end + end + + test "stores some user profile information to the database" do + stub_slack_client do |slack_client| + stubs { slack_client.users_info(user: "USER_ID") }.with do + slack_user_response("USER_ID") + end + + assert_difference -> { SlackUserProfile.count } do + @subject.call(user: "USER_ID") + end + + user_profile = SlackUserProfile.find_by(slack_user_id: "USER_ID") + assert_equal "Luke", user_profile.name + end + end + + private + + def slack_user_response(id) + Slack::Messages::Message.new( + id: id, + user: Slack::Messages::Message.new( + id: id, + profile: Slack::Messages::Message.new( + real_name: "Luke", + image_512: "https://example.com/x/512/512" + ) + ) + ) + end + end +end