Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions app/controllers/my_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions app/views/my/account.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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 %>
<meta name="turbo-cache-control" content="no-cache">
<% end %>

<%=
render(Primer::OpenProject::PageHeader.new) do |header|
header.with_title { t(:label_account) }
Expand Down
4 changes: 4 additions & 0 deletions app/views/users/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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 %>
<meta name="turbo-cache-control" content="no-cache">
<% end %>

<% tabs = tabs_for_key(:user, user: @user) %>

<%= render Users::EditPageHeaderComponent.new(user: @user, current_user:, tabs: tabs) %>
Expand Down
74 changes: 74 additions & 0 deletions spec/features/my/account_fresh_after_out_of_band_change_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# frozen_string_literal: true

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
Loading