From 9cae8afa0e812e4697dd466779799c07145d8b6b Mon Sep 17 00:00:00 2001 From: Sparky Autonomous Date: Wed, 4 Feb 2026 05:49:23 +0000 Subject: [PATCH] fix(github): resolve sync gaps and prevent data validation crashes --- app/models/github/issue.rb | 22 +++++++++++++------ app/models/github/repository.rb | 39 +++++++++++++++++++++------------ 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/app/models/github/issue.rb b/app/models/github/issue.rb index 022799e29..706899796 100644 --- a/app/models/github/issue.rb +++ b/app/models/github/issue.rb @@ -194,7 +194,6 @@ def self.extract_from_url(url) def update_from_github(options={}) # read synced_at before updating if_modified_since = options[:force] ? nil : self.synced_at.try(:httpdate) - update_attribute(:synced_at, Time.now) # add access token params = { @@ -209,7 +208,12 @@ def update_from_github(options={}) ) # trigger an update on the underlying object - self.class.update_attributes_from_github_data(api_response.data, obj: self) + if api_response.success? + update_attribute(:synced_at, Time.now) + self.class.update_attributes_from_github_data(api_response.data, obj: self) + else + Rails.logger.error "Github::Issue(#{id}) Update Failed: #{api_response.status} - #{api_response.data.inspect}" + end end # Sync comments with GitHub. Deletes comments that no longer exist. @@ -281,8 +285,11 @@ def github_api_path end def self.update_attributes_from_github_array(github_data, options={}) + # IMPROVEMENT: Guard against non-array data + return [] unless github_data.is_a?(Array) + # bulk load all issues - issue_hash = where("remote_id in (?)", github_data.map { |r| r['id'] }) + issue_hash = where("remote_id in (?)", github_data.map { |r| r['id'] if r.is_a?(Hash) }.compact) .inject({}) do |hash,obj| hash[obj.remote_id.to_i] = obj hash @@ -290,14 +297,15 @@ def self.update_attributes_from_github_array(github_data, options={}) # bulk update - repos = Github::Repository.update_attributes_from_github_array(github_data.map { |issue| issue['repo'] }.uniq.compact) + repos = Github::Repository.update_attributes_from_github_array(github_data.map { |issue| issue['repo'] if issue.is_a?(Hash) }.uniq.compact) repos_hash = repos.inject({}) { |hash,obj| hash[obj.remote_id.to_i] = obj; hash } # bulk update linked accounts to save lots of lookups - linked_accounts = LinkedAccount::Github.update_attributes_from_github_array(github_data.map { |issue| issue['user'] }.uniq.compact) - linked_accounts_hash = linked_accounts.inject({}) { |hash,obj| hash[obj.remote_id.to_i] = obj; hash } + linked_accounts = LinkedAccount::Github.update_attributes_from_github_array(github_data.map { |issue| issue['user'] if issue.is_a?(Hash) }.uniq.compact) + linked_accounts_hash = linked_accounts.inject({}) { |hash,obj| hash[obj.uid.to_i] = obj; hash } github_data.map do |issue_data| + next unless issue_data.is_a?(Hash) update_attributes_from_github_data( issue_data, options.merge( @@ -306,7 +314,7 @@ def self.update_attributes_from_github_array(github_data, options={}) tracker: (issue_data.has_key?('repo') ? repos_hash[issue_data['repo']['id'].to_i] : options[:tracker]) ) ) - end + end.compact end def self.update_attributes_from_github_data(github_data, options={}) diff --git a/app/models/github/repository.rb b/app/models/github/repository.rb index 00245bf57..e6749324a 100644 --- a/app/models/github/repository.rb +++ b/app/models/github/repository.rb @@ -125,7 +125,6 @@ def remote_sync(options={}) def update_from_github(options={}) # read synced_at before updating if_modified_since = options[:force] ? nil : self.synced_at.try(:httpdate) - update_attribute(:synced_at, Time.now) # add access token params = { @@ -139,7 +138,10 @@ def update_from_github(options={}) headers: { 'If-Modified-Since' => if_modified_since } ) # trigger an update on the underlying object if modified - self.class.update_attributes_from_github_data(api_response.data, obj: self) if api_response.modified? + if api_response.modified? + update_attribute(:synced_at, Time.now) + self.class.update_attributes_from_github_data(api_response.data, obj: self) + end end # Update the repo languages from GitHub @@ -188,29 +190,38 @@ def find_or_create_issues_from_github(options={}) # TODO: optimization: github api returns open_issues_count which we could compare to issues.where(state=open) if options[:state] + # IMPROVEMENT: Add a 10-minute buffer to account for clock drift/indexing delays + since_timestamp = options[:since] + since_timestamp -= 10.minutes if since_timestamp + # make a call to github API api_response = Github::API.call( url: File.join(self.github_api_path, "/issues"), params: { access_token: options[:person].try(:github_account).try(:oauth_token), - since: options[:since].try(:iso8601), + since: since_timestamp.try(:iso8601), state: options[:state], + per_page: 100, page: options[:page] || 1 - }.reject { |k,v| v.nil? }, - - # NOTE: this doesn't have any affect... and we don't want to store etags, so no API optimization here. - #headers: { - # 'If-Modified-Since' => options[:since].try(:httpdate) - #}.reject { |k,v| v.nil? } + }.reject { |k,v| v.nil? } ) - # Edge case: tracker is deleted, just raise for now - if !api_response.success? && api_response.status == 404 - raise "Github::Repository(#{id}) not found at GitHub(#{url}). Maybe it was deleted?" + # IMPROVEMENT: Robust error logging and status check + unless api_response.success? + Rails.logger.error "Github::Repository(#{id}) Issue Sync Failed: #{api_response.status} - #{api_response.data.inspect}" + if api_response.status == 404 + raise "Github::Repository(#{id}) not found at GitHub(#{url}). Maybe it was deleted?" + end + return false # Stop processing this branch end - # process these 100 issues - Github::Issue.update_attributes_from_github_array(api_response.data) + # IMPROVEMENT: Type validation before processing + if api_response.data.is_a?(Array) + Github::Issue.update_attributes_from_github_array(api_response.data) + else + Rails.logger.error "Github::Repository(#{id}) Expected Array from API, got: #{api_response.data.class}" + return false + end # if a page param wasn't passed in, load the rest of the pages if !options[:page] && api_response.link[:last]