Disable club sync on invalid_auth or not_in_channel.

dblock · Nov 29, 2024 · ef8c24b · ef8c24b
1 parent 441863a
commit ef8c24b
Show file tree

Hide file tree

Showing 8 changed files with 101 additions and 31 deletions.
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2024-11-24 17:28:28 UTC using RuboCop version 1.68.0.
+# on 2024-11-29 15:39:05 UTC using RuboCop version 1.68.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -78,7 +78,7 @@ Naming/VariableNumber:
     - 'spec/models/team_leaderboard_spec.rb'
     - 'spec/models/user_spec.rb'
 
-# Offense count: 140
+# Offense count: 145
 RSpec/AnyInstance:
   Enabled: false
 
@@ -114,7 +114,7 @@ RSpec/EmptyExampleGroup:
   Exclude:
     - 'spec/models/athlete_spec.rb'
 
-# Offense count: 165
+# Offense count: 167
 # Configuration parameters: CountAsOne.
 RSpec/ExampleLength:
   Max: 36
@@ -151,7 +151,7 @@ RSpec/InstanceVariable:
     - 'spec/slack-strava/service_spec.rb'
     - 'spec/support/api/endpoints/it_behaves_like_a_cursor_api.rb'
 
-# Offense count: 35
+# Offense count: 36
 RSpec/LetSetup:
   Enabled: false
 
@@ -161,11 +161,11 @@ RSpec/LetSetup:
 RSpec/MessageSpies:
   EnforcedStyle: receive
 
-# Offense count: 204
+# Offense count: 207
 RSpec/MultipleExpectations:
   Max: 11
 
-# Offense count: 32
+# Offense count: 33
 # Configuration parameters: AllowSubject.
 RSpec/MultipleMemoizedHelpers:
   Max: 14
@@ -179,7 +179,7 @@ RSpec/NamedSubject:
     - 'spec/api/endpoints/teams_endpoint_spec.rb'
     - 'spec/slack-strava/app_spec.rb'
 
-# Offense count: 106
+# Offense count: 109
 # Configuration parameters: AllowedGroups.
 RSpec/NestedGroups:
   Max: 7
@@ -331,7 +331,7 @@ Style/StringConcatenation:
     - 'slack-strava/api/helpers/error_helpers.rb'
     - 'slack-strava/models/team.rb'
 
-# Offense count: 232
+# Offense count: 233
 # This cop supports safe autocorrection (--autocorrect).
 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns.
 # URISchemes: http, https

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,6 @@
 ### Changelog
 
