Revert #7470 (default_url_options strip) -- exposes locale-leak regression

app/assets/javascripts/hyrax/select_work_type.es6

@@ -42,12 +42,9 @@ export default class SelectWorkType {
       let url = this.form.find('input[type="radio"]:checked').data(this.type)
       const select = this.form.find('select')
 
-      // Only append admin_set_id if the select exists and has a value.
-      // Pick the right separator: URLs lose their locale query string when
-      // I18n.locale == I18n.default_locale, so we cannot assume `?` is present.
+      // Only append admin_set_id if the select exists and has a value
       if (select.length && select.val()) {
-          const separator = url.indexOf('?') === -1 ? '?' : '&'
-          url = url + separator + "admin_set_id=" + select.val()
+          url = url + "&admin_set_id=" + select.val()
       }
       return url
   }

app/controllers/concerns/hyrax/controller.rb

@@ -27,15 +27,9 @@ def user_root_path
     hyrax.dashboard_path
   end
 
-  # Ensure that the locale choice is persistent across requests, but only
-  # when it differs from the default. Appending +?locale=+ for the default
-  # locale on every URL pollutes sitemaps, canonical tags, and outbound
-  # links, which causes search engines to index and rank duplicate
-  # locale-parameterized URLs as canonical.
+  # Ensure that the locale choice is persistent across requests
   def default_url_options
-    opts = super
-    opts[:locale] = I18n.locale unless I18n.locale == I18n.default_locale
-    opts
+    super.merge(locale: I18n.locale)
   end
 
   # @note for Blacklight 6/7 compatibility

app/presenters/hyrax/admin_set_presenter.rb

@@ -51,7 +51,7 @@ def collection_type_gid
     end
 
     def show_path
-      Hyrax::Engine.routes.url_helpers.admin_admin_set_path(id, locale: (I18n.locale unless I18n.locale == I18n.default_locale))
+      Hyrax::Engine.routes.url_helpers.admin_admin_set_path(id, locale: I18n.locale)
     end
 
     def available_parent_collections(*)

app/presenters/hyrax/collection_presenter.rb

@@ -138,7 +138,7 @@ def user_can_create_new_nest_collection?
     end
 
     def show_path
-      Hyrax::Engine.routes.url_helpers.dashboard_collection_path(id, locale: (I18n.locale unless I18n.locale == I18n.default_locale))
+      Hyrax::Engine.routes.url_helpers.dashboard_collection_path(id, locale: I18n.locale)
     end
 
     ##

app/presenters/hyrax/file_set_presenter.rb

@@ -69,7 +69,7 @@ def license
     end
 
     def stats_path
-      Hyrax::Engine.routes.url_helpers.stats_file_path(self, locale: (I18n.locale unless I18n.locale == I18n.default_locale))
+      Hyrax::Engine.routes.url_helpers.stats_file_path(self, locale: I18n.locale)
     end
 
     def events(size = 100)

app/presenters/hyrax/work_show_presenter.rb

@@ -199,7 +199,7 @@ def display_unfeature_link?
     end
 
     def stats_path
-      Hyrax::Engine.routes.url_helpers.stats_work_path(self, locale: (I18n.locale unless I18n.locale == I18n.default_locale))
+      Hyrax::Engine.routes.url_helpers.stats_work_path(self, locale: I18n.locale)
     end
 
     def model

app/renderers/hyrax/renderers/faceted_attribute_renderer.rb

@@ -9,7 +9,7 @@ def li_value(value)
       end
 
       def search_path(value)
-        Rails.application.routes.url_helpers.search_catalog_path("f[#{search_field}][]": value, locale: (I18n.locale unless I18n.locale == I18n.default_locale))
+        Rails.application.routes.url_helpers.search_catalog_path("f[#{search_field}][]": value, locale: I18n.locale)
       end
 
       def search_field

app/renderers/hyrax/renderers/linked_attribute_renderer.rb

@@ -10,7 +10,7 @@ def li_value(value)
 
       def search_path(value)
         Rails.application.routes.url_helpers.search_catalog_path(
-          search_field: search_field, q: ERB::Util.h(value), locale: (I18n.locale unless I18n.locale == I18n.default_locale)
+          search_field: search_field, q: ERB::Util.h(value), locale: I18n.locale
         )
       end
 

