Fix logic around default locales

app/controllers/script_versions_controller.rb

@@ -51,7 +51,7 @@ def create
 
 		@script_version.script = @script
 		@script.script_type_id = params['script']['script_type_id']
-		@script.locale_id = params['script']['locale_id']
+		@script.locale_id = params['script']['locale_id'] if params['script'].has_key?('locale_id')
 		# a bit of weirdness because one of the radio buttons maps to nil
 		@script.approve_redistribution = params['script']['approve_redistribution'] == 'on' ? nil : params['script']['approve_redistribution']
 		@script.adult_content_self_report = params['script']['adult_content_self_report'] == '1'

app/models/script.rb

@@ -126,11 +126,18 @@ def set_locale
 		# The API is limited.
 		return unless description.present?
 
-		self.locale = detect_locale 
+		self.locale = detect_locale
 		localized_attributes.select{|la| la.locale.nil?}.each{|la| la.locale = self.locale}
 		true
 	end
 
+	# If the locale has changed, update the default localized attributes' locale
+	before_validation :update_localized_attribute_locales
+	def update_localized_attribute_locales
+		return if !locale_id_changed?
+		localized_attributes.select(&:attribute_default).each{|la| la.locale = self.locale}
+	end
+
 	before_validation :set_sensitive_flag
 	def set_sensitive_flag
 		self.sensitive ||= (adult_content_self_report || for_sensitive_site?)
@@ -332,6 +339,7 @@ def deleted?
 	def detect_locale
 		ft = full_text
 		return if ft.nil?
+
 		if Greasyfork::Application.config.enable_detect_locale
 			Logger.new("#{Rails.root}/log/detectlanguage.log").info("Sending DetectLanguage request for #{id ? "script #{id}" : "a new script"} - #{full_text[0..50]}...")
 			begin

app/models/script_version.rb

@@ -72,6 +72,7 @@ def number_of_screenshots
 		}
 	end
 
+	before_validation :set_locale
 	before_save :set_locale
 	def set_locale
 		localized_attributes.select{|la| la.locale.nil?}.each{|la| la.locale = script.locale}

app/views/script_versions/_form.html.erb

@@ -144,7 +144,7 @@ if !script_version.script.script_sync_type_id.nil? and script_version.script.scr
 		</div>
 	<% end %>
 
-	<% if true || @script.new_record? %>
+	<% if @script.new_record? %>
 		<div class="form-control radio-group">
 			<label><%=t('activerecord.attributes.script.locale')%></label> <span class="label-note"><%=t('scripts.locale_explanation_html')%></span><br>
 			<%= render partial: 'scripts/default_locale_selector', locals: {script: @script} %>

test/fixtures/locales.yml

@@ -3,53 +3,61 @@ english:
   english_name: English
   native_name: English
   code: en
+  detect_language_code: en
   ui_available: true
 
 french:
   id: 2
   english_name: French
   native_name: Français
   code: fr
+  detect_language_code: fr
   ui_available: true
 
 spanish:
   id: 3
   english_name: Spanish
   native_name: Español
   code: es
+  detect_language_code: es
   ui_available: true
 
 chinese traditional:
   id: 4
   english_name: Chinese (Traditional)
   native_name: 繁體中文
   code: zh-TW
+  detect_language_code: zh-TW
   ui_available: true
 
 chinese simplified:
   id: 5
   english_name: Chinese (Simplified)
   native_name: 简体中文
   code: zh-CN
+  detect_language_code: zh-CN
   ui_available: true
 
 portuguese:
   id: 6
   english_name: Portuguese
   native_name: Português
   code: pt
+  detect_language_code: pt
   ui_available: false
 
 portuguese brazil:
   id: 7
   english_name: Portuguese (Brazil)
   native_name: Português do Brasil
   code: pt-BR
+  detect_language_code: pt-BR
   ui_available: true
 
 zulu:
   id: 8
   english_name: Zulu
   native_name: isiZulu
   code: zu
+  detect_language_code: zu
   ui_available: false

test/fixtures/localized_script_attributes.yml

@@ -311,21 +311,21 @@ script_14_name_fr:
   script_id: 14
   attribute_key: name
   attribute_value: Un test!
-  attribute_default: true
+  attribute_default: false
   locale_id: 2
   value_markup: text
 script_14_description_fr:
   script_id: 14
   attribute_key: description
   attribute_value: Test d'unit.
-  attribute_default: true
+  attribute_default: false
   locale_id: 2
   value_markup: text
 script_14_additional_info_fr:
   script_id: 14
   attribute_key: additional_info
   attribute_value: MonTexte
-  attribute_default: true
+  attribute_default: false
   locale_id: 2
   value_markup: markdown
   sync_identifier: MonNouveauTexte

test/models/script_localization_test.rb

@@ -21,9 +21,11 @@ class ScriptLocalizationTest < ActiveSupport::TestCase
 // ==/UserScript==
 var foo = "bar";
 EOF
+		sv.localized_attributes.build(attribute_key: 'additional_info', attribute_value: 'Additional info', locale: Locale.find_by(code: :en), attribute_default: true, value_markup: 'text')
+		sv.localized_attributes.build(attribute_key: 'additional_info', attribute_value: 'Info additionelle', locale: Locale.find_by(code: :fr), attribute_default: false, value_markup: 'text')
 		sv.calculate_all
 		script.apply_from_script_version(sv)
-		assert script.valid?, sv.errors.full_messages.inspect
+		assert script.valid?, script.errors.full_messages.inspect
 		assert_equal 'A Test!', script.name
 		assert_equal 'Unit test', script.description
 		available_locale_codes = script.available_locales.map{|l| l.code}
