Fix CR suggestions

app/models/carto/client_application.rb

@@ -1,14 +1,7 @@
 require 'active_record'
 require 'oauth'
 require_dependency 'cartodb_config_utils'
-
-class DomainPatcherRequestProxy < OAuth::RequestProxy::RackRequest
-
-  def uri
-    super.sub('carto.com', 'cartodb.com')
-  end
-
-end
+require_dependency 'carto/domain_patcher_request_proxy'
 
 module Carto
   class ClientApplication < ActiveRecord::Base
@@ -37,7 +30,7 @@ module Carto
       value = OAuth::Signature.build(request, options, &block).verify
       if !value && !cartodb_com_hosted?
         # Validation failed, try to see if it has been signed for cartodb.com
-        cartodb_request = DomainPatcherRequestProxy.new(request, options)
+        cartodb_request = Carto::DomainPatcherRequestProxy.new(request, options)
         value = OAuth::Signature.build(cartodb_request, options, &block).verify
       end
       value
@@ -48,8 +41,8 @@ module Carto
     private
 
     def initialize_entity
-      self.key = OAuth::Helper.generate_key(40)[0, 40]
-      self.secret = OAuth::Helper.generate_key(40)[0, 40]
+      self.key = OAuth::Helper.generate_key(40)[0, 40] unless key.present?
+      self.secret = OAuth::Helper.generate_key(40)[0, 40] unless secret.present?
     end
 
   end

app/models/carto/helpers/user_commons.rb

@@ -383,12 +383,12 @@ module Carto::UserCommons
   end
 
   def new_client_application
-    Carto::ClientApplication.create(user_id: id)
+    Carto::ClientApplication.create!(user_id: id)
   end
 
   def reset_client_application!
     client_application&.destroy
-    Carto::ClientApplication.create(user_id: id)
+    Carto::ClientApplication.create!(user_id: id)
   end
 
 end

app/models/user.rb

@@ -436,7 +436,7 @@ class User < Sequel::Model
 
       assign_search_tweets_to_organization_owner
 
-      Carto::ClientApplication.where(user_id: id).each(&:destroy)
+      Carto::ClientApplication.where(user_id: id).destroy_all
     rescue StandardError => exception
       error_happened = true
       log_error(message: 'Error destroying user', current_user: self, exception: exception)

app/services/carto/user_metadata_export_service.rb

@@ -230,7 +230,7 @@ module Carto
     end
 
     def build_client_application_from_hash(client_app_hash)
-      return nil unless client_app_hash
+      return unless client_app_hash
 
       client_application = Carto::ClientApplication.create(
         name: client_app_hash[:name],
@@ -239,11 +239,11 @@ module Carto
         callback_url: client_app_hash[:callback_url],
         oauth_tokens: client_app_hash[:oauth_tokens].map { |t| build_oauth_token_fom_hash(t) },
         access_tokens: client_app_hash[:access_tokens].map { |t| build_oauth_token_fom_hash(t) },
-        user_id: client_app_hash[:user_id]
+        user_id: client_app_hash[:user_id],
+        key: client_app_hash[:key],
+        secret: client_app_hash[:secret]
       )
       # Overwrite fields that were created with ORM lifecycle callbacks
-      client_application.key = client_app_hash[:key]
-      client_application.secret = client_app_hash[:secret]
       client_application.created_at = client_app_hash[:created_at]
       client_application.updated_at = client_app_hash[:updated_at]
       client_application

lib/carto/domain_patcher_request_proxy.rb

@@ -0,0 +1,11 @@
+require 'oauth'
+
+module Carto
+  class DomainPatcherRequestProxy < OAuth::RequestProxy::RackRequest
+
+    def uri
+      super.sub('carto.com', 'cartodb.com')
+    end
+
+  end
+end

spec/services/carto/user_metadata_export_service_spec.rb

@@ -172,7 +172,6 @@ describe Carto::UserMetadataExportService do
           st.destroy
         end
       end
-      Carto::ClientApplication.where(user_id: @user.id).each(&:destroy)
 
       @user.oauth_app_users.each do |oau|
         unless oau.oauth_access_tokens.blank?