spec/controllers/concerns/hyrax/works_controller_behavior_spec.rb

@@ -70,7 +70,7 @@
     it 'redirects to new user login' do
       get :create, params: {}
 
-      expect(response).to redirect_to paths.new_user_session_path
+      expect(response).to redirect_to paths.new_user_session_path(locale: :en)
     end
 
     context 'with a logged in user' do
@@ -82,7 +82,7 @@
         get :create, params: { test_simple_work: { title: 'comet in moominland' } }
 
         expect(response)
-          .to redirect_to paths.send(work_route_path, id: assigns(:curation_concern).id)
+          .to redirect_to paths.send(work_route_path, id: assigns(:curation_concern).id, locale: :en)
       end
 
       it 'sets current user as depositor' do
@@ -306,7 +306,7 @@
   describe '#destroy' do
     it 'redirect to user login' do
       delete :destroy, params: { id: work.id }
-      expect(response).to redirect_to paths.new_user_session_path
+      expect(response).to redirect_to paths.new_user_session_path(locale: :en)
     end
 
     context 'with a logged in user' do
@@ -361,7 +361,7 @@
     it 'redirects to new user login' do
       get :edit, params: { id: work.id }
 
-      expect(response).to redirect_to paths.new_user_session_path
+      expect(response).to redirect_to paths.new_user_session_path(locale: :en)
     end
 
     context 'with a logged in user' do
@@ -433,7 +433,7 @@
   describe '#new' do
     it 'redirect to user login' do
       get :new
-      expect(response).to redirect_to paths.new_user_session_path
+      expect(response).to redirect_to paths.new_user_session_path(locale: :en)
     end
 
     context 'with a logged in user' do
@@ -521,7 +521,7 @@
     it 'redirects to new user login' do
       get :show, params: { id: work.id }
 
-      expect(response).to redirect_to paths.new_user_session_path
+      expect(response).to redirect_to paths.new_user_session_path(locale: :en)
     end
 
     context 'when indexed as public' do
@@ -549,7 +549,7 @@
     it 'redirects to new user login' do
       patch :update, params: { id: work.id }
 
-      expect(response).to redirect_to paths.new_user_session_path
+      expect(response).to redirect_to paths.new_user_session_path(locale: :en)
     end
 
     context 'when the user has edit access' do
@@ -561,7 +561,7 @@
         patch :update, params: { id: work.id, test_simple_work: { title: 'new title' } }
 
         expect(response)
-          .to redirect_to paths.send(work_route_path, id: work.id)
+          .to redirect_to paths.send(work_route_path, id: work.id, locale: :en)
       end
 
       it 'updates the work metadata' do

spec/controllers/hyrax/admin/permission_template_accesses_controller_spec.rb

@@ -69,6 +69,7 @@
 
             expect(response)
               .to redirect_to(hyrax.edit_admin_admin_set_path(source_id,
+                                                              locale: 'en',
                                                               anchor: 'participants'))
           end
 
@@ -106,6 +107,7 @@
 
             expect(response)
               .to redirect_to(hyrax.edit_admin_admin_set_path(source_id,
+                                                                   locale: 'en',
                                                                    anchor: 'participants'))
           end
 
@@ -148,6 +150,7 @@
 
             expect(response)
               .to redirect_to(hyrax.edit_dashboard_collection_path(source_id,
+                                                                   locale: 'en',
                                                                    anchor: 'sharing'))
           end
 
@@ -179,6 +182,7 @@
 
             expect(response)
               .to redirect_to(hyrax.edit_dashboard_collection_path(source_id,
+                                                                   locale: 'en',
                                                                    anchor: 'sharing'))
           end
 

spec/controllers/hyrax/admin/permission_templates_controller_spec.rb

@@ -49,7 +49,7 @@
         expect(controller).to receive(:authorize!).with(:update, Hyrax::PermissionTemplate)
         expect(form).to receive(:update).with(ActionController::Parameters.new(form_attributes).permit!).and_return(updated: true, content_tab: 'participants')
         put :update, params: input_params
