Merge pull request #164 from prograils/fix_duplication_detection_while_synchronizing

pnikrat · web-flow · commit 4b15672f4c21 · 2021-03-08T12:59:35.000+01:00
When synchronizing with remote record duplication was not properly checked
diff --git a/app/controllers/lit/api/v1/localizations_controller.rb b/app/controllers/lit/api/v1/localizations_controller.rb
@@ -2,34 +2,26 @@ module Lit
   class Api::V1::LocalizationsController < Api::V1::BaseController
     def index
       @localizations = fetch_localizations
-      render json: @localizations.as_json(
-        root: false,
-        only: %i[id localization_key_id locale_id],
-        methods: %i[
-          value localization_key_str locale_str
-          localization_key_is_deleted
-        ]
-      )
+      render json:
+               @localizations.as_json(
+                 root: false,
+                 only: %i[id localization_key_id locale_id],
+                 methods: %i[value localization_key_str locale_str localization_key_is_deleted],
+               )
     end
 
     def last_change
       @localization = Localization.order(updated_at: :desc).first
-      render json: @localization.as_json(
-        root: false, only: [], methods: [:last_change]
-      )
+      render json: @localization.as_json(root: false, only: [], methods: [:last_change])
     end
 
     private
 
     def fetch_localizations
-      scope = Localization.includes(
-        :locale,
-        :localization_key
-      )
+      scope = Localization.includes(:locale, :localization_key)
 
       if params[:after].present?
-        after_date = Time.parse("#{params[:after]} #{Time.zone.name}")
-                         .in_time_zone
+        after_date = Time.parse("#{params[:after]} #{Time.zone.name}").in_time_zone
         scope.after(after_date).to_a
       else
         scope.all
diff --git a/app/models/lit/incomming_localization.rb b/app/models/lit/incomming_localization.rb
@@ -36,39 +36,27 @@ def accept
     def duplicated?(val)
       set_localization
       return false if localization_has_changed?
-      translated_value =
-        localization.read_attribute_before_type_cast('translated_value')
-      if localization.is_changed? && !translated_value.nil?
-        translated_value == val
-      else
-        localization.read_attribute_before_type_cast('default_value') == val
-      end
+
+      localization.translation == val
     end
 
     private
 
     def set_localization
       return if locale.blank? || localization_key.blank?
-      self.localization = localization_key.localizations
-                                          .find_by(locale_id: locale_id)
+      self.localization = localization_key.localizations.find_by(locale_id: locale_id)
     end
 
     def localization_has_changed?
-      localization.blank? ||
-        localization.is_deleted != localization_key_is_deleted
+      localization.blank? || localization.is_deleted != localization_key_is_deleted
     end
 
     def update_existing_localization_data
-      localization.update!(
-        translated_value: translated_value,
-        is_changed: true
-      )
+      localization.update!(translated_value: translated_value, is_changed: true)
     end
 
     def update_existing_localization_key_data
-      localization_key.update!(
-        is_deleted: localization_key_is_deleted
-      )
+      localization_key.update!(is_deleted: localization_key_is_deleted)
     end
 
     def assign_new_localization_data
@@ -83,25 +71,23 @@ def assign_new_locale
 
     def assign_new_localization_key
       self.localization_key =
-        Lit::LocalizationKey.where(
-          localization_key: localization_key_str,
-          is_deleted: localization_key_is_deleted
-        ).first_or_create!
+        Lit::LocalizationKey.where(localization_key: localization_key_str, is_deleted: localization_key_is_deleted)
+          .first_or_create!
     end
 
     def assign_new_localization
       self.localization =
-        Lit::Localization.where(localization_key_id: localization_key.id)
-                         .where(locale_id: locale.id)
-                         .first_or_initialize
+        Lit::Localization
+          .where(localization_key_id: localization_key.id)
+          .where(locale_id: locale.id)
+          .first_or_initialize
       localization.translated_value = translated_value
       localization.is_changed = true
       localization.save!
     end
 
     def update_cache
-      Lit.init.cache.update_cache localization.full_key,
-                                  localization.translation
+      Lit.init.cache.update_cache localization.full_key, localization.translation
     end
   end
 end