@@ -33,6 +35,7 @@ class ScriptLocalizationTest < ActiveSupport::TestCase
 		assert available_locale_codes.include?('es')
 		assert available_locale_codes.include?('zh-TW')
 		assert_equal 'Un test!', script.localized_value_for(:name, 'fr')
+		assert_equal 'Info additionelle', script.localized_value_for(:additional_info, 'fr')
 		assert_equal 'Unidad de prueba', script.localized_value_for(:description, 3)
 		assert_equal 'A Test!', script.localized_value_for('name', Locale.find(1))
 		assert_equal '本地化測試腳本', script.localized_value_for(:description, 'zh-TW')
@@ -128,4 +131,85 @@ class ScriptLocalizationTest < ActiveSupport::TestCase
 		assert sv.valid?
 	end
 
+	test 'changing the default locale' do
+		script = Script.new(user: User.find(1), locale: Locale.find_by(code: :en))
+		sv = ScriptVersion.new
+		sv.script = script
+		sv.code = <<EOF
+// ==UserScript==
+// @name		Una prueba!
+// @name:fr		Un test!
+// @description		Unidad de prueba
+// @description:fr	Test d'unit
+// @namespace http://greasyfork.local/users/1
+// @version 1.0
+// ==/UserScript==
+var foo = "bar";
+EOF
+		sv.localized_attributes.build(attribute_key: 'additional_info', attribute_value: 'Additional info en espanol', locale: Locale.find_by(code: :en), attribute_default: true, value_markup: 'text')
+		sv.localized_attributes.build(attribute_key: 'additional_info', attribute_value: 'Additional info en francais', locale: Locale.find_by(code: :fr), attribute_default: false, value_markup: 'text')
+		sv.calculate_all
+		script.apply_from_script_version(sv)
+		script.script_versions << sv
+		sv.save!
+		script.save!
+
+		available_locale_codes = script.available_locales.map{|l| l.code}
+		assert_equal available_locale_codes, ['en', 'fr']
+
+		assert_equal 'Una prueba!', script.localized_value_for(:name, 'en')
+		assert_equal 'Un test!', script.localized_value_for(:name, 'fr')
+		assert_equal 'Unidad de prueba', script.localized_value_for(:description, 'en')
+		assert_equal 'Test d\'unit', script.localized_value_for(:description, 'fr')
+		assert_equal 'Additional info en espanol', script.localized_value_for(:additional_info, 'en')
+		assert_equal 'Additional info en francais', script.localized_value_for(:additional_info, 'fr')
+
+		script.locale = Locale.find_by(code: 'es')
+		script.save!
+
+		available_locale_codes = script.available_locales.map{|l| l.code}
+		assert_equal available_locale_codes, ['es', 'fr']
+
+		assert_equal 'Una prueba!', script.localized_value_for(:name, 'es')
+		assert_equal 'Un test!', script.localized_value_for(:name, 'fr')
+		assert_equal 'Unidad de prueba', script.localized_value_for(:description, 'es')
+		assert_equal 'Test d\'unit', script.localized_value_for(:description, 'fr')
+		assert_equal 'Additional info en espanol', script.localized_value_for(:additional_info, 'es')
+		assert_equal 'Additional info en francais', script.localized_value_for(:additional_info, 'fr')
+	end
+
+	test 'detecting default locale' do
+
+		script = Script.new(user: User.find(1))
+		sv = ScriptVersion.new
+		sv.script = script
+		sv.code = <<EOF
+// ==UserScript==
+// @name		Una prueba!
+// @name:fr		Un test!
+// @description		Unidad de prueba
+// @description:fr	Test d'unit
+// @namespace http://greasyfork.local/users/1
+// @version 1.0
+// ==/UserScript==
+var foo = "bar";
+EOF
+		sv.localized_attributes.build(attribute_key: 'additional_info', attribute_value: 'en espanol', attribute_default: true, value_markup: 'text')
+		sv.localized_attributes.build(attribute_key: 'additional_info', attribute_value: 'en francais', locale: Locale.find_by(code: :fr), attribute_default: false, value_markup: 'text')
+		sv.calculate_all
+		script.apply_from_script_version(sv)
+		script.script_versions << sv
+
+		Greasyfork::Application.config.enable_detect_locale = true
+		DetectLanguage.stub(:simple_detect, 'es') do
+			script.valid?
+			sv.valid?
+			sv.save!
+			script.save!
+		end
+
+		assert_equal 'es', script.locale.code
+		available_locale_codes = script.available_locales.map{|l| l.code}
+		assert_equal available_locale_codes, ['es', 'fr']
+	end
 end

test/models/script_version_test.rb

@@ -828,7 +828,7 @@ class ScriptVersionTest < ActiveSupport::TestCase
 		sv.code = script.script_versions.first.code
 		sv.rewritten_code = script.script_versions.first.rewritten_code
 		sv.localized_attributes.build({:attribute_key => 'additional_info', :attribute_value => 'New', :attribute_default => true, :locale => script.locale, :value_markup => 'html'})
-		sv.localized_attributes.build({:attribute_key => 'additional_info', :attribute_value => 'Nouveau', :attribute_default => true, :locale => Locale.where(:code => 'fr').first, :value_markup => 'html'})
+		sv.localized_attributes.build({:attribute_key => 'additional_info', :attribute_value => 'Nouveau', :attribute_default => false, :locale => Locale.where(:code => 'fr').first, :value_markup => 'html'})
 		sv.calculate_all
 		assert sv.valid?, sv.errors.full_messages
 		script.apply_from_script_version(sv)

test/test_helper.rb

@@ -1,6 +1,7 @@
 ENV["RAILS_ENV"] ||= "test"
 require File.expand_path('../../config/environment', __FILE__)
 require 'rails/test_help'
+require 'minitest/autorun'
 
 class ActiveSupport::TestCase
   ActiveRecord::Migration.check_pending!