diff --git a/.gitignore b/.gitignore index 9731a5d18d..ce4de8869e 100644 --- a/.gitignore +++ b/.gitignore @@ -43,3 +43,6 @@ # Random .DS_Store *.swp + +# Local SQLite databases +/db/*.sqlite3* diff --git a/CHANGELOG b/CHANGELOG index cb2f9d7210..0f3415fa01 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,5 @@ [v#.#.#] ([month] [YYYY]) - - [entity]: - - [future tense verb] [feature] + - Issues: prevent editing a record while another user is already editing it - Upgraded gems: - rails, rails-html-sanitizer, sqlite3, websocket-driver - Bugs fixes: diff --git a/app/assets/javascripts/hera.js b/app/assets/javascripts/hera.js index b1b2bf9044..c95b32e70f 100644 --- a/app/assets/javascripts/hera.js +++ b/app/assets/javascripts/hera.js @@ -57,6 +57,7 @@ //= require hera/modules/liquid_async //= require hera/modules/nodes //= require hera/modules/project_board_colors +//= require hera/modules/release_edit_lock //= require hera/modules/search //= require hera/modules/sidebar //= require hera/modules/try_pro diff --git a/app/assets/javascripts/hera/behaviors.js b/app/assets/javascripts/hera/behaviors.js index 3fa9179268..66ff916519 100644 --- a/app/assets/javascripts/hera/behaviors.js +++ b/app/assets/javascripts/hera/behaviors.js @@ -93,6 +93,13 @@ document.addEventListener('turbo:load', function () { new LocalAutoSave(this); }); + // Release the edit lock when the Cancel link is used to leave an edit form + $(parentElement) + .find('[data-behavior~=release-edit-lock]') + .each(function () { + new ReleaseEditLock(this); + }); + // Fetch content $(parentElement) .find('[data-behavior~=fetch]') diff --git a/app/assets/javascripts/hera/modules/release_edit_lock.js b/app/assets/javascripts/hera/modules/release_edit_lock.js new file mode 100644 index 0000000000..f531dda9c2 --- /dev/null +++ b/app/assets/javascripts/hera/modules/release_edit_lock.js @@ -0,0 +1,26 @@ +class ReleaseEditLock { + constructor(link) { + this.link = link; + this.path = link.dataset.releaseEditLockPath; + + this.behaviors(); + } + + behaviors() { + this.link.addEventListener('click', () => this.release()); + } + + // Uses keepalive so the request still completes even though the click + // also triggers a full-page navigation away from the current document. + release() { + if (!this.path) { return; } + + const csrfToken = document.querySelector('meta[name="csrf-token"]')?.content; + + fetch(this.path, { + method: 'DELETE', + keepalive: true, + headers: { 'X-CSRF-Token': csrfToken }, + }); + } +} diff --git a/app/assets/stylesheets/hera/modules.scss b/app/assets/stylesheets/hera/modules.scss index 68229d7459..45f7586ece 100644 --- a/app/assets/stylesheets/hera/modules.scss +++ b/app/assets/stylesheets/hera/modules.scss @@ -8,6 +8,7 @@ @import 'hera/modules/divider'; @import 'hera/modules/dropdown'; @import 'hera/modules/dots_menu'; +@import 'hera/modules/edit_locked'; @import 'hera/modules/editor_toolbar'; @import 'hera/modules/icons'; @import 'hera/modules/inline_editable'; diff --git a/app/assets/stylesheets/hera/modules/_edit_locked.scss b/app/assets/stylesheets/hera/modules/_edit_locked.scss new file mode 100644 index 0000000000..d1fb4eae03 --- /dev/null +++ b/app/assets/stylesheets/hera/modules/_edit_locked.scss @@ -0,0 +1,85 @@ +.edit-locked { + padding: 3rem 2rem; + text-align: center; + + .actions { + display: flex; + gap: 0.75rem; + justify-content: center; + } + + .description { + color: var(--text-muted); + font-size: 1rem; + line-height: 1.6; + margin: 0 auto 2rem; + max-width: 28rem; + } + + .editor { + align-items: center; + background: var(--secondary-bg); + border-radius: 0.5rem; + display: flex; + gap: 0.75rem; + justify-content: center; + margin: 0 auto 2rem; + max-width: 22rem; + padding: 1rem 1.5rem; + } + + .editor-info { + text-align: left; + } + + .editor-name { + display: block; + font-size: 1rem; + font-weight: 600; + } + + .editor-status { + align-items: center; + color: var(--text-muted); + display: flex; + font-size: 0.85rem; + gap: 0.35rem; + } + + .heading { + color: var(--text-default); + font-size: 1.4rem; + font-weight: 700; + margin-bottom: 0.75rem; + } + + .icon { + align-items: center; + background: $orange-100; + border-radius: 50%; + display: flex; + height: 4.5rem; + justify-content: center; + margin: 0 auto 1.5rem; + width: 4.5rem; + + i { + color: $orange-500; + font-size: 1.8rem; + } + } + + .pulse { + animation: edit-locked-pulse 2s infinite; + background: var(--brand-bg); + border-radius: 50%; + display: inline-block; + height: 0.5rem; + width: 0.5rem; + } +} + +@keyframes edit-locked-pulse { + 0%, 100% { opacity: 1; } + 50% { opacity: 0.4; } +} diff --git a/app/controllers/concerns/edit_lockable.rb b/app/controllers/concerns/edit_lockable.rb new file mode 100644 index 0000000000..c295501c5c --- /dev/null +++ b/app/controllers/concerns/edit_lockable.rb @@ -0,0 +1,34 @@ +module EditLockable + extend ActiveSupport::Concern + + protected + + def acquire_edit_session(record) + EditingSession.acquire!(record_type: record.class.name, record_id: record.id, user: current_user) + end + + def check_edit_lock + record = lockable_record + competing_sessions = EditingSession.for_record(record).active.by_others(current_user) + + if competing_sessions.any? && params[:force] != 'true' + @locked_by = competing_sessions.includes(:user).map(&:user) + @locked_record = record + @back_path = url_from(request.referer) || root_path + render 'shared/edit_locked' + return + end + + acquire_edit_session(record) + end + + def lockable_record + @lockable_record ||= + instance_variable_get("@#{controller_name.singularize}") || + send("set_or_initialize_#{controller_name.singularize}") + end + + def release_edit_session(record) + EditingSession.for_record(record).where(user: current_user).destroy_all + end +end diff --git a/app/controllers/issues/editing_sessions_controller.rb b/app/controllers/issues/editing_sessions_controller.rb new file mode 100644 index 0000000000..e62a985595 --- /dev/null +++ b/app/controllers/issues/editing_sessions_controller.rb @@ -0,0 +1,15 @@ +class Issues::EditingSessionsController < AuthenticatedController + include EditLockable + include ProjectScoped + + def destroy + release_edit_session(issue) + head :no_content + end + + private + + def issue + @issue ||= current_project.issues.find(params[:issue_id]) + end +end diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 90b1bc0011..adb0e43ddf 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -2,6 +2,7 @@ class IssuesController < AuthenticatedController include ConflictResolver include ContentFromTemplate include DynamicFieldNamesCacher + include EditLockable include EventPublisher include IssuesHelper include LiquidEnabledResource @@ -16,6 +17,7 @@ class IssuesController < AuthenticatedController before_action :set_columns, only: :index before_action :set_or_initialize_issue, except: [:import, :index] + before_action :check_edit_lock, only: :edit before_action :set_auto_save_key, only: [:new, :create, :edit, :update] before_action :set_affected_nodes, only: [:show] before_action :set_form_cancel_path, only: [:new, :edit] @@ -31,8 +33,8 @@ def show .group('nodes.id') .sort_by { |node, _| node.label } - @first_node = @affected_nodes.first - @first_evidence = Evidence.where(node: @first_node, issue: @issue) + @first_node = @affected_nodes.first + @first_evidence = Evidence.where(node: @first_node, issue: @issue) load_conflicting_revisions(@issue) end @@ -81,6 +83,7 @@ def update updated_at_before_save = @issue.updated_at.to_i if @issue.update(issue_params) + release_edit_session(@issue) @modified = true check_for_edit_conflicts(@issue, updated_at_before_save) format.html { redirect_to_main_or_qa } diff --git a/app/models/configuration.rb b/app/models/configuration.rb index 96d52dd2a8..3e943c4bb7 100644 --- a/app/models/configuration.rb +++ b/app/models/configuration.rb @@ -15,6 +15,11 @@ class Configuration < ApplicationRecord # -- Class Methods -------------------------------------------------------- # --------------------------------------------------------------- Misc admin: + def self.editing_session_stale_after + create_with(value: 1440) + .find_or_create_by(name: 'admin:editing_session_stale_after').value.to_i + end + def self.max_deleted_inline create_with(value: 15) .find_or_create_by(name: 'admin:max_deleted_inline').value.to_i diff --git a/app/models/editing_session.rb b/app/models/editing_session.rb new file mode 100644 index 0000000000..a93b96b534 --- /dev/null +++ b/app/models/editing_session.rb @@ -0,0 +1,31 @@ +class EditingSession < ApplicationRecord + ALLOWED_RECORD_TYPES = %w[Issue].freeze + + belongs_to :user + belongs_to :record, polymorphic: true + + validates :record_type, presence: true, inclusion: { in: ALLOWED_RECORD_TYPES } + + scope :active, -> { where(created_at: stale_after.ago..) } + scope :by_others, ->(user) { where.not(user: user) } + # `where(record: record)` won't work here: Issue is an STI subclass of Note, + # so Rails' polymorphic query would look up record_type: 'Note' (the base + # class), not 'Issue' (what we actually store, see .acquire! below). + scope :for_record, ->(record) { + where(record_type: record.class.name, record_id: record.id) + } + scope :stale, -> { where(created_at: ...stale_after.ago) } + + def self.acquire!(record_type:, record_id:, user:) + purge_stale_for(record_type: record_type, record_id: record_id) + create_or_find_by!(record_type: record_type, record_id: record_id) { |session| session.user = user } + end + + def self.purge_stale_for(record_type:, record_id:) + where(record_type: record_type, record_id: record_id).stale.destroy_all + end + + def self.stale_after + Configuration.editing_session_stale_after.minutes + end +end diff --git a/app/views/issues/_form.html.erb b/app/views/issues/_form.html.erb index e051ed9dbc..551ee69af2 100644 --- a/app/views/issues/_form.html.erb +++ b/app/views/issues/_form.html.erb @@ -37,15 +37,15 @@
<%= render partial: 'qa/state_button', locals: { f: f, record: @issue } %> + or + <%= + link_to 'Cancel', + @form_cancel_path, + class: 'cancel-link', + data: { + behavior: class_names('clear-local-auto-save', 'release-edit-lock': @issue.persisted?), + 'release-edit-lock-path': @issue.persisted? ? project_issue_editing_session_path(current_project, @issue) : nil + } + %>
- or - <%= - link_to 'Cancel', - @form_cancel_path, - class: 'cancel-link', - data: { - behavior: 'clear-local-auto-save' - } - %> - <% end %> diff --git a/app/views/qa/_state_button.html.erb b/app/views/qa/_state_button.html.erb index 1ddb5aef34..280df302a8 100644 --- a/app/views/qa/_state_button.html.erb +++ b/app/views/qa/_state_button.html.erb @@ -18,3 +18,4 @@ <% end %> <% end %> + diff --git a/app/views/shared/edit_locked.html.erb b/app/views/shared/edit_locked.html.erb new file mode 100644 index 0000000000..0dbb839b79 --- /dev/null +++ b/app/views/shared/edit_locked.html.erb @@ -0,0 +1,40 @@ +<% content_for :title, 'Record locked' %> + +
+
+
+ +
+ +