diff --git a/app/models/lit/localization.rb b/app/models/lit/localization.rb
@@ -6,19 +6,14 @@ class Localization < Lit::Base
     ## SCOPES
     scope :changed, -> { where is_changed: true }
     scope :not_changed, -> { where is_changed: false }
+
     # @HACK: dirty, find a way to round date to full second
-    scope :after, lambda { |dt|
-      where('updated_at >= ?', dt + 1.second)
-        .where(is_changed: true)
-    }
-    scope :active, lambda {
-      joins(:localization_key)
-        .where(Lit::LocalizationKey.table_name => { is_deleted: false })
-    }
+    scope :after, lambda { |dt| where('updated_at >= ?', dt + 1.second).where(is_changed: true) }
+    scope :active, lambda { joins(:localization_key).where(Lit::LocalizationKey.table_name => { is_deleted: false }) }
 
     ## ASSOCIATIONS
-    belongs_to :locale
-    belongs_to :localization_key, touch: true
+    belongs_to :locale, required: true
+    belongs_to :localization_key, touch: true, required: true
     has_many :localization_versions, dependent: :destroy
     has_many :versions, class_name: '::Lit::LocalizationVersion'
 
@@ -31,9 +26,7 @@ class Localization < Lit::Base
     ## ACCESSORS
     attr_accessor :full_key_str
 
-    unless defined?(::ActionController::StrongParameters)
-      attr_accessible :translated_value, :locale_id
-    end
+    attr_accessible :translated_value, :locale_id unless defined?(::ActionController::StrongParameters)
 
     ## BEFORE & AFTER
     with_options if: :translated_value_changed? do |o|
@@ -94,6 +87,5 @@ def create_version
       translated_value = translated_value_was || default_value
       localization_versions.new(translated_value: translated_value)
     end
-
   end
 end
diff --git a/app/services/remote_interactor_service.rb b/app/services/remote_interactor_service.rb
@@ -27,7 +27,7 @@ def initialize_uri(path, query_values)
 
   def initialize_request(uri)
     req = Net::HTTP::Get.new(uri.request_uri)
-    req.add_field('Authorization', %(Token token="#{@source.api_key}"))
+    req.add_field('Authorization', "Token token=\"#{@source.api_key}\"")
     req
   end
 
@@ -41,6 +41,7 @@ def initialize_connection(uri)
   def perform_request(connection, request)
     res = connection.start { |http| http.request(request) }
     return res unless res.is_a?(Net::HTTPSuccess)
+
     JSON.parse(res.body)
   end
 end
diff --git a/app/services/synchronize_source_service.rb b/app/services/synchronize_source_service.rb
@@ -12,8 +12,7 @@ def execute
 
   def synchronize_localizations
     after_date = @source.last_updated_at&.to_s(:db)
-    result = interactor.send_request Lit::Source::LOCALIZATIONS_PATH,
-                                     after: after_date
+    result = interactor.send_request Lit::Source::LOCALIZATIONS_PATH, after: after_date
     return unless result&.is_a?(Array)
     result.each { |loc| synchronize_localization loc }
   end
@@ -28,19 +27,16 @@ def synchronize_localization(loc)
     inc_loc.localization_key = find_localization_key(inc_loc)
     inc_loc.translated_value = loc['value']
     return if inc_loc.duplicated?(loc['value'])
+
     inc_loc.save!
   end
 
   def find_incomming_localization(localization)
-    Lit::IncommingLocalization.find_or_initialize_by(
-      incomming_id: localization['id']
-    )
+    Lit::IncommingLocalization.find_or_initialize_by(incomming_id: localization['id'])
   end
 
   def find_localization_key(inc_loc)
-    Lit::LocalizationKey.find_by(
-      localization_key: inc_loc.localization_key_str
-    )
+    Lit::LocalizationKey.find_by(localization_key: inc_loc.localization_key_str)
   end
 
   def localization_key_deleted?(loc)