-        expect(response).to redirect_to(hyrax.edit_admin_admin_set_path(permission_template.source_id, anchor: 'participants'))
+        expect(response).to redirect_to(hyrax.edit_admin_admin_set_path(permission_template.source_id, locale: 'en', anchor: 'participants'))
         expect(flash[:notice]).to eq(I18n.t('participants', scope: 'hyrax.admin.admin_sets.form.permission_update_notices'))
       end
     end
@@ -64,7 +64,7 @@
         expect(controller).to receive(:authorize!).with(:update, Hyrax::PermissionTemplate)
         expect(form).to receive(:update).with(ActionController::Parameters.new(form_attributes).permit!).and_return(updated: true, content_tab: 'sharing')
         put :update, params: input_params
-        expect(response).to redirect_to(hyrax.edit_dashboard_collection_path(permission_template.source_id, anchor: 'sharing'))
+        expect(response).to redirect_to(hyrax.edit_dashboard_collection_path(permission_template.source_id, locale: 'en', anchor: 'sharing'))
         expect(flash[:notice]).to eq(I18n.t('sharing', scope: 'hyrax.dashboard.collections.form.permission_update_notices'))
       end
 
@@ -79,7 +79,7 @@
           expect(controller).to receive(:authorize!).with(:update, Hyrax::PermissionTemplate)
           expect(form).to receive(:update).with(ActionController::Parameters.new(form_attributes).permit!).and_return(updated: true, content_tab: 'sharing')
           put :update, params: input_params
-          expect(response).to redirect_to(hyrax.edit_dashboard_collection_path(permission_template.source_id, anchor: 'sharing'))
+          expect(response).to redirect_to(hyrax.edit_dashboard_collection_path(permission_template.source_id, locale: 'en', anchor: 'sharing'))
 
           updated = Hyrax::PermissionTemplate.find_by(id: permission_template.id)
           expect(updated.release_date.strftime('%Y-%m-%d')).to eq(release_date)

spec/controllers/hyrax/admin/strategies_controller_spec.rb

@@ -35,7 +35,7 @@
         patch :update, params: { feature_id: feature.id, id: strategy }
 
         expect(response.location)
-          .to include Hyrax::Engine.routes.url_helpers.admin_features_path
+          .to include Hyrax::Engine.routes.url_helpers.admin_features_path(locale: 'en')
       end
     end
   end

spec/controllers/hyrax/admin/workflow_roles_controller_spec.rb

@@ -7,9 +7,9 @@
       end
 
       it "is successful" do
-        expect(controller).to receive(:add_breadcrumb).with('Home', root_path)
-        expect(controller).to receive(:add_breadcrumb).with('Dashboard', dashboard_path)
-        expect(controller).to receive(:add_breadcrumb).with('Workflow Roles', admin_workflow_roles_path)
+        expect(controller).to receive(:add_breadcrumb).with('Home', root_path(locale: 'en'))
+        expect(controller).to receive(:add_breadcrumb).with('Dashboard', dashboard_path(locale: 'en'))
+        expect(controller).to receive(:add_breadcrumb).with('Workflow Roles', admin_workflow_roles_path(locale: 'en'))
         get :index
         expect(response).to be_successful
         expect(assigns[:presenter]).to be_kind_of Hyrax::Admin::WorkflowRolesPresenter
@@ -20,7 +20,7 @@
     context "when they don't have permission" do
       it "throws a CanCan error" do
         get :index
-        expect(response).to redirect_to main_app.new_user_session_path
+        expect(response).to redirect_to main_app.new_user_session_path(locale: 'en')
       end
     end
   end
@@ -46,7 +46,7 @@
     context "when they don't have permission" do
       it 'throws a CanCan error' do
         post :create, params: { sipity_workflow_responsibility: {} }
-        expect(response).to redirect_to main_app.new_user_session_path
+        expect(response).to redirect_to main_app.new_user_session_path(locale: 'en')
       end
     end
   end
@@ -70,7 +70,7 @@
     context "when they don't have permission" do
       it "throws a CanCan error" do
         get :index
-        expect(response).to redirect_to main_app.new_user_session_path
+        expect(response).to redirect_to main_app.new_user_session_path(locale: 'en')
       end
     end
   end

spec/controllers/hyrax/api/zotero_controller_spec.rb

@@ -112,7 +112,7 @@
 
       specify do
         expect(subject).to have_http_status(:found)
