Fixed error handling during updates for multiple identical users acro…

…ss teams.

CHANGELOG.md

@@ -1,5 +1,6 @@
 ### Changelog
 
+* 2024/07/14: Fixed error handling during updates for multiple identical users across teams - [@dblock](https://github.com/dblock).
 * 2024/07/07: Fixed duplicate activities from concurrent updates - [@dblock](https://github.com/dblock).
 * 2024/02/17: Added `leaderboard` - [@dblock](https://github.com/dblock).
 * 2024/01/07: Added Title, Description, Url, User, Athlete and Date field options - [@dblock](https://github.com/dblock).

slack-strava/api/endpoints/strava_endpoint.rb

@@ -37,7 +37,11 @@ class StravaEndpoint < Grape::API
                     Api::Middleware.logger.info "Team #{user.team} subscription expired, user #{user}, #{user.athlete}, #{params['object_type']}=#{params['object_id']}."
                   else
                     Api::Middleware.logger.info "Syncing activity for team #{user.team}, user #{user}, #{user.athlete}, #{params['object_type']}=#{params['object_id']}."
-                    user.sync_and_brag!
+                    begin
+                      user.sync_and_brag!
+                    rescue StandardError => e
+                      Api::Middleware.logger.warn "Error syncing new activity for team #{user.team}, user #{user}, #{user.athlete}, #{params['object_type']}=#{params['object_id']}: #{e}."
+                    end
                   end
                 end
               when 'update'
@@ -48,7 +52,11 @@ class StravaEndpoint < Grape::API
                     activity = user.activities.where(strava_id: params['object_id']).first
                     if activity
                       Api::Middleware.logger.info "Updating activity team #{user.team}, user #{user}, #{user.athlete}, #{params['object_type']}=#{params['object_id']}, #{params['updates']}."
-                      user.rebrag_activity!(activity)
+                      begin
+                        user.rebrag_activity!(activity)
+                      rescue StandardError => e
+                        Api::Middleware.logger.warn "Error rebgragging new activity for team #{user.team}, user #{user}, #{user.athlete}, #{params['object_type']}=#{params['object_id']}: #{e}."
+                      end
                     else
                       Api::Middleware.logger.info "Ignoring activity team #{user.team}, user #{user}, #{user.athlete}, #{params['object_type']}=#{params['object_id']}, #{params['updates']}."
                     end

spec/api/endpoints/strava_endpoint_spec.rb

@@ -77,6 +77,55 @@
             expect(response['ok']).to be true
           end
         end
+        context 'with multiple connected users with the same athlete id' do
+          let!(:team2) { Fabricate(:team) }
+          let!(:user2) do
+            Fabricate(
+              :user,
+              team: team2,
+              athlete: Fabricate.build(:athlete, athlete_id: user.athlete.athlete_id.to_s),
+              access_token: 'token'
+            )
+          end
+          it 'syncs both users' do
+            users = []
+            allow_any_instance_of(User).to receive(:sync_and_brag!) do |a_user, _args|
+              users << a_user
+            end
+            post '/api/strava/event',
+                 JSON.dump(
+                   event_data.merge(
+                     aspect_type: 'create',
+                     owner_id: user.athlete.athlete_id.to_s
+                   )
+                 ),
+                 'CONTENT_TYPE' => 'application/json'
+            expect(last_response.status).to eq 200
+            response = JSON.parse(last_response.body)
+            expect(response['ok']).to be true
+            expect(users).to eq([user, user2])
+          end
+          it 'skips over failures' do
+            users = []
+            allow_any_instance_of(User).to receive(:sync_and_brag!) do |a_user, _args|
+              raise 'error' if a_user == user
+
+              users << a_user
+            end
+            post '/api/strava/event',
+                 JSON.dump(
+                   event_data.merge(
+                     aspect_type: 'create',
+                     owner_id: user.athlete.athlete_id.to_s
+                   )
+                 ),
+                 'CONTENT_TYPE' => 'application/json'
+            expect(last_response.status).to eq 200
+            response = JSON.parse(last_response.body)
+            expect(response['ok']).to be true
+            expect(users).to eq([user2])
+          end
+        end
         context 'with an existing activity' do
           let!(:activity) { Fabricate(:user_activity, user: user, map: nil) }
           it 'rebrags the existing activity' do
@@ -131,6 +180,58 @@
             response = JSON.parse(last_response.body)
             expect(response['ok']).to be true
           end
+          context 'with multiple connected users with the same athlete id' do
+            let!(:team2) { Fabricate(:team) }
+            let!(:user2) do
+              Fabricate(
+                :user,
+                team: team2,
+                athlete: Fabricate.build(:athlete, athlete_id: user.athlete.athlete_id.to_s),
+                access_token: 'token'
+              )
+            end
+            let!(:activity2) { Fabricate(:user_activity, user: user2, strava_id: activity.strava_id, map: nil) }
+            it 'rebrags both activities' do
+              users = []
+              allow_any_instance_of(User).to receive(:rebrag_activity!) do |a_user, _args|
+                users << a_user
+              end
+              post '/api/strava/event',
+                   JSON.dump(
+                     event_data.merge(
+                       aspect_type: 'update',
+                       object_id: activity.strava_id,
+                       owner_id: user.athlete.athlete_id.to_s
+                     )
+                   ),
+                   'CONTENT_TYPE' => 'application/json'
+              expect(last_response.status).to eq 200
+              response = JSON.parse(last_response.body)
+              expect(response['ok']).to be true
+              expect(users).to eq([user, user2])
+            end
+            it 'skips over failures' do
+              users = []
+              allow_any_instance_of(User).to receive(:rebrag_activity!) do |a_user, _args|
+                raise 'error' if a_user == user
+
+                users << a_user
+              end
+              post '/api/strava/event',
+                   JSON.dump(
+                     event_data.merge(
+                       aspect_type: 'update',
+                       object_id: activity.strava_id,
+                       owner_id: user.athlete.athlete_id.to_s
+                     )
+                   ),
+                   'CONTENT_TYPE' => 'application/json'
+              expect(last_response.status).to eq 200
+              response = JSON.parse(last_response.body)
+              expect(response['ok']).to be true
+              expect(users).to eq([user2])
+            end
+          end
           it 'ignores delete' do
             expect_any_instance_of(Logger).to receive(:info).with(/Ignoring aspect type 'delete'/).and_call_original
             expect_any_instance_of(User).to_not receive(:sync_and_brag!)