+* 2024/11/29: Disable club sync on `invalid_auth` or `not_in_channel` - [@dblock](https://github.com/dblock).
 * 2024/11/23: Include primary activity photo - [@dblock](https://github.com/dblock).
 * 2024/11/23: Render activities as blocks - [@dblock](https://github.com/dblock).
 * 2024/11/10: Added medal field - [@dblock](https://github.com/dblock).

diff --git a/slack-strava/api/endpoints/requests/command.rb b/slack-strava/api/endpoints/requests/command.rb
@@ -101,17 +101,19 @@ def club_connect_channel!
 
           strava_id = arg
           strava_club = Club.attrs_from_strava(user.strava_client.club(strava_id))
-          club = Club.create!(
+          club = user.team.clubs.where(channel_id: channel_id).first
+          club ||= Club.new(team: user.team, channel_id: channel_id)
+          club.assign_attributes(
             strava_club.merge(
+              sync_activities: true,
               access_token: user.access_token,
               refresh_token: user.refresh_token,
               token_expires_at: user.token_expires_at,
               token_type: user.token_type,
-              team: user.team,
-              channel_id: channel_id,
               channel_name: channel_name
             )
           )
+          club.save!
           logger.info "Connected #{club}, #{user}, #{user.team}."
           user.team.slack_client.chat_postMessage(
             club.to_slack.merge(

diff --git a/slack-strava/models/club.rb b/slack-strava/models/club.rb
@@ -83,12 +83,18 @@ def channel_mention
     "<##{channel_id}>"
   end
 
+  def disabled_s
+    return unless persisted? && !sync_activities?
+
+    'Sync disabled.'
+  end
+
   def to_slack
     {
       attachments: [{
         title: name,
         title_link: strava_url,
-        text: [description, location, member_count_s].compact.join("\n"),
+        text: [description, location, member_count_s, disabled_s].compact.join("\n"),
         thumb_url: logo,
         color: '#FC4C02'
       }]
@@ -100,13 +106,13 @@ def connect_to_slack
       attachments: [{
         title: name,
         title_link: strava_url,
-        text: [description, location, member_count_s].compact.join("\n"),
+        text: [description, location, member_count_s, disabled_s].compact.join("\n"),
         thumb_url: logo,
         color: '#FC4C02',
-        callback_id: "club-#{persisted? ? 'disconnect' : 'connect'}-channel",
+        callback_id: "club-#{persisted? && sync_activities? ? 'disconnect' : 'connect'}-channel",
         actions: [{
           name: 'strava_id',
-          text: persisted? ? 'Disconnect' : 'Connect',
+          text: persisted? && sync_activities? ? 'Disconnect' : 'Connect',
           type: 'button',
           value: strava_id
         }]

diff --git a/slack-strava/models/club_activity.rb b/slack-strava/models/club_activity.rb
@@ -40,17 +40,19 @@ def brag!
     case e.message
     when 'is_archived'
       logger.warn "Bragging to #{club} failed, #{e.message}."
-      club.team.inform_admin!(text: "I couldn't post an activity from #{club.name} into #{club.channel_mention} because the channel was archived, please reconnect that club in a different channel.")
       club.update_attributes!(sync_activities: false)
+      club.team.inform_admin!(text: "I couldn't post an activity from #{club.name} into #{club.channel_mention} because the channel was archived, please reconnect that club in a different channel.")
       NewRelic::Agent.notice_error(e, custom_params: { team: club.team.to_s, self: club.to_s })
       nil
     when 'restricted_action'
       logger.warn "Bragging to #{club} failed, #{e.message}."
+      club.update_attributes!(sync_activities: false)
       club.team.inform_admin!(text: "I wasn't allowed to post into #{club.channel_mention} because of a Slack workspace preference, please contact your Slack admin.")
       NewRelic::Agent.notice_error(e, custom_params: { team: club.team.to_s, self: club.to_s })
       nil
     when 'not_in_channel', 'account_inactive'
       logger.warn "Bragging to #{club} failed, #{e.message}."
+      club.update_attributes!(sync_activities: false)
       NewRelic::Agent.notice_error(e, custom_params: { team: club.team.to_s, self: club.to_s })
       nil
     else

diff --git a/spec/api/endpoints/slack_endpoint_spec.rb b/spec/api/endpoints/slack_endpoint_spec.rb
@@ -18,24 +18,24 @@
     context 'interactive buttons' do
       let(:user) { Fabricate(:user, team: team, access_token: 'token', token_expires_at: Time.now + 1.day) }
 
-      context 'without a club' do
-        let(:club) do
-          Club.new(
-            name: 'Orchard Street Runners',
-            description: 'www.orchardstreetrunners.com',
-            url: 'OrchardStreetRunners',
-            city: 'New York',
-            state: 'New York',
-            country: 'United States',
-            member_count: 146,
-            logo: 'https://dgalywyr863hv.cloudfront.net/pictures/clubs/43749/1121181/4/medium.jpg'
-          )
-        end
+      let(:osr) do
+        Club.new(
+          name: 'Orchard Street Runners',
+          description: 'www.orchardstreetrunners.com',
+          url: 'OrchardStreetRunners',
+          city: 'New York',
+          state: 'New York',
+          country: 'United States',
+          member_count: 146,
+          logo: 'https://dgalywyr863hv.cloudfront.net/pictures/clubs/43749/1121181/4/medium.jpg'
+        )
+      end
 
+      context 'without a club' do
         it 'connects club', vcr: { cassette_name: 'strava/retrieve_a_club' } do
           expect {
             expect_any_instance_of(Slack::Web::Client).to receive(:chat_postMessage).with(
-              club.to_slack.merge(
+              osr.to_slack.merge(
                 as_user: true,
                 channel: 'C12345',
                 text: "A club has been connected by #{user.slack_mention}."
@@ -61,6 +61,38 @@
       context 'with a club' do
         let!(:club) { Fabricate(:club, team: team) }
 
+        context 'with sync disabled' do
+          before do
+            club.update_attributes!(sync_activities: false)
+          end
+
+          it 'reconnects the existing club', vcr: { cassette_name: 'strava/retrieve_a_club' } do
+            expect {
+              expect_any_instance_of(Slack::Web::Client).to receive(:chat_postMessage).with(
+                osr.to_slack.merge(
+                  as_user: true,
+                  channel: club.channel_id,
+                  text: "A club has been connected by #{user.slack_mention}."
+                )
+              )
+              expect_any_instance_of(Strava::Api::Client).to receive(:paginate)
+              expect_any_instance_of(Club).to receive(:sync_last_strava_activity!)
+              post '/api/slack/action', payload: {
+                actions: [{ name: 'strava_id', value: '43749' }],
+                channel: { id: club.channel_id, name: 'runs' },
+                user: { id: user.user_id },
+                team: { id: team.team_id },
+                token: token,
+                callback_id: 'club-connect-channel'
+              }.to_json
+              expect(last_response.status).to eq 201
+              response = JSON.parse(last_response.body)
+              expect(response['attachments'][0]['actions'][0]['text']).to eq 'Disconnect'
+            }.not_to change(Club, :count)
+            expect(club.reload.sync_activities).to be true
+          end
+        end
+
         it 'disconnects club' do
           expect {
             expect_any_instance_of(Strava::Api::Client).to receive(:paginate)
@@ -259,6 +291,30 @@
               expect(response['attachments'].count).to eq 6
               expect(response['attachments'][0]['title']).to eq nyrr_club.name
               expect(response['attachments'][1]['title']).to eq club.name
+              expect(response['attachments'][1]['actions'].first['text']).to eq 'Disconnect'
+              expect(response['attachments'][1]['text']).not_to include "\nSync disabled."
+            end
+          end
+
+          context 'with another disabled club in the channel' do
+            let!(:club_in_another_channel) { Fabricate(:club, team: team, channel_id: 'another') }
+            let!(:club) { Fabricate(:club, team: team, channel_id: 'channel', sync_activities: false) }
+
+            it 'lists both clubs a user is a member of and the connected club', vcr: { cassette_name: 'strava/list_athlete_clubs' } do
+              post '/api/slack/command',
+                   command: '/slava',
+                   text: 'clubs',
+                   channel_id: 'channel',
+                   channel_name: 'channel_name',
+                   user_id: user.user_id,
+                   team_id: team.team_id,
+                   token: token
+              response = JSON.parse(last_response.body)
+              expect(response['attachments'].count).to eq 6
+              expect(response['attachments'][0]['title']).to eq nyrr_club.name
+              expect(response['attachments'][1]['title']).to eq club.name
+              expect(response['attachments'][1]['actions'].first['text']).to eq 'Connect'
+              expect(response['attachments'][1]['text']).to include "\nSync disabled."
             end
           end
 

diff --git a/spec/models/club_activity_spec.rb b/spec/models/club_activity_spec.rb
@@ -34,6 +34,7 @@
         }
         expect(activity.brag!).to be_nil
       }.not_to change(Club, :count)
+      expect(club.reload.sync_activities).to be false
     end
 
     it 'warns if the account goes inactive' do
@@ -46,6 +47,7 @@
           expect(activity.brag!).to be_nil
         }.not_to change(Club, :count)
       }.not_to change(ClubActivity, :count)
+      expect(club.reload.sync_activities).to be false
     end
 
     it 'informs admin on restricted_action' do
@@ -57,6 +59,7 @@
         }
         expect(activity.brag!).to be_nil
       }.not_to change(Club, :count)
+      expect(club.reload.sync_activities).to be false
     end
 
     it 'informs admin on is_archived channel' do

diff --git a/spec/models/club_spec.rb b/spec/models/club_spec.rb
@@ -125,7 +125,7 @@
         Faraday::ResourceNotFound.new(404, body: { 'message' => 'Not Found', 'errors' => [] })
       )
       expect(club.team.slack_client).to receive(:chat_postMessage).with(
-        club.to_slack.merge(
+        hash_including(
           text: 'Your club can no longer be found on Strava. Please disconnect and reconnect it via /slava clubs.',
           channel: club.channel_id,
           as_user: true