+ This <%= @locked_record.model_name.human.downcase %> is currently being edited +

+ +

+ Another team member is currently editing this record. Go back, or edit + anyway and risk overwriting their changes. +

+ + <% @locked_by.each do |user| %> +
+ <%= avatar_image(user, size: 48) %> +
+ <%= user.name %> + + Currently editing + +
+
+ <% end %> + +
+ <%= link_to @back_path, class: 'btn btn-primary' do %> + Go back + <% end %> + + <%= link_to url_for(request.parameters.merge(force: 'true')), class: 'btn btn-outline-danger' do %> + Edit anyway + <% end %> +
+
+
diff --git a/config/routes.rb b/config/routes.rb index 6bbfe11a41..0c1275b4af 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -79,6 +79,7 @@ resources :evidence, concerns: :multiple_destroy, controller: 'issues/evidence', only: [:index, :new] resources :nodes, only: [:show], controller: 'issues/nodes' resources :revisions, only: [:index, :show] + resource :editing_session, only: :destroy, controller: 'issues/editing_sessions' end resources :methodologies do diff --git a/db/migrate/20260722100000_create_editing_sessions.rb b/db/migrate/20260722100000_create_editing_sessions.rb new file mode 100644 index 0000000000..43fd48a40c --- /dev/null +++ b/db/migrate/20260722100000_create_editing_sessions.rb @@ -0,0 +1,11 @@ +class CreateEditingSessions < ActiveRecord::Migration[8.0] + def change + create_table :editing_sessions do |t| + t.references :user, null: false, foreign_key: true + t.references :record, polymorphic: true, null: false, index: false + t.datetime :created_at, null: false + + t.index [:record_type, :record_id], unique: true, name: 'index_editing_sessions_uniqueness' + end + end +end diff --git a/db/schema.rb b/db/schema.rb index cbce9e5bf5..1d58c9d717 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2026_05_21_000001) do +ActiveRecord::Schema[8.0].define(version: 2026_07_22_100000) do create_table "active_storage_attachments", force: :cascade do |t| t.string "name", null: false t.string "record_type", null: false @@ -140,6 +140,15 @@ t.datetime "updated_at", null: false end + create_table "editing_sessions", force: :cascade do |t| + t.integer "user_id", null: false + t.string "record_type", null: false + t.integer "record_id", null: false + t.datetime "created_at", null: false + t.index ["record_type", "record_id"], name: "index_editing_sessions_uniqueness", unique: true + t.index ["user_id"], name: "index_editing_sessions_on_user_id" + end + create_table "evidence", force: :cascade do |t| t.integer "node_id" t.integer "issue_id" @@ -302,6 +311,7 @@ add_foreign_key "comments", "users", on_delete: :nullify add_foreign_key "dradis_plugins_echo_agents", "dradis_plugins_echo_providers", column: "provider_id" add_foreign_key "dradis_plugins_echo_prompts", "users" + add_foreign_key "editing_sessions", "users" add_foreign_key "inline_threads", "users" add_foreign_key "inline_threads", "users", column: "resolved_by_id" add_foreign_key "mapping_fields", "mappings" diff --git a/spec/factories/editing_sessions.rb b/spec/factories/editing_sessions.rb new file mode 100644 index 0000000000..d2acbebf94 --- /dev/null +++ b/spec/factories/editing_sessions.rb @@ -0,0 +1,7 @@ +FactoryBot.define do + factory :editing_session do + association :user + record_type { 'Issue' } + record_id { create(:issue).id } + end +end diff --git a/spec/features/edit_locking_spec.rb b/spec/features/edit_locking_spec.rb new file mode 100644 index 0000000000..12a1b651a4 --- /dev/null +++ b/spec/features/edit_locking_spec.rb @@ -0,0 +1,75 @@ +require 'rails_helper' + +describe 'Edit locking multi-actor flow' do + let(:project) { Project.new } + let(:issue) { create(:issue, node: project.issue_library) } + let(:password) { 'spec-password' } + + before do + Configuration.find_or_create_by(name: 'admin:password') + .update!(value: BCrypt::Password.create(password)) + end + + def sign_in_as(username) + visit login_path + fill_in 'Username', with: username + fill_in 'Password', with: password + click_button 'Log in' + end + + it 'locks the record for a second editor, lets them bypass it, and releases the lock on save' do + Capybara.using_session(:user_a) { sign_in_as('user-a@example.com') } + Capybara.using_session(:user_b) { sign_in_as('user-b@example.com') } + + Capybara.using_session(:user_a) do + visit edit_project_issue_path(project, issue) + expect(page).to have_content('Edit issue') + end + + Capybara.using_session(:user_b) do + visit edit_project_issue_path(project, issue) + expect(page).to have_content('currently being edited') + expect(page).to have_content('user-a@example.com') + + click_link 'Go back' + expect(page).not_to have_content('currently being edited') + end + + Capybara.using_session(:user_b) do + visit edit_project_issue_path(project, issue) + click_link 'Edit anyway' + expect(page).to have_content('Edit issue') + end + + Capybara.using_session(:user_a) do + find('.btn-states button[type="submit"]').click + expect(page).to have_content('Issue updated.') + end + + user_a = User.find_by(email: 'user-a@example.com') + expect(EditingSession.for_record(issue).where(user: user_a)).not_to exist + end + + it 'releases the lock when the editor clicks cancel', js: true do + Capybara.using_session(:user_a) { sign_in_as('user-a@example.com') } + + Capybara.using_session(:user_a) do + visit edit_project_issue_path(project, issue) + expect(page).to have_content('Edit issue') + + click_link 'Cancel' + expect(page).to have_current_path(project_issue_path(project, issue)) + end + + user_a = User.find_by(email: 'user-a@example.com') + + # The lock release request is fired with `fetch(..., { keepalive: true })` + # alongside the Cancel link's navigation, so it may still be in flight + # once the browser lands on the next page. + Timeout.timeout(Capybara.default_max_wait_time) do + sleep 0.1 while EditingSession.for_record(issue).where(user: user_a).exists? + end + + expect(EditingSession.for_record(issue).where(user: user_a)).not_to exist + end +end diff --git a/spec/models/editing_session_spec.rb b/spec/models/editing_session_spec.rb new file mode 100644 index 0000000000..2918160f2c --- /dev/null +++ b/spec/models/editing_session_spec.rb @@ -0,0 +1,148 @@ +require 'rails_helper' + +describe EditingSession do + let(:user) { create(:user) } + let(:other_user) { create(:user) } + let(:project) { Project.new } + let(:issue) { create(:issue, node: project.issue_library) } + + describe 'associations' do + it { is_expected.to belong_to(:user) } + it { is_expected.to belong_to(:record) } + end + + describe '#record' do + it 'resolves the polymorphic association from record_type and record_id' do + session = create(:editing_session, user: user, record_type: 'Issue', record_id: issue.id) + + expect(session.record).to eq(issue) + end + end + + describe 'validations' do + subject { create(:editing_session, user: user, record_type: 'Issue', record_id: issue.id) } + it { is_expected.to validate_presence_of(:record_type) } + end + + describe '.for_record' do + it 'returns sessions for the given record' do + session = create(:editing_session, user: user, record_type: 'Issue', record_id: issue.id) + other_issue = create(:issue, node: project.issue_library) + create(:editing_session, user: user, record_type: 'Issue', record_id: other_issue.id) + + expect(EditingSession.for_record(issue)).to eq([session]) + end + end + + describe '.by_others' do + it 'excludes sessions belonging to the given user' do + create(:editing_session, user: user, record_type: 'Issue', record_id: issue.id) + other_issue = create(:issue, node: project.issue_library) + other_session = create(:editing_session, user: other_user, record_type: 'Issue', record_id: other_issue.id) + + expect(EditingSession.by_others(user)).to contain_exactly(other_session) + end + end + + describe '.active' do + it 'excludes sessions older than the staleness threshold' do + fresh_session = create(:editing_session, + user: user, + record_type: 'Issue', + record_id: issue.id, + created_at: 1.minute.ago + ) + other_issue = create(:issue, node: project.issue_library) + create(:editing_session, + user: other_user, + record_type: 'Issue', + record_id: other_issue.id, + created_at: EditingSession.stale_after.ago - 1.minute + ) + + expect(EditingSession.active).to eq([fresh_session]) + end + end + + describe '.purge_stale_for' do + it 'destroys stale sessions for the given record only' do + create(:editing_session, + user: user, + record_type: 'Issue', + record_id: issue.id, + created_at: EditingSession.stale_after.ago - 1.minute + ) + other_issue = create(:issue, node: project.issue_library) + stale_elsewhere = create(:editing_session, + user: other_user, + record_type: 'Issue', + record_id: other_issue.id, + created_at: EditingSession.stale_after.ago - 1.minute + ) + + EditingSession.purge_stale_for(record_type: 'Issue', record_id: issue.id) + + expect(EditingSession.all).to contain_exactly(stale_elsewhere) + end + end + + describe '.acquire!' do + it 'creates a session for the user and record' do + session = EditingSession.acquire!(record_type: 'Issue', record_id: issue.id, user: user) + + expect(session).to be_persisted + expect(EditingSession.where(user: user, record_type: 'Issue', record_id: issue.id)).to exist + end + + it 'returns the existing session instead of raising when one already exists' do + existing = create(:editing_session, user: user, record_type: 'Issue', record_id: issue.id) + + session = EditingSession.acquire!(record_type: 'Issue', record_id: issue.id, user: user) + + expect(session).to eq(existing) + end + + it 'returns the existing owner session instead of creating one for another user' do + existing = create(:editing_session, user: user, record_type: 'Issue', record_id: issue.id) + + session = EditingSession.acquire!(record_type: 'Issue', record_id: issue.id, user: other_user) + + expect(session).to eq(existing) + expect(EditingSession.for_record(issue).count).to eq(1) + end + + it 'purges stale sessions for the record before acquiring' do + stale = create(:editing_session, + user: other_user, + record_type: 'Issue', + record_id: issue.id, + created_at: EditingSession.stale_after.ago - 1.minute + ) + + EditingSession.acquire!(record_type: 'Issue', record_id: issue.id, user: user) + + expect(EditingSession.where(id: stale.id)).not_to exist + end + end + + describe '.stale_after' do + it 'defaults to 1 day' do + expect(EditingSession.stale_after).to eq(1.day) + end + + it 'is configurable instance-wide via Configuration' do + Configuration.find_or_create_by(name: 'admin:editing_session_stale_after').update(value: 30) + + expect(EditingSession.stale_after).to eq(30.minutes) + end + end + + describe 'unique constraint' do + it 'prevents more than one session for the same record at the database level' do + create(:editing_session, user: user, record_type: 'Issue', record_id: issue.id) + + duplicate = EditingSession.new(user: other_user, record_type: 'Issue', record_id: issue.id) + expect { duplicate.save(validate: false) }.to raise_error(ActiveRecord::RecordNotUnique) + end + end +end diff --git a/spec/requests/edit_locking_spec.rb b/spec/requests/edit_locking_spec.rb new file mode 100644 index 0000000000..1c344b82ac --- /dev/null +++ b/spec/requests/edit_locking_spec.rb @@ -0,0 +1,169 @@ +require 'rails_helper' + +describe 'EditLockable concern' do + let(:user_a) { create(:user) } + let(:user_b) { create(:user) } + let(:project) { Project.new } + let(:issue) { create(:issue, node: project.issue_library) } + + before { @project = project } + + describe '#check_edit_lock' do + context 'when another user has an active editing session' do + before do + create(:editing_session, + user: user_a, + record_type: 'Issue', + record_id: issue.id + ) + end + + it 'renders the lockout page' do + login_as_user(user_b) + get edit_project_issue_path(project, issue) + expect(response.body).to include('currently being edited') + end + + it 'does not create a session for the locked-out user' do + login_as_user(user_b) + get edit_project_issue_path(project, issue) + expect(EditingSession.where(user: user_b)).to be_empty + end + end + + context 'when no other user is editing' do + it 'renders the edit page' do + login_as_user(user_a) + get edit_project_issue_path(project, issue) + expect(response.body).to include('Edit issue') + end + + it 'creates an editing session' do + login_as_user(user_a) + get edit_project_issue_path(project, issue) + expect(EditingSession.for_record(issue).where(user: user_a)).to exist + end + end + + context 'when another user has a stale editing session' do + before do + create(:editing_session, + user: user_a, + record_type: 'Issue', + record_id: issue.id, + created_at: EditingSession.stale_after.ago - 1.minute + ) + end + + it 'renders the edit page instead of the lockout page' do + login_as_user(user_b) + get edit_project_issue_path(project, issue) + expect(response.body).to include('Edit issue') + end + + it 'purges the stale session' do + login_as_user(user_b) + get edit_project_issue_path(project, issue) + expect(EditingSession.where(user: user_a)).not_to exist + end + end + + context 'when the request has a cross-origin referer' do + before do + create(:editing_session, + user: user_a, + record_type: 'Issue', + record_id: issue.id + ) + end + + it 'does not use the referer as the back path' do + login_as_user(user_b) + get edit_project_issue_path(project, issue), headers: { 'HTTP_REFERER' => 'https://evil.example.com/phish' } + expect(response.body).not_to include('https://evil.example.com/phish') + end + end + + context 'when force=true' do + before do + create(:editing_session, + user: user_a, + record_type: 'Issue', + record_id: issue.id + ) + end + + it 'bypasses the lock and renders the edit page' do + login_as_user(user_b) + get edit_project_issue_path(project, issue, force: 'true') + expect(response.body).to include('Edit issue') + end + + it 'does not create a new session, leaving the original owner as the lock holder' do + login_as_user(user_b) + get edit_project_issue_path(project, issue, force: 'true') + + expect(EditingSession.for_record(issue).count).to eq(1) + expect(EditingSession.for_record(issue).first.user).to eq(user_a) + end + end + end + + describe '#release_edit_session' do + it 'destroys the editing session on update' do + login_as_user(user_a) + create(:editing_session, + user: user_a, + record_type: 'Issue', + record_id: issue.id + ) + + patch project_issue_path(project, issue), + params: { issue: { text: '#[Title]#\nUpdated' } } + + expect(EditingSession.for_record(issue).where(user: user_a)).not_to exist + end + + it 'keeps the editing session when the update fails validation' do + login_as_user(user_a) + create(:editing_session, + user: user_a, + record_type: 'Issue', + record_id: issue.id + ) + + patch project_issue_path(project, issue), + params: { issue: { text: 'a' * (DB_MAX_TEXT_LENGTH + 1) } } + + expect(response.body).to include('Edit issue') + expect(EditingSession.for_record(issue).where(user: user_a)).to exist + end + end + + describe 'DELETE project_issue_editing_session_path' do + before do + create(:editing_session, + user: user_a, + record_type: 'Issue', + record_id: issue.id + ) + end + + it 'releases the current user\'s editing session' do + login_as_user(user_a) + delete project_issue_editing_session_path(project, issue) + + expect(EditingSession.for_record(issue).where(user: user_a)).not_to exist + end + + it 'does not release another user\'s editing session on a different record' do + other_issue = create(:issue, node: project.issue_library) + create(:editing_session, user: user_b, record_type: 'Issue', record_id: other_issue.id) + + login_as_user(user_a) + delete project_issue_editing_session_path(project, issue) + + expect(EditingSession.for_record(other_issue).where(user: user_b)).to exist + end + end +end