From 665964bcbba69829a4ff1e7d7bd936f90d49b3f7 Mon Sep 17 00:00:00 2001 From: elijah Date: Sun, 22 Dec 2013 01:45:41 -0800 Subject: locale prefix support: * set locale based on request header * enforce locale path prefix when current locale is not the default * note: don't use root_path anymore, instead use home_path --- DEVELOP.md | 17 ++++++ Gemfile | 2 + Gemfile.lock | 2 + README.md | 2 + app/controllers/application_controller.rb | 68 ++++++++++++++++++++-- billing/config/routes.rb | 33 ++++++----- billing/test/integration/subscription_test.rb | 2 +- config.ru | 3 + config/defaults.yml | 8 ++- config/initializers/i18n.rb | 10 +++- config/routes.rb | 57 ++---------------- core/app/views/common/_home_page_buttons.html.haml | 4 +- core/config/routes.rb | 1 - help/app/views/tickets/new.html.haml | 2 +- help/config/routes.rb | 6 +- help/test/functional/tickets_controller_test.rb | 2 +- test/integration/locale_path_test.rb | 51 ++++++++++++++++ .../controller_extension/authentication.rb | 2 +- users/app/controllers/sessions_controller.rb | 4 +- users/app/controllers/users_controller.rb | 2 +- users/app/views/sessions/new.html.haml | 2 +- users/app/views/users/new.html.haml | 2 +- users/config/routes.rb | 18 +++--- users/test/functional/sessions_controller_test.rb | 6 +- users/test/functional/users_controller_test.rb | 2 +- users/test/support/auth_test_helper.rb | 2 +- 26 files changed, 207 insertions(+), 103 deletions(-) create mode 100644 test/integration/locale_path_test.rb diff --git a/DEVELOP.md b/DEVELOP.md index a35ce06..3603dc5 100644 --- a/DEVELOP.md +++ b/DEVELOP.md @@ -78,4 +78,21 @@ CouchRest Model behaved strangely when using a model without a design block. So From that point on you should be able to use the standart persistance and querying methods such as create, find, destroy and so on. +## Writing Tests ## +### Locale + +The ApplicationController defines a before filter #set_locale that will set +the default_url_options to include the appropriate default {:locale => x} param. + +However, paths generated in tests don't use default_url_options. This can +create failures for certain nested routes unless you explicitly provide +:locale => nil to the path helper. This is not needed for actual path code in +the controllers or views, only when generating paths in tests. + +For example: + + test "robot" do + login_as @user + visit robot_path(@robot, :locale => nil) + end \ No newline at end of file diff --git a/Gemfile b/Gemfile index 2e0b8d0..e475bad 100644 --- a/Gemfile +++ b/Gemfile @@ -12,6 +12,8 @@ gem 'leap_web_certs', :path => 'certs' gem 'leap_web_help', :path => 'help' gem 'leap_web_billing', :path => 'billing' +gem 'http_accept_language' + # To use debugger gem 'debugger', :platforms => :mri_19 # ruby 1.8 is not supported anymore diff --git a/Gemfile.lock b/Gemfile.lock index 8047594..cc549ee 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -146,6 +146,7 @@ GEM haml (~> 3.1) railties (>= 3.1, < 4.1) hike (1.2.3) + http_accept_language (2.0.0) i18n (0.6.9) journey (1.0.4) jquery-rails (3.0.4) @@ -273,6 +274,7 @@ DEPENDENCIES faker haml (~> 3.1.7) haml-rails (~> 0.3.4) + http_accept_language jquery-rails kaminari (= 0.13.0) launchy diff --git a/README.md b/README.md index 1d6016c..ec1ea69 100644 --- a/README.md +++ b/README.md @@ -104,3 +104,5 @@ To run all tests To run an individual test: rake test TEST=certs/test/unit/client_certificate_test.rb + or + ruby -Itest certs/test/unit/client_certificate_test.rb \ No newline at end of file diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index de8d06b..35d6cb4 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,13 +1,14 @@ class ApplicationController < ActionController::Base protect_from_forgery + before_filter :set_locale before_filter :no_cache_header before_filter :no_frame_header + before_filter :language_header ActiveSupport.run_load_hooks(:application_controller, self) protected - rescue_from StandardError do |e| respond_to do |format| format.json { render_json_error(e) } @@ -32,13 +33,59 @@ class ApplicationController < ActionController::Base end helper_method :bold + ## + ## LOCALE + ## + # - # we want to prevent the browser from caching anything, just to be safe. + # URL paths for which we don't enforce the locale as the prefix of the path. # - def no_cache_header - response.headers["Cache-Control"] = "no-cache, no-store, must-revalidate" - response.headers["Pragma"] = "no-cache" - response.headers["Expires"] = "0" + NON_LOCALE_PATHS = /^\/(assets|webfinger|.well-known|rails|key|[0-9]+)($|\/)/ + + # + # Before filter to set the current locale. Possible outcomes: + # + # (a) do nothing for certain routes and requests. + # (b) if path already starts with locale, set I18n.locale and default_url_options. + # (c) otherwise, redirect so that path starts with locale. + # + # If the locale happens to be the default local, no paths are prefixed with the locale. + # + def set_locale + if request.method == "GET" && request.format == 'text/html' && request.path !~ NON_LOCALE_PATHS + if params[:locale] && LOCALES_STRING.include?(params[:locale]) + I18n.locale = params[:locale] + update_default_url_options + else + I18n.locale = http_accept_language.compatible_language_from(I18n.available_locales) || I18n.default_locale + update_default_url_options + if I18n.locale != I18n.default_locale + redirect_to url_for(params.merge(:locale => I18n.locale)) + end + end + else + update_default_url_options + end + end + + def update_default_url_options + if I18n.locale != I18n.default_locale + self.default_url_options[:locale] = I18n.locale + else + self.default_url_options[:locale] = nil + end + end + + ## + ## HTTP HEADERS + ## These are in individual helpers so that controllers can disable them if needed. + ## + + # + # Not necessary, but kind to let the browser know the current locale. + # + def language_header + response.headers["Content-Language"] = I18n.locale.to_s end # @@ -48,4 +95,13 @@ class ApplicationController < ActionController::Base response.headers["X-Frame-Options"] = "DENY" end + # + # we want to prevent the browser from caching anything, just to be safe. + # + def no_cache_header + response.headers["Cache-Control"] = "no-cache, no-store, must-revalidate" + response.headers["Pragma"] = "no-cache" + response.headers["Expires"] = "0" + end + end diff --git a/billing/config/routes.rb b/billing/config/routes.rb index dbdc24b..7263dff 100644 --- a/billing/config/routes.rb +++ b/billing/config/routes.rb @@ -1,24 +1,25 @@ Rails.application.routes.draw do - match 'payments/new' => 'payments#new', :as => :new_payment - match 'payments/confirm' => 'payments#confirm', :as => :confirm_payment - resources :users do - resources :payments, :only => [:index] - resources :subscriptions, :only => [:index, :show, :destroy] - end - - resources :customer, :only => [:new, :edit] - resources :credit_card_info, :only => [:edit] + scope "(:locale)", :locale => MATCH_LOCALE do + match 'payments/new' => 'payments#new', :as => :new_payment + match 'payments/confirm' => 'payments#confirm', :as => :confirm_payment + resources :users do + resources :payments, :only => [:index] + resources :subscriptions, :only => [:index, :show, :destroy] + end - match 'customer/confirm/' => 'customer#confirm', :as => :confirm_customer - match 'customer/show/:id' => 'customer#show', :as => :show_customer - match 'credit_card_info/confirm' => 'credit_card_info#confirm', :as => :confirm_credit_card_info + resources :customer, :only => [:new, :edit] + resources :credit_card_info, :only => [:edit] - resources :subscriptions, :only => [:new, :create, :update] # index, show & destroy are within users path - match 'billing_admin' => 'billing_admin#show', :as => :billing_admin + match 'customer/confirm/' => 'customer#confirm', :as => :confirm_customer + match 'customer/show/:id' => 'customer#show', :as => :show_customer + match 'credit_card_info/confirm' => 'credit_card_info#confirm', :as => :confirm_credit_card_info - #match 'transactions/:product_id/new' => 'transactions#new', :as => :new_transaction - #match 'transactions/confirm/:product_id' => 'transactions#confirm', :as => :confirm_transaction + resources :subscriptions, :only => [:new, :create, :update] # index, show & destroy are within users path + match 'billing_admin' => 'billing_admin#show', :as => :billing_admin + #match 'transactions/:product_id/new' => 'transactions#new', :as => :new_transaction + #match 'transactions/confirm/:product_id' => 'transactions#confirm', :as => :confirm_transaction + end end diff --git a/billing/test/integration/subscription_test.rb b/billing/test/integration/subscription_test.rb index 6356177..b95bfac 100644 --- a/billing/test/integration/subscription_test.rb +++ b/billing/test/integration/subscription_test.rb @@ -29,7 +29,7 @@ class SubscriptionTest < ActionDispatch::IntegrationTest login_as @admin @customer.stubs(:subscriptions).returns([@subscription]) @subscription.stubs(:balance).returns 0 - visit user_subscriptions_path(@customer.user_id) + visit user_subscriptions_path(@customer.user_id, :locale => nil) assert page.has_content?("Subscriptions") assert page.has_content?("Status: Active") page.save_screenshot('/tmp/subscriptions.png') diff --git a/config.ru b/config.ru index e798eef..7b4c2bd 100644 --- a/config.ru +++ b/config.ru @@ -1,4 +1,7 @@ # This file is used by Rack-based servers to start the application. +require 'http_accept_language' +use HttpAcceptLanguage::Middleware + require ::File.expand_path('../config/environment', __FILE__) run LeapWeb::Application diff --git a/config/defaults.yml b/config/defaults.yml index eb31301..98dc334 100644 --- a/config/defaults.yml +++ b/config/defaults.yml @@ -45,7 +45,9 @@ common: &common user_actions: ['destroy_account'] admin_actions: ['change_pgp_key', 'change_service_level', 'destroy_account'] billing: ~ - + default_locale: :en + available_locales: + - :en service_levels: &service_levels service_levels: 0: @@ -89,6 +91,10 @@ test: secret_token: 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx' reraise_errors: true billing: {} + default_locale: :en + available_locales: + - :en + - :de production: <<: *downloads diff --git a/config/initializers/i18n.rb b/config/initializers/i18n.rb index 574d169..c277a22 100644 --- a/config/initializers/i18n.rb +++ b/config/initializers/i18n.rb @@ -1,2 +1,10 @@ +I18n.enforce_available_locales = true +I18n.available_locales = APP_CONFIG[:available_locales] +I18n.default_locale = APP_CONFIG[:default_locale] -I18n.available_locales = ['en'] +# Used to match locales route prefixes +MATCH_LOCALE = /(#{I18n.available_locales.join('|')})/ + +# I18n.available_locales is always an array of symbols, but for comparison with +# params we need it to be an array of strings. +LOCALES_STRING = I18n.available_locales.map(&:to_s) diff --git a/config/routes.rb b/config/routes.rb index 3b29251..12bfe93 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,56 +1,9 @@ LeapWeb::Application.routes.draw do - root :to => "home#index" - - # The priority is based upon order of creation: - # first created -> highest priority. - - # Sample of regular route: - # match 'products/:id' => 'catalog#view' - # Keep in mind you can assign values other than :controller and :action - - # Sample of named route: - # match 'products/:id/purchase' => 'catalog#purchase', :as => :purchase - # This route can be invoked with purchase_url(:id => product.id) - - # Sample resource route (maps HTTP verbs to controller actions automatically): - # resources :products - - # Sample resource route with options: - # resources :products do - # member do - # get 'short' - # post 'toggle' - # end # - # collection do - # get 'sold' - # end - # end - - # Sample resource route with sub-resources: - # resources :products do - # resources :comments, :sales - # resource :seller - # end - - # Sample resource route with more complex sub-resources - # resources :products do - # resources :comments - # resources :sales do - # get 'recent', :on => :collection - # end - # end - - # Sample resource route within a namespace: - # namespace :admin do - # # Directs /admin/products/* to Admin::ProductsController - # # (app/controllers/admin/products_controller.rb) - # resources :products - # end - - # See how all your routes lay out with "rake routes" + # Please do not use root_path or root_url. Use home_path and home_url instead, + # so that the path will be correctly prefixed with the locale. + # + root :to => "home#index" + get '(:locale)' => 'home#index', :locale => MATCH_LOCALE, :as => 'home' - # This is a legacy wild controller route that's not recommended for RESTful applications. - # Note: This route will make all actions in every controller accessible via GET requests. - # match ':controller(/:action(/:id))(.:format)' end diff --git a/core/app/views/common/_home_page_buttons.html.haml b/core/app/views/common/_home_page_buttons.html.haml index 3be12e2..4c3fc5b 100644 --- a/core/app/views/common/_home_page_buttons.html.haml +++ b/core/app/views/common/_home_page_buttons.html.haml @@ -15,8 +15,8 @@ %span.link= link_to(icon('ok-sign', icon_color) + t(:login), login_path, :class => 'btn') %span.info= t(:login_info) .signup.span4 - %span.link= link_to(icon('user', icon_color) + t(:signup), new_user_path, :class => 'btn') + %span.link= link_to(icon('user', icon_color) + t(:signup), signup_path, :class => 'btn') %span.info= t(:signup_info) .help.span4 - %span.link= link_to(icon('question-sign', icon_color) + t(:get_help), "/tickets/new", :class => 'btn') + %span.link= link_to(icon('question-sign', icon_color) + t(:get_help), new_ticket_path, :class => 'btn') %span.info= t(:help_info) diff --git a/core/config/routes.rb b/core/config/routes.rb index 63f644c..1daf9a4 100644 --- a/core/config/routes.rb +++ b/core/config/routes.rb @@ -1,3 +1,2 @@ Rails.application.routes.draw do - root :to => "home#index" end diff --git a/help/app/views/tickets/new.html.haml b/help/app/views/tickets/new.html.haml index 393e5d6..8f217a5 100644 --- a/help/app/views/tickets/new.html.haml +++ b/help/app/views/tickets/new.html.haml @@ -27,4 +27,4 @@ - if logged_in? = link_to t(:cancel), auto_tickets_path, :class => :btn - else - = link_to t(:cancel), root_path, :class => 'btn' \ No newline at end of file + = link_to t(:cancel), home_path, :class => 'btn' \ No newline at end of file diff --git a/help/config/routes.rb b/help/config/routes.rb index 8f3241c..23e0c11 100644 --- a/help/config/routes.rb +++ b/help/config/routes.rb @@ -1,6 +1,8 @@ Rails.application.routes.draw do - resources :tickets, :except => :edit - resources :users do + scope "(:locale)", :locale => MATCH_LOCALE do resources :tickets, :except => :edit + resources :users do + resources :tickets, :except => :edit + end end end diff --git a/help/test/functional/tickets_controller_test.rb b/help/test/functional/tickets_controller_test.rb index 0f56e6e..2b30f66 100644 --- a/help/test/functional/tickets_controller_test.rb +++ b/help/test/functional/tickets_controller_test.rb @@ -49,7 +49,7 @@ class TicketsControllerTest < ActionController::TestCase login get :show, :id => ticket.id assert_response :redirect - assert_redirected_to root_url + assert_redirected_to home_url end test "should create unauthenticated ticket" do diff --git a/test/integration/locale_path_test.rb b/test/integration/locale_path_test.rb new file mode 100644 index 0000000..2c43ab2 --- /dev/null +++ b/test/integration/locale_path_test.rb @@ -0,0 +1,51 @@ +require 'test_helper' + +# +# Test how we handle redirections and locales. +# +# The basic rules are: +# +# (1) If the browser header Accept-Language matches default locale, then don't do a locale prefix. +# (2) If browser locale is supported in available_locales, but is not the default, then redirect. +# (3) If browser locale is not in available_locales, use the default locale with no prefix. +# +# Settings in defaults.yml +# +# default_locale: :en +# available_locales: +# - :en +# - :de +# +# NOTE: Although the browser sends the header Accept-Language, this is parsed by +# ruby as HTTP_ACCEPT_LANGUAGE +# + +class LocalePathTest < ActionDispatch::IntegrationTest + test "redirect if accept-language is not default locale" do + get_via_redirect '/', {}, 'HTTP_ACCEPT_LANGUAGE' => 'de' + assert_equal '/de', path + assert_equal({:locale => :de}, @controller.default_url_options) + end + + test "no locale prefix" do + get_via_redirect '/', {}, 'HTTP_ACCEPT_LANGUAGE' => 'en' + assert_equal '/', path + assert_equal({:locale => nil}, @controller.default_url_options) + + get_via_redirect '/', {}, 'HTTP_ACCEPT_LANGUAGE' => 'pt' + assert_equal '/', path + assert_equal({:locale => nil}, @controller.default_url_options) + end + + test "no redirect if locale explicit" do + get_via_redirect '/de', {}, 'HTTP_ACCEPT_LANGUAGE' => 'en' + assert_equal '/de', path + assert_equal({:locale => :de}, @controller.default_url_options) + end + + test "strip prefix from url options if locale is default" do + get_via_redirect '/en', {}, 'HTTP_ACCEPT_LANGUAGE' => 'en' + assert_equal '/en', path + assert_equal({:locale => nil}, @controller.default_url_options) + end +end \ No newline at end of file diff --git a/users/app/controllers/controller_extension/authentication.rb b/users/app/controllers/controller_extension/authentication.rb index dca3664..d831fbe 100644 --- a/users/app/controllers/controller_extension/authentication.rb +++ b/users/app/controllers/controller_extension/authentication.rb @@ -23,7 +23,7 @@ module ControllerExtension::Authentication respond_to do |format| format.html do if logged_in? - redirect_to root_url, :alert => t(:not_authorized) + redirect_to home_url, :alert => t(:not_authorized) else redirect_to login_url, :alert => t(:not_authorized_login) end diff --git a/users/app/controllers/sessions_controller.rb b/users/app/controllers/sessions_controller.rb index ca228c2..0195f30 100644 --- a/users/app/controllers/sessions_controller.rb +++ b/users/app/controllers/sessions_controller.rb @@ -1,7 +1,7 @@ class SessionsController < ApplicationController def new - redirect_to root_path if logged_in? + redirect_to home_url if logged_in? @session = Session.new if authentication_errors @errors = authentication_errors @@ -11,7 +11,7 @@ class SessionsController < ApplicationController def destroy logout - redirect_to root_path + redirect_to home_url end # diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb index 0b32ec7..a874772 100644 --- a/users/app/controllers/users_controller.rb +++ b/users/app/controllers/users_controller.rb @@ -61,7 +61,7 @@ class UsersController < UsersBaseController else # let's remove the invalid session logout - redirect_to root_url + redirect_to home_url end end diff --git a/users/app/views/sessions/new.html.haml b/users/app/views/sessions/new.html.haml index 0939e00..771dc97 100644 --- a/users/app/views/sessions/new.html.haml +++ b/users/app/views/sessions/new.html.haml @@ -7,4 +7,4 @@ = f.input :password, :required => false, :input_html => { :id => :srp_password } .form-actions = f.button :submit, :value => t(:login), :class => 'btn-primary' - = link_to t(:cancel), root_path, :class => 'btn' + = link_to t(:cancel), home_path, :class => 'btn' diff --git a/users/app/views/users/new.html.haml b/users/app/views/users/new.html.haml index f8d14b5..aecf831 100644 --- a/users/app/views/users/new.html.haml +++ b/users/app/views/users/new.html.haml @@ -15,5 +15,5 @@ = f.input :password_confirmation, :required => false, :validate => true, :input_html => { :id => :srp_password_confirmation } .form-actions = f.button :submit, :value => t(:signup), :class => 'btn btn-primary' - = link_to t(:cancel), root_url, :class => 'btn' + = link_to t(:cancel), home_path, :class => 'btn' diff --git a/users/config/routes.rb b/users/config/routes.rb index de2ff37..736b283 100644 --- a/users/config/routes.rb +++ b/users/config/routes.rb @@ -8,15 +8,17 @@ Rails.application.routes.draw do resources :users, :only => [:create, :update, :destroy, :index] end - get "login" => "sessions#new", :as => "login" - delete "logout" => "sessions#destroy", :as => "logout" + scope "(:locale)", :locale => MATCH_LOCALE do + get "login" => "sessions#new", :as => "login" + delete "logout" => "sessions#destroy", :as => "logout" - get "signup" => "users#new", :as => "signup" - resources :users, :except => [:create, :update] do - # resource :email_settings, :only => [:edit, :update] - # resources :email_aliases, :only => [:destroy], :id => /.*/ - post 'deactivate', on: :member - post 'enable', on: :member + get "signup" => "users#new", :as => "signup" + resources :users, :except => [:create, :update] do + # resource :email_settings, :only => [:edit, :update] + # resources :email_aliases, :only => [:destroy], :id => /.*/ + post 'deactivate', on: :member + post 'enable', on: :member + end end get "/.well-known/host-meta" => 'webfinger#host_meta' diff --git a/users/test/functional/sessions_controller_test.rb b/users/test/functional/sessions_controller_test.rb index 8b49005..fe7903f 100644 --- a/users/test/functional/sessions_controller_test.rb +++ b/users/test/functional/sessions_controller_test.rb @@ -17,11 +17,11 @@ class SessionsControllerTest < ActionController::TestCase assert_template "sessions/new" end - test "redirect to root_url if logged in" do + test "redirect to home_url if logged in" do login get :new assert_response :redirect - assert_redirected_to root_url + assert_redirected_to home_url end test "renders json" do @@ -53,7 +53,7 @@ class SessionsControllerTest < ActionController::TestCase expect_logout delete :destroy assert_response :redirect - assert_redirected_to root_url + assert_redirected_to home_url end end diff --git a/users/test/functional/users_controller_test.rb b/users/test/functional/users_controller_test.rb index 9c5f8d9..002a011 100644 --- a/users/test/functional/users_controller_test.rb +++ b/users/test/functional/users_controller_test.rb @@ -103,7 +103,7 @@ class UsersControllerTest < ActionController::TestCase delete :destroy, :id => @current_user.id assert_response :redirect - assert_redirected_to root_path + assert_redirected_to home_path end test "non-admin can't destroy user" do diff --git a/users/test/support/auth_test_helper.rb b/users/test/support/auth_test_helper.rb index 50e9453..57f9f9b 100644 --- a/users/test/support/auth_test_helper.rb +++ b/users/test/support/auth_test_helper.rb @@ -27,7 +27,7 @@ module AuthTestHelper else if logged_in assert_equal({:alert => I18n.t(:not_authorized)}, flash.to_hash) - assert_redirected_to root_url + assert_redirected_to home_url else assert_equal({:alert => I18n.t(:not_authorized_login)}, flash.to_hash) assert_redirected_to login_url -- cgit v1.2.3