diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 2964d20def35..398d2c5c8118 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -178,6 +178,13 @@ def set_cache_buster end end + # Firefox serves these pages stale from the HTTP cache and bfcache on reload and + # history-back (the turbo-cache-control meta only governs Turbo's snapshot). + # no-store opts out of both so a fresh copy is always fetched. + def prevent_response_caching + response.cache_control.merge!(no_store: true) + end + def tag_request context = { controller: self, request: } ::OpenProject::Appsignal.tag_request(context) diff --git a/app/controllers/my_controller.rb b/app/controllers/my_controller.rb index 473d736b251b..e6e379dae0a2 100644 --- a/app/controllers/my_controller.rb +++ b/app/controllers/my_controller.rb @@ -41,6 +41,7 @@ class MyController < ApplicationController before_action :require_login before_action :set_current_user before_action :check_password_confirmation, only: %i[update_account] + before_action :prevent_response_caching, only: :account no_authorization_required! :account, :update_account, diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 8a3bf1a126b2..ab3040ad831b 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -36,6 +36,7 @@ class UsersController < ApplicationController layout "admin" before_action :authorize_global, except: %i[show deletion_info destroy] + before_action :prevent_response_caching, only: :edit # rubocop:disable Rails/LexicallyScopedActionFilter before_action :find_user, only: %i[show diff --git a/app/views/my/account.html.erb b/app/views/my/account.html.erb index cfe54c27176e..539acb470f3d 100644 --- a/app/views/my/account.html.erb +++ b/app/views/my/account.html.erb @@ -29,6 +29,10 @@ See COPYRIGHT and LICENSE files for more details. <% html_title(t(:label_my_account), t(:label_account)) -%> +<% content_for :header_tags do %> + +<% end %> + <%= render(Primer::OpenProject::PageHeader.new) do |header| header.with_title { t(:label_account) } diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index d97a0481878b..c6688033a316 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -29,6 +29,10 @@ See COPYRIGHT and LICENSE files for more details. <% html_title(t(:label_administration), "#{t(:label_edit)} #{User.model_name.human} #{h(@user.name)}") -%> +<% content_for :header_tags do %> + +<% end %> + <% tabs = tabs_for_key(:user, user: @user) %> <%= render Users::EditPageHeaderComponent.new(user: @user, current_user:, tabs: tabs) %> diff --git a/spec/features/my/account_fresh_after_out_of_band_change_spec.rb b/spec/features/my/account_fresh_after_out_of_band_change_spec.rb new file mode 100644 index 000000000000..022c678385e6 --- /dev/null +++ b/spec/features/my/account_fresh_after_out_of_band_change_spec.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +# After an administrator changes a user's attributes out-of-band (e.g. their department, +# or the "Job title" custom field), the account page must not keep showing the old values. +# The examples below cover each way of returning to the page. +RSpec.describe "My account is never served stale after an out-of-band change", :js do + shared_let(:admin) { create(:admin) } + shared_let(:section) { create(:user_custom_field_section, name: "Professional info") } + shared_let(:job_title) { create(:user_custom_field, :string, name: "Job title", user_custom_field_section: section) } + shared_let(:alpha) { create(:department, name: "Alpha department", members: [admin]) } + shared_let(:beta) { create(:department, name: "Beta department") } + + before do + section.update!(attribute_order: ["department", job_title.column_name]) + admin.update!(custom_field_values: { job_title.id => "Old title" }) + login_as admin + visit my_account_path + end + + # Confirms the page rendered the current values, then has an administrator change the + # department and job title out-of-band so that any browser/Turbo cache now holds stale data. + def change_department_and_job_title_out_of_band + expect(page).to have_select("user[department_id]", disabled: true, selected: "Alpha department") + expect(page).to have_field("Job title", with: "Old title") + + Departments::AddUserService.new(beta, user: admin) + .call(user_id: admin.id, remove_from_previous_department: true) + admin.update!(custom_field_values: { job_title.id => "New title" }) + end + + def expect_fresh_account + expect(page).to have_select("user[department_id]", disabled: true, selected: "Beta department") + expect(page).to have_field("Job title", with: "New title") + end + + # Fixed by the `turbo-cache-control: no-cache` meta: a restoration visit would + # otherwise re-render Turbo's cached snapshot. + it "is fresh after navigating away and back through browser history" do + change_department_and_job_title_out_of_band + + click_on "Notification and email" + expect(page).to have_current_path(my_notifications_path, wait: 10, ignore_query: true) + + page.go_back + + expect_fresh_account + end + + # Green in Chrome (it revalidates on reload) but stale in Firefox, where the HTTP + # cache / bfcache serves the old page; fixed by `Cache-Control: no-store`. + it "is fresh after a reload" do + change_department_and_job_title_out_of_band + + page.refresh + + expect_fresh_account + end + + # Navigating back to the account page through the menu is a fresh Turbo visit; + # this guards that it keeps fetching current data. + it "is fresh after navigating away and back via the menu" do + change_department_and_job_title_out_of_band + + click_on "Notification and email" + expect(page).to have_current_path(my_notifications_path, wait: 10, ignore_query: true) + + click_on "Account" + expect(page).to have_current_path(my_account_path, wait: 10, ignore_query: true) + + expect_fresh_account + end +end