Refactor link processing logic in RichTextLinkIdentifier for clarity and maintainability

Author: rsmithlal

app/services/better_together/metrics/rich_text_link_identifier.rb

@@ -57,67 +57,65 @@ def process_rich_text(rt)
         return [0, 0] if links.empty?
 
         links.each_with_index do |link, index|
-          begin
-            uri = parse_uri(link)
-            if uri.nil?
+          uri = parse_uri(link)
+          if uri.nil?
+            create_invalid(rt, index, link, 'undetermined')
+            invalid_count += 1
+            next
+          end
+
+          canonical_host = uri.host
+          if canonical_host.nil? && uri.scheme.nil?
+            if link.start_with?('/')
+              canonical_host = rt_platform_host
+            else
               create_invalid(rt, index, link, 'undetermined')
               invalid_count += 1
               next
             end
+          end
 
-            canonical_host = uri.host
-            if canonical_host.nil? && uri.scheme.nil?
-              if link.start_with?('/')
-                canonical_host = rt_platform_host
-              else
-                create_invalid(rt, index, link, 'undetermined')
-                invalid_count += 1
-                next
-              end
-            end
-
-            bt_link = BetterTogether::Content::Link.find_or_initialize_by(url: link)
-            bt_link.host ||= canonical_host
-            bt_link.scheme ||= uri.scheme
-            bt_link.external = (canonical_host.present? && (rt_platform_host != canonical_host))
-            bt_link.save! if bt_link.changed?
-
-            # Persist the RichTextLink depending on the schema available.
-            if BetterTogether::Metrics::RichTextLink.column_names.include?('link_id')
-              attrs = {
-                link_id: bt_link.id,
-                rich_text_id: rt.id,
-                rich_text_record_id: rt.record_id,
-                rich_text_record_type: rt.record_type,
-                position: index,
-                locale: rt.locale
-              }
-
-              BetterTogether::Metrics::RichTextLink.find_or_create_by!(attrs)
-            else
-              # Fallback schema: metrics rich_text_links store URL and metadata inline
-              # Some legacy schemas use column names (for example `valid`) that clash
-              # with Active Record method names. To avoid DangerousAttributeError we
-              # perform a raw INSERT unless a record already exists for this
-              # (rich_text_id, url) tuple.
-              model = BetterTogether::Metrics::RichTextLink
-              unless model.where(rich_text_id: rt.id, url: link).exists?
-                conn = model.connection
-                table = model.table_name
-                now = Time.current
-                cols = %w[rich_text_id url link_type external valid host error_message created_at updated_at]
-                vals = [rt.id, link, bt_link.link_type || 'website', bt_link.external || false,
-                        bt_link.valid_link || false, bt_link.host, bt_link.error_message, now, now]
-                sql = "INSERT INTO #{table} (#{cols.join(',')}) VALUES (#{vals.map { |v| conn.quote(v) }.join(',')})"
-                conn.execute(sql)
-              end
+          bt_link = BetterTogether::Content::Link.find_or_initialize_by(url: link)
+          bt_link.host ||= canonical_host
+          bt_link.scheme ||= uri.scheme
+          bt_link.external = (canonical_host.present? && (rt_platform_host != canonical_host))
+          bt_link.save! if bt_link.changed?
+
+          # Persist the RichTextLink depending on the schema available.
+          if BetterTogether::Metrics::RichTextLink.column_names.include?('link_id')
+            attrs = {
+              link_id: bt_link.id,
+              rich_text_id: rt.id,
+              rich_text_record_id: rt.record_id,
+              rich_text_record_type: rt.record_type,
+              position: index,
+              locale: rt.locale
+            }
+
+            BetterTogether::Metrics::RichTextLink.find_or_create_by!(attrs)
+          else
+            # Fallback schema: metrics rich_text_links store URL and metadata inline
+            # Some legacy schemas use column names (for example `valid`) that clash
+            # with Active Record method names. To avoid DangerousAttributeError we
+            # perform a raw INSERT unless a record already exists for this
+            # (rich_text_id, url) tuple.
+            model = BetterTogether::Metrics::RichTextLink
+            unless model.where(rich_text_id: rt.id, url: link).exists?
+              conn = model.connection
+              table = model.table_name
+              now = Time.current
+              cols = %w[rich_text_id url link_type external valid host error_message created_at updated_at]
+              vals = [rt.id, link, bt_link.link_type || 'website', bt_link.external || false,
+                      bt_link.valid_link || false, bt_link.host, bt_link.error_message, now, now]
+              sql = "INSERT INTO #{table} (#{cols.join(',')}) VALUES (#{vals.map { |v| conn.quote(v) }.join(',')})"
+              conn.execute(sql)
             end
-
-            valid_count += 1
-          rescue URI::InvalidURIError
-            create_invalid(rt, index, link, 'invalid_uri')
-            invalid_count += 1
           end
+
+          valid_count += 1
+        rescue URI::InvalidURIError
+          create_invalid(rt, index, link, 'invalid_uri')
+          invalid_count += 1
         end
 
         [valid_count, invalid_count]

spec/factories/better_together/content/rich_texts.rb

@@ -3,7 +3,7 @@
 FactoryBot.define do
   factory :content_rich_text, class: 'ActionText::RichText' do
     association :record, factory: :platform
-  name { 'body' }
+    name { 'body' }
     locale { I18n.default_locale }
     body { '' }
   end