diff --git a/lit.gemspec b/lit.gemspec
@@ -3,21 +3,20 @@ $LOAD_PATH.push File.expand_path('../lib', __FILE__)
 require 'lit/version'
 
 Gem::Specification.new do |s|
-  s.name        = 'lit'
-  s.version     = Lit::VERSION
-  s.authors     = ['Maciej Litwiniuk', 'Piotr Boniecki', 'Michał Buszkiewicz',
-                   'Szymon Soppa']
-  s.email       = ['maciej@litwiniuk.net']
-  s.license     = 'MIT'
-  s.homepage    = 'https://github.com/prograils/lit'
-  s.summary     = 'Database powered i18n backend with web gui'
-  s.description = "Translate your apps with pleasure (sort of...) and for free.
+  s.name = 'lit'
+  s.version = Lit::VERSION
+  s.authors = ['Maciej Litwiniuk', 'Piotr Boniecki', 'Michał Buszkiewicz', 'Szymon Soppa']
+  s.email = ['maciej@litwiniuk.net']
+  s.license = 'MIT'
+  s.homepage = 'https://github.com/prograils/lit'
+  s.summary = 'Database powered i18n backend with web gui'
+  s.description =
+    "Translate your apps with pleasure (sort of...) and for free.
                    It's simple i18n web interface, build on top of twitter
                    bootstrap, that one may find helpful in translating app by
                    non-technicals. "
 
-  s.files = Dir['{app,config,db,lib}/**/*'] + ['MIT-LICENSE', 'Rakefile',
-                                               'README.md']
+  s.files = Dir['{app,config,db,lib}/**/*'] + %w[MIT-LICENSE Rakefile README.md]
   s.add_dependency 'rails', '>= 4.2.0'
   s.add_dependency 'jquery-rails'
   s.add_dependency 'coffee-rails'
@@ -33,4 +32,3 @@ Gem::Specification.new do |s|
   s.add_development_dependency 'vcr', '~> 4.0.0'
   s.add_development_dependency 'webmock', '~> 3.4.2'
 end
-
diff --git a/test/jobs/lit/synchronize_source_job_test.rb b/test/jobs/lit/synchronize_source_job_test.rb
diff --git a/test/services/lit/synchronize_source_service_test.rb b/test/services/lit/synchronize_source_service_test.rb
@@ -0,0 +1,50 @@
+require 'test_helper'
+
+# Applicable only in an ActiveJob-enabled environment (Rails 4.2+ or 4.0/4.1
+# with additional gem)
+if defined?(ActiveJob)
+  module Lit
+    class SynchronizeSourceServiceTest < ActiveSupport::TestCase
+      fixtures :all
+
+      def setup
+        after = 3.hours.ago
+        after_str = after.strftime('%F %T')
+        after_param = Rack::Utils.escape(after_str)
+        @source = Source.first
+        @source.update_column(:last_updated_at, after)
+        localizations_addr = "http://testhost.com/lit/api/v1/localizations.json?after=#{after_param}"
+        last_change_addr = 'http://testhost.com/lit/api/v1/last_change.json'
+        stub_request(:get, localizations_addr).to_return(
+          body:
+            Localization
+              .all
+              .as_json(
+                root: false,
+                only: %i[id localization_key_id locale_id],
+                methods: %i[value localization_key_str locale_str localization_key_is_deleted],
+              )
+              .to_json,
+        )
+        stub_request(:get, last_change_addr).to_return(body: { last_change: after_str }.to_json)
+      end
+
+      test 'synchronization works for different values' do
+        assert_equal 0, @source.incomming_localizations.count
+        Localization.all.each do |lc|
+          lc.translated_value << 'test'
+          lc.is_changed = true
+          lc.save
+        end
+        SynchronizeSourceService.new(@source).execute
+        assert_equal 5, @source.incomming_localizations.count
+      end
+
+      test 'synchronization ignores same values' do
+        assert_equal 0, @source.incomming_localizations.count
+        SynchronizeSourceService.new(@source).execute
+        assert_equal 0, @source.incomming_localizations.count
+      end
+    end
+  end
+end
diff --git a/test/unit/lit/incomming_localization_test.rb b/test/unit/lit/incomming_localization_test.rb
