Skip to content

Commit

Permalink
Fixes language switching to default language
Browse files Browse the repository at this point in the history
Closes #2632

From this commit on the language id won't be stored in the session any more.

Upon every page visit the language id got stored in the session. When switching from a secondary language to the default language no language param is passed to the controller. The order how the language is found prioritized the session and the default was just a fallback. Coming from a secondary language and therefore it's id in the session, it would never load the default language, but the secondary.

Storing the language in the session was most likely a performance optimization. Removing this should not impact current behaviour.
  • Loading branch information
robinboening committed Jan 16, 2024
1 parent f2d045e commit 1c2b53e
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 84 deletions.
21 changes: 4 additions & 17 deletions lib/alchemy/controller_actions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,15 @@ def set_current_alchemy_site
Site.current = current_alchemy_site
end

# Try to find and stores current language for Alchemy.
# Sets the current language for Alchemy.
#
def set_alchemy_language(lang = nil)
@language = if lang
lang.is_a?(Language) ? lang : load_alchemy_language_from_id_or_code(lang)
else
# find the best language and remember it for later
load_alchemy_language_from_params ||
load_alchemy_language_from_session ||
Language.default
load_alchemy_language_from_params || Language.default
end

store_current_alchemy_language(@language)
end

Expand All @@ -80,26 +78,15 @@ def load_alchemy_language_from_params
end
end

# Load language from session if it's present on current site.
# Otherwise return nil so we can load the default language from current site.
def load_alchemy_language_from_session
if session[:alchemy_language_id].present? && Site.current
Site.current.languages.find_by(id: session[:alchemy_language_id])
end
end

def load_alchemy_language_from_id_or_code(id_or_code)
Language.find_by(id: id_or_code) ||
Language.find_by_code(id_or_code)
end

# Stores language's id in the session.
#
# Also stores language in +Language.current+
# Stores language in +Language.current+
#
def store_current_alchemy_language(language)
if language&.id
session[:alchemy_language_id] = language.id
Language.current = language
end
end
Expand Down
5 changes: 0 additions & 5 deletions spec/controllers/alchemy/admin/languages_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,6 @@

let(:language) { create(:alchemy_language, :klingon) }

it "should store the current language in session" do
switch
expect(session[:alchemy_language_id]).to eq(language.id)
end

context "having a referer" do
before do
expect_any_instance_of(ActionDispatch::Request).to receive(:referer) do
Expand Down
5 changes: 0 additions & 5 deletions spec/controllers/alchemy/pages_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,6 @@ module Alchemy
end

context "with no lang parameter present" do
it "should store defaults language id in the session." do
get :show, params: {urlname: page.urlname}
expect(controller.session[:alchemy_language_id]).to eq(Language.default.id)
end

it "should store default language as class var." do
get :show, params: {urlname: page.urlname}
expect(Language.current).to eq(Language.default)
Expand Down
1 change: 0 additions & 1 deletion spec/helpers/alchemy/base_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ module Alchemy

context "of an existing page" do
it "should return the page object" do
session[:alchemy_language_id] = page.language_id
expect(helper.page_or_find(page.page_layout)).to eq(page)
end
end
Expand Down
62 changes: 6 additions & 56 deletions spec/libraries/controller_actions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,24 +55,24 @@
RequestStore.store[:alchemy_current_language] = nil
end

context "with a Language argument" do
it "should set the language to the passed Language instance" do
context "with a Language object given" do
it "should set the language to the Language instance" do
controller.send :set_alchemy_language, klingon
expect(assigns(:language)).to eq(klingon)
expect(Alchemy::Language.current).to eq(klingon)
end
end

context "with a language id argument" do
it "should set the language to the language specified by the passed id" do
context "with a language id given" do
it "should find and set the language by the id" do
controller.send :set_alchemy_language, klingon.id
expect(assigns(:language)).to eq(klingon)
expect(Alchemy::Language.current).to eq(klingon)
end
end

context "with a language code argument" do
it "should set the language to the language specified by the passed code" do
context "with a locale given" do
it "should find and set the language by the locale" do
controller.send :set_alchemy_language, klingon.code
expect(assigns(:language)).to eq(klingon)
expect(Alchemy::Language.current).to eq(klingon)
Expand All @@ -85,55 +85,6 @@
controller.send :set_alchemy_language
expect(assigns(:language)).to eq(default_language)
expect(Alchemy::Language.current).to eq(default_language)
expect(controller.session).to include_language_information_for(default_language)
end
end

context "with language in the session" do
before do
allow(controller).to receive(:session).and_return(alchemy_language_id: klingon.id)
end

it "should use the language from the session" do
controller.send :set_alchemy_language
expect(assigns(:language)).to eq(klingon)
expect(Alchemy::Language.current).to eq(klingon)
end

context "if no current site exists" do
before do
expect(Alchemy::Site).to receive(:current).exactly(:twice) { nil }
end

it "should set the default language" do
controller.send :set_alchemy_language

expect(assigns(:language)).to eq(default_language)
expect(Alchemy::Language.current).to eq(default_language)
end
end

context "if the language is not on the current site" do
let(:french_site) do
create(:alchemy_site, host: "french.fr")
end

let(:french_language) do
create(:alchemy_language, site: french_site, code: :fr)
end

before do
expect(controller).to receive(:session).at_least(:once) do
{alchemy_language_id: french_language.id}
end
end

it "should set the default language" do
controller.send :set_alchemy_language

expect(assigns(:language)).to eq(default_language)
expect(Alchemy::Language.current).to eq(default_language)
end
end
end

Expand All @@ -143,7 +94,6 @@
controller.send :set_alchemy_language
expect(assigns(:language)).to eq(klingon)
expect(Alchemy::Language.current).to eq(klingon)
expect(controller.session).to include_language_information_for(klingon)
end

context "for language that does not exist" do
Expand Down

0 comments on commit 1c2b53e

Please sign in to comment.