-        expect(subject).to redirect_to(routes.url_helpers.edit_dashboard_profile_path(user))
+        expect(subject).to redirect_to(routes.url_helpers.edit_dashboard_profile_path(user, locale: 'en'))
         expect(flash[:alert]).to eq 'Malformed request from Zotero'
       end
     end
@@ -125,7 +125,7 @@
 
       specify do
         expect(subject).to have_http_status(:found)
-        expect(subject).to redirect_to(routes.url_helpers.edit_dashboard_profile_path(user))
+        expect(subject).to redirect_to(routes.url_helpers.edit_dashboard_profile_path(user, locale: 'en'))
         expect(flash[:alert]).to eq 'You have not yet connected to Zotero'
       end
     end
@@ -153,7 +153,7 @@
       specify do
         expect(subject).to have_http_status(:found)
         expect(Hyrax::Arkivo::CreateSubscriptionJob).to have_received(:perform_later)
-        expect(subject).to redirect_to(routes.url_helpers.dashboard_profile_path(user))
+        expect(subject).to redirect_to(routes.url_helpers.dashboard_profile_path(user, locale: 'en'))
         expect(flash[:alert]).to be_nil
         expect(flash[:notice]).to eq 'Successfully connected to Zotero!'
         expect(user.reload.zotero_userid).to eq zuserid

spec/controllers/hyrax/batch_edits_controller_spec.rb

@@ -12,9 +12,9 @@
 
   shared_examples('tests that edit page loads') do
     it "is successful" do
-      expect(controller).to receive(:add_breadcrumb).with('Home', Hyrax::Engine.routes.url_helpers.root_path)
-      expect(controller).to receive(:add_breadcrumb).with(I18n.t('hyrax.dashboard.title'), Hyrax::Engine.routes.url_helpers.dashboard_path)
-      expect(controller).to receive(:add_breadcrumb).with(I18n.t('hyrax.dashboard.my.works'), Hyrax::Engine.routes.url_helpers.my_works_path)
+      expect(controller).to receive(:add_breadcrumb).with('Home', Hyrax::Engine.routes.url_helpers.root_path(locale: 'en'))
+      expect(controller).to receive(:add_breadcrumb).with(I18n.t('hyrax.dashboard.title'), Hyrax::Engine.routes.url_helpers.dashboard_path(locale: 'en'))
+      expect(controller).to receive(:add_breadcrumb).with(I18n.t('hyrax.dashboard.my.works'), Hyrax::Engine.routes.url_helpers.my_works_path(locale: 'en'))
       get :edit
       expect(response).to be_successful
       expect(response).to render_template('dashboard')
@@ -75,15 +75,15 @@
 
       it "is successful" do
         put :update, params: { update_type: "delete_all" }
-        expect(response).to redirect_to(dashboard_path)
+        expect(response).to redirect_to(dashboard_path(locale: 'en'))
         expect { GenericWork.find(one.id) }.to raise_error(Ldp::Gone)
         expect { GenericWork.find(two.id) }.to raise_error(Ldp::Gone)
         expect(GenericWork).to exist(three.id)
       end
 
       it "redirects to the return controller" do
         put :update, params: { update_type: "delete_all", return_controller: mycontroller }
-        expect(response).to redirect_to(Hyrax::Engine.routes.url_for(controller: mycontroller, only_path: true))
+        expect(response).to redirect_to(Hyrax::Engine.routes.url_for(controller: mycontroller, only_path: true, locale: 'en'))
       end
 
       it "updates the records" do
@@ -190,15 +190,15 @@
 
       it "is successful" do
         put :update, params: { update_type: "delete_all" }
-        expect(response).to redirect_to(dashboard_path)
+        expect(response).to redirect_to(dashboard_path(locale: 'en'))
         expect { work1 }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
         expect { work2 }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
         expect { work3 }.not_to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
       end
 
       it "redirects to the return controller" do
         put :update, params: { update_type: "delete_all", return_controller: mycontroller }
-        expect(response).to redirect_to(Hyrax::Engine.routes.url_for(controller: mycontroller, only_path: true))
+        expect(response).to redirect_to(Hyrax::Engine.routes.url_for(controller: mycontroller, only_path: true, locale: 'en'))
       end
 
       it "updates the records" do